Repository navigation
feat(ppt): Add Types and Validation for PPT generation - #784
Conversation
WalkthroughAdds PowerPoint (PPT) as a new output modality: new Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Validator as Validator
participant ErrorFactory as ErrorFactory
participant Generator as Generator
participant Storage as Storage
Client->>Validator: validatePPTGenerationInput(options)
Validator->>Validator: check mode === "ppt" and validate ppt.options (pages, format, theme, audience, tone, aspectRatio, logoPath, prompt length)
alt validation fails
Validator->>ErrorFactory: select PPT error factory (e.g., invalidPPTPages)
ErrorFactory-->>Client: NeuroLinkError (code, message, context, suggestions)
else validation succeeds
Validator-->>Client: EnhancedValidationResult (isValid, warnings, suggestions)
Client->>Generator: generate(options with ppt config)
Generator->>Generator: build slides, apply theme/audience/tone, include images/logo
Generator->>Storage: write .pptx file
Storage-->>Generator: filePath, fileSize
Generator-->>Client: PPTGenerationResult (filePath, totalSlides, format, metadata)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@src/lib/types/generateTypes.ts`:
- Around line 349-366: The JSDoc example incorrectly references a non-existent
slides array on PPTGenerationResult; update the example under the ppt?:
PPTGenerationResult docblock to use the actual properties (e.g.,
result.ppt.filePath and result.ppt.totalSlides) and any appropriate access
pattern (such as logging the file path and totalSlides) so the sample matches
the PPTGenerationResult shape.
In `@src/lib/utils/errorHandling.ts`:
- Around line 563-601: Both missingPPTPages and invalidPPTPages contain
inconsistent guidance (mentioning 1–50, 1–30 and a default) that conflicts with
the enforced constraint that pages are required with a minimum of 5; update
these functions so messaging and suggestions reflect the true limits (min 5, max
30), remove any references to a default slide count, and make error messages
explicitly state "pages is required and must be between 5 and 30". Specifically,
edit NeuroLinkError returned by missingPPTPages to change suggestions and
message to require pages and show "5 to 30" range, and edit invalidPPTPages
(parameters pages, reason) to update the message template and suggestions array
to "Use a number between 5 and 30" and remove/default-related guidance while
preserving providedValue in context.
In `@src/lib/utils/parameterValidation.ts`:
- Around line 1098-1106: The current logoPath validation in the options block
incorrectly rejects Buffer and ImageWithAltText types; update the checks for
options.logoPath so it accepts either a non-empty string, a Buffer (use
Buffer.isBuffer(options.logoPath)), or an ImageWithAltText-shaped object
(validate the expected properties on the object, e.g., has binary/data/image
property and optional alt text) and only call
ErrorFactory.invalidPPTLogoPath(options.logoPath, ...) when none of these shapes
match; keep the existing empty-string trim check for string values and reuse
ErrorFactory.invalidPPTLogoPath for invalid cases.
- Around line 1160-1183: When mode is 'ppt' the code currently only validates
options.output.ppt if present, so missing output.ppt slips through; add an
explicit check when options.mode === 'ppt' to require options.output?.ppt and
push a validation error via errors.push(toValidationError(...)) if it's absent.
Locate the PPT block around validatePPTOutputOptions and extend it: if
options.mode === 'ppt' && !options.output?.ppt then create and push a validation
error (use the same error shape you return from validatePPTOutputOptions or
toValidationError) so missing pages/config is enforced before accessing pages or
includeImages.
- Around line 996-1000: The PPT page limit is inconsistent; choose 30 as the
intended max and update the constant and messages accordingly: change
MAX_PPT_PAGES from 50 to 30 (symbol MAX_PPT_PAGES), adjust the warning threshold
check that currently tests for pages > 30 to reference the same threshold or a
named PPT_PAGE_WARNING_THRESHOLD (so there is a single source of truth), and
update ErrorFactory.invalidPPTPages and ErrorFactory.missingPPTPages messages to
state "Use a number between 1 and 30" / "Valid range: 1 to 30 slides"
respectively so all four locations (MAX_PPT_PAGES, the warning condition,
ErrorFactory.invalidPPTPages, ErrorFactory.missingPPTPages) are consistent.
| /** | ||
| * PowerPoint generation result (present when output.mode is "ppt") | ||
| * | ||
| * @example | ||
| * ```typescript | ||
| * const result = await neurolink.generate({ | ||
| * input: { text: "Introducing Our New Product" }, | ||
| * model: "gemini-pro", | ||
| * output: { mode: "ppt", ppt: { pages: 10, theme: "modern" } } | ||
| * }); | ||
| * | ||
| * if (result.ppt) { | ||
| * console.log(`Generated ${result.ppt.slides.length} slides`); | ||
| * console.log(`Title: ${result.ppt.slides[0].title}`); | ||
| * } | ||
| * ``` | ||
| */ | ||
| ppt?: PPTGenerationResult; |
There was a problem hiding this comment.
JSDoc example references non-existent slides.
PPTGenerationResult exposes filePath and totalSlides, not slides. Update the example to avoid confusion.
📝 Suggested doc fix
- * if (result.ppt) {
- * console.log(`Generated ${result.ppt.slides.length} slides`);
- * console.log(`Title: ${result.ppt.slides[0].title}`);
- * }
+ * if (result.ppt) {
+ * console.log(`Presentation saved: ${result.ppt.filePath}`);
+ * console.log(`Total slides: ${result.ppt.totalSlides}`);
+ * }📝 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.
| /** | |
| * PowerPoint generation result (present when output.mode is "ppt") | |
| * | |
| * @example | |
| * ```typescript | |
| * const result = await neurolink.generate({ | |
| * input: { text: "Introducing Our New Product" }, | |
| * model: "gemini-pro", | |
| * output: { mode: "ppt", ppt: { pages: 10, theme: "modern" } } | |
| * }); | |
| * | |
| * if (result.ppt) { | |
| * console.log(`Generated ${result.ppt.slides.length} slides`); | |
| * console.log(`Title: ${result.ppt.slides[0].title}`); | |
| * } | |
| * ``` | |
| */ | |
| ppt?: PPTGenerationResult; | |
| /** | |
| * PowerPoint generation result (present when output.mode is "ppt") | |
| * | |
| * `@example` | |
| * |
🤖 Prompt for AI Agents
In `@src/lib/types/generateTypes.ts` around lines 349 - 366, The JSDoc example
incorrectly references a non-existent slides array on PPTGenerationResult;
update the example under the ppt?: PPTGenerationResult docblock to use the
actual properties (e.g., result.ppt.filePath and result.ppt.totalSlides) and any
appropriate access pattern (such as logging the file path and totalSlides) so
the sample matches the PPTGenerationResult shape.
| static missingPPTPages(): NeuroLinkError { | ||
| return new NeuroLinkError({ | ||
| code: ERROR_CODES.MISSING_PPT_PAGES, | ||
| message: | ||
| "PPT generation requires 'pages' field to specify number of slides", | ||
| category: ErrorCategory.VALIDATION, | ||
| severity: ErrorSeverity.MEDIUM, | ||
| retriable: false, | ||
| context: { | ||
| field: "output.ppt.pages", | ||
| suggestions: [ | ||
| "Provide the number of slides: output.ppt.pages = 10", | ||
| "Valid range: 1 to 50 slides", | ||
| "Recommended: 10 slides for most presentations", | ||
| ], | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Create an invalid PPT pages error | ||
| */ | ||
| static invalidPPTPages(pages: unknown, reason: string): NeuroLinkError { | ||
| return new NeuroLinkError({ | ||
| code: ERROR_CODES.INVALID_PPT_PAGES, | ||
| message: `Invalid pages value '${pages}': ${reason}`, | ||
| category: ErrorCategory.VALIDATION, | ||
| severity: ErrorSeverity.MEDIUM, | ||
| retriable: false, | ||
| context: { | ||
| field: "output.ppt.pages", | ||
| providedValue: pages, | ||
| suggestions: [ | ||
| "Use a number between 1 and 30", | ||
| "Default is 10 slides if not specified", | ||
| "For longer presentations, consider breaking into multiple decks", | ||
| ], | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Page-range guidance doesn’t match the actual constraints.
Messages suggest 1–50 / 1–30 and a default slide count even though pages are required and min is 5. Align messaging with the enforced limits.
📝 Suggested alignment (update to match validation limits)
- "Valid range: 1 to 50 slides",
+ "Valid range: 5 to 30 slides",
...
- "Use a number between 1 and 30",
- "Default is 10 slides if not specified",
+ "Use a number between 5 and 30",
+ "Recommended: 10 slides for most presentations",📝 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.
| static missingPPTPages(): NeuroLinkError { | |
| return new NeuroLinkError({ | |
| code: ERROR_CODES.MISSING_PPT_PAGES, | |
| message: | |
| "PPT generation requires 'pages' field to specify number of slides", | |
| category: ErrorCategory.VALIDATION, | |
| severity: ErrorSeverity.MEDIUM, | |
| retriable: false, | |
| context: { | |
| field: "output.ppt.pages", | |
| suggestions: [ | |
| "Provide the number of slides: output.ppt.pages = 10", | |
| "Valid range: 1 to 50 slides", | |
| "Recommended: 10 slides for most presentations", | |
| ], | |
| }, | |
| }); | |
| } | |
| /** | |
| * Create an invalid PPT pages error | |
| */ | |
| static invalidPPTPages(pages: unknown, reason: string): NeuroLinkError { | |
| return new NeuroLinkError({ | |
| code: ERROR_CODES.INVALID_PPT_PAGES, | |
| message: `Invalid pages value '${pages}': ${reason}`, | |
| category: ErrorCategory.VALIDATION, | |
| severity: ErrorSeverity.MEDIUM, | |
| retriable: false, | |
| context: { | |
| field: "output.ppt.pages", | |
| providedValue: pages, | |
| suggestions: [ | |
| "Use a number between 1 and 30", | |
| "Default is 10 slides if not specified", | |
| "For longer presentations, consider breaking into multiple decks", | |
| ], | |
| }, | |
| }); | |
| static missingPPTPages(): NeuroLinkError { | |
| return new NeuroLinkError({ | |
| code: ERROR_CODES.MISSING_PPT_PAGES, | |
| message: | |
| "PPT generation requires 'pages' field to specify number of slides", | |
| category: ErrorCategory.VALIDATION, | |
| severity: ErrorSeverity.MEDIUM, | |
| retriable: false, | |
| context: { | |
| field: "output.ppt.pages", | |
| suggestions: [ | |
| "Provide the number of slides: output.ppt.pages = 10", | |
| "Valid range: 5 to 30 slides", | |
| "Recommended: 10 slides for most presentations", | |
| ], | |
| }, | |
| }); | |
| } | |
| /** | |
| * Create an invalid PPT pages error | |
| */ | |
| static invalidPPTPages(pages: unknown, reason: string): NeuroLinkError { | |
| return new NeuroLinkError({ | |
| code: ERROR_CODES.INVALID_PPT_PAGES, | |
| message: `Invalid pages value '${pages}': ${reason}`, | |
| category: ErrorCategory.VALIDATION, | |
| severity: ErrorSeverity.MEDIUM, | |
| retriable: false, | |
| context: { | |
| field: "output.ppt.pages", | |
| providedValue: pages, | |
| suggestions: [ | |
| "Use a number between 5 and 30", | |
| "Recommended: 10 slides for most presentations", | |
| "For longer presentations, consider breaking into multiple decks", | |
| ], | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In `@src/lib/utils/errorHandling.ts` around lines 563 - 601, Both missingPPTPages
and invalidPPTPages contain inconsistent guidance (mentioning 1–50, 1–30 and a
default) that conflicts with the enforced constraint that pages are required
with a minimum of 5; update these functions so messaging and suggestions reflect
the true limits (min 5, max 30), remove any references to a default slide count,
and make error messages explicitly state "pages is required and must be between
5 and 30". Specifically, edit NeuroLinkError returned by missingPPTPages to
change suggestions and message to require pages and show "5 to 30" range, and
edit invalidPPTPages (parameters pages, reason) to update the message template
and suggestions array to "Use a number between 5 and 30" and
remove/default-related guidance while preserving providedValue in context.
| const VALID_PPT_ASPECT_RATIOS = ["16:9", "4:3"] as const; | ||
| const VALID_PPT_FORMATS = ["pptx"] as const; | ||
| const MIN_PPT_PAGES = 5; | ||
| const MAX_PPT_PAGES = 50; | ||
| const MAX_PPT_PROMPT_LENGTH = 1000; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the parameterValidation.ts file around the mentioned lines
head -n 1010 src/lib/utils/parameterValidation.ts | tail -n 50Repository: juspay/neurolink
Length of output: 1333
🏁 Script executed:
# Search for references to PPT page limits, error messages, or documentation
rg -i "ppt.*page|page.*ppt|5.*30|5.*50" --type ts --context 3Repository: juspay/neurolink
Length of output: 41761
🏁 Script executed:
# Search for where MAX_PPT_PAGES is used
rg "MAX_PPT_PAGES" --type ts --context 5Repository: juspay/neurolink
Length of output: 1635
🏁 Script executed:
# Search for MIN_PPT_PAGES usage as well
rg "MIN_PPT_PAGES" --type ts --context 5Repository: juspay/neurolink
Length of output: 1649
🏁 Script executed:
# Look for error messages or validation messages mentioning page counts
rg -i "ppt.*pages|slide.*count|page.*limit" --type ts --context 3Repository: juspay/neurolink
Length of output: 12395
🏁 Script executed:
# Check for any API documentation, README, or comments defining the requirement
rg -i "5.*30.*ppt|5.*50.*ppt|ppt.*5.*30|ppt.*5.*50" --type ts --type md --context 3Repository: juspay/neurolink
Length of output: 42
🏁 Script executed:
# Get the complete errorHandling context for both error functions
sed -n '1350,1450p' src/lib/utils/errorHandling.tsRepository: juspay/neurolink
Length of output: 42
🏁 Script executed:
# Check if there are any docs or comments about the 30-page limit
rg -i "30.*slide|slide.*30|performance|time.*estimate" --type ts src/lib/utils/parameterValidation.ts -A 2 -B 2Repository: juspay/neurolink
Length of output: 442
Reconcile inconsistent PPT page limit across validation and error messages.
MAX_PPT_PAGES=50 conflicts with the warning threshold (pages > 30) and with ErrorFactory.invalidPPTPages which suggests "Use a number between 1 and 30". Meanwhile, ErrorFactory.missingPPTPages says "Valid range: 1 to 50 slides". This creates conflicting guidance. Align the constant, warning threshold, and both error messages to reflect the intended maximum.
Issues to reconcile
- Max pages constant: MAX_PPT_PAGES = 50
- Warning threshold: Warns when pages > 30 with time-estimate escalation
- invalidPPTPages error: Suggests max 30
- missingPPTPages error: Suggests max 50
Choose the intended limit and update all four locations consistently.
🤖 Prompt for AI Agents
In `@src/lib/utils/parameterValidation.ts` around lines 996 - 1000, The PPT page
limit is inconsistent; choose 30 as the intended max and update the constant and
messages accordingly: change MAX_PPT_PAGES from 50 to 30 (symbol MAX_PPT_PAGES),
adjust the warning threshold check that currently tests for pages > 30 to
reference the same threshold or a named PPT_PAGE_WARNING_THRESHOLD (so there is
a single source of truth), and update ErrorFactory.invalidPPTPages and
ErrorFactory.missingPPTPages messages to state "Use a number between 1 and 30" /
"Valid range: 1 to 30 slides" respectively so all four locations (MAX_PPT_PAGES,
the warning condition, ErrorFactory.invalidPPTPages,
ErrorFactory.missingPPTPages) are consistent.
12c1ad7 to
47e22e0
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/lib/types/pptTypes.ts`:
- Around line 13-32: The JSDoc for the PPTOutputOptions.pages field incorrectly
indicates a default value; update the comment on the pages property in the
PPTOutputOptions type to remove the "default: 10" text (keep the "max: 50" if
desired) so it no longer implies it's optional, and ensure consistency with
validatePPTOutputOptions which enforces pages as required.
In `@src/lib/utils/parameterValidation.ts`:
- Around line 1234-1240: The warning message pushed when options.input.images
exists is grammatically incorrect; update the warnings.push string to a
professional, clear sentence (e.g., "Images may be unused in PPT generation if
they fail quality standards and can increase generation time.") and ensure
"unUsed" is corrected to "unused"; keep the suggestions.push unchanged but
verify phrasing matches the revised warning for consistency.
- Around line 1218-1232: The code uses options.input.text.trim() without null
checks which can throw when options.input or options.input.text is missing;
update the validation in the block that defines trimmedPrompt to safely access
the text (e.g., guard options.input and options.input.text or coerce to an empty
string) before calling trim, and if missing push a validation error via
toValidationError(ErrorFactory.invalidPPTPrompt(...)); ensure the rest of the
logic still uses trimmedPrompt and compares against MAX_PPT_PROMPT_LENGTH.
♻️ Duplicate comments (2)
src/lib/utils/errorHandling.ts (1)
580-596: Page range in error message should match the warning threshold for clarity.The suggestion says "Use a number between 5 and 50", but
validatePPTGenerationInputemits a warning when pages > 30. This creates mixed signals for users. Consider aligning the guidance with the practical recommendation (30) or clarifying that 50 is the hard limit while 30+ triggers performance warnings.📝 Suggested alignment
context: { field: "output.ppt.pages", providedValue: pages, suggestions: [ - "Use a number between 5 and 50", + "Use a number between 5 and 30 for optimal generation time", + "Maximum supported: 50 slides (may take significant time)", "For longer presentations, consider breaking into multiple decks", ], },src/lib/types/generateTypes.ts (1)
349-366: JSDoc example references non-existentslidesproperty.The example accesses
result.ppt.slides.lengthandresult.ppt.slides[0].title, butPPTGenerationResultonly hasfilePath,totalSlides, andmetadata. This will mislead developers.📝 Suggested doc fix
* if (result.ppt) { - * console.log(`Generated ${result.ppt.slides.length} slides`); - * console.log(`Title: ${result.ppt.slides[0].title}`); + * console.log(`Presentation saved: ${result.ppt.filePath}`); + * console.log(`Total slides: ${result.ppt.totalSlides}`); * }
dc32f47 to
c8b4080
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
-- added types for ppt generation -- added type validation for ppt gen
c8b4080 to
1e48fd3
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
🎉 This PR is included in version 8.36.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Pull Request
Description
What does this PR do?
Implements Phase 1 of PPT generation feature: type definitions and comprehensive validation layer. Adds PPTOutputOptions and PPTGenerationResult types following the video generation pattern. Includes validatePPTOutputOptions() and validatePPTGenerationInput() functions with 21 passing tests. Pages field is mandatory (5-30 slides), supports configurable theme, audience, tone, aspectRatio, and other presentation options. Validates prompt length and provides helpful error messages via 11 new error codes. Sets foundation for AI-powered presentation creation for next step.
Related Issues
Does this PR close any issues?
Fixes #(issue number)
Closes #(issue number)
Relates to #(issue number)
Type of Change
Please select the type of change:
Motivation and Context
Why is this change needed? What problem does it solve?
Provide context for reviewers:
Changes Made
What specific changes were made?
Provide a bullet-point list of the key changes:
Breaking Changes
Does this PR introduce breaking changes?
If yes, describe:
Testing
How has this been tested?
Please describe the tests you ran and their results:
Test Coverage
Manual Testing Steps
Provide steps for manual testing:
Code Quality
Have you followed code quality standards?
Documentation
Have you updated documentation?
Commit Message Format
Does your commit follow semantic commit conventions?
type(scope): descriptionExample:
feat(providers): add support for LiteLLM proxyDependencies
Does this PR add, update, or remove dependencies?
If yes, list dependencies and justification:
Performance Impact
Does this change affect performance?
If applicable, provide benchmark results:
Security Considerations
Are there any security implications?
If applicable, describe:
Deployment Notes
Special deployment instructions?
Screenshots / Videos
If applicable, add screenshots or videos to demonstrate changes:
[Add screenshots or videos here]
Reviewer Checklist
For reviewers:
Additional Notes
Any additional information for reviewers:
[Add any extra context, concerns, or questions here]
Pre-submission Checklist
Before submitting, ensure you have:
pnpm testpnpm buildpnpm run validate:alland all checks passThank you for contributing to NeuroLink!
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.