Repository navigation
feat(ppt): Add PPT generation content planner - #786
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds a PPT generation feature: new public types and validation, theme/layout/constants, AI-driven content planning with parsing/validation/post-processing, central export barrel, input validation tweaks, and extensive unit tests for the PPT surface. Changes
Sequence DiagramsequenceDiagram
participant Client
participant ContentPlanner as Content Planner
participant Constants
participant AIProvider as AI Provider
participant Parser
participant Validator
Client->>ContentPlanner: generateContentPlan(context, provider)
ContentPlanner->>Constants: buildContentPlanningPrompt(context)
Constants-->>ContentPlanner: structured prompt
ContentPlanner->>AIProvider: call with prompt
AIProvider-->>ContentPlanner: raw AI response
ContentPlanner->>Parser: parseAIResponse(response)
Parser-->>ContentPlanner: parsed JSON
ContentPlanner->>Validator: validateContentPlan(parsed)
Validator-->>ContentPlanner: normalized ContentPlan
ContentPlanner->>ContentPlanner: postProcessPlan (title + thank you slides)
ContentPlanner-->>Client: validated ContentPlan
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/lib/features/ppt/constants.ts`:
- Around line 441-459: Update the prompt comments and constant identifiers in
src/lib/features/ppt/constants.ts to fix typos and clarify instructions: correct
the misspelled constant name VLAID_THEMES to VALID_THEMES (and update any
references), fix the unclear sentence "if these properties are not mention ,
then chose from the things , else use the user-defined values" to a clear
statement like "If audience, tone, or theme are not provided, select defaults
from VALID_AUDIENCES, VALID_TONES, and VALID_THEMES; otherwise use the
user-provided values", and clean up other minor typos (e.g., "VLAID_THEMES" →
"VALID_THEMES", "mention" → "mentioned", "chose" → "choose", "VLAID"
occurrences) so the SLIDE STRUCTURE and VALID_* arrays (VALID_AUDIENCES,
VALID_TONES) are grammatically correct and unambiguous for the planner.
In `@src/lib/features/ppt/contentPlanner.ts`:
- Around line 347-367: Replace the raw string prompt and direct await of
provider.generate with a MessageBuilder-constructed message and wrap the
provider call with withTimeout; specifically, stop using
buildContentPlanningPrompt to pass a raw prompt into provider.generate and
instead create the messages via MessageBuilder (include the same content that
buildContentPlanningPrompt produced and set the system message to
CONTENT_PLANNING_SYSTEM_PROMPT), then call withTimeout(provider.generate({...})
, CONTENT_PLANNING_TIMEOUT_MS) so the call respects the enforced timeout; keep
the existing provider options (temperature, maxTokens, disableTools) when
invoking provider.generate and reference MessageBuilder, withTimeout,
CONTENT_PLANNING_SYSTEM_PROMPT, CONTENT_PLANNING_TIMEOUT_MS,
buildContentPlanningPrompt (for content to port over), and provider.generate to
locate the changes.
In `@src/lib/types/pptTypes.ts`:
- Around line 661-728: The extraction currently only preserves Buffer logos and
drops string logo paths/URLs; update the PPTGenerationContext.logo type to allow
Buffer | string and modify extractPPTContext to assign pptOptions.logoPath when
it's a string (e.g., else if typeof pptOptions.logoPath === "string" then logo =
pptOptions.logoPath), while keeping the existing branches that handle Buffer and
file-like objects (check "data" field). Ensure the returned object from
extractPPTContext uses this logo value and update any relevant type references
(PPTGenerationContext and usages of logo) so downstream code can accept a string
URL or Buffer.
In `@test/unit/ppt-generation.test.ts`:
- Around line 7-50: The import of PPTOutputOptions in the test file should be a
type-only import because it’s only used in type positions; update the import
statement that currently lists PPTOutputOptions among the values imported from
"../../src/lib/types/pptTypes.js" to use a type-only import (e.g., import type {
PPTOutputOptions } from "...") while leaving the rest of the runtime imports
unchanged so only PPTOutputOptions is imported purely for types.
0a34107 to
45c51d6
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/lib/features/ppt/constants.ts`:
- Around line 18-149: THEMES currently stores color strings with a leading "#"
which is incompatible with pptxgenjs's expected 6-character hex format; update
the THEMES object (symbol THEMES in src/lib/features/ppt/constants.ts) so all
color values are stored without the "#" prefix, or alternatively ensure any
consumer of themes (e.g., where themes are applied to slides) calls the existing
normalizeHexColor helper before passing values to pptxgenjs; locate uses of
THEMES and the theme application code and either strip the "#" in the THEMES
entries or add normalizeHexColor(...) calls where colors are handed to pptxgenjs
APIs.
♻️ Duplicate comments (2)
src/lib/features/ppt/contentPlanner.ts (1)
347-367: Adopt MessageBuilder + withTimeout for the planning call.The prompt is built as raw strings and
provider.generateis awaited directly. The project guidelines require MessageBuilder for message construction andwithTimeoutto enforce timeouts even if a provider ignores the timeout option. Based on coding guidelines, route prompt construction through MessageBuilder and wrap the async call withwithTimeout.src/lib/features/ppt/constants.ts (1)
441-459: Fix prompt typos to avoid confusing the planner.The prompt still contains unclear grammar on line 442: "if these properties are not mention , then chose from the things". This was flagged in a previous review and acknowledged but may not be fully addressed.
🧹 Nitpick comments (4)
src/lib/types/pptTypes.ts (1)
1236-1244: Hex color validation may reject valid shorthand colors.The
isValidHexColorfunction only accepts 6-character hex codes. While pptxgenjs typically uses 6-character hex without#, consider documenting this constraint clearly or handling 3-character shorthand expansion if users might provide shorthand.src/lib/features/ppt/contentPlanner.ts (2)
36-125: Duplicate type/layout sets create maintenance risk.
VALID_SLIDE_TYPESandVALID_SLIDE_LAYOUTSduplicate the type definitions frompptTypes.ts. If theSlideTypeorSlideLayoutunion types are updated, these sets must be manually synchronized.Consider deriving these sets from the source of truth or exporting them from
pptTypes.tsto avoid drift.♻️ Suggested approach
Export the valid sets from
pptTypes.tsalongside the types:// In pptTypes.ts export const ALL_SLIDE_TYPES: readonly SlideType[] = [ "title", "section-header", /* ... all types */ ] as const; export const ALL_SLIDE_LAYOUTS: readonly SlideLayout[] = [ "title-centered", "title-bottom", /* ... all layouts */ ] as const;Then import and use in contentPlanner.ts:
import { ALL_SLIDE_TYPES, ALL_SLIDE_LAYOUTS } from "./types.js"; const VALID_SLIDE_TYPES = new Set(ALL_SLIDE_TYPES); const VALID_SLIDE_LAYOUTS = new Set(ALL_SLIDE_LAYOUTS);
424-448: Helper functions mutate input parameter.
ensureTitleSlideandensureThankYouSlidemodify theplan.slidesarray in-place via spread assignment to array indices. While the functions return the plan, the mutation side-effect can surprise callers who expect pure functions.♻️ Suggested immutable approach
export function ensureTitleSlide(plan: ContentPlan): ContentPlan { if (plan.slides.length > 0 && plan.slides[0].type !== "title") { - plan.slides[0] = { - ...plan.slides[0], - type: "title", - layout: "title-centered", - }; + const newSlides = [...plan.slides]; + newSlides[0] = { + ...newSlides[0], + type: "title", + layout: "title-centered", + }; + return { ...plan, slides: newSlides }; } return plan; }test/unit/ppt-generation.test.ts (1)
89-99: Use explicit invalid literal types instead ofas anyfor cleaner test assertions.Static analysis flags the
as anycasts. While acceptable in tests for invalid inputs, using explicit string literals with type assertions is more precise and avoids the linter warnings.♻️ Suggested fix
it("should reject invalid theme/audience/tone", () => { expect( - validatePPTOutputOptions({ pages: 10, theme: "invalid" as any }), + validatePPTOutputOptions({ pages: 10, theme: "invalid" as PPTOutputOptions["theme"] }), ).not.toBeNull(); expect( - validatePPTOutputOptions({ pages: 10, audience: "invalid" as any }), + validatePPTOutputOptions({ pages: 10, audience: "invalid" as PPTOutputOptions["audience"] }), ).not.toBeNull(); expect( - validatePPTOutputOptions({ pages: 10, tone: "invalid" as any }), + validatePPTOutputOptions({ pages: 10, tone: "invalid" as PPTOutputOptions["tone"] }), ).not.toBeNull(); });Or alternatively, cast the entire object:
validatePPTOutputOptions({ pages: 10, theme: "invalid" } as PPTOutputOptions)
- Implement AI content planner generating structured ContentPlan from topics - Add 5 presentation themes (modern, corporate, creative, minimal, dark) - Add validation with default value suggestions for theme/audience/tone - Add comprehensive type system aligned with pptxgenjs API (charts, tables, timelines)
|
🎉 This PR is included in version 8.38.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
2 participants
Pull Request
Description
What does this PR do?
Adds comprehensive foundation for AI-powered PowerPoint presentation generation using pptxgenjs. This PR establishes constants, content planning, and validation needed for the PPT generation pipeline.
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.