fix: only select songs from songs/ directory

[sweetmantech]

Summary

• listArtistSongs searched all .mp3 files under artists/{slug}/ which included clips from the content/ directory (e.g. content/social-pack-bedroom-lip-sync/assets/clip_bridge_15s.mp3)
• These are short asset clips, not full songs, causing fal.ai transcription to fail with Bad Request
• Fix: narrow the search prefix to artists/{slug}/songs/

Test plan

• Added unit tests verifying only songs/ mp3s are returned
• Content directory mp3s are excluded

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• Tests

  • Added comprehensive test suite for artist song listing functionality with multiple test cases covering filtering behavior, accurate song detection across different directory structures, and appropriate fallback handling.

• Bug Fixes

  • Refined song detection scanning to target a more specific directory location, ensuring only songs from the designated path are included in the returned results.

[coderabbitai]

📝 Walkthrough

Walkthrough

A test suite was added for the listArtistSongs function that verifies MP3 filtering behavior with mocked GitHub API responses. The function's path scanning logic was refined to search specifically within the songs/ subdirectory rather than the broader artists/ directory.

Changes

Cohort / File(s)	Summary
Test Suite Addition src/content/__tests__/listArtistSongs.test.ts	New Vitest suite with global fetch mocking and two test cases: validating MP3 filtering from artists/<artist>/songs/ paths and handling failures when fetching submodule content.
Path Prefix Refinement src/content/listArtistSongs.ts	Narrowed GitHub tree API path scan from artists/{artistSlug}/ to artists/{artistSlug}/songs/ to filter MP3 files more precisely.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A carrot-loving song finder hops with glee,
Tests now ensure the MP3s flow quite free,
From songs folder deep, we pluck with care,
No stray melodies hiding anywhere!
The paths grow narrow, the logic refined,
A tidier codebase, one hop at a time. 🎵

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and accurately describes the main change: narrowing the search path for songs from 'artists/{slug}/' to 'artists/{slug}/songs/', which is the core fix addressed in this PR.

_{✏️ 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 fix/song-selection-path

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (2)

src/content/__tests__/listArtistSongs.test.ts (2)

24-51: Tests correctly validate the fix. Consider adding coverage for the submodule fallback path.

The current tests verify the core filtering behavior. For more complete coverage, consider adding tests for:

• The parseSongPath helper (also exported)
• Successful org submodule fallback when songs are found there

💡 Example additional test case

it("falls back to org submodule when main repo has no songs", async () => {
  // Main repo has no songs/ mp3s
  mockFetch.mockResolvedValueOnce(makeTreeResponse([]));
  // .gitmodules returns a submodule
  mockFetch.mockResolvedValueOnce({
    ok: true,
    text: () => Promise.resolve("url = https://github.com/org/artist-repo"),
  });
  // Submodule repo has songs
  mockFetch.mockResolvedValueOnce(
    makeTreeResponse(["artists/gatsby-grace/songs/track.mp3"]),
  );

  const songs = await listArtistSongs("https://github.com/org/repo", "gatsby-grace");

  expect(songs).toEqual([
    "__ORG_REPO__https://github.com/org/artist-repo__artists/gatsby-grace/songs/track.mp3",
  ]);
});

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/content/__tests__/listArtistSongs.test.ts` around lines 24 - 51, Add
tests to cover the submodule fallback and parseSongPath helper: create a new
test that mocks fetch to return an empty tree for the main repo, then a
successful .gitmodules response (ok: true with text() returning a submodule
URL), then a tree response from the submodule containing a songs mp3 and assert
listArtistSongs returns the prefixed submodule path (use listArtistSongs and the
same makeTreeResponse helper); also add unit tests for parseSongPath exported
function to validate it parses/filters paths correctly (e.g., accepts
songs/.../*.mp3 and rejects content/ mp3s) so both the fallback path and helper
are covered.

---

7-7: Clean up environment variable after tests to prevent leakage.

Setting process.env.GITHUB_TOKEN at module level without cleanup could affect other test files running in the same process.

♻️ Suggested fix

+const originalToken = process.env.GITHUB_TOKEN;
 process.env.GITHUB_TOKEN = "test-token";
+
+afterAll(() => {
+  process.env.GITHUB_TOKEN = originalToken;
+});

You'll also need to add afterAll to the import:

-import { describe, it, expect, vi, beforeEach } from "vitest";
+import { describe, it, expect, vi, beforeEach, afterAll } from "vitest";

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/content/__tests__/listArtistSongs.test.ts` at line 7, The test currently
sets process.env.GITHUB_TOKEN = "test-token" at module scope and never cleans it
up, so add lifecycle cleanup: save the original value (const _origGithubToken =
process.env.GITHUB_TOKEN) before overwriting, then ensure you import and use
afterAll to restore or delete process.env.GITHUB_TOKEN (e.g., in afterAll
restore _origGithubToken or delete process.env.GITHUB_TOKEN) so the environment
change does not leak to other tests; also add afterAll to the test imports if
your test runner requires explicit import.

🤖 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__/listArtistSongs.test.ts`:
- Around line 24-51: Add tests to cover the submodule fallback and parseSongPath
helper: create a new test that mocks fetch to return an empty tree for the main
repo, then a successful .gitmodules response (ok: true with text() returning a
submodule URL), then a tree response from the submodule containing a songs mp3
and assert listArtistSongs returns the prefixed submodule path (use
listArtistSongs and the same makeTreeResponse helper); also add unit tests for
parseSongPath exported function to validate it parses/filters paths correctly
(e.g., accepts songs/.../*.mp3 and rejects content/ mp3s) so both the fallback
path and helper are covered.
- Line 7: The test currently sets process.env.GITHUB_TOKEN = "test-token" at
module scope and never cleans it up, so add lifecycle cleanup: save the original
value (const _origGithubToken = process.env.GITHUB_TOKEN) before overwriting,
then ensure you import and use afterAll to restore or delete
process.env.GITHUB_TOKEN (e.g., in afterAll restore _origGithubToken or delete
process.env.GITHUB_TOKEN) so the environment change does not leak to other
tests; also add afterAll to the test imports if your test runner requires
explicit import.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 69086aa9-94d4-407e-a360-5e4dfbb86951

📥 Commits

Reviewing files that changed from the base of the PR and between 3bf9da9 and 5b07781.

📒 Files selected for processing (2)

• src/content/__tests__/listArtistSongs.test.ts
• src/content/listArtistSongs.ts