Repository navigation
feat(ppt): Implement CLI support for PPT Gen - #808
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds PowerPoint (PPT) generation as a first-class CLI output mode: new PPT flags and options, type declarations for PPT inputs/results, CLI wiring in the command factory to build PPT output config and handle results, new PPT feature docs, redirects, and extensive unit tests for PPT flags and generation utilities. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as Client (CLI)
participant CF as CommandFactory
participant GS as GenerationService
participant SG as SlideGenerator
participant FS as FileSystem
CLI->>CF: parse args (outputMode=ppt + ppt flags)
CF->>GS: build multimodal input + PPT output config
GS->>SG: request PPT generation (plan slides, images, metadata)
SG->>FS: write PPTX file (filePath)
FS-->>SG: file saved
SG-->>GS: return PPTGenerationResult (filePath,totalSlides,format,metadata)
GS-->>CF: GenerateResult (includes ppt)
CF->>CLI: display PPT output info (file path, total slides, format)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
f85f261 to
68d0628
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/cli/factories/commandFactory.ts`:
- Around line 311-316: The GenerateCommandArgs.outputMode union type is missing
"ppt" which mismatches the CLI choices and forces unsafe casts; update the
outputMode declaration in the GenerateCommandArgs type to include "ppt" (i.e.,
change the union to "text" | "video" | "ppt") and then remove any
now-unnecessary type casts/usages that coerced strings to the old narrower type
(e.g., the cast at the location where outputMode is read and cast to the
enum/union) so callers and consumers use the widened, type-safe
GenerateCommandArgs.outputMode directly.
- Around line 347-385: Remove the unintended defaults that cause PPT mode to
auto-enable: delete the default values from the pptPages and pptAspectRatio
option definitions in commandFactory (remove default: 10 from pptPages and
default: "16:9" from pptAspectRatio) so they are undefined unless the user
explicitly sets them; ensure hasPPTFlags/isPPTMode logic continues to rely on
presence of these argv properties (or fallback behavior in
buildGenerateOutputConfig) rather than yargs-applied defaults.
68d0628 to
bd0d916
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
bd0d916 to
278e63b
Compare
|
@Tafheem-Ahemad-Juspay you will need to update the documentation also for all these features that we are adding. Can you figure out from previous full requests that I have merged and understand what kind of documentation you need to update and in which places. |
|
@Tafheem-Ahemad-Juspay is attempting to deploy a commit to the Sachin Sharma's projects Team on Vercel. A member of the Team first needs to authorize it. |
c911c6e to
7f5709a
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@murdore I updated all the docs and added test case for ppt gen , please review |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
docs/use-cases.md (1)
115-117: Remove unusedwriteFileimport in the PPT example.Line 116 imports
writeFile, but the snippet never uses it (output is already handled viaoutputPath).✂️ Small cleanup
import { NeuroLink } from "@juspay/neurolink"; -import { writeFile } from "fs/promises";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/use-cases.md` around lines 115 - 117, Remove the unused writeFile import from the PPT example: delete the "import { writeFile } from 'fs/promises'" import (the symbol writeFile is never referenced; output is handled via outputPath) so only "import { NeuroLink } from '@juspay/neurolink'" remains.docs/sdk/api-reference.md (1)
170-173: Add an inlinePPTOutputOptionsdefinition (or direct link) nearGenerateOptions.Line 172 introduces
ppt?: PPTOutputOptions, but this section only definesVideoOutputOptionslocally. Adding the PPT type here will make the API section self-contained.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/sdk/api-reference.md` around lines 170 - 173, The docs currently reference ppt?: PPTOutputOptions in GenerateOptions but do not define PPTOutputOptions; add an inline type definition (or a direct link) adjacent to the GenerateOptions/VideoOutputOptions block that defines PPTOutputOptions, including its key fields (e.g., slideSize, template, includeNotes, assets[]) and short descriptions so the API reference is self-contained and mirrors the existing VideoOutputOptions style; ensure the symbol name PPTOutputOptions matches exactly and update cross-references if you add a link instead of an inline type.test/unit/ppt-generation.test.ts (1)
1991-2325: Strengthen render-coverage tests with output assertions, not only non-throw checks.Most cases in this block only assert
not.toThrow(). Add a few targeted assertions on rendered slide content/shape to catch silent rendering regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/ppt-generation.test.ts` around lines 1991 - 2325, Tests only check that rendering doesn't throw; add targeted assertions that validate the rendered slide structure to catch silent regressions. Update the tests that call render (which wraps gen.renderSlide) to capture its return or inspect the PptxGenJS instance and assert expected properties for a few representative cases (e.g., verify returned slide object has expected type/layout/title/content or that the PptxGenJS slides array length increased and slide objects contain the expected text fields). Locate usages of createMockSlideSchema, gen, render, PptxGenJS and gen.renderSlide and in a few tests (e.g., "section-header", "image-left", "chart-line") add one or two assertions each checking concrete output fields instead of only .not.toThrow().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/cli-guide.md`:
- Around line 137-144: The CLI docs omit several PPT flags and incomplete alias
coverage: update the PPT Generation Options section to list all flags including
--pptAudience, --pptTone, and --pptAspectRatio, and ensure aliases are
documented (e.g., show that --pptOutput has any alias such as --output if
applicable and that --pptPages is aliased to --pages). Edit the block that
currently lists --outputMode, --pptOutput, --pptTheme, --pptPages/--pages, and
--pptNoImages to also include short descriptions and valid values/defaults for
--pptAudience, --pptTone, and --pptAspectRatio, and add the correct alias text
for --pptOutput and --pptPages so the CLI guide matches the CLI reference.
In `@docs/cli-reference.md`:
- Around line 353-356: In the CLI example command (the npx `@juspay/neurolink`
generate line) fix the model slug typo by replacing "claude-3.5-sonnet" with the
canonical hyphenated slug "claude-3-5-sonnet" so the example matches other docs
and won't fail on copy-paste.
In `@docs/configuration.md`:
- Around line 217-237: The "PPT Generation (PowerPoint Presentations)" section
currently implies universal provider compatibility; update the text to
explicitly list only the supported providers (e.g., Google Vertex via
GOOGLE_VERTEX_PROJECT/VERTEX_IMAGE_MODEL, OpenAI via OPENAI_API_KEY, Anthropic
via ANTHROPIC_API_KEY, and Google AI Studio via GOOGLE_AI_API_KEY) and remove
language suggesting any text AI model will work; adjust the example env var
block and the opening sentence to state "Supported providers: ..." and note that
other providers are not guaranteed to work.
In `@docs/features/ppt-generation.md`:
- Around line 35-37: Update the slide-count references so they are consistent:
change any occurrences of "31" (e.g., the phrase "All 31" found later in the
doc) to "35" to match the header "**35 slide types**" and ensure the
"Professional presentations" section and all other mentions (lines referencing
slide counts or "All 31") state "35" instead; scan the document for any
remaining "31" numeric mentions and replace them with "35" so all counts match.
In `@docs/getting-started/provider-setup.md`:
- Line 484: Update the "PPT Generation" sentence to clarify it's provider-gated
by replacing "with any text model" with a note that PPT output requires a
supported provider; mention the exact config key output.mode: "ppt" and either
list supported providers or add a link to provider support details (e.g.,
"requires a provider that supports PPT generation — see provider support
table"), so readers know PPT generation is not available for all text model
providers.
In `@src/cli/factories/commandFactory.ts`:
- Around line 2168-2194: detectGenerateOutputMode currently allows both
isVideoMode and isPPTMode to be true when conflicting flags are passed; update
it to detect mixed signals and fail fast: inside detectGenerateOutputMode (the
function that computes isVideoMode/isPPTMode/spinnerMessage) add logic to treat
"video" outputMode and any ppt* flags as mutually exclusive and throw or return
an explicit error when both are present, and update the caller that runs the
Video and PPT configurators (the block that invokes the configurator code paths
after detection) to check for that error and abort before running either
configurator to prevent ambiguous behavior.
- Around line 1138-1160: The PPT/video generation branch currently skips
handleOutput() and only prints PPT details when options.quiet is false, hiding
essential info; update the PPT/video flow to always emit the generated artifact
details (at minimum the ppt.filePath and ppt.totalSlides/format) regardless of
options.quiet or call handleOutput() for PPT/video branches so the standard
output handling is reused; locate the PPT/video branch that sets ppt and the
conditional around options.quiet in commandFactory (symbols: handleOutput, ppt,
options.quiet, and the PPT/video generation block) and either invoke
handleOutput(ppt...) for those types or move the file-path/slide/format
logger.always lines out of the !options.quiet guard so they always run while
keeping other verbose tips gated by quiet.
In `@test/unit/cli/ppt-flags.test.ts`:
- Around line 9-120: The tests are self-referential and assert local constants
instead of exercising the real CLI wiring; update the tests to use the actual
command definitions and parsing so regressions are caught: import the
commandFactory (or the function that builds the generate command) and/or call
buildGenerateOutputConfig (the symbols referenced in the comment) to construct
the generate command, invoke its parser with sample argv for PPT flags
(including aliases like --ppt-pages/--pages and --ppt-output/--po), and assert
the normalized yargs properties (outputMode, pptPages, pptOutput,
pptAspectRatio, pptNoImages) and final output config/defaults (pages,
aspectRatio, generateAiImages) returned by buildGenerateOutputConfig; ensure
tests cover supported theme/audience/tone/aspect values and provider-related
code paths by parsing with different provider-related flags to validate mapping
and multimodal handling.
In `@test/unit/ppt-generation.test.ts`:
- Around line 2037-2039: The test case in ppt-generation.test.ts constructs a
slide with type: "closing" but uses layout: "contact-info", which maps to
thank-you layouts; change the slide's layout to a valid closing layout (e.g.,
replace "contact-info" with the correct closing layout name used by the renderer
or theme) so the `type: "closing"` branch is exercised correctly, and update any
associated snapshot/expected output for the closing layout if required; look for
the slide object in the test where type: "closing" and adjust its layout
property accordingly.
---
Nitpick comments:
In `@docs/sdk/api-reference.md`:
- Around line 170-173: The docs currently reference ppt?: PPTOutputOptions in
GenerateOptions but do not define PPTOutputOptions; add an inline type
definition (or a direct link) adjacent to the GenerateOptions/VideoOutputOptions
block that defines PPTOutputOptions, including its key fields (e.g., slideSize,
template, includeNotes, assets[]) and short descriptions so the API reference is
self-contained and mirrors the existing VideoOutputOptions style; ensure the
symbol name PPTOutputOptions matches exactly and update cross-references if you
add a link instead of an inline type.
In `@docs/use-cases.md`:
- Around line 115-117: Remove the unused writeFile import from the PPT example:
delete the "import { writeFile } from 'fs/promises'" import (the symbol
writeFile is never referenced; output is handled via outputPath) so only "import
{ NeuroLink } from '@juspay/neurolink'" remains.
In `@test/unit/ppt-generation.test.ts`:
- Around line 1991-2325: Tests only check that rendering doesn't throw; add
targeted assertions that validate the rendered slide structure to catch silent
regressions. Update the tests that call render (which wraps gen.renderSlide) to
capture its return or inspect the PptxGenJS instance and assert expected
properties for a few representative cases (e.g., verify returned slide object
has expected type/layout/title/content or that the PptxGenJS slides array length
increased and slide objects contain the expected text fields). Locate usages of
createMockSlideSchema, gen, render, PptxGenJS and gen.renderSlide and in a few
tests (e.g., "section-header", "image-left", "chart-line") add one or two
assertions each checking concrete output fields instead of only .not.toThrow().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 94798b51-350c-43d6-88d2-f458c43fb6c6
📒 Files selected for processing (28)
README.mddocs-site/config/redirects.tsdocs-site/scripts/sync-docs.tsdocs-site/sidebars.tsdocs/cli-guide.mddocs/cli-reference.mddocs/configuration.mddocs/error-handling.mddocs/features/audio-input.mddocs/features/index.mddocs/features/multimodal-chat.mddocs/features/multimodal.mddocs/features/ppt-generation.mddocs/features/tts.mddocs/getting-started/provider-setup.mddocs/getting-started/quick-start.mddocs/performance-optimization.mddocs/reference/error-codes.mddocs/sdk/api-reference.mddocs/troubleshooting.mddocs/tutorials.mddocs/use-cases.mdsrc/cli/factories/commandFactory.tssrc/lib/features/ppt/index.tssrc/lib/types/cli.tssrc/lib/types/generateTypes.tstest/unit/cli/ppt-flags.test.tstest/unit/ppt-generation.test.ts
7f5709a to
1252d28
Compare
|
🎉 This PR is included in version 9.24.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Pull Request
Description
What does this PR do?
Implements Step 5 of the PPT feature enhancement - CLI Interface for PowerPoint generation. This enables users to generate presentations directly from the command line.
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
New Features
Documentation
Tests