feat: add feature for generating presentation files #57
Conversation
… all related namespaces
…ding unique id in the filename
…ve generated token to redis
WalkthroughRebrands DocumentService to OsmoDoc, replaces legacy generators and models with new OsmoDoc PDF/Word/PPTX implementations, adds Redis-backed JWT token storage and token revocation, integrates OpenAI-based slide extraction, updates configuration/Docker/VSCode tooling, expands DocFX docs, and restructures projects and DI registrations. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant LoginAPI as LoginController
participant Auth as AuthenticationHelper
participant Store as RedisTokenStoreService
note over LoginAPI: POST /api/login
Client->>LoginAPI: LoginRequestDTO (Email)
LoginAPI->>Auth: JwtTokenGenerator(Email)
Auth-->>LoginAPI: JWT
LoginAPI->>Store: StoreTokenAsync(token, email)
Store-->>LoginAPI: ack
LoginAPI-->>Client: 200 BaseResponse{AuthToken}
note over LoginAPI: POST /api/revoke (Authorized)
Client->>LoginAPI: RevokeTokenRequestDTO (Token)
LoginAPI->>Store: RevokeTokenAsync(token)
Store-->>LoginAPI: ack
LoginAPI-->>Client: 200 BaseResponse{Success}
sequenceDiagram
autonumber
actor Client
participant API as PdfController
participant Gen as PdfDocumentGenerator
participant Tool as wkhtmltopdf
note over API: POST /pdf/GeneratePdfUsingHtml or /GeneratePdfUsingEjs
Client->>API: PdfGenerationRequestDTO
API->>API: validate request & config
API->>Gen: GeneratePdf(templatePath, metaData, outPath, isEjs, ejsJson)
alt isEjs
Gen->>Gen: ConvertEjsToHTML(ejsJson)
end
Gen->>Gen: Replace placeholders -> tmp HTML
Gen->>Tool: ConvertHtmlToPdf(tmpHtml, outputPdf)
Tool-->>Gen: PDF file
Gen-->>API: success
API-->>Client: 200 BaseResponse{Base64 PDF}
note right of API: finally -> optional cleanup of temp files
sequenceDiagram
autonumber
actor Client
participant API as PresentationController
participant PGen as PptxGenerator
participant LLM as LlmSlideExtractorService
participant OpenAI as OpenAI API
participant HTML as HtmlGeneratorService
participant PPTX as PptxService
Client->>API: PresentationRequest / PresentationWithImagesRequest
API->>PGen: Generate...(text, images?)
PGen->>LLM: ExtractSlidesAsync(text, images?)
LLM->>OpenAI: POST /v1/chat/completions (OPENAI_API_KEY, OPENAI_MODEL)
OpenAI-->>LLM: JSON slide data
LLM-->>PGen: SlideExtractionResult
PGen->>HTML: GenerateHtml(slides, imageMap?)
HTML-->>PGen: html
PGen->>PPTX: GeneratePptxFromHtmlString(html, file)
PPTX-->>PGen: filePath
PGen-->>API: path.pptx
API-->>Client: FileResult (pptx)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learningsThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 33
🧹 Nitpick comments (80)
docs/site/10.0.2/api/OsmoDoc.API.Helpers.AuthenticationHelper.html (6)
8-11: Replace placeholder site title/metadata with brand-accurate values."Some Documentation" appears in title/meta. Use the correct product/site name (e.g., "OsmoDoc Documentation") for consistency and SEO.
73-76: Add class summary/conceptual docs.These sections are empty. Add XML docs on AuthenticationHelper so DocFX renders meaningful summaries.
113-152: Document method properly and align parameter casing.
- Summary, parameter description, and returns are empty. Add XML docs for JwtTokenGenerator, its parameter, and return value.
- Parameter name "LoginEmail" is PascalCase. Prefer camelCase (loginEmail) per C# conventions. If named-args stability matters, keep the name but still add XML docs; otherwise consider renaming.
Is JwtTokenGenerator part of the intended public API surface, or should it be internal?
107-111: Consider making the helper class static or moving to a service interface.AuthenticationHelper is a non-static class with static methods. Either mark the class static or refactor to an IAuthenticationTokenService with DI and hide helper details from docs if not public-facing.
Based on learnings
37-39: Provide accessible alt text for the brand logo.
should have meaningful alt text (e.g., "OsmoDoc") or aria-hidden="true" if purely decorative.
54-56: Fix empty breadcrumb entry.Empty
is rendered. Ensure DocFX breadcrumb shows the actual trail or omit the element to avoid confusing screen readers. .gitignore (2)
356-360: Anchor ignore patterns to repo root + fix typo.Unanchored Temp/ and Generated/ will match at any depth and could hide legitimate nested folders. Also, “Additonal” is misspelled.
Apply:
-# Docker mount volumes -Temp/ +# Docker mount volumes (root-only) +/Temp/ -# Additonal Folders -Generated/ +# Additional folders (generated artifacts, root-only) +/Generated/
356-360: Mirror these ignores in .dockerignore (build context hygiene).To reduce build context size and accidental inclusion in images, add the same entries to .dockerignore.
Do you want me to create a .dockerignore patch mirroring these rules?
README.md (4)
5-23: Add “Presentation (PPTX) Generation” to Key Features.PR adds PPTX, but README lacks it. Add a concise section noting text-only and text+images flows.
## Key Features @@ ### PDF Document Generation @@ ### EJS to PDF Support @@ * JSON string data is passed to the EJS template at runtime. + +### Presentation (PPTX) Generation + +* Generate PowerPoint presentations from structured text. +* Supports both text-only and text+images generation flows. +* Built using OpenXML SDK for compatibility with Microsoft Office.Based on learnings
32-37: Clarify Node/EJS are only needed when using EJS templates.Currently reads as mandatory. Mark as conditional to avoid unnecessary installs for plain-HTML users.
- * Install [Node.js](https://nodejs.org/) and npm. - * Install EJS globally via npm: + * If you plan to use EJS templates: install [Node.js](https://nodejs.org/) and npm. + * Then install EJS globally via npm: @@ -sudo apt-get update && sudo apt-get install -y --no-install-recommends \ - wkhtmltopdf \ - nodejs \ - npm +sudo apt-get update && sudo apt-get install -y --no-install-recommends wkhtmltopdf +## Only if you plan to use EJS templates: +sudo apt-get install -y --no-install-recommends nodejs npm @@ -npm install -g --only=prod ejs +## Only if you plan to use EJS templates: +npm install -g --only=prod ejsAlso applies to: 41-50
87-89: Document presentation endpoints in API Access (or link Swagger section).Add the two new endpoints (text-only and text+images) or a note pointing to Swagger paths so users can discover them quickly.
116-120: Add EJS project link to Citations (used in README).Since EJS is referenced and optionally required, include its homepage for completeness.
* [OpenXML SDK](https://github.com/dotnet/Open-XML-SDK) * [wkhtmltopdf](https://wkhtmltopdf.org/) +* [EJS](https://ejs.co/)docs/site/10.0.2/api/OsmoDoc.API.Models.RevokeTokenRequestDTO.html (5)
8-11: Replace placeholder site title with product brandingThe page title/meta still say “Some Documentation”. Set the DocFX site/app title to “OsmoDoc Documentation” (or your preferred branding) in configuration/template so generated pages inherit it.
Example (docfx.json):
{ "build": { "globalMetadata": { "_appTitle": "OsmoDoc Documentation", "title": "OsmoDoc Documentation" } } }
38-39: Add descriptive alt text for logo (a11y)Logo image alt is empty. Provide meaningful alt (or role="img" with aria-label) for accessibility.
Example (template):
<img id="logo" class="svg" src="{{_rel}}logo.svg" alt="OsmoDoc" />
43-46: Add aria-label to search input (a11y)Search input lacks an accessible name. Add aria-label or associate a visible label.
Example:
<input type="text" class="form-control" id="search-query" placeholder="Search" autocomplete="off" aria-label="Search documentation">
74-76: Add XML documentation to surface summaries/descriptionsClass and property summaries are empty. Add XML docs in the source model so DocFX generates meaningful text and property descriptions.
Example (OsmoDoc.API.Models.RevokeTokenRequestDTO.cs):
namespace OsmoDoc.API.Models { /// <summary> /// Request payload to revoke an existing JWT/session token. /// Use this to invalidate a token via the revocation endpoint. /// </summary> public class RevokeTokenRequestDTO { /// <summary> /// The token string to revoke (exact JWT as issued). Do not log this value. /// </summary> [Required(ErrorMessage = "Token is required")] public string Token { get; set; } } }Additionally consider adding remarks on security hygiene (avoid logging tokens).
Also applies to: 115-116, 132-134
119-121: Consider localization for validation messagesHard-coded error message in [Required] may hinder i18n. Centralize messages or use resource files if localization is planned.
Example:
[Required(ErrorMessageResourceType = typeof(Resources.Validation), ErrorMessageResourceName = "TokenRequired")] public string Token { get; set; }docs/site/styles/docfx.css (1)
1037-1041: Good addition: print page margins; adjust to satisfy linterThe @page margin improves printed layout. Biome flags a shorthand “margin” here; expand to side-specific margins (or add a biome-ignore) to avoid CI failure.
Apply this diff:
@media print { @page { - margin: .4in; + margin-top: .4in; + margin-right: .4in; + margin-bottom: .4in; + margin-left: .4in; } }Optionally, co-locate this with the existing print styles at lines 959-974 for cohesion.
.vscode/launch.json (3)
11-11: Set working directory to the web project folder.Prevents contentRoot/relative-path issues (appsettings, static files, Views).
- "cwd": "${workspaceFolder}", + "cwd": "${workspaceFolder}/OsmoDoc.API",
20-22: Verify Views mapping (path likely under OsmoDoc.API).If Views reside in OsmoDoc.API/Views, adjust mapping; or remove if not needed locally.
- "sourceFileMap": { - "/Views": "${workspaceFolder}/Views" - } + "sourceFileMap": { + "/Views": "${workspaceFolder}/OsmoDoc.API/Views" + }
8-11: Avoid hardcoding Debug/net8.0 in program path (make config resilient).Option A (prefer): launch via dotnet run for portability.
- "preLaunchTask": "build", - "program": "${workspaceFolder}/OsmoDoc.API/bin/Debug/net8.0/OsmoDoc.API.dll", - "args": [], - "cwd": "${workspaceFolder}", + "preLaunchTask": "build", + "program": "dotnet", + "args": [ + "run", + "--project", + "${workspaceFolder}/OsmoDoc.API/OsmoDoc.API.csproj" + ], + "cwd": "${workspaceFolder}/OsmoDoc.API",Option B: keep dll but parameterize configuration.
- "program": "${workspaceFolder}/OsmoDoc.API/bin/Debug/net8.0/OsmoDoc.API.dll", + "program": "${workspaceFolder}/OsmoDoc.API/bin/${config:dotnet.defaultConfiguration:Debug}/net8.0/OsmoDoc.API.dll",docs/site/10.0.2/api/OsmoDoc.API.Helpers.Base64StringHelper.html (3)
121-152: Add XML docs to populate parameter/return descriptions.Tables are empty. Add
, , and comments in Base64StringHelper.cs; re-run DocFX to generate richer docs.
Example:
/// <summary>Converts a file at the given path to a Base64 string.</summary> /// <param name="filePath">Absolute or relative path to the source file.</param> /// <returns>Base64 representation of the file contents.</returns> public static Task<string> ConvertFileToBase64String(string filePath) { ... } /// <summary>Saves a Base64 string to a file at the given path.</summary> /// <param name="base64String">Input Base64 content (no data: URI prefix).</param> /// <param name="filePath">Target output path (created/overwritten).</param> /// <param name="configuration">App configuration for I/O settings if applicable.</param> /// <returns>A task that completes when write finishes.</returns> public static Task SaveBase64StringToFilePath(string base64String, string filePath, IConfiguration configuration) { ... }Also applies to: 162-202
38-39: Improve accessibility: add descriptive alt text to the logo.The brand image has empty alt. Provide meaningful alt text for screen readers.
Apply at generation source; if patching HTML, minimal diff:
-<img id="logo" class="svg" src="../../logo.svg" alt=""> +<img id="logo" class="svg" src="../../logo.svg" alt="OsmoDoc">
1-236: Confirm policy on committing generated DocFX site artifacts.docs/site/10.0.2 appears to be build output. If the repo prefers CI‑published docs (e.g., gh-pages or Pages from /docs), exclude build artifacts from PRs to reduce noise. If versioned static docs are intentionally tracked, ignore this.
If desired, I can propose a lightweight GH Actions workflow to build and publish DocFX to Pages and .gitignore updates for local builds.
docs/site/styles/docfx.js (2)
167-170: Add try-catch around Worker creation and consider user feedback.While the
onerrorhandler logs worker failures, the Worker constructor itself should be wrapped in try-catch to handle cases where:
- The worker file is missing or blocked
- The browser blocks Worker creation due to CSP or security policies
- Network issues prevent loading the worker script
Additionally, consider providing user-visible feedback when search fails, rather than silently logging to console.
Apply this diff to add try-catch and improve error handling:
function webWorkerSearch() { var indexReady = $.Deferred(); - var worker = new Worker(relHref + 'styles/search-worker.min.js'); - worker.onerror = function (oEvent) { - console.error('Error occurred at search-worker. message: ' + oEvent.message); - } + var worker; + try { + worker = new Worker(relHref + 'styles/search-worker.min.js'); + worker.onerror = function (oEvent) { + console.error('Error occurred at search-worker. message: ' + oEvent.message); + // Consider showing user-visible error: $('#search-results>.sr-items').html('<p>Search is temporarily unavailable</p>'); + } + } catch (e) { + console.error('Failed to initialize search worker:', e); + // Consider showing user-visible error: $('#search-results>.sr-items').html('<p>Search is not available</p>'); + return; + }
131-134: Confirm Web Worker support & fallback strategy
Dedicated Web Workers have ~99–100% coverage in modern browsers [MDN], but can be blocked in very old browsers, JS-disabled contexts, restrictive CSPs (worker-src/script-src), sandboxed iframes, or privacy extensions. If these edge cases are out of scope, the removal is acceptable; otherwise, reintroduce a local-search fallback for unsupported environments..env.example (1)
7-10: Consider consolidating Redis configuration.REDIS_URL (line 7) duplicates the information from REDIS_HOST and REDIS_PORT (lines 9-10). Having both composite and separate values increases maintenance burden and risk of inconsistency.
If both formats are needed for different components, consider documenting why both are required. Otherwise, use only one format consistently.
OsmoDoc.API/DotEnv.cs (1)
26-46: Consider supporting comment lines in .env files.The Load method currently processes all lines containing '=' but doesn't skip comment lines (typically starting with '#'). While not breaking for the current use case, supporting comments would improve maintainability of .env files.
Apply this diff to skip comment lines:
foreach (string line in File.ReadAllLines(filePath)) { + string trimmedLine = line.Trim(); + // Skip empty lines and comments + if (string.IsNullOrWhiteSpace(trimmedLine) || trimmedLine.StartsWith("#")) + { + continue; + } + // Check if the line contains '=' - int equalsIndex = line.IndexOf('='); + int equalsIndex = trimmedLine.IndexOf('='); if (equalsIndex == -1) { continue; // Skip lines without '=' } - string key = line.Substring(0, equalsIndex).Trim(); - string value = line.Substring(equalsIndex + 1).Trim(); + string key = trimmedLine.Substring(0, equalsIndex).Trim(); + string value = trimmedLine.Substring(equalsIndex + 1).Trim();OsmoDoc.API/Helpers/AuthenticationHelper.cs (1)
11-27: Consider validating the LoginEmail parameter.The method generates a JWT token using the provided email but doesn't validate it. While the non-expiry token design with Redis validation is intentional (based on learnings), the email parameter could be null, empty, or malformed.
Apply this diff to add parameter validation:
public static string JwtTokenGenerator(string LoginEmail) { + if (string.IsNullOrWhiteSpace(LoginEmail)) + { + throw new ArgumentException("Email cannot be null or empty", nameof(LoginEmail)); + } + SymmetricSecurityKey secretKey = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(OsmoDoc/OsmoDoc.csproj (1)
23-29: Bump outdated NuGet package versions
- DocumentFormat.OpenXml from 3.0.2 to 3.3.0
- Newtonsoft.Json from 13.0.3 to 13.0.4
- StackExchange.Redis from 2.8.41 to 2.9.25
- HtmlAgilityPack from 1.11.46 to 1.12.4
- Microsoft.AspNetCore.OpenApi from 8.0.19 to 9.0.9
No security advisories affect the current versions. Microsoft.Extensions.Http (9.0.9) and Microsoft.Extensions.Logging.Abstractions (9.0.9) are already at or above the latest stable.
OsmoDoc.API/Models/RevokeTokenRequestDTO.cs (1)
1-9: LGTM!The DTO is correctly implemented with appropriate validation. The
[Required]attribute ensures the Token property is validated by ASP.NET Core's model validation system, which aligns with the project's approach to validation.Consider adding XML documentation comments to describe the class and property purpose:
+/// <summary> +/// Request DTO for revoking authentication tokens. +/// </summary> public class RevokeTokenRequestDTO { + /// <summary> + /// The authentication token to be revoked. + /// </summary> [Required(ErrorMessage = "Token is required")] public string Token { get; set; } = string.Empty; }OsmoDoc.API/Models/BaseResponse.cs (1)
12-20: LGTM!The BaseResponse class design is solid with a clear constructor pattern and nullable properties for flexible response construction. The nullable properties allow for optional fields to be omitted when not needed.
Consider adding XML documentation comments to improve API discoverability:
+/// <summary> +/// Base response model for API endpoints. +/// </summary> public class BaseResponse { + /// <summary> + /// Initializes a new instance of BaseResponse with the specified status. + /// </summary> + /// <param name="status">The response status.</param> public BaseResponse(ResponseStatus status) => this.Status = status; + + /// <summary> + /// Gets or sets the response status. + /// </summary> public ResponseStatus? Status { get; set; } + + /// <summary> + /// Gets or sets the Base64-encoded content (e.g., for file downloads). + /// </summary> public string? Base64 { get; set; } // ... and so on for other properties }docs/guides/usage_guide.md (4)
12-14: Fix nested list indentation under wkhtmltopdf prerequisitesIndent by 2 spaces for nested bullets to satisfy MD007.
- * **Windows:** Download and install `wkhtmltopdf` from their official website. You'll need to set the `OsmoDocPdfConfig.WkhtmltopdfPath` property to the executable's location. - * **Linux:** Install `wkhtmltopdf` via your distribution's package manager (e.g., `sudo apt-get install wkhtmltopdf` for Debian/Ubuntu). No explicit path setting is needed if it's in your system's PATH. + * **Windows:** Download and install `wkhtmltopdf` from their official website. You'll need to set the `OsmoDocPdfConfig.WkhtmltopdfPath` property to the executable's location. + * **Linux:** Install `wkhtmltopdf` via your distribution's package manager (e.g., `sudo apt-get install wkhtmltopdf` for Debian/Ubuntu). No explicit path setting is needed if it's in your system's PATH.
60-71: De-duplicate “Explanation” headingsRename the second “Explanation” to avoid MD024 duplicate headings and improve skimmability.
-### Explanation +### HTML to PDF — Explanation-### Explanation +### EJS to PDF — ExplanationAlso applies to: 97-107
267-267: Normalize horizontal ruleTrim trailing space to comply with MD035.
---- +---
270-271: Use descriptive link textReplace “here” with descriptive text to satisfy MD059.
-- Templates used in the above examples are available [here](docs/templates). +- Templates used in the above examples are available in the [templates directory](docs/templates).OsmoDoc/Pptx/Services/LlmSlideExtractorService.cs (3)
37-46: Model default and configurabilityDefaulting to “gpt-5-mini” is likely invalid. Prefer a configurable default (e.g., via IConfiguration) and validate model compatibility (vision vs. non‑vision) when images are present.
Please confirm the intended default model and whether vision models are guaranteed in deployments.
69-75: Guard against large image payloadsReading entire files into memory can spike RAM. Add a per‑image size cap (e.g., 5–10 MB) and short‑circuit with a clear error when exceeded.
160-168: Defensive JSON accesschoices[0].message.content may be missing on edge cases. Prefer TryGetProperty checks with bounds guard to avoid KeyNotFound/IndexOutOfRange.
OsmoDoc/Pptx/Services/PptxService.cs (3)
64-64: Avoid orphaned slide partsClearing SlideIdList leaves template slide parts in the package unreferenced, increasing file size. Consider removing the original SlidePart instances or starting from a clean clone of the template’s first slide/layout.
307-321: Bullets formatting likely missingSetting only BulletFont doesn’t emit bullets. Add bullet markers (e.g., A.BuChar or auto‑numbering) in ParagraphProperties so list items render as bullets.
132-240: Deduplicate HTML→PPTX flowsGeneratePptx and GeneratePptxFromHtmlString duplicate logic. Extract a common private method to reduce drift and ease maintenance.
OsmoDoc/Pdf/Models/ContentMetaData.cs (1)
3-7: Consider adding XML documentation comments.The class implementation is clean and appropriate for a DTO. However, adding XML documentation comments would improve code maintainability and auto-generated API documentation.
Apply this diff to add XML documentation:
+/// <summary> +/// Represents metadata for content placeholders in PDF documents. +/// </summary> public class ContentMetaData { + /// <summary> + /// Gets or sets the placeholder identifier. + /// </summary> public string Placeholder { get; set; } = string.Empty; + /// <summary> + /// Gets or sets the content to replace the placeholder. + /// </summary> public string Content { get; set; } = string.Empty; }OsmoDoc.API/Helpers/AutoMappingProfile.cs (1)
1-3: Remove unused using directives.The constructor is empty (which is acceptable based on learnings), but the imports for
OsmoDoc.Word.ModelsandOsmoDoc.API.Modelson Lines 2-3 are not being used.Apply this diff to remove unused imports:
using AutoMapper; -using OsmoDoc.Word.Models; -using OsmoDoc.API.Models; namespace OsmoDoc.API.Helpers;OsmoDoc/Pdf/Models/DocumentData.cs (2)
1-3: Remove unused using directives.The
SystemandSystem.Textimports are not used in this file. OnlySystem.Collections.Genericis needed forList<T>.Apply this diff to remove unused imports:
-using System; using System.Collections.Generic; -using System.Text; namespace OsmoDoc.Pdf.Models;
7-10: Consider consistency improvements for maintainability.Two optional improvements for consistency with
OsmoDoc.Word.Models.DocumentData:
- Use
new()instead ofnew List<ContentMetaData>()for conciseness (modern C# syntax)- Add XML documentation comments to match the Word version
Apply this diff for consistency:
+/// <summary> +/// Represents the data structure for PDF document generation. +/// </summary> public class DocumentData { + /// <summary> + /// Gets or sets the list of content placeholders in the document. + /// </summary> - public List<ContentMetaData> Placeholders { get; set; } = new List<ContentMetaData>(); + public List<ContentMetaData> Placeholders { get; set; } = new(); }OsmoDoc.API/Helpers/CommonMethodsHelper.cs (1)
28-32: Consider validating the fileExtension parameter.The method doesn't validate the
fileExtensionparameter, which could lead to issues if null or malicious input is provided (e.g., path traversal characters).Apply this diff to add basic validation:
public static string GenerateRandomFileName(string fileExtension) { + if (string.IsNullOrWhiteSpace(fileExtension)) + { + throw new ArgumentException("File extension cannot be null or empty.", nameof(fileExtension)); + } + string randomFileName = Path.GetRandomFileName().Replace(".", string.Empty); return $"{randomFileName}-{Guid.NewGuid()}.{fileExtension}"; }OsmoDoc/Word/Models/DocumentData.cs (1)
1-3: Remove unused using directive.
System.Textappears to be unused in this file.Apply this diff:
using System; using System.Collections.Generic; -using System.Text;Dockerfile (1)
5-7: Consider making the environment configurable.The
ASPNETCORE_ENVIRONMENTis hardcoded toProductionin the build stage. While this is appropriate for production builds, consider whether this should be configurable via build arguments for different deployment scenarios (staging, QA, etc.).If flexibility is needed, apply this diff:
+ARG ASPNETCORE_ENVIRONMENT=Production # Build configuration variable. Debug for development, Release for production ENV BUILD_CONFIGURATION=Release -ENV ASPNETCORE_ENVIRONMENT=Production +ENV ASPNETCORE_ENVIRONMENT=${ASPNETCORE_ENVIRONMENT}This allows overriding the environment during build:
docker build --build-arg ASPNETCORE_ENVIRONMENT=Staging .docs/site/10.0.2/api/OsmoDoc.API.Models.PdfGenerationRequestDTO.html (1)
73-183: Enrich DTO docs (summaries, examples, cross-field guidance).
- Add XML-doc summaries on the source DTO to populate Summary/Conceptual sections and include a sample JSON payload.
- Clarify interplay: when SerializedEjsDataJson is provided, whether DocumentData is optional/ignored.
- Consider updating site title/metadata from “Some Documentation” to “OsmoDoc” globally (DocFX config).
docs/site/10.0.2/api/OsmoDoc.Word.Models.DocumentData.html (1)
12-12: Update description to include Images.DocumentData now exposes Images, but the description still mentions only placeholders and tables. Please update the summary/meta description accordingly.
Also applies to: 73-75
OsmoDoc.API/Models/WordGenerationRequestDTO.cs (2)
11-13: [Required] + initializer defeats validation; pick one.If DocumentData can be empty (preferred in OsmoDoc), drop [Required]. If it must be supplied, remove the initializer (and consider [ValidateComplexType]).
Option A (allow empty; consistent with past learnings):
- [Required(ErrorMessage = "Data to be modified in Word file is required")] - public WordDocumentDataRequestDTO DocumentData { get; set; } = new(); + public WordDocumentDataRequestDTO DocumentData { get; set; } = new();Option B (enforce presence and validate nested):
- [Required(ErrorMessage = "Data to be modified in Word file is required")] - public WordDocumentDataRequestDTO DocumentData { get; set; } = new(); + [Required(ErrorMessage = "Data to be modified in Word file is required")] + [ValidateComplexType] + public WordDocumentDataRequestDTO DocumentData { get; set; }Based on learnings
15-20: Align naming with domain model and unify initializers.
- Rename ImagesData -> Images for consistency with OsmoDoc.Word.Models.DocumentData.
- Use target‑typed new for brevity.
public class WordDocumentDataRequestDTO { - public List<ContentData> Placeholders { get; set; } = new List<ContentData>(); - public List<TableData> TablesData { get; set; } = new List<TableData>(); - public List<ImageData> ImagesData { get; set; } = new List<ImageData>(); + public List<ContentData> Placeholders { get; set; } = new(); + public List<TableData> TablesData { get; set; } = new(); + public List<ImageData> Images { get; set; } = new(); }docs/site/10.0.2/api/OsmoDoc.API.Helpers.CommonMethodsHelper.html (2)
111-177: Doc gaps: missing parameter/returns descriptions for both methods.Source XML comments appear incomplete, leading to empty parameter/returns sections. Please add XML doc summaries for params and return values so DocFX renders meaningful descriptions.
8-11: Branding/metadata polish.Page title/meta still say “Some Documentation”. Update to project naming (OsmoDoc) for consistency across docs.
OsmoDoc.API/Controllers/PdfController.cs (3)
95-126: Use structured logging; avoid logging stack trace as a separate message.Prefer
LogError(ex, "...")once instead of two calls (message + stack trace). Apply across both actions’ catch blocks.- response.Status = ResponseStatus.Error; - response.Message = ex.Message; - this._logger.LogError(ex.Message); - this._logger.LogError(ex.StackTrace); + response.Status = ResponseStatus.Error; + response.Message = ex.Message; + this._logger.LogError(ex, "Bad request while generating PDF");Repeat similarly for other catch blocks with context-specific messages.
- response.Status = ResponseStatus.Error; - response.Message = "Error converting base64 string to file"; - this._logger.LogError(ex.Message); - this._logger.LogError(ex.StackTrace); + response.Status = ResponseStatus.Error; + response.Message = "Error converting base64 string to file"; + this._logger.LogError(ex, "Base64 conversion failed");- response.Status = ResponseStatus.Error; - response.Message = "Unable to load file saved from base64 string"; - this._logger.LogError(ex.Message); - this._logger.LogError(ex.StackTrace); + response.Status = ResponseStatus.Error; + response.Message = "Unable to load file saved from base64 string"; + this._logger.LogError(ex, "File not found after base64 save");- response.Status = ResponseStatus.Error; - response.Message = ex.Message; - this._logger.LogError(ex.Message); - this._logger.LogError(ex.StackTrace); + response.Status = ResponseStatus.Error; + response.Message = ex.Message; + this._logger.LogError(ex, "Unhandled error while generating PDF");Also applies to: 227-258
185-197: Nit: comment refers to “html template” but this block handles EJS.Rename to avoid confusion.
41-76: Reduce duplication in path-building and cleanup.Path construction and cleanup blocks are near-identical across actions. Factor into private helpers (e.g., BuildInputPath, BuildOutputPath, TryDeleteIfExists) to simplify and reduce error surface.
Also applies to: 174-208, 127-154, 259-286
OsmoDoc/Word/WordDocumentGenerator.cs (3)
189-201: Replacing entire paragraph text can wipe run-level formatting.The current approach clears all Text nodes and sets one, losing formatting (bold/italic/runs). Consider a run-aware replacement that edits only the affected runs, preserving styling.
31-41: Compile and reuse regex for performance.Use a static readonly compiled Regex instead of calling Regex.IsMatch with a pattern string repeatedly.
- private const string PlaceholderPattern = @"{[a-zA-Z][a-zA-Z0-9_-]*}"; + private static readonly Regex PlaceholderRegex = new Regex(@"{[a-zA-Z][a-zA-Z0-9_-]*}", RegexOptions.Compiled);And replace usages:
- if (string.IsNullOrEmpty(paragraphText) || !Regex.IsMatch(paragraphText, PlaceholderPattern)) + if (string.IsNullOrEmpty(paragraphText) || !PlaceholderRegex.IsMatch(paragraphText))
87-95: Optional: also process headers/footers if placeholders can appear there.Currently only the main document body is handled. If templates place placeholders in headers/footers, include those parts.
Also applies to: 203-227
OsmoDoc/Word/Models/ImageData.cs (1)
21-38: Optional validation hardening.Consider adding lightweight checks:
- For Url: ensure Data starts with http/https.
- For LocalFile: reject absolute paths or enforce a safe base directory (or disallow at API level).
These guardrails complement generator-side defenses.docs/site/10.0.2/api/OsmoDoc.API.Controllers.LoginController.html (2)
8-11: Update doc site branding.Meta title still shows “Some Documentation”. Replace with the OsmoDoc documentation title for consistency.
669-676: Add XML doc comments in source to populate summaries.Class/method summaries are empty. Add XML comments in LoginController to improve generated docs (purpose, request/response schemas, error cases).
docs/site/10.0.2/api/OsmoDoc.Word.WordDocumentGenerator.html (1)
139-147: Document usage and behavior details.Add a short code example and list possible exceptions/requirements (e.g., template path existence, output path permissions). Note cleanup expectations (only intermediates; inputs/outputs managed by caller).
OsmoDoc.API/Controllers/LoginController.cs (1)
38-45: Log exceptions with context, not just messages/stack.Use LogError(ex, ...) to capture stack traces and structured info in one call.
- response.Status = ResponseStatus.Error; - response.Message = ex.Message; - this._logger.LogError(ex.Message); - this._logger.LogError(ex.StackTrace); + response.Status = ResponseStatus.Error; + response.Message = ex.Message; + this._logger.LogError(ex, "Error generating token in Login"); return this.StatusCode(StatusCodes.Status500InternalServerError, response);- response.Status = ResponseStatus.Error; - response.Message = ex.Message; - this._logger.LogError(ex.Message); - this._logger.LogError(ex.StackTrace); + response.Status = ResponseStatus.Error; + response.Message = ex.Message; + this._logger.LogError(ex, "Error revoking token in RevokeToken"); return this.StatusCode(StatusCodes.Status500InternalServerError, response);Also applies to: 62-69
OsmoDoc/Pptx/Services/HtmlGeneratorService.cs (1)
82-91: Harden image handling: filter null/empty IDs and rels; avoid nullable loop var.Ensure only valid image IDs/URLs are emitted.
- if (slide.Images != null && slide.Images.Count > 0 && imageMap != null) - { - foreach (string? imgId in slide.Images.Distinct()) - { - if (imageMap.TryGetValue(imgId, out string? rel)) - { - html.AppendLine($" <img data-image-id='{Escape(imgId)}' src='{Escape(rel)}' alt='{Escape(imgId)}' />"); - } - } - } + if (slide.Images != null && slide.Images.Count > 0 && imageMap != null) + { + foreach (string imgId in slide.Images.Where(id => !string.IsNullOrWhiteSpace(id)).Distinct()!) + { + if (imageMap.TryGetValue(imgId, out string? rel) && !string.IsNullOrWhiteSpace(rel)) + { + html.AppendLine($" <img data-image-id='{Escape(imgId)}' src='{Escape(rel)}' alt='{Escape(imgId)}' />"); + } + } + }- if (slide.Images != null && slide.Images.Count > 0 && imageMap != null) - { - foreach (string? imgId in slide.Images.Distinct()) - { - if (imageMap.TryGetValue(imgId, out string? rel)) - { - html.AppendLine($" <img data-image-id='{Escape(imgId)}' src='{Escape(rel)}' alt='{Escape(imgId)}' />"); - } - } - } + if (slide.Images != null && slide.Images.Count > 0 && imageMap != null) + { + foreach (string imgId in slide.Images.Where(id => !string.IsNullOrWhiteSpace(id)).Distinct()!) + { + if (imageMap.TryGetValue(imgId, out string? rel) && !string.IsNullOrWhiteSpace(rel)) + { + html.AppendLine($" <img data-image-id='{Escape(imgId)}' src='{Escape(rel)}' alt='{Escape(imgId)}' />"); + } + } + }Also applies to: 113-122
OsmoDoc/DependencyInjection/ServiceCollectionExtensions.cs (1)
17-21: Configure typed HttpClient and verify service lifetimes.
- Configure AddHttpClient() to set BaseAddress from IConfiguration and a sensible Timeout (e.g., 30 s).
- Ensure PptxService is thread-safe as a Singleton; if not, switch to Scoped or Transient.
OsmoDoc/Pptx/PptxGenerator.cs (2)
50-53: Gate debug.html persistence behind configPersisting full HTML (user content) on disk by default can leak PII and bloat storage. Guard with a config flag (default false).
Apply:
- string debugHtmlPath = Path.Combine(requestDir, "debug.html"); - await File.WriteAllTextAsync(debugHtmlPath, filledHtml, ct); + if (bool.TryParse(Environment.GetEnvironmentVariable("SAVE_DEBUG_HTML"), out var saveDebug) && saveDebug) + { + string debugHtmlPath = Path.Combine(requestDir, "debug.html"); + await File.WriteAllTextAsync(debugHtmlPath, filledHtml, ct); + }Also applies to: 103-106
73-95: Basic file-name sanitization is good; consider minimal extra validation
- Optionally enforce allowed image extensions/MIME and cap filename length to prevent path abuses and weird filenames.
- Consider deduplicating collisions if multiple files share the same name.
Do you want me to add a small validation helper (allowed mime/ext, unique naming)?
OsmoDoc.API/Controllers/WordController.cs (1)
114-121: Prefer structured logging with exception parameterUse LogError(Exception, message) to keep stack traces and enrich logs.
Example:
this._logger.LogError(ex, "Bad request");Also applies to: 122-129, 130-137, 138-145
OsmoDoc.API/Controllers/PresentationController.cs (2)
30-36: Stream the PPTX instead of buffering entire file into memoryUse PhysicalFile/FileStreamResult to reduce memory pressure for large presentations.
Apply:
- byte[] bytes = await System.IO.File.ReadAllBytesAsync(pptxPath, ct); - string fileName = Path.GetFileName(pptxPath); - return this.File(bytes, - "application/vnd.openxmlformats-officedocument.presentationml.presentation", - fileName); + string fileName = Path.GetFileName(pptxPath); + return PhysicalFile(pptxPath, + "application/vnd.openxmlformats-officedocument.presentationml.presentation", + fileName);And similarly for GenerateWithImages.
Also applies to: 54-57
20-36: Add optional cleanup of per-request folder after response completesTo prevent disk bloat, conditionally delete the request folder once the response is sent, reusing CONFIG:CLEAN_RESOURCES_GENERATED_BY_BASE64_STRINGS.
Apply:
- public PresentationController(PptxGenerator pptxGenerator) + private readonly IConfiguration _configuration; + public PresentationController(PptxGenerator pptxGenerator, IConfiguration configuration) { this._pptxGenerator = pptxGenerator; + this._configuration = configuration; } @@ - string pptxPath = await this._pptxGenerator.GeneratePresentationFromTextAsync(request.UserText, ct); + string pptxPath = await this._pptxGenerator.GeneratePresentationFromTextAsync(request.UserText, ct); + bool cleanup = this._configuration.GetValue("CONFIG:CLEAN_RESOURCES_GENERATED_BY_BASE64_STRINGS", false); + if (cleanup) + { + HttpContext.Response.OnCompleted(() => + { + try + { + string? dir = Path.GetDirectoryName(pptxPath); + if (System.IO.File.Exists(pptxPath)) System.IO.File.Delete(pptxPath); + if (!string.IsNullOrEmpty(dir) && Directory.Exists(dir)) Directory.Delete(dir, true); + } + catch { /* swallow cleanup failures */ } + return Task.CompletedTask; + }); + }Repeat the same pattern in GenerateWithImages.
Also applies to: 42-57
docs/site/10.0.2/api/OsmoDoc.Word.Models.TableData.html (1)
8-11: DocFX branding: replace “Some Documentation” with OsmoDocThe HTML title/SEO meta still says “Some Documentation”. Update DocFX/global metadata (e.g., docfx.json site title) so pages render OsmoDoc branding instead of patching generated HTML.
OsmoDoc.API/Program.cs (4)
73-86: Use Swagger “bearer” HTTP scheme instead of ApiKeySwashbuckle treats HTTP bearer specially (Authorize button UX, header formatting). Replace ApiKey “oauth2” with HTTP bearer.
Apply this diff:
- options.AddSecurityDefinition("oauth2", new OpenApiSecurityScheme - { - Name = "Authorization", - Description = "Standard Authorization header using the Bearer Scheme (\"bearer {token}\")", - In = ParameterLocation.Header, - Type = SecuritySchemeType.ApiKey - }); + options.AddSecurityDefinition("Bearer", new OpenApiSecurityScheme + { + Description = "JWT Authorization header using the Bearer scheme. Example: \"Bearer {token}\"", + Name = "Authorization", + In = ParameterLocation.Header, + Type = SecuritySchemeType.Http, + Scheme = "bearer", + BearerFormat = "JWT" + });
124-143: Fallback to token.RawData when Authorization header is absent; remove unused localUse JwtSecurityToken.RawData as a fallback and avoid false negatives. Also utilize the existing ‘token’ variable.
Apply this diff:
- JwtSecurityToken? token = context.SecurityToken as JwtSecurityToken; - string tokenString = string.Empty; + JwtSecurityToken? token = context.SecurityToken as JwtSecurityToken; + string tokenString = string.Empty; if (context.Request.Headers.TryGetValue("Authorization", out Microsoft.Extensions.Primitives.StringValues authHeader) && authHeader.ToString().StartsWith("Bearer ", StringComparison.OrdinalIgnoreCase)) { tokenString = authHeader.ToString().Substring("Bearer ".Length).Trim(); } + // Fallback to RawData if header is missing + if (string.IsNullOrWhiteSpace(tokenString) && token != null) + { + tokenString = token.RawData ?? string.Empty; + }
156-164: Ensure log directory exists; avoid hardcoded pathCreating the directory avoids startup/runtime failures if wwwroot/logs is missing and keeps paths environment‑aware.
Apply this diff:
-builder.Logging.AddSerilog(); -Log.Logger = new LoggerConfiguration() - .WriteTo.File( - path: "wwwroot/logs/log-.txt", +builder.Logging.AddSerilog(); +string logDir = Path.Combine(builder.Environment.WebRootPath ?? Path.Combine(AppContext.BaseDirectory, "wwwroot"), "logs"); +Directory.CreateDirectory(logDir); +Log.Logger = new LoggerConfiguration() + .WriteTo.File( + path: Path.Combine(logDir, "log-.txt"), outputTemplate: "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u3}] {Message:lj}{NewLine}{Exception}{NewLine}{NewLine}", rollingInterval: RollingInterval.Day, restrictedToMinimumLevel: LogEventLevel.Information ).CreateLogger();
146-153: Avoid mixing AddControllers and AddMvc; configure ApiBehavior on ControllersRegistering both is redundant. Configure InvalidModelStateResponseFactory on AddControllers to keep a single MVC registration.
Apply this diff to remove the duplicate registration:
-// Configure Error Response from Model Validations -builder.Services.AddMvc().ConfigureApiBehaviorOptions(options => -{ - options.InvalidModelStateResponseFactory = actionContext => - { - return ModelValidationBadRequest.ModelValidationErrorResponse(actionContext); - }; -});Then adjust the earlier AddControllers call (outside this hunk) like:
builder.Services.AddControllers(options => { options.Filters.Add(new ProducesAttribute("application/json")); }) .ConfigureApiBehaviorOptions(options => { options.InvalidModelStateResponseFactory = actionContext => ModelValidationBadRequest.ModelValidationErrorResponse(actionContext); }) .AddJsonOptions(options => { options.JsonSerializerOptions.DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull; options.JsonSerializerOptions.Converters.Add(new JsonStringEnumConverter()); });docs/site/10.0.2/api/OsmoDoc.API.DotEnv.html (2)
8-11: DocFX branding: replace “Some Documentation” with OsmoDocUpdate the site title/global metadata (docfx.json) so pages render the OsmoDoc brand consistently.
74-75: Add XML docs for DotEnv to populate API documentationEmpty summaries for class and methods degrade docs. Add XML doc comments in OsmoDoc.API/DotEnv.cs; DocFX will populate these sections.
Example:
/// <summary> /// Utilities to load environment variables from a .env file or a specific file path. /// </summary> public static class DotEnv { /// <summary> /// Loads a .env file by searching upward from the current directory. /// Throws FileNotFoundException if not found. /// </summary> /// <param name="fileName">The .env file name to search for. Defaults to ".env".</param> public static void LoadEnvFile(string fileName = ".env") { /* ... */ } /// <summary> /// Loads environment variables from the specified file path. /// Lines are parsed as KEY=VALUE, supporting quoted values. /// </summary> /// <param name="filePath">Absolute path to the env file.</param> public static void Load(string filePath) { /* ... */ } }Also applies to: 115-116, 140-145
| JWT_KEY=xxx | ||
| OPENAI_API_KEY=PLACEHOLDER_REPLACE_WITH_YOUR_OPENAI_API_KEY_BEFORE_USE | ||
| MAX_IMAGES=5 | ||
| OPENAI_MODEL=gpt-5-mini |
There was a problem hiding this comment.
🧩 Analysis chain
Invalid OpenAI model name.
The model name "gpt-5-mini" does not exist in OpenAI's API. As of my knowledge cutoff (March 2025), valid models include gpt-4o, gpt-4o-mini, gpt-4-turbo, gpt-3.5-turbo, etc. This will cause runtime errors when the PPTX generation service attempts to call the OpenAI API.
Please verify the correct model name by searching for the latest OpenAI models:
Apply this diff to use a valid model name:
-OPENAI_MODEL=gpt-5-mini
+OPENAI_MODEL=gpt-4o-mini🌐 Web query:
What are the current OpenAI chat completion model names available in 2025?
💡 Result:
Here are the main OpenAI chat-completion models available (2025) — concise list of current public models and families:
- GPT-5. [1]
- GPT-4.1 family: gpt-4.1, gpt-4.1-mini, gpt-4.1-nano. [2][3]
- GPT-4 / GPT-4-turbo (and GPT-4 variants such as 32k). [4]
- GPT-4o family: gpt-4o, gpt-4o-mini (and audio-preview variants). [5][6]
- GPT-3.5 family: gpt-3.5-turbo (and 16k/other variants). [6]
- “o‑series” models (OpenAI o1, o1-pro, o3, o3-pro, o3-mini, o4-mini, o3-mini variants). [4][6]
Sources:
[1] Axios / news on GPT-5 (Oct 2025).
[2] OpenAI / GPT-4.1 announcement and coverage.
[3] Reuters / reporting on GPT-4.1 (Apr 2025).
[4] OpenAI release pages for o‑series (o3, o4‑mini) and GPT-4 family.
[5] OpenAI blog: GPT‑4o mini announcement.
[6] OpenAI/API model list and community references showing chat-completions endpoint model names.
If you want, I can fetch and show the exact API model IDs and links to the official OpenAI docs for each item.
Invalid OpenAI model name.
“gpt-5-mini” isn’t a valid model. As of 2025, use “gpt-5” (or a supported mini variant such as “gpt-4.1-mini”).
-OPENAI_MODEL=gpt-5-mini
+OPENAI_MODEL=gpt-5📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| OPENAI_MODEL=gpt-5-mini | |
| OPENAI_MODEL=gpt-5 |
🤖 Prompt for AI Agents
In .env.example around line 3, the OPENAI_MODEL value is set to an invalid model
name ("gpt-5-mini"); replace it with a supported model name such as "gpt-5" (or
another valid mini variant like "gpt-4.1-mini") to ensure the environment file
uses a recognized OpenAI model identifier.
| "serverReadyAction": { | ||
| "action": "openExternally", | ||
| "pattern": "\\\\bNow listening on:\\\\s+(https?://\\\\S+)" | ||
| }, |
There was a problem hiding this comment.
Fix over‑escaped serverReadyAction regex (prevents auto-open).
Pattern is over-escaped; VS Code expects double backslashes in JSON. Use this:
- "serverReadyAction": {
- "action": "openExternally",
- "pattern": "\\\\bNow listening on:\\\\s+(https?://\\\\S+)"
- },
+ "serverReadyAction": {
+ "action": "openExternally",
+ "pattern": "\\bNow listening on:\\s+(https?://\\S+)"
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "serverReadyAction": { | |
| "action": "openExternally", | |
| "pattern": "\\\\bNow listening on:\\\\s+(https?://\\\\S+)" | |
| }, | |
| "serverReadyAction": { | |
| "action": "openExternally", | |
| "pattern": "\\bNow listening on:\\s+(https?://\\S+)" | |
| }, |
🤖 Prompt for AI Agents
.vscode/launch.json around lines 13-16: the serverReadyAction regex is
over-escaped and fails to match, preventing automatic opening; replace the
current over-escaped pattern with a correctly escaped JSON string that uses
double backslashes for the word-boundary, whitespace and non-space character
classes (i.e. use \\b for \b, \\s for \s, and \\S for \S), keep the capturing
group for the URL and the http/https alternation so the "Now listening on:
<url>" line is matched and VS Code can auto-open the server URL.
| If you find a bug in the source code, you can help us by [submitting an issue](https://github.com/OsmosysSoftware/osmodoc/issues/new) | ||
| to our GitHub Repository. Even better, you can submit a [pull request](https://github.com/OsmosysSoftware/osmodoc/pulls) with a fix. |
There was a problem hiding this comment.
Minor style/capitalization nits.
Lowercase “repository”; tighten wording.
-If you find a bug in the source code, you can help us by [submitting an issue](https://github.com/OsmosysSoftware/osmodoc/issues/new)
-to our GitHub Repository. Even better, you can submit a [pull request](https://github.com/OsmosysSoftware/osmodoc/pulls) with a fix.
+If you find a bug, please [submit an issue](https://github.com/OsmosysSoftware/osmodoc/issues/new)
+to our GitHub repository. Even better, submit a [pull request](https://github.com/OsmosysSoftware/osmodoc/pulls) with a fix.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| If you find a bug in the source code, you can help us by [submitting an issue](https://github.com/OsmosysSoftware/osmodoc/issues/new) | |
| to our GitHub Repository. Even better, you can submit a [pull request](https://github.com/OsmosysSoftware/osmodoc/pulls) with a fix. | |
| If you find a bug, please [submit an issue](https://github.com/OsmosysSoftware/osmodoc/issues/new) | |
| to our GitHub repository. Even better, submit a [pull request](https://github.com/OsmosysSoftware/osmodoc/pulls) with a fix. |
🤖 Prompt for AI Agents
In CONTRIBUTING.md around lines 26 to 27, the wording uses “Repository” and is
slightly loose; change “Repository” to lowercase “repository” and tighten the
sentence for concision — e.g., rephrase to “submit an issue to our GitHub
repository” and streamline the follow-up sentence to “or, even better, open a
pull request with a fix.” Ensure punctuation and capitalization follow the
project style.
| For us to address and fix a bug, we need to reproduce it. Thus when submitting a bug report, we will ask for a minimal reproduction scenario using a repository or [Gist](https://gist.github.com/). Providing a live, reproducible scenario helps us understand the issue better. Information to include: | ||
|
|
||
| - The version of the document-service you are using. | ||
| - The version of the osmodoc you are using. |
There was a problem hiding this comment.
Grammar: drop article and fix casing.
-- The version of the osmodoc you are using.
+- The version of OsmoDoc you are using.Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In CONTRIBUTING.md around line 37, the sentence "The version of the osmodoc you
are using." should drop the leading article and fix casing; replace it with
"Version of OsmoDoc you are using." to remove "The" and capitalize the project
name consistently.
| git push origin my-fix-branch | ||
| ``` | ||
| 7. Send a pull request to the `document-service:main`. | ||
| 7. Send a pull request to the `osmodoc:main`. |
There was a problem hiding this comment.
Fix garbled bullets and minor wording; current HTML/� chars won’t render correctly.
Replace with plain Markdown bullets and clarify target.
-7. Send a pull request to the `osmodoc:main`.
+7. Open a pull request against `osmodoc:main`.-- <strong style="color:black">�</strong> **If we suggest changes, then:**
- - � Make the required updates.
- - � Ensure that your changes do not break existing functionality or introduce new issues.
- - � Rebase your branch and force push to your GitHub repository. This will update your Pull Request.
+- **If we suggest changes, then:**
+ - Make the required updates.
+ - Ensure your changes do not break existing functionality or introduce issues.
+ - Rebase your branch and force-push to update your pull request.Also applies to: 68-71
🤖 Prompt for AI Agents
In CONTRIBUTING.md around line 66 (and similarly lines 68–71), the list item
contains garbled bullets/HTML characters; replace the offending line with a
plain Markdown bullet and clearer wording such as "Send a pull request to
osmodoc:main" (or "Create a pull request targeting the osmodoc/main branch") and
update the nearby bullet lines 68–71 to use standard Markdown hyphens or
asterisks and plain text only so they render correctly.
| Dictionary<string, string> textPlaceholders = documentData.Placeholders | ||
| .Where(content => content.ParentBody == ParentBody.None && content.ContentType == ContentType.Text) | ||
| .ToDictionary(content => "{" + content.Placeholder + "}", content => content.Content); | ||
|
|
||
| Dictionary<string, string> tableContentPlaceholders = documentData.Placeholders | ||
| .Where(content => content.ParentBody == ParentBody.Table && content.ContentType == ContentType.Text) | ||
| .ToDictionary(content => "{" + content.Placeholder + "}", content => content.Content); | ||
|
|
There was a problem hiding this comment.
Duplicate placeholders will throw due to ToDictionary.
ToDictionary throws on duplicate keys. Use last-write-wins to avoid runtime exceptions.
- Dictionary<string, string> textPlaceholders = documentData.Placeholders
- .Where(content => content.ParentBody == ParentBody.None && content.ContentType == ContentType.Text)
- .ToDictionary(content => "{" + content.Placeholder + "}", content => content.Content);
+ Dictionary<string, string> textPlaceholders = new();
+ foreach (var content in documentData.Placeholders.Where(c => c.ParentBody == ParentBody.None && c.ContentType == ContentType.Text))
+ {
+ textPlaceholders["{" + content.Placeholder + "}"] = content.Content;
+ }
- Dictionary<string, string> tableContentPlaceholders = documentData.Placeholders
- .Where(content => content.ParentBody == ParentBody.Table && content.ContentType == ContentType.Text)
- .ToDictionary(content => "{" + content.Placeholder + "}", content => content.Content);
+ Dictionary<string, string> tableContentPlaceholders = new();
+ foreach (var content in documentData.Placeholders.Where(c => c.ParentBody == ParentBody.Table && c.ContentType == ContentType.Text))
+ {
+ tableContentPlaceholders["{" + content.Placeholder + "}"] = content.Content;
+ }Also applies to: 83-86
🤖 Prompt for AI Agents
In OsmoDoc/Word/WordDocumentGenerator.cs around lines 79-86, the current
ToDictionary calls throw on duplicate placeholder keys; change each to group by
the placeholder key and build the dictionary using the group's last item (e.g.,
.Where(...).GroupBy(content => "{" + content.Placeholder + "}").ToDictionary(g
=> g.Key, g => g.Last().Content)) so duplicate placeholders resolve with
last-write-wins; apply this change to both the ParentBody.None and
ParentBody.Table dictionary constructions.
| string tempFilePath = IOPath.GetTempFileName(); | ||
|
|
||
| // Determine image extension | ||
| string extension = ".jpg"; // Default fallback | ||
|
|
||
| if (!string.IsNullOrEmpty(imageData.ImageExtension)) | ||
| { | ||
| extension = imageData.ImageExtension.StartsWith(".") | ||
| ? imageData.ImageExtension | ||
| : "." + imageData.ImageExtension; | ||
| } | ||
| else if (imageData.SourceType == ImageSourceType.LocalFile || imageData.SourceType == ImageSourceType.Url) | ||
| { | ||
| try | ||
| { | ||
| extension = System.IO.Path.GetExtension(imageData.Data); | ||
| if (string.IsNullOrEmpty(extension)) | ||
| { | ||
| extension = ".jpg"; // fallback if no extension in path/URL | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogWarning(ex, $"Failed to determine image extension from path: {imageData.Data}"); | ||
| extension = ".jpg"; // safe fallback on exception | ||
| } | ||
| } | ||
|
|
||
| // Define allowed image extensions | ||
| string[] allowedExtensions = new[] { ".jpg", ".jpeg", ".png", ".gif", ".bmp", ".tiff", ".svg" }; | ||
| if (!allowedExtensions.Contains(extension, StringComparer.OrdinalIgnoreCase)) | ||
| { | ||
| throw new ArgumentException($"Invalid image extension: {extension}"); | ||
| } | ||
|
|
||
| tempFilePath = IOPath.ChangeExtension(tempFilePath, extension); | ||
|
|
There was a problem hiding this comment.
Temp file leak: Path.GetTempFileName creates a file that isn’t deleted.
You call GetTempFileName then change the extension, leaving the original temp file behind. Generate a random path without creating a file.
- string tempFilePath = IOPath.GetTempFileName();
+ // Generate a unique temp file path without creating the file
+ string tempFilePath = IOPath.Combine(IOPath.GetTempPath(), IOPath.GetRandomFileName());
...
- tempFilePath = IOPath.ChangeExtension(tempFilePath, extension);
+ tempFilePath = IOPath.ChangeExtension(tempFilePath, extension);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| string tempFilePath = IOPath.GetTempFileName(); | |
| // Determine image extension | |
| string extension = ".jpg"; // Default fallback | |
| if (!string.IsNullOrEmpty(imageData.ImageExtension)) | |
| { | |
| extension = imageData.ImageExtension.StartsWith(".") | |
| ? imageData.ImageExtension | |
| : "." + imageData.ImageExtension; | |
| } | |
| else if (imageData.SourceType == ImageSourceType.LocalFile || imageData.SourceType == ImageSourceType.Url) | |
| { | |
| try | |
| { | |
| extension = System.IO.Path.GetExtension(imageData.Data); | |
| if (string.IsNullOrEmpty(extension)) | |
| { | |
| extension = ".jpg"; // fallback if no extension in path/URL | |
| } | |
| } | |
| catch (Exception ex) | |
| { | |
| _logger.LogWarning(ex, $"Failed to determine image extension from path: {imageData.Data}"); | |
| extension = ".jpg"; // safe fallback on exception | |
| } | |
| } | |
| // Define allowed image extensions | |
| string[] allowedExtensions = new[] { ".jpg", ".jpeg", ".png", ".gif", ".bmp", ".tiff", ".svg" }; | |
| if (!allowedExtensions.Contains(extension, StringComparer.OrdinalIgnoreCase)) | |
| { | |
| throw new ArgumentException($"Invalid image extension: {extension}"); | |
| } | |
| tempFilePath = IOPath.ChangeExtension(tempFilePath, extension); | |
| // Generate a unique temp file path without creating the file | |
| string tempFilePath = IOPath.Combine(IOPath.GetTempPath(), IOPath.GetRandomFileName()); | |
| // Determine image extension | |
| string extension = ".jpg"; // Default fallback | |
| if (!string.IsNullOrEmpty(imageData.ImageExtension)) | |
| { | |
| extension = imageData.ImageExtension.StartsWith(".") | |
| ? imageData.ImageExtension | |
| : "." + imageData.ImageExtension; | |
| } | |
| else if (imageData.SourceType == ImageSourceType.LocalFile || imageData.SourceType == ImageSourceType.Url) | |
| { | |
| try | |
| { | |
| extension = System.IO.Path.GetExtension(imageData.Data); | |
| if (string.IsNullOrEmpty(extension)) | |
| { | |
| extension = ".jpg"; // fallback if no extension in path/URL | |
| } | |
| } | |
| catch (Exception ex) | |
| { | |
| _logger.LogWarning(ex, $"Failed to determine image extension from path: {imageData.Data}"); | |
| extension = ".jpg"; // safe fallback on exception | |
| } | |
| } | |
| // Define allowed image extensions | |
| string[] allowedExtensions = new[] { ".jpg", ".jpeg", ".png", ".gif", ".bmp", ".tiff", ".svg" }; | |
| if (!allowedExtensions.Contains(extension, StringComparer.OrdinalIgnoreCase)) | |
| { | |
| throw new ArgumentException($"Invalid image extension: {extension}"); | |
| } | |
| tempFilePath = IOPath.ChangeExtension(tempFilePath, extension); |
🤖 Prompt for AI Agents
In OsmoDoc/Word/WordDocumentGenerator.cs around lines 412 to 448, you’re calling
Path.GetTempFileName which creates a real file and then changing its extension,
leaking the original temp file; instead generate a temp path without creating a
file by combining Path.GetTempPath() with Path.GetRandomFileName() (then change
the extension as you do now). Replace the GetTempFileName usage with a
non-creating random filename approach so no on-disk file is created, maintaining
the same extension logic and validations afterward.
| case ImageSourceType.Base64: | ||
| await File.WriteAllBytesAsync( | ||
| tempFilePath, | ||
| Convert.FromBase64String(imageData.Data)); | ||
| break; | ||
|
|
||
| case ImageSourceType.LocalFile: | ||
| string fullPath = IOPath.GetFullPath(imageData.Data); | ||
| if (!File.Exists(fullPath)) | ||
| { | ||
| throw new FileNotFoundException("Image file not found", fullPath); | ||
| } | ||
| File.Copy(fullPath, tempFilePath, true); | ||
| break; | ||
|
|
||
| case ImageSourceType.Url: | ||
| using (HttpClient httpClient = new HttpClient()) | ||
| { | ||
| byte[] bytes = await httpClient.GetByteArrayAsync(imageData.Data); | ||
| await File.WriteAllBytesAsync(tempFilePath, bytes); | ||
| } | ||
| break; | ||
|
|
||
| default: | ||
| throw new ArgumentOutOfRangeException(); | ||
| } |
There was a problem hiding this comment.
Security: SSRF via URL and arbitrary server file read via LocalFile. Add strict validation.
- Url: unbounded outbound fetch with no scheme/host/IP validation → SSRF risk.
- LocalFile: copies any server path provided by client → potential sensitive file exfiltration.
Mitigations:
- Allow only http/https; block private/loopback/link-local IPs; set timeouts; optionally cap download size.
- Restrict LocalFile to a whitelisted base directory (or disable in API).
Example hardened changes within this switch (minimal):
case ImageSourceType.Url:
- using (HttpClient httpClient = new HttpClient())
- {
- byte[] bytes = await httpClient.GetByteArrayAsync(imageData.Data);
- await File.WriteAllBytesAsync(tempFilePath, bytes);
- }
+ if (!Uri.TryCreate(imageData.Data, UriKind.Absolute, out var uri)
+ || (uri.Scheme != Uri.UriSchemeHttp && uri.Scheme != Uri.UriSchemeHttps))
+ {
+ throw new ArgumentException("Only HTTP/HTTPS URLs are allowed.");
+ }
+ if (IsPrivateNetwork(uri))
+ {
+ throw new ArgumentException("URL targets a private or loopback address.");
+ }
+ using (HttpClient httpClient = new HttpClient { Timeout = TimeSpan.FromSeconds(15) })
+ {
+ byte[] bytes = await httpClient.GetByteArrayAsync(uri);
+ await File.WriteAllBytesAsync(tempFilePath, bytes);
+ }
break;
case ImageSourceType.LocalFile:
- string fullPath = IOPath.GetFullPath(imageData.Data);
- if (!File.Exists(fullPath))
- {
- throw new FileNotFoundException("Image file not found", fullPath);
- }
- File.Copy(fullPath, tempFilePath, true);
+ // Prefer restricting to a known safe base directory. Example:
+ throw new NotSupportedException("LocalFile source type is disabled for security. Use Base64 or URL.");
break;Add this helper in the same class:
private static bool IsPrivateNetwork(Uri uri)
{
try
{
var addresses = System.Net.Dns.GetHostAddresses(uri.Host);
foreach (var ip in addresses)
{
if (System.Net.IPAddress.IsLoopback(ip)) return true;
if (ip.AddressFamily == System.Net.Sockets.AddressFamily.InterNetwork)
{
byte[] bytes = ip.GetAddressBytes();
// 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 169.254.0.0/16
if (bytes[0] == 10) return true;
if (bytes[0] == 172 && bytes[1] >= 16 && bytes[1] <= 31) return true;
if (bytes[0] == 192 && bytes[1] == 168) return true;
if (bytes[0] == 169 && bytes[1] == 254) return true;
}
}
}
catch { return true; }
return false;
}🤖 Prompt for AI Agents
In OsmoDoc/Word/WordDocumentGenerator.cs around lines 451-476, the switch
handling ImageSourceType.Url and ImageSourceType.LocalFile is vulnerable to SSRF
and arbitrary file read: for Url, ensure the URI uses only http/https, parse and
DNS-resolve the host and block loopback, link-local and private IP ranges (treat
resolution failures as disallowed), set an HttpClient timeout, use
response-streaming (ResponseHeadersRead) and enforce a maximum byte download
limit before writing to tempFilePath, and validate redirects do not lead to
disallowed hosts; for LocalFile, restrict access to a configured whitelist base
directory by calling Path.GetFullPath on the requested path and verifying it
starts with the canonical base directory path (deny traversal), check file
existence and permissions, and reject any paths outside the whitelist; replace
current unbounded File.Copy and GetByteArrayAsync calls with these validated,
size-limited streaming operations and throw clear exceptions when validation
fails.
| <!DOCTYPE html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8"> | ||
| <title>PPTX Template</title> | ||
| </head> | ||
| <body> | ||
|
|
||
| <div class="slide" data-template-slide="1"> | ||
| <h1>Welcome to OQSHA</h1> | ||
| <h3>Author: Jatin Gupta</h3> | ||
| </div> | ||
|
|
||
| <div class="slide" data-template-slide="2"> | ||
| <h1>Why OQSHA?</h1> | ||
| <ul> | ||
| <li>Streamlined workflow</li> | ||
| <li>Real-time notifications</li> | ||
| <li>Cloud & On-Premise</li> | ||
| </ul> | ||
| </div> | ||
|
|
||
| <div class="slide" data-template-slide="3"> | ||
| <h1>Thank You!</h1> | ||
| <h3>Contact support@oqsha.com for onboarding</h3> | ||
| </div> | ||
|
|
||
| </body> | ||
| </html> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Clarify template naming and branding inconsistencies.
This new file has several concerns:
- Misleading filename: Named "old_template.html" but is a new addition to the repository.
- Branding inconsistency: Contains "OQSHA" branding throughout (lines 10, 15, 19, 25) instead of "OsmoDoc".
- Hardcoded personal information: Author name "Jatin Gupta" (line 11) and email "support@oqsha.com" (line 25).
Please verify:
- Is this intended as an example template or actual production template?
- Should branding be updated to OsmoDoc or is OQSHA branding intentional?
- Should this be renamed to better reflect its purpose?
🏁 Script executed:
#!/bin/bash
set -e
echo "1) List all HTML files in Templates directory:"
fd --type f --extension html Templates
echo -e "\n2) References to 'old_template.html':"
rg -n "old_template.html" || echo "No references found"
echo -e "\n3) Occurrences of 'OQSHA' in repo:"
rg -n "OQSHA" || echo "No occurrences found"
echo -e "\n4) Occurrences of 'OsmoDoc' in repo:"
rg -n "OsmoDoc" || echo "No occurrences found"Length of output: 184104
🏁 Script executed:
#!/bin/bash
set -e
echo "Checking for author name in existing template:"
rg -n "Jatin Gupta" Templates/template.html || echo "No author name found in template.html"
echo -e "\nChecking for support email in existing template:"
rg -n "support@oqsha.com" Templates/template.html || echo "No support email found in template.html"Length of output: 195
🏁 Script executed:
#!/bin/bash
set -e
echo "Searching for author name in old_template.html:"
rg -n "Jatin Gupta" Templates/old_template.html || echo "No author name found in old_template.html"
echo -e "\nSearching for support email in old_template.html:"
rg -n "support@oqsha.com" Templates/old_template.html || echo "No support email found in old_template.html"Length of output: 199
Rename or remove Templates/old_template.html and update its contents.
old_template.htmlisn’t referenced by any code; if it’s not needed, delete it or rename to indicate it’s an example.- Change all “OQSHA” occurrences to “OsmoDoc” (e.g. in
<h1>headings). - Remove hard-coded author (“Jatin Gupta”) and support email (“support@oqsha.com”).
🤖 Prompt for AI Agents
Templates/old_template.html lines 1-29: this file appears unused—either delete
it or rename it to indicate it’s an example (e.g., old_template.example.html)
and update its content: replace every occurrence of “OQSHA” with “OsmoDoc” in
headings, remove the hard-coded author line and the hard-coded support email
line (either omit them or replace with placeholder tokens like {{author}} and
{{support_email}}), and ensure any remaining references or data attributes
remain consistent with the rest of the project; commit the file deletion/rename
or the cleaned-up example file as appropriate.
| <!-- Slide 4: Another Content Slide (Template Slide 2 again) --> | ||
| <div class="slide" data-template-slide="3"> | ||
| <h1>Our Mobile App</h1> | ||
| <p>Manage safety permits on the go using our dedicated mobile app.</p> | ||
| <!-- This will insert image into the only content area (body) if you want to handle images differently --> | ||
| <img src="images/mobile-app.png" placeholder="Picture 1" /> | ||
| </div> |
There was a problem hiding this comment.
Correct the duplicate data-template-slide value.
Slide 4 (line 38) uses data-template-slide="3", but this value is already used by Slide 3 (line 26). This duplication may cause confusion or incorrect slide mapping during PPTX generation.
Apply this diff to fix the duplicate:
- <div class="slide" data-template-slide="3">
+ <div class="slide" data-template-slide="2">
<h1>Our Mobile App</h1>
<p>Manage safety permits on the go using our dedicated mobile app.</p>
<!-- This will insert image into the only content area (body) if you want to handle images differently -->
<img src="images/mobile-app.png" placeholder="Picture 1" />
</div>Note: Verify the intended slide template mapping based on your PPTX template structure. If template slide 3 supports multiple instances, this may be intentional, otherwise adjust to the correct template slide number.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <!-- Slide 4: Another Content Slide (Template Slide 2 again) --> | |
| <div class="slide" data-template-slide="3"> | |
| <h1>Our Mobile App</h1> | |
| <p>Manage safety permits on the go using our dedicated mobile app.</p> | |
| <!-- This will insert image into the only content area (body) if you want to handle images differently --> | |
| <img src="images/mobile-app.png" placeholder="Picture 1" /> | |
| </div> | |
| <!-- Slide 4: Another Content Slide (Template Slide 2 again) --> | |
| <div class="slide" data-template-slide="2"> | |
| <h1>Our Mobile App</h1> | |
| <p>Manage safety permits on the go using our dedicated mobile app.</p> | |
| <!-- This will insert image into the only content area (body) if you want to handle images differently --> | |
| <img src="images/mobile-app.png" placeholder="Picture 1" /> | |
| </div> |
🤖 Prompt for AI Agents
In Templates/template.html around lines 37 to 43, the div for Slide 4 reuses
data-template-slide="3" which duplicates Slide 3’s template id and can break
PPTX mapping; update the attribute to the correct, unique template id (e.g.,
change to data-template-slide="4" or whatever unused template number your PPTX
expects) and verify against the PPTX template structure to ensure the slide maps
to the intended layout.
Task Link
REST-1961
Description
Summary by CodeRabbit
New Features
Refactor
Documentation
Chores