feat(library): add Star-Spangled Banner + Ode to Joy starter content - #744
Conversation
Add two more public-domain starter songs alongside Für Elise, wired into _BUILTIN_STARTER_SOURCES so they seed into DLC_DIR/starter/ on first run: - The Star-Spangled Banner (lead) — John Stafford Smith; cleaned the "Unknown" artist / placeholder album, author "Fee[dB]ack". - Ode to Joy (lead/rhythm/bass + drums) — Beethoven. Replaces the raw "Ode to Joy (VST Cover)_The Adicts.feedpak" that was committed to main but never added to the seed list (so it bundled 23 MB of dead weight and never appeared). Fixed metadata (artist Beethoven, year 1824, author "Fee[dB]ack"), and repointed the stem from the 22 MB editor WAV to the byte-identical-render full.ogg (both exactly 85.324 s), shrinking the pack 23.8 MB -> 1.7 MB. Add guard tests asserting every _BUILTIN_STARTER_SOURCES entry has its file committed and that a seed run lands them all — this catches exactly the listed-but-missing (or committed-but-unlisted) mismatch that left Ode to Joy un-seeded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughTwo new starter ChangesStarter Pack Seeding Update
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_builtin_starter_seed.py (1)
216-234: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueDirectory derivation assumes a single shared starter directory.
content_diris derived from the parent of an arbitrary listed entry (next(iter(listed))). If a future_BUILTIN_STARTER_SOURCESentry lives under a different directory, this test only globs one directory and would silently miss unlisted packs elsewhere — exactly the class of bug this guard test is meant to catch.Since the test already documents this assumption ("all sources share this dir"), this is low priority but worth hardening by grouping listed sources by parent directory and globbing each.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_builtin_starter_seed.py` around lines 216 - 234, The starter-pack guard test assumes all entries in _BUILTIN_STARTER_SOURCES share one parent directory, so it can miss stray .feedpak files if that assumption changes. Update test_no_unlisted_starter_pack_on_disk to derive content directories from each listed starter source in server_mod._BUILTIN_STARTER_SOURCES, group them by parent, and glob *.feedpak in every relevant directory instead of using next(iter(listed)); keep the existing unlisted assertion logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/test_builtin_starter_seed.py`:
- Around line 216-234: The starter-pack guard test assumes all entries in
_BUILTIN_STARTER_SOURCES share one parent directory, so it can miss stray
.feedpak files if that assumption changes. Update
test_no_unlisted_starter_pack_on_disk to derive content directories from each
listed starter source in server_mod._BUILTIN_STARTER_SOURCES, group them by
parent, and glob *.feedpak in every relevant directory instead of using
next(iter(listed)); keep the existing unlisted assertion logic unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 86f9ad71-d11c-4956-9094-c8654a3b6e5d
📒 Files selected for processing (5)
content/starter/Ode to Joy (VST Cover)_The Adicts.feedpakcontent/starter/beethoven-ode_to_joy.feedpakcontent/starter/star_spangled_banner.feedpakserver.pytests/test_builtin_starter_seed.py
There was a problem hiding this comment.
Pull request overview
Adds two additional bundled, public-domain starter song packs and ensures they are correctly wired into the one-time seeding mechanism so they appear in DLC_DIR/starter/ on first run, while preventing “bundled but unseeded” starter content regressions.
Changes:
- Extend
_BUILTIN_STARTER_SOURCESto include Star-Spangled Banner and Ode to Joy starter packs. - Add test guards to ensure
_BUILTIN_STARTER_SOURCESand on-diskcontent/starter/*.feedpakstay in sync, and that seeding lands every listed pack.
Reviewed changes
Copilot reviewed 2 out of 5 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
server.py |
Adds the two new starter packs to _BUILTIN_STARTER_SOURCES so they seed on first run. |
tests/test_builtin_starter_seed.py |
Adds coverage to prevent listed↔on-disk mismatches and to validate all listed packs seed successfully. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Adds two more public-domain starter songs alongside Für Elise, wired into
_BUILTIN_STARTER_SOURCESso they seed intoDLC_DIR/starter/on first run.Songs
Unknownartist / placeholder album (John Stafford Smith, The Anacreontic Song, 1814).Ode to Joy (VST Cover)_The Adicts.feedpakthat was committed tomainbut never added to the seed list — so it bundled 23 MB of dead weight into every build and never appeared. Fixed metadata (artist Beethoven, year 1824, authorFee[dB]ack) and repointed the stem from the 22 MB editor WAV to the byte-identical-renderfull.ogg(both exactly 85.324 s, so the chart stays aligned), shrinking the pack 23.8 MB → 1.7 MB.Guards
New tests assert (a) every listed source is present on disk and seeds, and (b) no on-disk
content/starter/*.feedpakis unlisted — the exact list↔file mismatch that left Ode to Joy un-seeded. All 3 packs verified to load through the app'sextract_meta.No seeding-code changes; no desktop change needed (the bundler already globs
content/starter/*.feedpak). Codex preflight: clean.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests