feat: add album-record-store content template#115
Conversation
New template for "album art on vinyl in a record store" content. Includes style guide, caption guide, example captions, video moods, and video movements. No code changes — template data only. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 9 minutes and 15 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds an album-record-store template (style guide, caption guide, examples, video moods, video movements), introduces resolveImageInstruction, adjusts resolveFaceGuide behavior to passthrough attached images, updates generateContentImage logging/error handling, and adds related tests for face-guide and instruction resolution. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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)
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/content/templates/album-record-store/style-guide.json`:
- Line 4: The template currently sets usesFaceGuide to true which conflicts with
the template's "no person in the shot" requirement; change the usesFaceGuide
boolean to false so face-guidance is disabled for this album template (update
the usesFaceGuide property in the template JSON to false and verify any
associated face-guidance flags or validators reference usesFaceGuide rather than
a separate flag).
- Line 5: The JSON key "customInstruction" contains contradictory directives
about text: change the wording to explicitly preserve any text that is part of
the original album art while removing only extraneous
overlays/captions/watermarks added to the image or scene; update the
"customInstruction" value to state “Do NOT alter or remove any text/design that
is part of the album cover art itself; however, remove any added captions,
watermarks, or overlays that are not part of the original artwork,” and keep the
rest of the scene instructions (vinyl on turntable, sleeve placement) unchanged
so callers know exactly what to preserve versus what to strip.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 79581e6e-9df5-4e09-b0a4-7d5977d10b02
📒 Files selected for processing (5)
src/content/templates/album-record-store/caption-guide.jsonsrc/content/templates/album-record-store/references/captions/examples.jsonsrc/content/templates/album-record-store/style-guide.jsonsrc/content/templates/album-record-store/video-moods.jsonsrc/content/templates/album-record-store/video-movements.json
9 reference images of vinyl records in record store settings for fal.ai scene composition guidance. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| "name": "album-record-store", | ||
| "description": "Album cover art displayed in a gritty New York record store — vinyl spinning on a turntable", | ||
| "usesFaceGuide": true, | ||
| "customInstruction": "Place the album cover art from the first image onto a vinyl record sleeve and display it prominently in the scene. The album art should be clearly visible — propped up on a shelf, leaning against a crate, or displayed on the counter next to the turntable. A vinyl record from the same album is spinning on a turntable nearby. Do NOT alter the album art — reproduce it exactly. Remove any text, captions, watermarks, or overlays from the generated scene. The album art itself should retain its original text/design.", |
There was a problem hiding this comment.
YAGNI - is customInstruction used anywhere?
There was a problem hiding this comment.
If no, what else is included, but unused, in this pull request?
Move the images[] check above the usesFaceGuide guard in resolveFaceGuide so album cover art flows through to fal.ai without triggering face-swap logic. Set usesFaceGuide to false for the album-record-store template. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/content/__tests__/resolveFaceGuide.test.ts (1)
41-53: Optionally harden the passthrough test with negative-path assertions.You can also assert that GitHub fetch/upload is not touched in this path, to make branch isolation explicit.
Proposed assertion additions
it("passes attached images through even when usesFaceGuide is false", async () => { vi.mocked(fetchImageFromUrl).mockResolvedValue("https://fal.ai/uploaded.png"); const result = await resolveFaceGuide({ usesFaceGuide: false, images: ["https://example.com/album-cover.png"], githubRepo: "https://github.com/test/repo", artistSlug: "artist", }); expect(fetchImageFromUrl).toHaveBeenCalledWith("https://example.com/album-cover.png"); + expect(fetchGithubFile).not.toHaveBeenCalled(); + expect(fal.storage.upload).not.toHaveBeenCalled(); expect(result).toBe("https://fal.ai/uploaded.png"); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/content/__tests__/resolveFaceGuide.test.ts` around lines 41 - 53, The passthrough test for resolveFaceGuide currently asserts fetchImageFromUrl was called and the returned value is forwarded; add negative-path assertions to ensure GitHub-related code paths are not exercised in this branch by asserting the mocked GitHub fetch/upload helpers (e.g., any mocks for functions like fetchImageFromGithub or uploadImageToGithub used in the module) were not called; keep using vi.mocked(...) to check those mocks were not invoked so the test explicitly isolates the usesFaceGuide=false branch alongside the existing fetchImageFromUrl and result assertions.src/content/resolveFaceGuide.ts (1)
24-30: Update the function contract comment to match the new behavior.The current docstring still reads like
usesFaceGuide: falsealways returnsnull, which is no longer true whenimages[0]is present.Proposed doc update
/** * Resolves the face guide URL for the content pipeline. - * Uses the first image from the images array if provided, - * otherwise fetches face-guide.png from the artist's GitHub repo. + * Uses the first image from the images array if provided (even when usesFaceGuide is false). + * Otherwise, if usesFaceGuide is true, fetches face-guide.png from the artist's GitHub repo. * - * `@returns` fal.ai storage URL, or null if template doesn't use face guide + * `@returns` fal.ai storage URL, or null when no image is provided and face guide is disabled */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/content/resolveFaceGuide.ts` around lines 24 - 30, The function comment for resolveFaceGuide is out of date: it states that usesFaceGuide: false always returns null, but the implementation first checks images?.[0] (imageUrl) and returns fetchImageFromUrl(imageUrl) even when usesFaceGuide is false. Update the docstring for resolveFaceGuide to describe the new behavior: that an attached image (images[0]) is always passed through via fetchImageFromUrl, and only when no image is present does the usesFaceGuide flag determine whether null is returned or face-guide logic runs; reference the imageUrl, images, usesFaceGuide, and fetchImageFromUrl symbols in the updated description.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/content/__tests__/resolveFaceGuide.test.ts`:
- Around line 41-53: The passthrough test for resolveFaceGuide currently asserts
fetchImageFromUrl was called and the returned value is forwarded; add
negative-path assertions to ensure GitHub-related code paths are not exercised
in this branch by asserting the mocked GitHub fetch/upload helpers (e.g., any
mocks for functions like fetchImageFromGithub or uploadImageToGithub used in the
module) were not called; keep using vi.mocked(...) to check those mocks were not
invoked so the test explicitly isolates the usesFaceGuide=false branch alongside
the existing fetchImageFromUrl and result assertions.
In `@src/content/resolveFaceGuide.ts`:
- Around line 24-30: The function comment for resolveFaceGuide is out of date:
it states that usesFaceGuide: false always returns null, but the implementation
first checks images?.[0] (imageUrl) and returns fetchImageFromUrl(imageUrl) even
when usesFaceGuide is false. Update the docstring for resolveFaceGuide to
describe the new behavior: that an attached image (images[0]) is always passed
through via fetchImageFromUrl, and only when no image is present does the
usesFaceGuide flag determine whether null is returned or face-guide logic runs;
reference the imageUrl, images, usesFaceGuide, and fetchImageFromUrl symbols in
the updated description.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1ecabce6-ff8f-4fc3-b29b-31751f61b286
⛔ Files ignored due to path filters (9)
src/content/templates/album-record-store/references/images/ref-01.pngis excluded by!**/*.pngsrc/content/templates/album-record-store/references/images/ref-02.pngis excluded by!**/*.pngsrc/content/templates/album-record-store/references/images/ref-03.pngis excluded by!**/*.pngsrc/content/templates/album-record-store/references/images/ref-04.pngis excluded by!**/*.pngsrc/content/templates/album-record-store/references/images/ref-05.pngis excluded by!**/*.pngsrc/content/templates/album-record-store/references/images/ref-06.pngis excluded by!**/*.pngsrc/content/templates/album-record-store/references/images/ref-07.pngis excluded by!**/*.pngsrc/content/templates/album-record-store/references/images/ref-08.pngis excluded by!**/*.pngsrc/content/templates/album-record-store/references/images/ref-09.pngis excluded by!**/*.png
📒 Files selected for processing (3)
src/content/__tests__/resolveFaceGuide.test.tssrc/content/resolveFaceGuide.tssrc/content/templates/album-record-store/style-guide.json
✅ Files skipped from review due to trivial changes (1)
- src/content/templates/album-record-store/style-guide.json
Log the full error response (status, body, imageUrls, prompt length) when fal.ai rejects the request so we can diagnose 422 errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/content/generateContentImage.ts`:
- Around line 56-62: The current logger.log call in generateContentImage.ts
exposes sensitive data by logging raw imageUrls and the full serialized input;
change the log to avoid printing imageUrls and the full input payload and
instead log safe metadata such as imageCount (imageUrls.length), promptLength
(prompt.length or prompt.slice info), and inputSize
(JSON.stringify(input).length) or flags (hasImages=true) via the existing
logger.log invocation; update the call referencing the logger.log invocation in
generateContentImage.ts so it removes imageUrls and full input serialization and
replaces them with derived metadata only.
- Around line 68-74: The catch block that logs fal.ai failures (where
logger.error is called with model: config.imageModel, status: err.status,
message: err.message, body: JSON.stringify(err.body ?? err).slice(0,1000),
imageUrls) can itself throw when JSON.stringify hits circular structures; change
the body serialization to a safe-serializer pattern: attempt
JSON.stringify(err.body ?? err) inside a try/catch (or use a safeStringify
utility), truncate the result to 1000 chars, and on failure fall back to a
non-throwing representation like String(err) or util.inspect(err) so the
original error is never masked. Ensure this safe serialization is used in the
logger.error call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 93435a80-3fe3-49a4-8926-e94e9d59391e
📒 Files selected for processing (1)
src/content/generateContentImage.ts
Extract resolveImageInstruction() that checks for a customInstruction field in the template's style guide before falling back to the default face-swap or no-face instructions. This allows the album-record-store template to tell fal.ai how to use the album cover + reference images. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update the prompt to explicitly require the provided album cover artwork on both the spinning vinyl record AND displayed nearby, so the model doesn't generate different art on the turntable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
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)
95-103:⚠️ Potential issue | 🟠 MajorInstruction/guide-image mismatch when attached images bypass
usesFaceGuidecheck.
resolveFaceGuide()passes through attached images regardless oftemplate.usesFaceGuide, butresolveImageInstruction()only checks that flag. If an attached image is provided butusesFaceGuideis false, the prompt will resolve toNO_FACE_INSTRUCTIONwhile a face guide image is still submitted to the generation API—creating a conflict between the prompt instruction and the actual inputs.Alignment fix
+import { NO_FACE_INSTRUCTION } from "../content/contentPrompts"; import { resolveImageInstruction } from "../content/resolveImageInstruction"; @@ const instruction = resolveImageInstruction({ styleGuide: template.styleGuide, usesFaceGuide: template.usesFaceGuide, }); + const effectiveFaceGuideUrl = + instruction === NO_FACE_INSTRUCTION ? undefined : faceGuideUrl ?? undefined; @@ let imageUrl = await generateContentImage({ - faceGuideUrl: faceGuideUrl ?? undefined, + faceGuideUrl: effectiveFaceGuideUrl, referenceImagePath, prompt: fullPrompt, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tasks/createContentTask.ts` around lines 95 - 103, The prompt/attachment mismatch comes from passing faceGuideUrl into generateContentImage even when template.usesFaceGuide is false while resolveImageInstruction uses usesFaceGuide to choose NO_FACE_INSTRUCTION; fix by making the behavior consistent: either (A) if template.usesFaceGuide is false, clear/ignore any attached faceGuideUrl before calling generateContentImage, or (B) if an attached face guide exists and should be honored, set usesFaceGuide=true when calling resolveImageInstruction; update the call sites around resolveImageInstruction and generateContentImage so both use the same usesFaceGuide decision (reference resolveImageInstruction, generateContentImage, faceGuideUrl, and template.usesFaceGuide).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/content/resolveImageInstruction.ts`:
- Around line 15-17: The current check returns styleGuide.customInstruction even
when it's only whitespace; update the early-return logic in
resolveImageInstruction to treat whitespace-only strings as empty by trimming
before length check: replace the condition that uses custom and custom.length
with a check like typeof custom === "string" && custom.trim().length > 0 so that
whitespace-only values fall through to the usesFaceGuide ? FACE_SWAP_INSTRUCTION
: NO_FACE_INSTRUCTION fallback.
---
Outside diff comments:
In `@src/tasks/createContentTask.ts`:
- Around line 95-103: The prompt/attachment mismatch comes from passing
faceGuideUrl into generateContentImage even when template.usesFaceGuide is false
while resolveImageInstruction uses usesFaceGuide to choose NO_FACE_INSTRUCTION;
fix by making the behavior consistent: either (A) if template.usesFaceGuide is
false, clear/ignore any attached faceGuideUrl before calling
generateContentImage, or (B) if an attached face guide exists and should be
honored, set usesFaceGuide=true when calling resolveImageInstruction; update the
call sites around resolveImageInstruction and generateContentImage so both use
the same usesFaceGuide decision (reference resolveImageInstruction,
generateContentImage, faceGuideUrl, and template.usesFaceGuide).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 71edae90-0c6d-4af1-9412-bfe0549bf176
📒 Files selected for processing (4)
src/content/__tests__/resolveImageInstruction.test.tssrc/content/resolveImageInstruction.tssrc/content/templates/album-record-store/style-guide.jsonsrc/tasks/createContentTask.ts
✅ Files skipped from review due to trivial changes (1)
- src/content/templates/album-record-store/style-guide.json
- Remove raw imageUrls and serialized input from success logs, log metadata (promptLength, hasFaceGuide, hasReferenceImage) instead - Wrap error body serialization in try/catch to handle circular refs - Clarify customInstruction text-preservation: preserve album art text/design, only suppress extra scene overlays/watermarks - Trim whitespace-only customInstruction values before using them Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
src/content/generateContentImage.ts
Outdated
| num_images: 1, | ||
| }; | ||
|
|
||
| logger.log("Generating image", { |
There was a problem hiding this comment.
Replace logger.log with logStep
| }): string { | ||
| const custom = styleGuide?.customInstruction; | ||
| if (typeof custom === "string" && custom.length > 0) return custom; | ||
| return usesFaceGuide ? FACE_SWAP_INSTRUCTION : NO_FACE_INSTRUCTION; |
There was a problem hiding this comment.
Why aren't we using rhe styleGuide customInstructions here?
src/tasks/createContentTask.ts
Outdated
| const instruction = resolveImageInstruction({ | ||
| styleGuide: template.styleGuide, | ||
| usesFaceGuide: template.usesFaceGuide, | ||
| }); |
There was a problem hiding this comment.
KISS - Why not simply pass the entire template object here?
src/content/generateContentImage.ts
Outdated
| let result; | ||
| try { | ||
| result = await fal.subscribe(config.imageModel, { input, logs: true }); | ||
| } catch (error: unknown) { | ||
| const err = error as Record<string, unknown>; | ||
| let body: string; | ||
| try { | ||
| body = JSON.stringify(err.body ?? err).slice(0, 1000); | ||
| } catch { | ||
| body = String(err).slice(0, 1000); | ||
| } | ||
| logger.error("fal.ai image generation failed", { | ||
| model: config.imageModel, | ||
| status: err.status, | ||
| message: err.message, | ||
| body, | ||
| promptLength: prompt.length, | ||
| }); | ||
| throw error; | ||
| } |
There was a problem hiding this comment.
SRP / OCP
- actual: new subscribe try catch added in generateContentImage function
- required: new lib file for the result try catch.
- Replace logger.log with logStep in generateContentImage for consistency - Simplify resolveImageInstruction to accept full TemplateData instead of destructured fields Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move fal.ai subscribe-with-error-logging into its own falSubscribe module so generateContentImage stays focused on image generation. falSubscribe handles try/catch, safe serialization, and structured error logging for any fal.ai model call. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Test plan
loadTemplate("album-record-store")🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests