Skip to content

feat: support attached audio/image in content pipeline#116

Merged
sweetmantech merged 5 commits intomainfrom
feature/slack-media-attachments
Mar 31, 2026
Merged

feat: support attached audio/image in content pipeline#116
sweetmantech merged 5 commits intomainfrom
feature/slack-media-attachments

Conversation

@recoup-coding-agent
Copy link
Copy Markdown
Collaborator

@recoup-coding-agent recoup-coding-agent commented Mar 31, 2026

Summary

  • Add attachedAudioUrl and attachedImageUrl optional fields to content creation schema
  • When attachedAudioUrl is provided, download and use that audio instead of selecting from Git
  • When attachedImageUrl is provided, use it as the face guide instead of fetching from GitHub
  • Companion to recoupable/api PR for Slack attachment extraction

Changes

  • New: selectAttachedAudioClip.ts — downloads user-attached audio, transcribes, analyzes clips
  • New: selectAttachedAudioClip.test.ts — 6 unit tests
  • Modified: contentCreationSchema.ts — adds optional URL fields
  • Modified: createContentTask.ts — routes to attached media handlers when URLs present

Test plan

  • 6 tests for selectAttachedAudioClip (all pass)
  • Full suite: 197 tests pass

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Attach public audio URLs: uploaded audio is downloaded, transcribed, and a best clip is selected (lipsync-aware).
    • Attach public images: images can be provided or fetched from a repo and uploaded for use as face guides.
    • Content payloads now accept an optional images array of validated URLs.
  • Tests

    • Added end-to-end tests covering audio/image attachment, selection, and resolver behaviors.

When attachedAudioUrl is provided, the pipeline downloads and uses that
audio instead of selecting a random song from the artist's Git repo.
When attachedImageUrl is provided, it replaces the face-guide.png.

New files:
- selectAttachedAudioClip.ts — downloads, transcribes, and selects clip
- selectAttachedAudioClip.test.ts — 6 tests for attached audio handling

Modified files:
- contentCreationSchema.ts — adds optional attachedAudioUrl/attachedImageUrl
- createContentTask.ts — routes to attached media handlers when URLs present

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 31, 2026

📝 Walkthrough

Walkthrough

Adds URL-based image and audio handling: download/upload image fetch, face-guide resolution, attached-audio downloading/transcription/clip selection, plus schema support for image URLs and task wiring to use the new resolvers. Several Vitest suites added for new behavior.

Changes

Cohort / File(s) Summary
Attached audio selector & tests
src/content/selectAttachedAudioClip.ts, src/content/__tests__/selectAttachedAudioClip.test.ts
New exported selector that fetches an audio URL, converts to Buffer, derives filename/title, calls transcribeSong and analyzeClips, enforces a fallback clip, applies lipsync selection preference, and returns SelectedAudioClip. Comprehensive tests exercise fetch errors, metadata extraction, transcription, and selection.
Audio resolution wrapper & tests
src/content/resolveAudioClip.ts, src/content/__tests__/resolveAudioClip.test.ts
New resolveAudioClip that routes to selectAttachedAudioClip when a songs entry is a URL, otherwise delegates to selectAudioClip. Tests verify routing and call behavior.
Face-guide resolver, image fetcher & tests
src/content/resolveFaceGuide.ts, src/content/fetchImageFromUrl.ts, src/content/__tests__/resolveFaceGuide.test.ts
resolveFaceGuide added to return either null or a fal storage URL, using images[0] via fetchImageFromUrl or fetching face-guide.png from GitHub and uploading; fetchImageFromUrl downloads and uploads an image to fal storage. Tests cover usesFaceGuide=false, image-provided, GitHub fallback, and missing file error.
Task integration
src/tasks/createContentTask.ts
Replaced direct face-guide and audio selection logic with calls to resolveFaceGuide(...) and resolveAudioClip(...); imports updated and downstream steps consume resolved values.
Schema and schema tests
src/schemas/contentCreationSchema.ts, src/schemas/__tests__/contentCreationSchema.test.ts
Schema extended with optional images: string[] (URL-validated). Tests added to assert songs can include URLs/slugs and images validation and ordering; also ensure removed fields (attachedAudioUrl, attachedImageUrl) are not preserved.

Sequence Diagram

sequenceDiagram
    participant Client as Client
    participant Task as createContentTask
    participant Resolver as resolveAudioClip
    participant Selector as selectAttachedAudioClip
    participant Network as HTTP(fetch)
    participant Transcriber as transcribeSong
    participant Analyzer as analyzeClips
    participant Storage as fal.storage

    Client->>Task: submit payload (songs/images, lipsync)
    Task->>Resolver: resolveAudioClip(payload)
    Resolver->>Selector: selectAttachedAudioClip(audioUrl, lipsync)
    Selector->>Network: fetch(audioUrl)
    Network-->>Selector: audio ArrayBuffer
    Selector->>Transcriber: transcribeSong(buffer, filename)
    Transcriber-->>Selector: lyrics & segments
    Selector->>Analyzer: analyzeClips(title, lyrics)
    Analyzer-->>Selector: candidate clips
    Selector->>Selector: choose clip (lipsync logic)
    Selector->>Storage: (if needed) upload clip metadata / notional
    Selector-->>Resolver: SelectedAudioClip
    Resolver-->>Task: SelectedAudioClip
    Task-->>Client: continue content creation
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 I fetched a tune, I fetched a face,
Buffer and bytes in a tidy little place.
I transcribed the chorus, I picked the best clip,
Uploaded a guide with a hop and a skip.
Hooray for attachments — now content can sing! 🎶

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'feat: support attached audio/image in content pipeline' accurately and concisely summarizes the main change: adding support for user-attached audio and image media in the content processing pipeline.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/slack-media-attachments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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/__tests__/selectAttachedAudioClip.test.ts`:
- Around line 24-33: The test sets a global fetch stub in the beforeEach using
vi.stubGlobal("fetch", ...) but never restores it, causing test pollution; add
an afterEach that calls vi.unstubAllGlobals() (or vi.restoreAllMocks() if
preferred) in the same test file to ensure the global stub is cleared after each
test; update the test suite around beforeEach in selectAttachedAudioClip.test.ts
to include afterEach(() => vi.unstubAllGlobals()) so fetch is restored between
tests.

In `@src/content/selectAttachedAudioClip.ts`:
- Around line 25-26: The logger currently prints the full attachedAudioUrl (see
logger.log and attachedAudioUrl in selectAttachedAudioClip.ts); change this to
log a redacted form that excludes query string and fragment (only host and path)
before calling fetch. Parse attachedAudioUrl (e.g., via new
URL(attachedAudioUrl)), build a redactedUrl from url.host + url.pathname (omit
url.search and url.hash), and pass that redactedUrl to logger.log while leaving
the original attachedAudioUrl unchanged for the subsequent fetch call.

In `@src/schemas/contentCreationSchema.ts`:
- Around line 19-22: The attachedAudioUrl and attachedImageUrl zod fields
currently accept any URL, creating SSRF risk; update these validators
(attachedAudioUrl and attachedImageUrl in contentCreationSchema) to only allow
safe public origins by rejecting local/private IPs and non-http(s) schemes
and/or enforcing a trusted hostname allowlist: parse the URL host and scheme in
a refinement or preprocess step (e.g., using new URL(...)) and fail validation
if the hostname is localhost, 127.0.0.1, any RFC1918/169.254/::1/fe80:: ranges,
or if the scheme is not http/https; alternatively apply an explicit allowlist of
trusted domains if available. Ensure the error message clearly indicates why the
URL was rejected.

In `@src/tasks/createContentTask.ts`:
- Around line 86-87: The code is forcing uploaded images to PNG by creating
File([...], "face-guide.png", { type: "image/png" }) which mislabels JPG/WebP
and can break downstream processing; in the createContentTask flow replace that
with logic that detects the actual MIME type from the imageBuffer (e.g., using a
buffer-based MIME detector like file-type.fromBuffer or reading the response
Content-Type), then construct the File with the detected MIME type and a
filename that uses the appropriate extension (e.g., face-guide.jpg,
face-guide.webp, etc.), validate the detected type is an allowed image type and
fallback to a safe default or throw an error if unsupported, and then call
fal.storage.upload(faceGuideFile) with the correctly typed File.
🪄 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: 1bfee268-2c70-4ddb-a423-cc0dc0e31353

📥 Commits

Reviewing files that changed from the base of the PR and between ce0efce and 9e1cef5.

📒 Files selected for processing (4)
  • src/content/__tests__/selectAttachedAudioClip.test.ts
  • src/content/selectAttachedAudioClip.ts
  • src/schemas/contentCreationSchema.ts
  • src/tasks/createContentTask.ts

Comment on lines +25 to +26
logger.log("Downloading attached audio", { url: attachedAudioUrl });
const response = await fetch(attachedAudioUrl);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid logging full attachment URLs.

Full URLs may carry signed tokens or user-identifying query params; log a redacted form (host/path only).

🧹 Suggested redaction
-  logger.log("Downloading attached audio", { url: attachedAudioUrl });
+  const parsedAudioUrl = new URL(attachedAudioUrl);
+  logger.log("Downloading attached audio", {
+    urlHost: parsedAudioUrl.host,
+    urlPath: parsedAudioUrl.pathname,
+  });
📝 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.

Suggested change
logger.log("Downloading attached audio", { url: attachedAudioUrl });
const response = await fetch(attachedAudioUrl);
const parsedAudioUrl = new URL(attachedAudioUrl);
logger.log("Downloading attached audio", {
urlHost: parsedAudioUrl.host,
urlPath: parsedAudioUrl.pathname,
});
const response = await fetch(attachedAudioUrl);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/content/selectAttachedAudioClip.ts` around lines 25 - 26, The logger
currently prints the full attachedAudioUrl (see logger.log and attachedAudioUrl
in selectAttachedAudioClip.ts); change this to log a redacted form that excludes
query string and fragment (only host and path) before calling fetch. Parse
attachedAudioUrl (e.g., via new URL(attachedAudioUrl)), build a redactedUrl from
url.host + url.pathname (omit url.search and url.hash), and pass that
redactedUrl to logger.log while leaving the original attachedAudioUrl unchanged
for the subsequent fetch call.

Comment on lines +19 to +22
/** Public URL of a user-attached audio file to use instead of selecting from Git songs. */
attachedAudioUrl: z.string().url().optional(),
/** Public URL of a user-attached image to use as the face guide instead of the repo face-guide.png. */
attachedImageUrl: z.string().url().optional(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Restrict attached media URLs to safe public origins (SSRF risk).

These fields accept any URL, but they are fetched server-side later. This permits internal/private-network targets unless explicitly blocked.

🔒 Suggested hardening
+const publicMediaUrlSchema = z
+  .string()
+  .url("Must be a valid URL")
+  .refine((raw) => {
+    try {
+      const u = new URL(raw);
+      const host = u.hostname.toLowerCase();
+      if (u.protocol !== "https:") return false;
+      if (host === "localhost" || host === "127.0.0.1" || host === "::1") return false;
+      return true;
+    } catch {
+      return false;
+    }
+  }, "Must be a public HTTPS URL");
+
 export const createContentPayloadSchema = z.object({
   accountId: z.string().min(1, "accountId is required"),
@@
-  attachedAudioUrl: z.string().url().optional(),
+  attachedAudioUrl: publicMediaUrlSchema.optional(),
@@
-  attachedImageUrl: z.string().url().optional(),
+  attachedImageUrl: publicMediaUrlSchema.optional(),
 });
📝 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.

Suggested change
/** Public URL of a user-attached audio file to use instead of selecting from Git songs. */
attachedAudioUrl: z.string().url().optional(),
/** Public URL of a user-attached image to use as the face guide instead of the repo face-guide.png. */
attachedImageUrl: z.string().url().optional(),
const publicMediaUrlSchema = z
.string()
.url("Must be a valid URL")
.refine((raw) => {
try {
const u = new URL(raw);
const host = u.hostname.toLowerCase();
if (u.protocol !== "https:") return false;
if (host === "localhost" || host === "127.0.0.1" || host === "::1") return false;
return true;
} catch {
return false;
}
}, "Must be a public HTTPS URL");
export const createContentPayloadSchema = z.object({
accountId: z.string().min(1, "accountId is required"),
// ... other fields ...
/** Public URL of a user-attached audio file to use instead of selecting from Git songs. */
attachedAudioUrl: publicMediaUrlSchema.optional(),
/** Public URL of a user-attached image to use as the face guide instead of the repo face-guide.png. */
attachedImageUrl: publicMediaUrlSchema.optional(),
// ... other fields ...
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/schemas/contentCreationSchema.ts` around lines 19 - 22, The
attachedAudioUrl and attachedImageUrl zod fields currently accept any URL,
creating SSRF risk; update these validators (attachedAudioUrl and
attachedImageUrl in contentCreationSchema) to only allow safe public origins by
rejecting local/private IPs and non-http(s) schemes and/or enforcing a trusted
hostname allowlist: parse the URL host and scheme in a refinement or preprocess
step (e.g., using new URL(...)) and fail validation if the hostname is
localhost, 127.0.0.1, any RFC1918/169.254/::1/fe80:: ranges, or if the scheme is
not http/https; alternatively apply an explicit allowlist of trusted domains if
available. Ensure the error message clearly indicates why the URL was rejected.

sweetmantech added a commit to recoupable/docs that referenced this pull request Mar 31, 2026
Document the new optional attachment parameters for POST /api/content/create.
These allow callers to provide custom audio and face-guide images that override
the defaults from the artist's GitHub repo.

Companion to recoupable/api#381 and recoupable/tasks#116.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Address CodeRabbit review feedback:
- Add afterEach(vi.unstubAllGlobals) to prevent test pollution
- Preserve actual content type for attached images instead of hardcoding PNG
- Fix fallback clip hasLyrics: false when no lyrics exist

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sweetmantech added a commit to recoupable/docs that referenced this pull request Mar 31, 2026
* docs: add attached_audio_url and attached_image_url to content/create

Document the new optional attachment parameters for POST /api/content/create.
These allow callers to provide custom audio and face-guide images that override
the defaults from the artist's GitHub repo.

Companion to recoupable/api#381 and recoupable/tasks#116.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: implement design feedback — songs accepts URLs, images array replaces attached_image_url

Address sweetmantech's review:
- Extend songs param to accept both slugs and public URLs (no separate attached_audio_url)
- Replace attached_image_url with images array to match songs pattern (KISS)
- Add Create Content to Guides navbar above Create Sandbox

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Align with docs design feedback from sweetmantech:
- songs param now accepts both slugs and public URLs (detected via http prefix)
- New images array replaces attachedImageUrl (KISS — matches songs pattern)
- Remove attachedAudioUrl and attachedImageUrl from schema
- Rename selectAttachedAudioClip param to audioUrl

TDD: 5 new schema tests + updated audio clip tests, 202/202 pass.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
const clipDuration = DEFAULT_PIPELINE_CONFIG.clipDuration;

// Download the attached audio
logger.log("Downloading attached audio", { url: audioUrl });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DRY - use shared logStep instead of logger.log

});
const faceGuideFile = new File([faceGuideBuffer], "face-guide.png", { type: "image/png" });
faceGuideUrl = await fal.storage.upload(faceGuideFile);
logStep("Face-guide uploaded", false, { faceGuideUrl });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OCP

  • actual: lots of inline code added to createContentTask
  • required: new lib to abstract net-new code

Address sweetmantech's review feedback:
- DRY: replace logger.log with shared logStep in selectAttachedAudioClip
- OCP: extract image download+upload into fetchFaceGuideFromUrl lib function
  to keep createContentTask closed for modification

202/202 tests pass.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DRY - how much does this function follow DRY with existing song selection logic?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right — the transcribe → analyze → pick clip → extract lyrics logic is duplicated between selectAudioClip and selectAttachedAudioClip. Extracting a shared pickBestClip helper that both use.

Comment on lines +76 to +93
const imageUrl = payload.images?.[0];
if (imageUrl) {
faceGuideUrl = await fetchFaceGuideFromUrl(imageUrl);
} else {
logStep("Fetching face-guide from GitHub");
const faceGuideBuffer = await fetchGithubFile(
payload.githubRepo,
`artists/${payload.artistSlug}/context/images/face-guide.png`,
);
if (!faceGuideBuffer) {
throw new Error(`face-guide.png not found for artist ${payload.artistSlug}`);
}
logStep("Uploading face-guide to fal.ai storage", true, {
sizeBytes: faceGuideBuffer.byteLength,
});
const faceGuideFile = new File([faceGuideBuffer], "face-guide.png", { type: "image/png" });
faceGuideUrl = await fal.storage.upload(faceGuideFile);
logStep("Face-guide uploaded", false, { faceGuideUrl });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SRP

  • actual: logic for face guide URL generation / retrieval happens within src/tasks/createContentTask.ts
  • required: abstract this logic, for both git and url based face guide url, into a standalone lib file.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — creating resolveFaceGuide.ts to handle both Git-based and URL-based face guide resolution. Task file stays focused on orchestration.

Comment on lines +99 to +110
const songUrl = payload.songs?.find(s => s.startsWith("http"));
let audioClip;
if (songUrl) {
logStep("Using song URL from songs array");
audioClip = await selectAttachedAudioClip({
audioUrl: songUrl,
lipsync: payload.lipsync,
});
} else {
logStep("Selecting audio clip from repo");
audioClip = await selectAudioClip(payload);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SRP

  • actual: logic for audioClip is in src/tasks/createContentTask.ts
  • required: new lib for audioClip logic

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — creating resolveAudioClip.ts to handle the URL-vs-repo branching. Task file will just call resolveAudioClip(payload).

…rename fetchImageFromUrl

Address sweetmantech's review:
- KISS: fetchFaceGuideFromUrl → fetchImageFromUrl (generic name)
- SRP: resolveFaceGuide.ts handles both URL and Git face guide resolution
- SRP: resolveAudioClip.ts handles URL-vs-repo audio clip branching
- createContentTask.ts now just orchestrates: 2 clean function calls

209/209 tests pass.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
src/content/selectAttachedAudioClip.ts (1)

24-26: ⚠️ Potential issue | 🟠 Major

Avoid logging the full URL with potential sensitive query parameters.

The audioUrl may contain signed tokens, user identifiers, or other sensitive data in query parameters. Log only the host and path.

🔒 Suggested fix
   // Download the attached audio
-  logStep("Downloading attached audio", { url: audioUrl });
+  const parsedUrl = new URL(audioUrl);
+  logStep("Downloading attached audio", {
+    host: parsedUrl.host,
+    path: parsedUrl.pathname,
+  });
   const response = await fetch(audioUrl);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/content/selectAttachedAudioClip.ts` around lines 24 - 26, The log
currently outputs the full audioUrl (in selectAttachedAudioClip), which can leak
sensitive query params; instead parse the URL (new URL(audioUrl)) and pass only
the host and pathname (e.g., `${url.host}${url.pathname}` or
`${url.origin}${url.pathname}`) into logStep so logStep("Downloading attached
audio", { url: safeUrl }) is used; keep the fetch(audioUrl) call unchanged and
ensure any try/catch that logs uses the same sanitized form.
🧹 Nitpick comments (3)
src/content/selectAttachedAudioClip.ts (1)

46-54: Minor: Avoid mutating the clips array directly.

Pushing to the array returned by analyzeClips works but is a subtle mutation. Consider using a local fallback instead.

♻️ Suggested fix
-  if (clips.length === 0) {
-    clips.push({
-      startSeconds: 0,
-      lyrics: "",
-      reason: "fallback — no clips analyzed",
-      mood: "unknown",
-      hasLyrics: false,
-    });
-  }
-
-  let selectedClip: SongClip;
-  if (lipsync) {
-    const lyricsClips = clips.filter(c => c.hasLyrics !== false);
-    selectedClip = lyricsClips.length > 0 ? lyricsClips[0] : clips[0];
-  } else {
-    selectedClip = clips[0];
-  }
+  const fallbackClip: SongClip = {
+    startSeconds: 0,
+    lyrics: "",
+    reason: "fallback — no clips analyzed",
+    mood: "unknown",
+    hasLyrics: false,
+  };
+
+  const effectiveClips = clips.length > 0 ? clips : [fallbackClip];
+
+  let selectedClip: SongClip;
+  if (lipsync) {
+    const lyricsClips = effectiveClips.filter(c => c.hasLyrics !== false);
+    selectedClip = lyricsClips.length > 0 ? lyricsClips[0] : effectiveClips[0];
+  } else {
+    selectedClip = effectiveClips[0];
+  }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/content/selectAttachedAudioClip.ts` around lines 46 - 54, The code in
selectAttachedAudioClip currently mutates the clips array returned by
analyzeClips by calling clips.push(...) when no clips are present; instead,
create a local fallbackClips (or fallbackClip) and use that for downstream logic
without modifying the original clips array. Locate the variable named clips in
function selectAttachedAudioClip (the result of analyzeClips) and replace the
push mutation with a conditional assignment that uses fallbackClip(s) only when
clips.length === 0, ensuring subsequent code references the new local variable
rather than mutating the analyzeClips return value.
src/content/fetchImageFromUrl.ts (1)

23-24: Rename misleading variable faceGuideFile to imageFile.

The variable name suggests this is specific to face guides, but this is a generic image upload function.

♻️ Suggested fix
-  const faceGuideFile = new File([new Uint8Array(imageBuffer)], originalName, { type: contentType });
-  const falUrl = await fal.storage.upload(faceGuideFile);
+  const imageFile = new File([new Uint8Array(imageBuffer)], originalName, { type: contentType });
+  const falUrl = await fal.storage.upload(imageFile);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/content/fetchImageFromUrl.ts` around lines 23 - 24, Rename the misleading
local variable faceGuideFile to imageFile inside
src/content/fetchImageFromUrl.ts: replace the declaration const faceGuideFile =
new File([new Uint8Array(imageBuffer)], originalName, { type: contentType });
and any subsequent uses (e.g., the argument passed to fal.storage.upload) so it
becomes const imageFile = new File(...); and await
fal.storage.upload(imageFile); also update any other references in the same
function to use imageFile instead of faceGuideFile.
src/content/__tests__/resolveFaceGuide.test.ts (1)

1-1: Remove unused afterEach import.

The afterEach hook is imported but never used in this test file.

♻️ Suggested fix
-import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
+import { describe, it, expect, vi, beforeEach } from "vitest";
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/content/__tests__/resolveFaceGuide.test.ts` at line 1, The import list in
the test file includes an unused symbol `afterEach`; remove `afterEach` from the
named imports in the top-level import that currently reads `import { describe,
it, expect, vi, beforeEach, afterEach } from "vitest";` so the import only
includes the used helpers (e.g., `describe, it, expect, vi, beforeEach`).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/content/selectAttachedAudioClip.ts`:
- Around line 24-26: The log currently outputs the full audioUrl (in
selectAttachedAudioClip), which can leak sensitive query params; instead parse
the URL (new URL(audioUrl)) and pass only the host and pathname (e.g.,
`${url.host}${url.pathname}` or `${url.origin}${url.pathname}`) into logStep so
logStep("Downloading attached audio", { url: safeUrl }) is used; keep the
fetch(audioUrl) call unchanged and ensure any try/catch that logs uses the same
sanitized form.

---

Nitpick comments:
In `@src/content/__tests__/resolveFaceGuide.test.ts`:
- Line 1: The import list in the test file includes an unused symbol
`afterEach`; remove `afterEach` from the named imports in the top-level import
that currently reads `import { describe, it, expect, vi, beforeEach, afterEach }
from "vitest";` so the import only includes the used helpers (e.g., `describe,
it, expect, vi, beforeEach`).

In `@src/content/fetchImageFromUrl.ts`:
- Around line 23-24: Rename the misleading local variable faceGuideFile to
imageFile inside src/content/fetchImageFromUrl.ts: replace the declaration const
faceGuideFile = new File([new Uint8Array(imageBuffer)], originalName, { type:
contentType }); and any subsequent uses (e.g., the argument passed to
fal.storage.upload) so it becomes const imageFile = new File(...); and await
fal.storage.upload(imageFile); also update any other references in the same
function to use imageFile instead of faceGuideFile.

In `@src/content/selectAttachedAudioClip.ts`:
- Around line 46-54: The code in selectAttachedAudioClip currently mutates the
clips array returned by analyzeClips by calling clips.push(...) when no clips
are present; instead, create a local fallbackClips (or fallbackClip) and use
that for downstream logic without modifying the original clips array. Locate the
variable named clips in function selectAttachedAudioClip (the result of
analyzeClips) and replace the push mutation with a conditional assignment that
uses fallbackClip(s) only when clips.length === 0, ensuring subsequent code
references the new local variable rather than mutating the analyzeClips return
value.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6811fd61-e419-4136-8216-12517932eaa3

📥 Commits

Reviewing files that changed from the base of the PR and between 9e1cef5 and 64c8f03.

📒 Files selected for processing (10)
  • src/content/__tests__/resolveAudioClip.test.ts
  • src/content/__tests__/resolveFaceGuide.test.ts
  • src/content/__tests__/selectAttachedAudioClip.test.ts
  • src/content/fetchImageFromUrl.ts
  • src/content/resolveAudioClip.ts
  • src/content/resolveFaceGuide.ts
  • src/content/selectAttachedAudioClip.ts
  • src/schemas/__tests__/contentCreationSchema.test.ts
  • src/schemas/contentCreationSchema.ts
  • src/tasks/createContentTask.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/schemas/contentCreationSchema.ts
  • src/tasks/createContentTask.ts
  • src/content/tests/selectAttachedAudioClip.test.ts

@sweetmantech sweetmantech merged commit 4d10af6 into main Mar 31, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants