-
Notifications
You must be signed in to change notification settings - Fork 2
agent: @U0AJM7X8FBR # Song Filtering for Content Creation PipelineProblemThe co #111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | ||
| import { selectAudioClip } from "../selectAudioClip"; | ||
|
|
||
| // Mock all external dependencies | ||
| vi.mock("@trigger.dev/sdk/v3", () => ({ | ||
| logger: { log: vi.fn() }, | ||
| })); | ||
|
|
||
| vi.mock("../listArtistSongs", () => ({ | ||
| listArtistSongs: vi.fn(), | ||
| parseSongPath: vi.fn((path: string) => ({ repoUrl: null, filePath: path })), | ||
| })); | ||
|
|
||
| vi.mock("../fetchGithubFile", () => ({ | ||
| fetchGithubFile: vi.fn(), | ||
| })); | ||
|
|
||
| vi.mock("../transcribeSong", () => ({ | ||
| transcribeSong: vi.fn(), | ||
| })); | ||
|
|
||
| vi.mock("../analyzeClips", () => ({ | ||
| analyzeClips: vi.fn(), | ||
| })); | ||
|
|
||
| import { listArtistSongs } from "../listArtistSongs"; | ||
| import { fetchGithubFile } from "../fetchGithubFile"; | ||
| import { transcribeSong } from "../transcribeSong"; | ||
| import { analyzeClips } from "../analyzeClips"; | ||
|
|
||
| const mockLyrics = { | ||
| fullLyrics: "test lyrics", | ||
| segments: [], | ||
| }; | ||
|
|
||
| const mockClip = { | ||
| startSeconds: 10, | ||
| lyrics: "verse lyrics", | ||
| reason: "good hook", | ||
| mood: "upbeat", | ||
| hasLyrics: true, | ||
| }; | ||
|
|
||
| const allSongPaths = [ | ||
| "artists/test-artist/songs/adhd/adhd.mp3", | ||
| "artists/test-artist/songs/hiccups/hiccups.mp3", | ||
| "artists/test-artist/songs/junk-drawer/junk-drawer.mp3", | ||
| ]; | ||
|
|
||
| describe("selectAudioClip", () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| vi.mocked(fetchGithubFile).mockResolvedValue(Buffer.from("fake-mp3")); | ||
| vi.mocked(transcribeSong).mockResolvedValue(mockLyrics); | ||
| vi.mocked(analyzeClips).mockResolvedValue([mockClip]); | ||
| }); | ||
|
|
||
| describe("song filtering", () => { | ||
| it("picks from all songs when songs is omitted", async () => { | ||
| vi.mocked(listArtistSongs).mockResolvedValue(allSongPaths); | ||
|
|
||
| await selectAudioClip({ | ||
| githubRepo: "https://github.com/org/repo", | ||
| artistSlug: "test-artist", | ||
| clipDuration: 15, | ||
| lipsync: false, | ||
| }); | ||
|
|
||
| // listArtistSongs was called — all 3 are eligible | ||
| expect(vi.mocked(listArtistSongs)).toHaveBeenCalledOnce(); | ||
| }); | ||
|
|
||
| it("filters to specified songs when songs array is provided", async () => { | ||
| vi.mocked(listArtistSongs).mockResolvedValue(allSongPaths); | ||
|
|
||
| await selectAudioClip({ | ||
| githubRepo: "https://github.com/org/repo", | ||
| artistSlug: "test-artist", | ||
| clipDuration: 15, | ||
| lipsync: false, | ||
| songs: ["adhd"], | ||
| }); | ||
|
|
||
| // Only adhd.mp3 is eligible — transcribeSong should be called with adhd path | ||
| expect(vi.mocked(fetchGithubFile)).toHaveBeenCalledWith( | ||
| expect.any(String), | ||
| "artists/test-artist/songs/adhd/adhd.mp3", | ||
| ); | ||
| }); | ||
|
|
||
| it("throws when none of the specified songs are found", async () => { | ||
| vi.mocked(listArtistSongs).mockResolvedValue(allSongPaths); | ||
|
|
||
| await expect( | ||
| selectAudioClip({ | ||
| githubRepo: "https://github.com/org/repo", | ||
| artistSlug: "test-artist", | ||
| clipDuration: 15, | ||
| lipsync: false, | ||
| songs: ["nonexistent-song"], | ||
| }), | ||
| ).rejects.toThrow("None of the specified songs [nonexistent-song] were found"); | ||
| }); | ||
|
|
||
| it("filters to multiple specified songs", async () => { | ||
| vi.mocked(listArtistSongs).mockResolvedValue(allSongPaths); | ||
|
|
||
| // Should not throw — both adhd and hiccups exist | ||
| await expect( | ||
| selectAudioClip({ | ||
| githubRepo: "https://github.com/org/repo", | ||
| artistSlug: "test-artist", | ||
| clipDuration: 15, | ||
| lipsync: false, | ||
| songs: ["adhd", "hiccups"], | ||
| }), | ||
| ).resolves.toBeDefined(); | ||
| }); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,25 +41,38 @@ export interface SelectedAudioClip { | |
| * @param artistSlug - Artist directory name | ||
| * @param clipDuration - How long the clip should be (seconds) | ||
| * @param lipsync - Whether to prefer clips with lyrics (for lipsync mode) | ||
| * @param songs - Optional list of song slugs to restrict selection to | ||
| * @returns Selected audio clip with all metadata | ||
| */ | ||
| export async function selectAudioClip({ | ||
| githubRepo, | ||
| artistSlug, | ||
| clipDuration, | ||
| lipsync, | ||
| songs, | ||
| }: { | ||
| githubRepo: string; | ||
| artistSlug: string; | ||
| clipDuration: number; | ||
| lipsync: boolean; | ||
| songs?: string[]; | ||
| }): Promise<SelectedAudioClip> { | ||
| // Step 1: List available songs | ||
| const songPaths = await listArtistSongs(githubRepo, artistSlug); | ||
| let songPaths = await listArtistSongs(githubRepo, artistSlug); | ||
| if (songPaths.length === 0) { | ||
| throw new Error(`No mp3 files found for artist ${artistSlug}`); | ||
| } | ||
|
|
||
| // Filter to allowed songs if specified | ||
| if (songs && songs.length > 0) { | ||
| songPaths = songPaths.filter(path => | ||
| songs.some(slug => path.includes(`/songs/${slug}/`)), | ||
| ); | ||
| if (songPaths.length === 0) { | ||
| throw new Error(`None of the specified songs [${songs.join(", ")}] were found`); | ||
| } | ||
| } | ||
|
Comment on lines
+67
to
+74
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Honor explicit empty Line 67 currently skips filtering when 💡 Suggested fix- if (songs && songs.length > 0) {
- songPaths = songPaths.filter(path =>
- songs.some(slug => path.includes(`/songs/${slug}/`)),
- );
+ if (songs !== undefined) {
+ const requestedSlugs = songs.map(slug => slug.trim()).filter(Boolean);
+ if (requestedSlugs.length === 0) {
+ throw new Error("songs must include at least one non-empty slug when provided");
+ }
+
+ songPaths = songPaths.filter(path =>
+ requestedSlugs.some(slug => path.includes(`/songs/${slug}/`)),
+ );
if (songPaths.length === 0) {
- throw new Error(`None of the specified songs [${songs.join(", ")}] were found`);
+ throw new Error(`None of the specified songs [${requestedSlugs.join(", ")}] were found`);
}
}🤖 Prompt for AI Agents |
||
|
|
||
| // Step 2: Pick a random song | ||
| const encodedPath = songPaths[Math.floor(Math.random() * songPaths.length)]; | ||
| const { repoUrl, filePath: songPath } = parseSongPath(encodedPath); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a regression test for
songs: []semantics.Given Line 67 in
src/content/selectAudioClip.ts, an explicit empty array currently behaves like “no filter.” Please add a test that locks the intended behavior forsongs: []so this edge case doesn’t regress.🤖 Prompt for AI Agents