fix: skip caption generation when captionLength is none#132
fix: skip caption generation when captionLength is none#132sweetmantech merged 2 commits intomainfrom
Conversation
Changes default captionLength from "short" to "none" in the content creation schema. When "none", the task skips the caption generation API call and passes an empty string to renderFinalVideo, producing a video without text overlay. Co-Authored-By: Paperclip <noreply@paperclip.ing>
📝 WalkthroughWalkthroughThe changes introduce a new Changes
Possibly Related PRs
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 docstrings
🧪 Generate unit tests (beta)
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 |
| let captionText = ""; | ||
| if (payload.captionLength !== "none") { | ||
| logStep("Generating caption via API"); | ||
| const captionTopic = [ | ||
| `Song: "${audioClip.songTitle}"`, | ||
| audioClip.clipLyrics ? `Lyrics: "${audioClip.clipLyrics}"` : null, | ||
| audioClip.clipMood ? `Mood: ${audioClip.clipMood}` : null, | ||
| artistContext ? `Artist: ${artistContext}` : null, | ||
| audienceContext ? `Audience: ${audienceContext}` : null, | ||
| ].filter(Boolean).join(". "); | ||
|
|
||
| captionText = await generateCaption({ | ||
| topic: captionTopic, | ||
| template: payload.template, | ||
| length: payload.captionLength, | ||
| }); | ||
| } else { | ||
| logStep("Skipping caption generation (captionLength=none)"); | ||
| } |
There was a problem hiding this comment.
SRP
- actual: caption generation / skip logic is in the createContentTask function file.
- required: new function file for caption generation / skip logic.
Moves the caption generation/skip logic out of createContentTask into its own module. The task now delegates to resolveCaptionText, which returns "" for "none" and calls generateCaption otherwise. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tasks/createContentTask.ts (1)
118-132:⚠️ Potential issue | 🟠 MajorCaption skip is too late; default path still does unnecessary context I/O.
Because
captionLengthnow defaults to"none", the task still pays forfetchArtistContext/fetchAudienceContext(Lines 77-84) before the skip branch is reached at Line 118. That adds avoidable latency and external calls on the default path.⚡ Suggested fix
- // --- Step 4: Fetch artist/audience context --- - logStep("Fetching artist context"); - const artistContext = await fetchArtistContext( - payload.githubRepo, payload.artistSlug, fetchGithubFile, - ); - const audienceContext = await fetchAudienceContext( - payload.githubRepo, payload.artistSlug, fetchGithubFile, - ); + let artistContext: string | null = null; + let audienceContext: string | null = null; @@ - // --- Step 9: Resolve caption text (skipped when captionLength === "none") --- - logStep("Resolving caption", true, { captionLength: payload.captionLength }); - const captionTopic = [ - `Song: "${audioClip.songTitle}"`, - audioClip.clipLyrics ? `Lyrics: "${audioClip.clipLyrics}"` : null, - audioClip.clipMood ? `Mood: ${audioClip.clipMood}` : null, - artistContext ? `Artist: ${artistContext}` : null, - audienceContext ? `Audience: ${audienceContext}` : null, - ].filter(Boolean).join(". "); - - const captionText = await resolveCaptionText({ - captionLength: payload.captionLength, - topic: captionTopic, - template: payload.template, - }); + // --- Step 9: Resolve caption text (skip all caption prep when captionLength === "none") --- + let captionText = ""; + if (payload.captionLength !== "none") { + logStep("Fetching artist context"); + artistContext = await fetchArtistContext( + payload.githubRepo, payload.artistSlug, fetchGithubFile, + ); + audienceContext = await fetchAudienceContext( + payload.githubRepo, payload.artistSlug, fetchGithubFile, + ); + + logStep("Resolving caption", true, { captionLength: payload.captionLength }); + const captionTopic = [ + `Song: "${audioClip.songTitle}"`, + audioClip.clipLyrics ? `Lyrics: "${audioClip.clipLyrics}"` : null, + audioClip.clipMood ? `Mood: ${audioClip.clipMood}` : null, + artistContext ? `Artist: ${artistContext}` : null, + audienceContext ? `Audience: ${audienceContext}` : null, + ].filter(Boolean).join(". "); + + captionText = await resolveCaptionText({ + captionLength: payload.captionLength, + topic: captionTopic, + template: payload.template, + }); + } else { + logStep("Resolving caption", true, { captionLength: payload.captionLength, skipped: true }); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tasks/createContentTask.ts` around lines 118 - 132, The code still calls fetchArtistContext/fetchAudienceContext and builds artistContext/audienceContext even when payload.captionLength defaults to "none", causing unnecessary I/O; change the flow in createContentTask so you check payload.captionLength === "none" (or use a guard like if (payload.captionLength === "none") skip caption resolution early) before invoking fetchArtistContext/fetchAudienceContext and before building captionTopic/logging; only call resolveCaptionText, build captionTopic, and fetch contexts when captionLength !== "none" (references: captionLength, resolveCaptionText, fetchArtistContext, fetchAudienceContext, captionTopic, logStep).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/tasks/createContentTask.ts`:
- Around line 118-132: The code still calls
fetchArtistContext/fetchAudienceContext and builds artistContext/audienceContext
even when payload.captionLength defaults to "none", causing unnecessary I/O;
change the flow in createContentTask so you check payload.captionLength ===
"none" (or use a guard like if (payload.captionLength === "none") skip caption
resolution early) before invoking fetchArtistContext/fetchAudienceContext and
before building captionTopic/logging; only call resolveCaptionText, build
captionTopic, and fetch contexts when captionLength !== "none" (references:
captionLength, resolveCaptionText, fetchArtistContext, fetchAudienceContext,
captionTopic, logStep).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4f572683-a2ea-46d1-bde8-efa71ad0a32d
📒 Files selected for processing (5)
src/content/__tests__/resolveCaptionText.test.tssrc/content/resolveCaptionText.tssrc/schemas/contentCreationSchema.tssrc/tasks/__tests__/createContentTask.test.tssrc/tasks/createContentTask.ts
Summary
Test plan
Generated with Claude Code
Summary by cubic
Skip caption generation when captionLength is "none" and make "none" the default. This reduces API calls and produces captionless videos unless a length is specified.
Refactors
resolveCaptionTexthelper that returns "" for "none" or callsgenerateCaptionfor other lengths.createContentTaskto use the helper; added unit tests for both paths.Migration
Written for commit 7927ca7. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests