CodeRabbit caught two real bugs in the first pass. Both are mine.
1. GRID RESIZE. perRow and rows were captured once when the list was filled, but
paint() also runs on resize — and resizing changes the grid's column count.
The window maths then sliced against the OLD column count: wrong songs on
screen, and padding sized for a row count the layout no longer had (so the
scrollbar lied). metrics() now recomputes perRow/itemH/rows together on every
paint, so the geometry can never disagree with itself.
2. STALE WINDOWS ON SHOW/HIDE. paint() only ran on scroll and resize. Expanding
or collapsing any section moves every list below it, and a windowed list's
contents are a function of its POSITION — so those lists kept the window from
their old position and showed blank padding where songs should be until the
user happened to scroll. Both toggles now call _repaintVirtualLists().
Re-opening an already-populated section had the same flaw.
Collapsed lists also kept doing layout work on every scroll tick. paint() now
bails early when the list is display:none or detached, and forgets its last
window so re-showing repaints from scratch instead of short-circuiting on a
stale memo.
Tests: grid re-window on a column-count change, the padding+rendered=rows
invariant at two different perRow values, and a test that PINS THE FAILURE MODE —
a mismatched perRow/rows pair must not silently look correct. 12/12.
Re-validated the DOM glue in real Chromium with 50k rows (25-31 rows rendered,
scroll height exact). eslint clean; JS 1189/1189; pytest 2597 passed.
CHANGELOG entry added (also flagged).
A song list rendered EVERY song it held. On a flat 50,944-song library that is
one <div> with 50,938 children and ~1,300,000 DOM nodes — ~4.2 GB of renderer
RSS, for a screen the user may not even be looking at (it was built while the
visible screen was v3-home).
It is not just this plugin's problem. A million-node document poisons unrelated
code: any `document.querySelector` that MISSES has to walk the whole tree before
returning null. That is exactly how song_preview's per-frame menu check ended up
consuming ~50% of the renderer and dropping the app to 2.7 fps
(feedBack-plugin-song-preview#7 fixes the per-frame walk; this fixes the tree it
was walking).
So render only what is on screen. Rows are uniform height (grid cards uniform
size), so the window is pure arithmetic — no per-row observers. Off-window songs
are represented by padding ON THE LIST rather than spacer elements: a spacer div
would become a grid ITEM in grid view and shift the columns, whereas padding
behaves identically in both layouts. Lists at or below VIRTUAL_MIN (200) render
in full exactly as before, so normal folders are untouched.
Two ordering fixes this forced, both real bugs waiting to happen:
- Both expand handlers populated the list BEFORE showing it. A windowed list
measures a real row and the scroller viewport, and both are zero under
display:none. Show first, then populate.
- _render() now tears down the previous render's scroll listeners. Without it
they survive against detached nodes and leak on every re-render.
Verified in real Chromium over CDP with 50,000 rows — the DOM glue, not just the
maths:
at top rendered= 25 rows scrollHeight=2,200,000px [0..24]
scroll 500k rendered= 31 rows scrollHeight=2,200,000px [11357..11387]
scroll 1,100k rendered= 31 rows scrollHeight=2,200,000px [24994..25024]
scroll to end rendered= 25 rows scrollHeight=2,200,000px [49975..49999]
25-31 rows in the DOM instead of 50,000; scroll height exact and constant (the
scrollbar stays honest); the last row lands on song 49,999.
Tests: _visibleWindow is pure and exposed via __test — top/middle/bottom/past-
the-end windows, the grid row-packing case, the padding-plus-rendered-equals-
total invariant that keeps the list from changing height as you scroll, and the
degenerate zero-height case (a list still display:none) falling back to
render-everything rather than to an empty list. eslint clean; full JS suite
1186/1186.
`Path.resolve()` is a filesystem call — it lstats every component of the path.
`_resolve_dlc_path` and `safe_join` both re-resolved their ROOT on every single
call, and those run once per song, per art fetch, per scanned row.
Found while profiling a 2-fps report: on a real 50,944-song library the server
was issuing ~23,500 stat/lstat calls per second, re-walking the same three
parent directories over and over, and burning ~50% of a core doing it. It is
worst exactly where big libraries live — the library was on an NTFS-3G (FUSE)
mount, where every stat is a userspace round trip through mount.ntfs-3g (itself
visible in top). The cost was the constant re-resolution, not the work.
A root is fixed for the life of the process, so resolve it once
(safepath.resolved_root, lru_cache). Measured, 5,000 lookups against a real
library path:
before: 15,264 stat syscalls (54.2 ms)
after: 277 stat syscalls ( 0.8 ms) 55x fewer
Containment is unchanged, which is the part that matters:
- safe_join still resolves the CANDIDATE on every call — following its
symlinks IS the zip-slip / traversal defence, so it is never cached. Only
the server-owned root is.
- _resolve_dlc_path keeps its lexical containment check (deliberately does not
follow symlinks, so junction-mounted libraries keep working).
Tradeoff, documented on resolved_root: if a root's symlink is re-pointed at a
NEW target while the server runs, the old target stays in effect until restart.
Fine for a library path fixed at startup; the cache is keyed on the Path, so
switching library dir is a different key.
Tests: root resolved once across 500 lookups (the regression), a different root
is a different entry, and the containment contract re-pinned — traversal,
Windows drive-absolute, backslash, NUL, empty, and a symlink escaping the root
must still be refused. Full suite green.
The Velvet Room (50 stars) now ships in every build like the bar and
arena: 4 reactive crowd loops, 2 stingers, and a balcony flyover intro
rendered from the AXA Music Stage scene (110 spectators, state-scaled
stage washes over the venue's own neon). Audio files are dive-bar
placeholders until club-scale recordings land.
The installed/delete test now asserts bundled-fallback semantics:
with every venue bundled, deleting a downloaded pack reveals the
bundled copy instead of uninstalling.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
CodeRabbit follow-up on #961: presence checks alone would pass with
swapped loop filenames; assert the full loops/stingers/sfx objects.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Feedback Arena (150 stars) now ships in every build like the bar:
4 reactive crowd loops, 2 stingers, and a flyover intro rendered
from the UE5 arena scene (200 spectators + 396-body intro fill,
state-reactive rig lighting). Served by the existing bundled-pack
fallback; venues.json unchanged. Audio files are dive-bar
placeholders until arena-scale recordings land.
Largest file is 89MB — future re-renders must stay under GitHub's
100MB hard limit.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* feat(career): Gold tier — a family-style goldImprov artifact upgrades an earned badge
The drill-state relay's goldImprov map (virtuoso gold_improv mints,
gained-only merged like drill nodes) turns an earned badge gold when the
passport's genre — or its genre family — has a verified improv artifact.
Gold never substitutes for the badge bar: gold-without-bronze stays
in_progress.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(career): Gold tier frontend — relay, ceremony, slam, gold ink everywhere
The drill-state relay now carries virtuoso's goldImprov map; a badge that
comes back gold gets its own ceremony + notification (tier-suffixed seen
ids — the bronze moment stays seen under its legacy id, a gold slam marks
both), a gold stamp slam in the book, gold ink on the shelf-cover mini
stamp, and the real gold foil chip. The bronze page's dashed 'Gold rung
coming' preview becomes a live invitation to jam the style in Virtuoso.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(career): gold review fixes — family-space style matching, intake guards, rail counter
The review's showstopper: virtuoso mints goldImprov under raw
STYLE_PALETTES ids ('punk', 'djent', 'disco'), which are mostly NOT
family keys — the tier check now matches in family space (artifact style
and passport genre bucket through the same _genre_family keyword match),
so a 'punk' gold reaches a 'punk rock' passport. Also: non-dict
goldImprov 400s loudly instead of silently dropping; evidence-free
artifacts (no verifier) never mint; goldImprov gets the same pre-merge
size bound byNode has (junk under the cap could otherwise persist
forever and wedge every later relay at the post-merge check); the
instrument-rail badge counter counts gold (earning gold no longer made a
badge vanish from the rail); first-artifact-wins is now asserted against
the persisted snapshot instead of vacuously.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix: remote transcription posts to /transcribe, not /align (stem-splitter#17)
transcribe_vocals_remote() POSTed the vocal stem to /align. That endpoint is FORCED ALIGNMENT —
"here are the lyrics, tell me when each word is sung" — and its `text` field is required. We have
no lyrics; transcribing them is the entire point. So the server rejected every request with a 422
from FastAPI's validation layer, before its handler ever ran, and remote transcription has never
worked for anyone.
It now posts to /transcribe (added in feedBack-demucs-server#14), which takes only the audio.
`language` moves from the query string to the FORM BODY, where the server actually reads it
(Form("")). As a query param it was silently ignored, so an explicit hint did nothing and
Whisper's auto-detection quietly decided instead — loading the wrong wav2vec2 aligner. It
"worked", it was just wrong, which is the failure mode that hides for months.
Error bodies are no longer cut at 300 chars. The body IS the diagnosis: a 422's JSON names the
field it rejected, a 500's traceback answers on its LAST line. Both got decapitated — which is
part of why this stayed invisible for so long. The message explaining the bug was inside the part
that got cut.
Nothing caught any of this because every test of this module tested the MAPPER, fed a hand-written
dict. The mapper was always fine. The request was never exercised, and the request was the bug.
tests/test_lyrics_transcribe_remote.py now pins it: the endpoint, the form field, the multipart
upload, the bearer token, an instrumental returning no lyrics rather than an error, and a 404
saying the server is too old. Verified they FAIL against /align + params.
Signed-off-by: topkoa <topkoa@gmail.com>
* fix: make the error-body cap an actual bound; correct the docstring's endpoint
- _err_body() appended the truncation marker AFTER slicing to _MAX_ERR_BODY, so the result could
exceed the cap it exists to enforce (4014 chars for a 4000 bound). A cap that is only a
suggestion surprises exactly the callers who trust it — a log line, a job record persisted to
disk and re-read on every load. The marker now fits inside the bound.
It also stripped after measuring, so a short JSON body followed by kilobytes of trailing
whitespace got truncated: real content cut to make room for blanks. Strip first, then measure.
- The public docstring still advertised /align — the exact contract this PR exists to change, in
the one place a reader would look for it. It now says what the function does and why, and that
an older server answers 404.
Found by Copilot and CodeRabbit on #959.
Signed-off-by: topkoa <topkoa@gmail.com>
* fix: keep the exception line when truncating — the tail is the answer
_err_body() kept only the HEAD of an over-long body. On a traceback the last line is the
diagnosis, and the docstring said exactly that while the code threw it away: a 4000-char window
holding "Traceback (most recent call last)" and none of the exception is a window onto nothing.
Same mistake as the 300-char cap it replaced, one level up — cutting off precisely the part the
function exists to preserve.
Head AND tail now, both inside the bound: two thirds head (what was being attempted), one third
tail (what actually went wrong), with the marker between them. Verified the test FAILS against
head-only truncation.
Found by Copilot on #959.
Signed-off-by: topkoa <topkoa@gmail.com>
* fix: every failure out of transcribe_vocals_remote() is a RuntimeError; 404 says why
The docstring promised one failure mode — RuntimeError — and the caller (_maybe_transcribe_lyrics)
catches exactly that so one song's failed lyrics don't take down the batch around it. But a DNS
failure, a timeout, a reset connection or an unreadable stem escaped as requests.RequestException
or OSError, walked straight past that handler, and turned "this song's lyrics failed" into "the
whole batch died".
A 404 now explains itself. Bare "404" sends someone hunting for a typo in their server URL; the
real answer is that their server predates /transcribe, and we are the only ones in a position to
know that.
Found by Copilot on #959.
Signed-off-by: topkoa <topkoa@gmail.com>
---------
Signed-off-by: topkoa <topkoa@gmail.com>
* feat(career): gigs frontend — poster, runner strip, summary, encore
Career v3, WS3 (frontend half), rebuilt cleanly on merged main (v3-a/b/c
in) after git interleaved the structurally-similar canvas functions:
- Book a gig from any opened passport: /gigs/propose renders as a GIG
POSTER (venue presents GENRE NIGHT, numbered bill) with re-roll and
Save/Copy poster (natively-drawn canvas via blob-io, audible failure
paths, slash-safe filenames).
- Play the gig: venue override + Venue viz handoff, then
playQueue.start(..., {source:'gig'}) — the queue's auto-advance runs
the set; zero new playback machinery.
- Floating gig strip (body-level, pointer-events none, z 35 per the
chrome invariant) tracks set position and names what's next.
- Completion = song:ended with an empty queue → POST /gigs → summary
poster overlay with per-song accuracies; encore fires the crowd
celebrate + confetti (reduced-motion: neither). song:stop with a dead
queue = abandoned (no log); end-of-song teardown (queue still active)
must NOT abandon.
- Gigs played render as dated rows in the passport book.
vm tests: runner advance/abandon semantics via the queue-state seam.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(career): stage restore survives a setViz throw; size-register row
CodeRabbit on #956: a setViz failure nulled the restore snapshot AFTER
the overrides were written, permanently borrowing the stage — snapshot
now captured before any write, write failures keep it intact. Also
registers career screen.js in docs/size-exemptions.md 'Planned, not
exempt' (1,516 lines; max-lines WARNS non-blocking — the split plan
needs Byron's sign-off).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs: register career screen.js in the size register (planned, not exempt)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* feat(career): gigs backend — propose a setlist, log the completed set
Career v3, WS3 (backend half). A gig is career's verb:
- POST /gigs/propose {instrument, genre, size}: setlist from the
passport's own stubs — qualifying songs (per the genre's badge bar,
family-aware) shuffled for a free re-roll, topped with the
highest-accuracy near-bar songs as stakes, and filled from UNPLAYED
genre songs when the passport is young (the first gig is how stubs
start). Names the highest venue the current stars can book.
- POST /gigs: logs a COMPLETED set only (abandoned sets never log — no
fail state). Per-song accuracy = MAX(last_accuracy) from song_stats,
freshly written by the set's own plays; encore = avg ≥ the data-driven
bar (passports.json gig.encore_accuracy, 0.75). Appends to the career
state file (same atomic _save_json pattern).
- Passports view: per-passport gigs (newest first, capped 20) and
per-instrument gig_count — the profile wall's gig line lights up.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(career): gig backfill offsets by qualifying taken, not picks length
CodeRabbit on #954: after the stakes loop appends near-bar songs,
qualifying[len(picks):] overshoots and skips eligible qualifying songs
— a stocked passport could still get a short set.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* feat(career): profile passport wall, home career card, shareable PNG card
Career v3, WS2. The identity artifact leaves the plugin tab:
- Profile: #v3-profile-passports-mount (core, one div) filled by career
on v3:profile-rendered — per-instrument shelves of earned covers,
hours, gig count, open-career link. Absent-not-empty.
- Home: the plugin-count stat tile becomes #v3-dash-career-slot with the
old stat as fallback content; career replaces it with a trading-card
tile (leather + foil shine, badge count, hours, closest-stamp ask) on
the existing v3:dashboard-rendered event.
- Shareable card: static/js/blob-io.js (downloadBlob lifts the idiom
duplicated verbatim in settings-io/diagnostics-export — both
refactored; copyImageBlob wraps ClipboardItem, returns false to signal
the download fallback). Earned passports get Save/Copy card: a
natively-drawn 480×640 canvas (leather, stamp ring, stubs+hours line);
copy falls back to download with a notice when the clipboard refuses.
- Mount-point convention documented in docs/plugin-v3-ui.md.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(career): external surfaces stay absent until a passport exists
CodeRabbit on #955: a bare commitment produced a zero-passport wall and
replaced the dashboard fallback. Docs also now say mounts may hold
fallback content and plugins REPLACE, never append.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* feat(career): practice invitations — closest stamps + bring-these-up
Career v3, WS1. The passport now points at the practice that pays:
- Stubs carry next_star_at (the same primitive _stars() uses) and each
passport exposes `nearest`: the top 3 non-qualifying songs by distance
to their next star, in the worklist order.
- "Closest stamps" strip above the shelf: the in-progress graded
passports nearest to minting, each row naming the ask — N more songs
(with the nearest title + best %) or the blocking Virtuoso drill.
Rows open the passport.
- "Bring these up" list on the stubs page of in-progress passports;
earned pages stay memorabilia (no homework on a won badge).
- Invitation-voiced throughout: no meters, no completion pressure.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(test): comment says qualifying-bar ranking, matching the assertion
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The enrichment fallback made the passport rack real (hundreds of MB
sub-genres) but only the five exact umbrella keys carried Virtuoso
drills. Genres now resolve to a family by keyword substring (MB's
vocabulary is open — 'metalcore' must hit metal without an alias),
first-match-wins in list order ('blues rock' → blues), and inherit the
family's requirement from the same genres map. Exact entries still win;
per-instrument scoping unchanged; unmatched genres stay songs-only.
Data: families for metal (incl. djent/grindcore/thrash/doom), blues,
jazz (bebop/swing/bossa), funk (disco), rock (punk/grunge/shoegaze).
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(sloppak): the full mix is a stem — drop the invented `original_audio` key (#933)
Core read, served, and depended on `original_audio:` — a top-level manifest key
this repo invented in #583 that the feedpak spec never defined. The format
already had a home for the pre-separation mixdown: it is a stem. feedpak 1.15.0
(feedpak-spec#53) RESERVES the id `full` for it, so read it from there.
The key existed to work around a bug in our own reader. The packer's comment
said so plainly: "we must NOT list the full mix as a playable stem — the player
sums every entry in `stems` and does not gate playback on `default`, so a listed
full mix plays on top of the stems". Faced with a reader that would double the
song, the packer put the mixdown outside `stems` and invented a key to point at
it. The fix belongs in the reader, and that is what this is.
load_song() now partitions the stem list: `full` comes out as
LoadedSloppak.full_mix, the instruments stay in .stems. Nothing that sums stems
or draws one fader per stem can see the mixdown, so retaining it is safe — which
is what lets the packer put it where the format says it goes.
- ws_highway: `song_info` gains full_mix_url / has_full_mix. The old
original_audio_url / has_original_audio remain as deprecated aliases for one
release so an older stems plugin keeps working (#945).
- `stems` on the wire, and stem_ids / stem_count in the library index, are now
INSTRUMENT stems only — a separated pack that retains its mixdown no longer
advertises a bogus "full" chip or an inflated stem count.
- enrichment: fingerprint against the mixdown wherever it lives. This widens
coverage — _song_audio_file() previously returned None for any pack without
the invented key, so fingerprinting silently did nothing for nearly every pack.
- sloppak: `original_audio:` is still READ as a deprecated fallback, because
every pack in the wild carries it and would otherwise lose its pristine mix.
tools/migrate_full_mix_stem.py rewrites those packs into the spec shape
(original/full.ogg -> stems/full.ogg, add the `full` stem at default:off, drop
the key); the fallback and the aliases die with #945.
The spec gate keeps the debt honest: the grandfather entry now tracks #945, and
the gate fails if it goes stale.
Verified: spec gate OK (4/4, incl. ingesting the spec's new example pack that
retains `full`); 2493 python tests, 995 js tests; migrator round-tripped over
real packs from the library and the results pass the spec's reference validator.
* fix(migrate): discover directory-form packs instead of silently skipping them
iter_packs() searched only files, so a directory-form pack (`song.sloppak/`, the
authoring shape) was walked INTO and never yielded — silently missed by a run
that's meant to be exhaustive. Discover suffix-named directories too (yielded
whole, not descended into), and route packs through migrate_pack/verify_pack.
Directory packs are REPORTED as `dir-form-unsupported`, not rewritten in place:
a single-file pack is replaced atomically (a fully-built temp archive swapped in
with one os.replace), but a populated directory can't be swapped that way, so an
interrupted in-place rewrite could leave an authoring pack half-migrated. The
status is a problem status, so it counts against the run's exit code and shows
in the summary — the operator re-packs or migrates it as a `.feedpak` instead of
it vanishing from the report. Addresses a CodeRabbit review finding.
Signed-off-by: Kris Anderson <topkoa@gmail.com>
* fix(migrate): verify requires an explicit `off` on a retained full mix
verify_zip accepted any non-truthy `default` on a multi-stem `full` (missing,
empty, boolean, `false`/`no`/`0`, malformed) as "ok". But core defaults an ABSENT
`default` to True — ON (lib/sloppak.py: `s.get("default", True)`) — and treats an
empty/unrecognized string as ON too, so a migrated-shape pack whose `full` stem
has a missing or blank default beside instrument stems would actually play the
mixdown on open and double the song. verify was certifying that as safe.
Require an explicit normalized `off` beside instrument stems: `on`-ish values are
reported `full-stem-default-on` (actively plays), everything that is not a
normalized `off` is reported `full-stem-default-not-off`. The migrator already
writes the literal `off`, so its own output is unaffected; this also certifies
the pack is in the tool's canonical, most-portable shape. The len>1 gate is kept,
so a sole `full` stem (which IS the audio) is not policed.
Adds parametrized coverage for missing / empty / boolean / off-ish / malformed
defaults, and a sole-full-stem case. Addresses a CodeRabbit review finding.
Signed-off-by: Kris Anderson <topkoa@gmail.com>
---------
Signed-off-by: Kris Anderson <topkoa@gmail.com>
Co-authored-by: Kris Anderson <topkoa@gmail.com>
* fix(sloppak): bound the unpack cache, and add a way to read a song without unpacking it
A tester's sloppak_cache reached 60 GB from an 1800-song library — his entire
library, unpacked, none of it played. Stems are already-compressed audio, so an
unpacked pack is ~1.1x its zip: the cache is a second, DECOMPRESSED copy of every
song it touches. It had no size cap, no LRU, and no cleanup of any kind — not even
when the song itself was deleted.
Two halves:
1. resolve_source_dir() now evicts least-recently-used songs to stay under a cap
(FEEDBACK_SLOPPAK_CACHE_MAX_MB, default 4 GB ≈ 130 songs of recency; 0 disables).
The sweep runs on unpack — the only moment the cache grows — so it can't drift.
An evicted song is dropped from _source_cache too: get_cached_source_dir() is
the only thing media.py consults before falling back, so a stale path there
would 404 every stem for the rest of the process instead of re-unpacking.
get_cached_source_dir() now also verifies the dir still exists, which makes
"just delete sloppak_cache/ to reclaim disk" safe advice.
2. read_member_bytes() reads ONE file out of a pack without unpacking it — the
same trick read_cover_bytes() uses so the library grid doesn't explode every
pack to show a cover. Unpacking a whole song to read a few KB of JSON is ~45x
write amplification; doing it in a loop over the library is what produced the
60 GB. rig_builder's library-wide tone batch is the caller that did exactly
that (fixed separately); this gives it, and everyone else, the right primitive.
Eviction is concurrency-safe: unpacks run 2-at-a-time, so a dir being written is
marked in-flight and the sweep skips it — checked and rmtree'd under one hold of
the guard, and the marker is released even if the unpack raises (a leaked marker
would make that dir permanently un-evictable).
read_member_bytes normalizes both the requested path AND the archive's stored
member names through safe_join, taking the last match — so './arrangements/x.json',
backslash members from Windows tooling, and duplicate members that normalize to
the same path all read back exactly as unpack-then-read did. Zip-slip is rejected
before anything is opened.
Tests: tests/test_sloppak_unpack_cache.py. All bite-tested (reverted each fix,
watched it fail) — including one that was passing vacuously: a freshly-unpacked
dir is the most-recently-used, so the LRU never reaches it and the in-flight race
test proved nothing until the packs were sized to force the sweep that far.
* test: split semicolon-joined statements (E702)
CodeRabbit on #950. Style only; no behaviour change.
* feat(library): effective genre falls back to MusicBrainz enrichment
Converted packs rarely carry a genres manifest key — on Byron's real
library 1188/1190 songs had no genre, starving the genre facet and the
career passport rack (2 usable genres) while song_enrichment already
held MB genres for 636 matched songs.
The effective-genre expression now resolves: per-song override → pack
genre → json_extract(enrichment.genres, '$[0]') for MATCHED rows only
(review/failed candidates could carry the wrong recording's genres).
Same fast-path gating as before: the plain indexed column is used
unless overrides or enrichment genres actually exist; stand-in DBs
without the table degrade via the OperationalError guard. Career
passports pick this up automatically through _effective_genre_expr().
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs: changelog names manual rows in the enrichment fallback
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
CodeRabbit on #947: the prior lastPlayPosition POST already stamped
last_played_at, so the assertion passed even if the seconds-only path
left it unchanged — a guard that cannot fail. Assert on a fresh row.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
CodeRabbit follow-up on #942:
- add_play_seconds() now stamps last_played_at (like touch_position):
an unscored play that ran to the natural end WAS played — recent /
Continue ordering must see it. Resume position stays untouched.
- stats-recorder post() treats non-2xx as failure: a 4xx/5xx JSON error
body parsed as an object read as success, silently dropping the
accrued seconds instead of re-queuing them.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* feat(career): passport visuals pack — tilt, emerging ink, gold foil
Career v2, WS4. All CSS + a rAF-throttled pointer handler, no deps:
- Trading-card tilt on EARNED artifacts only (shelf covers + the badge
page stamp): pointer-tracked perspective rotateX/Y with a glint sweep
following the pointer. The cover's jitter rotation moves into a CSS
var so the tilt transform composes with it; the blanket cover-hover
translate is scoped :not(.pp-tilt) so it can't fight the tilt.
Hover-capable pointers only; off under prefers-reduced-motion.
- Emerging-stamp ink: the ghost stamp fills by qualifying/required via
a conic-gradient — the stamp visibly carves in, no numbers added.
- Gold foil preview: the "coming" note gains a dashed foil chip with a
periodic shimmer — honest, never earnable-looking.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(career): tilt rAF race + freshly-slammed stamp becomes a card
CodeRabbit on #944: a queued tilt frame closed over the departed card
and re-applied vars after pointerleave; and the just-slammed stamp
never regained pp-tilt until the next open.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Career v2, WS3. Bronze in blues/rock/metal/funk/jazz now also asks for
the genre's signature Virtuoso drill, data-driven in passports.json:
blues_shuffle, rock_power_backbeat, melodic_metal_gallop,
sixteenth_pocket, vl_shells — with career-side display labels the
passport page renders instead of raw node ids.
- virtuoso_nodes becomes {instrument: [node_ids]} so a keys passport
never demands a guitar drill; a flat list keeps meaning guitar
(virtuoso's content is guitar-first).
- _node_cleared also accepts keysCleared (a top-tier clean pass in one
key — virtuoso's FIRST gained-only artifact). The depth rungs
additionally require a maxed speed tier, too high a bar for Bronze.
- Genres without a curated entry stay songs-only.
Note: pre-release behavior change — v1 passports aren't in any shipped
build, so no earned badge can demote in the wild.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Career v2, WS2. Nothing measured play time before (the achievements
plugin's final-position shortcut double-counts loops and mis-reads
seeks). Now:
- stats-recorder.js accrues WALL-CLOCK seconds across song:play/resume ↔
pause/stop/ended spans (single spans clamp at 2h against suspend
inflation) and piggybacks them as `seconds` on the POSTs it already
sends; failed POSTs restore the accumulator; a session reset flushes
first so time can't re-attribute to the next song/arrangement.
- POST /api/stats accepts optional `seconds` (finite, 0 < s ≤ 6h) on the
scored and position branches, plus a new seconds-only branch for
unscored plays that ran to the natural end — banks time WITHOUT
touching the resume position (song:ended must not overwrite Continue)
and still counts as playing today for the streak.
- song_stats gains additive idempotent `seconds_total`; record_session/
touch_position accrue, new add_play_seconds() for the seconds-only
path; the legacy-encoding stats merge sums seconds across duplicates.
- Passports surface it: "14.2 h in Blues" under the badge stamp and on
the shelf cover sub-line — a true fact that only grows, never a
target or a meter (Stage 5 post-cap, per the career design).
Tests: seconds accrual/validation/seconds-only branch (stats API),
per-instrument-and-genre summing (career), fmtHours formatting (vm).
Full suites: pytest 2480, JS 1165.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Earning a genre badge now stages the full moment (career v2, WS1):
- venue-crowd.js gains a public celebrate(): machine.force('ecstatic')
commits instantly (bypassing STABLE_MS/DWELL_MS; the stamped
lastSwitchAt makes the dwell window HOLD the forced state before the
real perf machine reasserts) + a cheer stinger via the same
_lastStingerAt=-Infinity bypass the end-of-song reaction uses. If a
stinger/intro owns the idle layer, the ecstatic loop is queued via
_pendingLoop exactly like onPerformanceState. No-op without a
manifest/active venue.
- career detectNewBadges() calls badgeCeremony(): crowd first, then a
body-appended full-screen overlay 300ms later (it cannot live in
#pp-overlay — #plugin-career is display:none during playback): dimmed
backdrop, the bronze stamp slamming in with a shine sweep, a 42-piece
canvas confetti burst, click-or-4s dismiss.
- prefers-reduced-motion: chime + fbNotify only, no overlay.
Tests: machine.force commit/dwell-hold/bogus-state, celebrate export +
no-manifest no-op, celebrate-called-once-per-badge, crowd-absent and
crowd-throwing degradation.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* chore(career): refresh bar pack to v4 + restore crowd-SFX setting
Pack v4: per-character desynced animation starts, flyover intro,
per-venue reaction sounds (sfx-up/sfx-down in manifest).
Settings: re-add the crowd sound reactions toggle that was dropped
when settings.html became the passports data panel.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(v3): crowd mood-change sound reactions (cheer up / boo down)
Port the venue-crowd SFX runtime that the settings toggle and the
pack's sfx-up/sfx-down files were built for: on a committed mood
transition, play the venue's own cheer (up) or boo (down) one-shot,
gated by the feedBack-venue-crowd-sfx setting (default off).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* chore: rebuild tailwind.min.css for settings toggle classes
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Retitle the toggle "Fullscreen" (from "Start in fullscreen") and reword
the description to "Run fee[dB]ack in fullscreen mode. On macOS, changes
take effect on the next launch."
The macOS note is honest about a native-fullscreen limitation: AppKit
drops the first programmatic fullscreen-enter on a window created
windowed, so on macOS the desktop side applies the pref at next launch
rather than live. Windows/Linux apply it live on the first toggle. The
note is self-scoping text (no platform-detection code needed).
Signed-off-by: gionnibgud <gionnibgud@gmail.com>
Adds a "Start in fullscreen" toggle to the Settings → System panel,
addressing the desktop request in feedBack-desktop#97: users want the
app to launch fullscreen without hitting the OS hotkey every time.
The block ships hidden and is gated exactly like the App-updates block:
setupWindowOptions() only unhides + wires it when the feedBack-desktop
bridge exposes window.feedBackDesktop.window.{getStartFullscreen,
setStartFullscreen}. Web/Docker builds have no such bridge, so the
section never appears there. Persistence lives desktop-side because
only the Electron main process can read the pref at window-creation
time — core just proxies through the bridge.
The desktop bridge + launch behaviour land in a follow-up
feedBack-desktop PR.
Signed-off-by: gionnibgud <gionnibgud@gmail.com>
CodeRabbit follow-up on #936 (the one Major — overlay outside the click
root — was verified false: the host mounts every screen.html root inside
#plugin-career, ✕-close confirmed working live):
- Tabs: aria-selected/aria-controls + role=tabpanel/aria-labelledby.
- Book overlay: role=dialog + aria-modal + aria-label; focus moves to
the close button on open and returns to the opener on close.
- seenBadges(): guard non-object JSON so a corrupt stored value cannot
throw on every passport refresh (covered by a new corruption test).
- Fresh-session suppression test (badge seen → no re-notification).
- Stylelint declaration-empty-line-before nit in .pp-stamp.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The Passports tab beside Venues renders the badge journey physically:
- Per-instrument passport book: embossed CSS-leather cover, 3D page-turn
spread (badge page left, ticket stubs right), Escape/backdrop close.
- Wax-seal commitment ceremony (Stage 0) — pressing the seal commits the
instrument; opening a first passport runs the ceremony implicitly.
- Rubber-stamp badge slam: earned badges chime + notify immediately, the
slam (with ink bleed, page shake, deterministic sin-hash jitter) plays
when the passport is next opened, then the badge is marked seen.
- Ticket-stub repertoire: qualifying songs as collected stubs.
- Brochure rack: unopened genres as "Explore next" invitations — no
greyed slots, no completion meters (the anti-list as layout).
- Drill relay: on virtuoso:progress bus events the career screen posts
the full virtuoso.progress localStorage snapshot to the drill-state
intake (debounced; one-time bootstrap when the server has none).
- Four synthesized sfx (stamp/seal/page/chime, 13 KB total) as plugin
assets; prefers-reduced-motion disables the theatrics.
Pure logic (ppKey, ppJitter, badge diff/seen) is covered by a bare-vm
node --test suite via a window.__careerPassportTest seam.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The badge-journey layer on top of career stars (Christian's career-mode
v2 design, composed with the shipped venue system). Badges are computed
on read from song_stats × the library's effective genre — never stored:
Bronze = N genre songs at min_stars (data-driven in passports.json,
default 5 songs at 2★) plus any configured virtuoso drill nodes.
New endpoints under /api/plugins/career/:
- GET /passports passport walls per instrument: badges, ticket
stubs (qualifying songs), library genres, drills
- POST /passports/commit instrument commitment (idempotent wax seal)
- POST /passports/open open a genre passport (implies commitment)
- POST /drill-state intake for the relayed virtuoso.progress
snapshot (career's frontend listens on the bus)
Instrument attribution reuses progression.instrument_for_arrangement via
the song_stats arrangement index; the genre column goes through the
host's override-aware effective-genre SQL. Non-graded instruments (bass,
drums) render shown-not-judged — repertoire, never a false badge denial.
Persisted state (commitments, opened passports, drill snapshot) lives
under CONFIG_DIR/career/ and rides the settings export bundle via
settings.server_files; a minimal settings.html documents it.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Two review nits: check_allowlist_closed() raised a traceback when
--baseline-exceptions pointed at a missing file (the error now says CI
derives it from the base branch and local runs should omit the flag), and
check_key_coverage() would traceback on a reader with a SyntaxError (now a
::error:: naming the module — belt-and-braces, since such a module can't
pass pytest either, but this job may run first).
Signed-off-by: topkoa <topkoa@gmail.com>
Design change, at the maintainer's direction: the gate now checks out
feedpak-spec at HEAD instead of a pinned SHA. .feedpak-spec-ref, its
40-char validation step, and the pin-bump machinery are gone.
Rationale: it is vital that the app conforms to the spec — the current
spec, not a snapshot. The pin bought determinism at the cost of a
maintenance loop (bump PRs, a PAT, weekly latency) and a window where the
gate verified against a stale spec. Tracking HEAD makes the dev flow fully
self-serve with zero upkeep: gated PR -> FEP -> spec merge -> re-run
checks -> green. Nothing to bump.
The trade-off is accepted with eyes open, and the docs state it: the
normal FEP is additive and can only loosen the gate, so it cannot redden
anyone's PR. Only a breaking spec change (rare, deliberate, MAJOR per the
spec's compatibility policy) turns PRs red repo-wide — which is the
correct org-wide signal that the app is out of conformance. The CI job
logs the spec SHA each run verified against, so any red run is
reproducible.
Failure messages now also say why it matters beyond the one PR (also at
the maintainer's direction): non-conformance that lands shows up as red CI
on every teammate's PR until it is resolved, and only its author can clear
it — the FEP route keeps everyone else unblocked. Tone softened throughout
(the exceptions-file header now explains rather than shouts).
Signed-off-by: topkoa <topkoa@gmail.com>
_parse_exceptions() now validates the document shape — top level must be a
mapping, 'exceptions' must be a list, each entry a mapping, and YAML parse
errors are caught — each failing with a ::error:: instead of an
AttributeError traceback. CI output must say what to fix. Parametrised
tests cover all four malformed shapes.
docs/feedpak-spec-gate.md: the Limitations section still described the
pre-flow-aware scanner (KEY_OPS, name-list-only receivers). Now states the
actual residual gaps: function-parameter manifests are recognised by name
only, and helper-mediated literal keys (song.py's
_gap_fill_manifest_absent(manifest, "album")) are unseen by both the scan
and the readers-complete guard, since they share one detector.
Signed-off-by: topkoa <topkoa@gmail.com>
Review found lib/routers/chart.py binding `m = load_manifest(p) or {}` and
reading eight manifest keys through it. `m` was not in MANIFEST_VARS and
`m.get` did not match the readers-complete regex, so the module was
invisible to BOTH halves of the gate — unlisted and unscanned. Same for
lib/routers/song.py (binds `manifest` from load_manifest for enrichment
gap-fill). Both are now in READERS.
The structural fix, not the name-list patch: keys_touched() now discovers
receivers flow-aware — any local assigned from load_manifest(...) is a
manifest dict, whatever it is called. MANIFEST_VARS remains only as the
fallback for manifests that arrive as function parameters (ws_highway).
A plain `m = {}` is not a receiver; test pins that.
readers-complete now reuses keys_touched() itself instead of a parallel
KEY_OPS regex — the two detectors diverged once already (that is exactly
how chart.py slipped through), so now there is one detector and one truth.
check_reverse() gets a 300s subprocess timeout: the validator executes at a
pinned SHA, but a pathological pack or validator bug should fail the job,
not hang the runner to the Actions-level timeout.
Tests: flow-aware receiver under an arbitrary name (read + write), and the
negative — a plain dict named `m` stays out of the scan. 17 pass.
Signed-off-by: topkoa <topkoa@gmail.com>
The CI gate catches spec drift at merge time; these two additions catch it
at write time, which is where "developer didn't read the spec first"
actually happens.
CLAUDE.md (Song Formats): a spec-is-sacrosanct paragraph next to the spec
pointer — the spec defines the format, the app implements it, any new
manifest key/file/directory lands in the spec first via the FEP process,
and the gate has no in-repo bypass. AI agents and contributors both hit
this while writing feedpak-touching code, not after CI reddens.
.github/pull_request_template.md (new — the repo had only issue templates):
a feedpak-surface section requiring either "doesn't touch pack I/O" or a
link to the landed FEP + the .feedpak-spec-ref bump, plus the standing
changelog/tests/DCO checklist.
Signed-off-by: topkoa <topkoa@gmail.com>
Self-review finding: the gate is what keeps the app from drifting off the
feedpak spec, but the gate itself had zero pytest coverage — a refactor
could quietly weaken keys_touched or the allowlist logic and nothing would
notice. The protector needs protecting.
tests/test_spec_gate.py pins the load-bearing behaviours:
- read/write classification: get() reads; subscript Store and setdefault()
write (the two forms that were blind spots in review); the
load_manifest-wrapped get; unrelated dicts and non-literal keys ignored.
- exceptions file: duplicate keys and issue-less entries rejected.
- the closed allowlist: growth fails, shrink and steady state pass,
bootstrap skips.
- live-tree checks, same as CI: READERS matches the codebase, and the only
non-spec key core touches is the grandfathered original_audio.
Also fixes stale "Layer 2/3" docstrings on check_forward/check_reverse
(they are layers 3/4 since allowlist-closed landed) — the same
docs-lag-the-code class this PR's review kept catching; now the numbering
is asserted by the printed [n/4] headers next to them.
Signed-off-by: topkoa <topkoa@gmail.com>
The allowlist-closed diff hardcoded `origin main`, but ship-ci.yml also runs
this workflow for PRs into release/** and for pushes to release/**, where a
main baseline diffs against the wrong branch and can fail changes that have
nothing to do with the allowlist. It now resolves the base:
PR -> github.event.pull_request.base.ref (the branch it merges into)
push -> github.ref_name (the branch itself; its tip already contains the
change, so the diff is a no-op — enforcement happens at PR time)
Also from review, all documentation drift introduced by my own earlier
commits:
- The layer count said "three" in the module docstring, the workflow comment,
the docs, and the changelog. There are four (allowlist-closed was added).
- The changelog listed three scanned modules; there are five.
- The docs and changelog stated the rule for keys core *reads*, omitting
writes — which are equally gated, and land in every pack we emit.
- The CI summary line labelled grandfathered keys "pending spec", implying
adoption is the only resolution. For original_audio it is not: the fix is
removal. Relabelled "grandfathered (tracked debt)".
- feedpak-spec-exceptions.yml said an entry clears when core "stops reading"
the key; the rule is "no longer reads or writes".
- Replaced a bitwise `&` over two bools with two named results and an
explicit `and` — both checks must run (a stale READERS list and an
undeclared key are separate failures; short-circuiting would hide one), and
`&` reads like a typo.
Signed-off-by: topkoa <topkoa@gmail.com>
Review found the gate was scanning less than it claimed.
READERS missed two modules that genuinely touch feedpak manifests:
lib/routers/ws_highway.py (reads `authors`) and lib/gp2notation.py (loads
manifest.yaml, stamps feedpak_version, writes the file back). Keys touched
there were going entirely unchecked.
The scan also only recognised writes done via subscript, so
`manifest.setdefault("k", v)` — exactly how gp2notation.py stamps
feedpak_version — was invisible. setdefault with a literal key now counts as
a write.
The deeper problem is that READERS is hand-maintained, and a hand-maintained
list rots; that is how both modules went unnoticed. check_readers_complete()
now re-derives the set: any module under lib/ (or server.py) that both
touches manifest keys and shows a feedpak signal must be listed, or the build
fails. It is a guard on the gate itself.
The list stays explicit rather than becoming a glob, because `manifest` is
overloaded here: lib/loosefolder.py (the loose-folder manifest.json) and
lib/diagnostics_bundle.py (the diagnostics bundle manifest) have their own
unrelated manifests, and scanning those would flag *their* keys as feedpak
drift. Both score zero on the feedpak signals, which is what keeps them out.
Now scanning 5 modules: 20 reads, 2 writes, all spec-declared.
Signed-off-by: topkoa <topkoa@gmail.com>
The gate's purpose is to make a non-conforming change *not merge*, so the
person merging must stop and decide whether to take it through the format
process. The escape hatches defeated exactly that: a developer who did not
want to write a FEP could name their key `x-whatever`, or append an entry to
feedpak-spec-exceptions.yml with any issue link, and merge. Both were
self-serve and in-repo. That is a speed bump with a signed excuse note, not a
gate.
The relief valve is the FEP process itself, not something in this repo. The
spec's governance already says so: "A change is not part of the format until
it lands here."
Removed the `x-` prefix bypass. It was invented here, not in the spec — the
spec reserves no experimental namespace. Its "unknown keys are reserved for
forward-compatibility" rule is about *tolerating* other implementations'
keys, not a licence for core to mint its own.
feedpak-spec-exceptions.yml is now a CLOSED grandfather list. A new check
(allowlist-closed) diffs it against the base branch and fails any PR that
ADDS an entry; removal stays allowed, so the list can only shrink. Deleting
an entry does not by itself pass the gate — key-coverage still fails while
core reads the key, so the entry goes when the code goes.
Every failure message now points at the FEP process and at bumping
.feedpak-spec-ref to the merged spec SHA, which is the one supported way a
new manifest key reaches core.
CI fetches the base branch to diff the allowlist; the bootstrap flag covers
the one case with no baseline — the PR introducing the gate.
Signed-off-by: topkoa <topkoa@gmail.com>
Six more findings from CodeRabbit and Copilot on #934. All valid; four were
my own docs lagging the write-checking change in d0626f5.
check_forward() now discovers example packs recursively, so a pack nested
under examples/<group>/ can't slip past the "every example pack" contract.
Taken WITHOUT the suggested is_file() filter, which would have broken it: a
feedpak is dual-form — a zip (foo.feedpak) or a directory (foo.feedpak/) —
and the spec's own examples ship as directories, so is_file() would have
matched zero packs. Suffix matching covers both forms.
load_exceptions() rejects duplicate keys instead of silently keeping the
last one, which would quietly retarget the tracking issue for a piece of
debt this file exists to track.
The sloppak import is wrapped so a missing dependency produces a CI-legible
::error:: rather than a bare traceback.
Docs caught up with the code: the exceptions file header, its stale-entry
rule, and the changelog all said "reads" when the gate checks reads AND
writes.
Signed-off-by: topkoa <topkoa@gmail.com>
Review feedback from CodeRabbit and Copilot on #934. All six findings were
valid; one is fixed the other way round from how it was suggested.
Key-coverage now checks manifest WRITES as well as reads. Copilot correctly
spotted that `ast.walk` ignored subscript context, so
`manifest["year"] = ...` (lib/songmeta.py) scored as a read — but the fix is
not to drop writes. A key core *writes* is spec surface pointed outward: it
lands in every pack we emit, so an undeclared one seeds the ecosystem with
non-spec data. Subscripts are now classified by ctx (Store = write, Load =
read) and both sets are checked, with distinct error messages. Today: 19
reads, 2 writes, all declared.
Workflow:
- persist-credentials: false on the repo checkout — the job runs repository
code and never pushes (CodeRabbit / zizmor artipacked).
- .feedpak-spec-ref must be a full 40-char SHA. actions/checkout resolves
branches and tags in `ref` too, so a non-SHA there would silently un-pin
the spec — precisely what the file exists to prevent.
- Pin jsonschema==4.26.0, for the same reason the spec SHA is pinned: an
upstream release must not redden this job on a PR that changed neither
this repo nor the spec.
Script:
- check_forward() guards a missing examples/ dir instead of raising an
unhandled FileNotFoundError.
- TemporaryDirectory() instead of mkdtemp(), so a local run doesn't leak.
Signed-off-by: topkoa <topkoa@gmail.com>
The spec already carries the pre-separation mixdown as a stem
({id: full, file: stems/full.ogg}), so the key added a second, redundant
location for audio to a format that already had one. Adopting it into the
spec would make that permanent; the resolution in #933 is to remove it.
No behaviour change — the gate is agnostic about which way a violation
resolves, and only insists that one of the two happens deliberately and in
the open before the code merges. This just stops the exception entry, the
changelog, and the docs from presupposing adoption.
Signed-off-by: topkoa <topkoa@gmail.com>
feedpak is published as an open format with its own repo, normative spec,
JSON Schemas, and reference validator. That makes the spec a contract with
everyone outside this repo: third-party packers, converters, and players
build against it, and it is meant to be the complete description of a pack.
Nothing enforced that. #583 added a manifest key (`original_audio`) that
core, lib/enrichment.py, and the stems plugin all now depend on, but which
was never added to the spec — so a spec-compliant pack stopped being a
fully-working pack, the reference validator could not warn authors about a
key it had never heard of, and third-party tooling began emitting an
`original/` directory reverse-engineered from an example in a code comment.
See #933.
We cannot mechanically prove core interprets a key the way the spec means.
We can prove three surface properties, and they cover the drift that
actually happens:
1. key-coverage — every manifest key core reads is declared in the spec's
manifest.schema.json (AST scan of lib/sloppak.py, lib/enrichment.py,
lib/songmeta.py).
2. forward — core's load_song() ingests every example pack the spec ships.
3. reverse — every pack committed here passes the spec's own
tools/validate.py (7/7 pass today).
The spec is pinned by SHA in .feedpak-spec-ref rather than tracked from its
default branch, so a change over there cannot redden an unrelated PR here;
bump it in its own PR, where a red result is precisely the signal that core
does not satisfy the new spec.
A gate with no legitimate way to say "yes, deliberately, not yet" gets
switched off the first time it blocks a release, so there are two escape
hatches: the reserved `x-` key prefix (always permitted, and it tells every
third-party packer the key is not stable surface), and
feedpak-spec-exceptions.yml, which requires a tracking issue per entry. An
exception that goes stale — the spec caught up, or core stopped reading the
key — fails the build, so the allowlist cannot become somewhere drift
quietly accumulates. `original_audio` is seeded there against #933 so the
gate lands green and starts blocking the next instance immediately, rather
than requiring #933 to be resolved first.
Dev/CI tooling only; never on the serve or Docker path (constitution
Principle I). jsonschema is installed in the CI job, not added to
requirements.txt.
Signed-off-by: topkoa <topkoa@gmail.com>
Three more rules, all learned by shipping the bug first. Every one of them
produced a symptom that pointed nowhere near its cause.
RULE 5 REWRITTEN — `isConnected` lies about a panel that is a pane, in BOTH
directions:
- true when the panel is not here (it is in a pane window)
- FALSE when the panel is perfectly fine — the host detaches the element the
moment a pop-out starts, before the new window has loaded
Code that rebuilds on that `false` builds a SECOND panel while the host still
holds the first. Docking brings both home. The one the user can see is the
original, which the module no longer points at — so its close button closes the
other, invisible panel ("the X doesn't work"), and the chip gets re-attached to
the impostor ("the pop-out icon vanished"). Two baffling symptoms, one duplicate,
nothing in the stack trace.
Ask the pane system where the element is (`panes.isOpen(id)`), not the DOM.
RULE 6 (new) — a plugin that can be re-injected must be able to remove itself.
Without a teardown the second run duplicates every observer, timer and listener —
and leaves a stale pane registration, which is worse than untidy: `element` is
resolved LAZILY at open time, so the host gets a node from a dead instance. Pop
out, and it moves a panel nobody owns. Includes the teardown people forget:
panes.unregister().
RULE 1 EXTENDED — panel-internal id lookups. document.getElementById returns null
once the panel has moved, so every update it guards silently stops happening
while the user is looking at the panel. Search from the panel instead. Elements
outside the panel never move and are fine as they are — audit which is which.
Signed-off-by: topkoa <topkoa@gmail.com>
Every rule here is something that has already gone wrong on this feature —
mostly in core's own code, twice in the two plugins that adopted it first.
They are cheap to get right up front and miserable to diagnose later,
because a broken pane almost always LOOKS perfect.
The traps, and why each one is easy to walk into:
- Your code still runs in the main window. That is exactly why moving the
element works at all — and exactly why `document.body.appendChild(tooltip)`
inside a popped-out panel lands in the window the user is NOT looking at.
- Don't hide your own panel when it pops out. Core hides it and leaves a
stub. A plugin that also hides it hides the node that just moved — which is
precisely how core's own chip shipped a blank pop-out window.
- Use `hidden` or a class, not inline `display`, for show/hide. `.fb-paned`
forces the panel visible while it is out; when it docks and that class is
removed, an inline `display:none` reasserts itself and the panel returns
invisible.
- `isConnected` does not mean "docked". A panel in a pane window IS connected,
just not to this document. The test you meant is
`el.ownerDocument === document`.
- `element` is a function so it can be resolved late: return the LIVE node, and
re-attach the chip if you rebuild your panel (Camera Director rebuilds on
every mode change).
- rAF is throttled while the main window is backgrounded — which it is, whenever
the user is looking at your pane. Event-driven panels don't care; continuously
animating ones will stutter exactly when they are the only thing on screen.
- Don't synchronise anything. One realm, one panel. Writing sync code means
you have misunderstood the model.
Also states what core guarantees back, including the one that cost the most to
learn: the element is evacuated BEFORE the pane window's document is destroyed,
so it comes home alive rather than as a photograph of a panel with every
listener in its subtree silently gone.
Signed-off-by: topkoa <topkoa@gmail.com>
