26350517 Rabbithole player audit
An audit of cryptograss/rabbithole at commit c1a5fa8 (5 November 2026), asked for by Justin and Skyler in #rabbithole after three fixes in one day turned out to be patches over a missing rule rather than the rule itself. The question for each item is the same: what should always be true here, and what would make it true by construction instead of cleaning up after it isn't.[unverified]
Confidence is marked on each: confirmed means the path was traced or tried in a browser, likely means read but not run, smell means it works today and is standing on the wrong leg.[unverified]
Ranked findings
1. A song ending never plays the next one (confirmed; fixed in rabbithole#12)
Done, the playlist way (#12, block 26,351,0xx): the queue is Webamp's playlist, the chart follows Webamp's current track, the shell keeps no queue. Checked on pickipedia.xyz: Swords seeked to 3s before its end, and Victory came on by itself with its cards, header and URL. While checking it, a sixteenth finding: openWikiPage wired the pane's links twice since 18 Oct (once in wirePaneChrome, once after it), so one click on a pane Listen button loaded its song twice 20ms apart. That is the pair of loads behind item 10's doubled cards. Fixed in the same PR.[unverified]
{{{1}}}[unverified]
What a person sees: the trail button on every song page queues a chain of songs, and the chain never plays past the first one. Add-to-queue did the same, which is part of why it looked like it did nothing.[unverified]
Invariant: when a song ends, the next queued song plays.[unverified]
By construction: make the queue Webamp's own playlist (appendTracks) and swap the chart on onTrackDidChange, so Webamp advances and we follow. The smaller fix is getDuration() in place of length, but that keeps the "guess the end from the clock" pattern.[unverified]
2. Two wiki pages in flight, and the slower one wins (confirmed; fixed in rabbithole#16)
Done (#16, block 26,354,4xx): each request aborts the one before and becomes the newest; a response that is no longer the newest is dropped. The pane records what was asked for apart from what is shown. Tried in a browser with one page held back 2.5 seconds: the later tap wins and stays.[unverified]
pickipedia-demo.html openWikiPage, lines 2826–2874. Each solo change fetches a page and writes it into the pane when it arrives. Nothing cancels an older request or drops its response. Seek across two solos, or hit Cicada Song's markers at 7.7 and 7.8, and two fetches race; the pane ends up showing whichever landed last, and pane.dataset.wikiTitle records it as current, so the right page is then refused as "already here".[unverified]
Invariant: the pane shows the page most recently asked for.[unverified]
By construction: a request counter (or AbortController) in openWikiPage; a response whose number is no longer the newest is discarded.[unverified]
3. Webamp drawn in one place, then moved (confirmed; fixed in rabbithole#13)
Done (#13, block 26,351,3xx), by a different route than the one below: the home position is a stylesheet rule with !important on the window's wrapper, written by the player before Webamp exists, so the first paint is already home and a drag, zoom or rotation has nothing to move; windowLayout alone would not have held through those. The rest hangs off renderWhenReady() as proposed: the move into our container and the ensemble build happen in the render's own task. Checked on pickipedia.xyz, recording the window's position every animation frame from the first frame it exists: one position ever, in both layouts. One correction to the text below: Webamp does not render into the container it is given; it appends #webamp to body and uses the container only to centre its windows, so the move into our container stays, and is now one line after the render. Item 11 goes with this: there is no observer now.[unverified]
Five separate loops poll for Webamp's window and then move it: createEnsembleInsideWebamp (player 397–413), the 500ms and 100ms timers in initWebamp (256–264), pinWebamp and holdWebampStill in the shell (3521–3601), wireVolume (3368) and waitForAudio (3408). Webamp renders itself at its default spot first; our pin lands a tick or more later. That is "webamp is in the wrong place and jumps into position".[unverified]
Invariant: our layout runs once Webamp has rendered, before a frame is painted.[unverified]
By construction: renderWhenReady() already returns a promise that resolves after render. One promise, awaited by everything that needs the window, and no polling. The window's home position belongs in the windowLayout option Webamp takes at construction, not in a MutationObserver that writes it back.[unverified]
4. The lead card's scale fights Packery for the same transform (confirmed)
Player 1081–1107 and 1374. Packery positions every card with an inline transform: translate(). The lead card's scale(1.05) is on the same property, so every layout() wipes it, and fixLeadMusicianTransforms runs on a requestAnimationFrame after every tick to write it back. This is the mechanism behind Maddie's pulse: rabbithole#7 stopped one trigger of it (the re-sort), not the fight.[unverified]
Invariant: Packery owns a card's transform; nothing else writes it.[unverified]
By construction: put the scale on something Packery doesn't touch, such as .musician-card.lead .musician-name, and delete fixLeadMusicianTransforms.[unverified]
5. A pickup re-sorts the cards ten times a second for a weight that never changes (confirmed)
{{{1}}}[unverified]
Invariant: the cards are re-ordered when the computed order changes.[unverified]
By construction: compute the order from the weights every tick (cheap), remember the last order, and touch the DOM only when it differs. rabbithole#7 is near this but compares against the DOM; comparing against the last computed order is the same check without reading the page. Either fix calculatePickupWeight to read pickup or delete the slide.[unverified]
6. Card clicks depend on a timer winning a race (confirmed; fixed in rabbithole#17)
Done (#17, block 26,354,5xx): one delegated click listener on the document; the function and all three timers are gone. Tried in a browser: a card clicked 59ms after the cards first existed opened its page.[unverified]
Shell 3154, 3320, 3627: setTimeout(addMusicianClickHandlers, 500) after a song load, and once at 2000ms. The cards are built on their own timers (100ms, then a poll for Webamp's window). If the cards land after the timer, they are never clickable. Also player 994: updateMusicianCard calls makeMusiciansClickable(), which exists only in index.html; in this shell it is never defined.[unverified]
Invariant: a card is clickable whenever it exists.[unverified]
By construction: one delegated click listener on a stable ancestor (the dock, or document), matching .musician-card. No per-card handlers, no timers.[unverified]
7. A card is rebuilt from scratch to change a class (confirmed)
Player 930–996. updateMusicianCard already sets the name text at 965 and the classes at 953–959, then at 978 rewrites innerHTML anyway, which throws away the card's children. The rabbit icon is then re-created by hand (984–991), and again in resetEnsembleDisplay (2006–2013) "in case the card was fully rebuilt". Any click handler on a child is lost each time.[unverified]
Invariant: a card's children persist; an update changes classes and text only.[unverified]
By construction: delete the innerHTML write; the lines above it already do the job.[unverified]
8. Loading a song starts it, then a 50ms poll stops it (likely)
Player 2050–2085. setTracksToPlay autoplays, so _autoplayGuard polls every 50ms for up to 3s and pauses the first time it sees PLAYING. A listener who presses Play within those 3s gets paused by the guard.[unverified]
Invariant: loading a song never starts audio; only a person or the queue does.[unverified]
By construction: the store subscription at 642 already sees every status change; handle "first PLAYING after this load" there, once, instead of a second poller. Better still, load through the playlist (see 1) so there is no autoplay to undo.[unverified]
9. Pausing clears the cards (likely; design question)
Player 651–657: any status other than PLAYING calls resetEnsembleDisplay, so pausing mid-solo removes the soloist's highlight until the first tick after resume. isLoadingNewSong (2022, 1991) exists only to stop this reset from running during a song load.[unverified]
Invariant: the cards show the arrangement at the playhead, playing or paused.[unverified]
By construction: render from (time, arrangement) only, never from "status changed". That retires resetEnsembleDisplay, isLoadingNewSong, and the rabbit re-adding in both places. Skyler's call whether a paused song should keep its highlight; I think it should.[unverified]
10. One ensemble, built by whoever gets there (confirmed; rabbithole#9)
Player 2093–2101 and 397–413. Every song load removes the ensemble and schedules a rebuild; two loads inside the window both build. Known from today; recorded here for the structural fix below. The embed path (populateEnsembleContainer, 302–327) already does it the right way: one container, contents replaced.[unverified]
11. A new MutationObserver on every song load, never disconnected (smell)
Player 558–604. Each build creates an observer on Webamp's windows and keeps none of the references; after ten song changes there are ten observers writing the same transform back. The shell has a second copy of the same idea for bottom-bar mode (3586–3601).[unverified]
Invariant: one observer per player, or none (see 3).[unverified]
12. Polling for cards that were just appended (smell)
Player 614–627. waitForCardsAndUpdate polls up to 100 times for the first card, which the same function appended synchronously twenty lines earlier. updateEnsembleDisplay(0) can be called directly.[unverified]
13. The song-switch confirm comes after the switch (confirmed by reading)
Shell 4051–4076. With the editor open, loadSelectedSong switches the song first and asks "You have unsaved changes. Switch songs anyway?" afterwards. Cancel leaves the player on the new song and the editor on the old chart.[unverified]
Invariant: nothing switches until the charter agrees.[unverified]
By construction: ask first, then call the original.[unverified]
14. August's section1–section10 keys through the editor (likely; re's open question from Day 7)
EditableTrackData.getTimelineEntries (line 86–93) does parseFloat(timeStr), so a section key becomes NaN in the marker list. The save (trackDataToWikiFormat) writes the working timeline as-is, so an untouched August survives a save; editing or moving one of those markers can write a NaN key.[unverified]
Invariant: the editor edits the timeline the player plays.[unverified]
By construction: resolve section keys to seconds on load, the way processTimelineKeys does, and keep standardSectionLength as the grid.[unverified]
15. Moments don't survive a seek (smell)
Player 1774–1802. Moments fire when the clock is within a second past them; a seek backwards never re-arms the ones already passed, a seek forwards culls them silently.[unverified]
Invariant: a moment fires when the playhead crosses it.[unverified]
By construction: remember the last tick's time, fire every moment in (last, now], and re-arm on seek.[unverified]
Dead or misleading, for the record
- Player 1264–1275:
oldPositionsmeasures every card's bounding box on each solo change and is never read. A forced layout for nothing.[unverified] - Player 1959–1968:
seekTocomputes a percent fromtrackData.duration, which is a 180s placeholder on the built-in songs. Nothing calls it.[unverified] - The console noise Justin saw is upstream, not ours: "Cannot cancel a locked stream" is Webamp's bundled music-metadata cancelling a fetch body it has already locked (vendor bundle line 28943), and the "11 close listeners" warning is the stream polyfill inside the same tag parsing. Both appear on a plain first load in Chromium too. Harmless, and not fixable from our side short of not asking Webamp to parse tags.[unverified]
Three changes that retire most of this
- One
renderEnsemble(time)into a container created once. Cards are created at construction and on song change, then only their classes and text move. Retires 6, 7, 9, 10, 11, 12 and the rabbit re-adding, and turns rabbithole#9 into a deletion.[unverified] - One "Webamp is ready" promise. Await
renderWhenReady()once; everything that needs the window hangs off it; the home position goes inwindowLayout. Retires 3, the two 500/100ms timers, both pin loops and both observers.[unverified] - The queue is Webamp's playlist.
appendTracksplusonTrackDidChange. Retires 1 and 8, and gives add-to-queue something visible to show: Webamp's own playlist window.[unverified]
Four is smaller but worth doing with the first: the lead scale on an inner element, so fixLeadMusicianTransforms and the rAF per tick go away.[unverified]
A correction
In #rabbithole on 5 Nov I said the 100ms ticker keeps running while paused. It doesn't: the store subscription stops it on any status other than PLAYING (player 651–652). I was wrong.[unverified]
After the Vue conversion (rabbithole#18, 6 November 2026)
rabbithole#18 rebuilds the shell and the player as a Vue app: the chart is a function of (song, time), so the findings that were "two things synced by time" stop existing rather than being fixed. By construction in #18: 4 (no Packery, so no transform to fight), 5 (the order is a computed; the DOM is touched only when it differs), 7 (a card's children persist: Vue patches), 8 (a loaded song is paused the first time Webamp reports it playing, from the store subscription, no poll), 9 (the cards render from time, never from "status changed"), 12 (nothing polls for cards). Already done before it: 1, 2, 3, 6, 10, 11. Left: 13, 14, 15, and the editor's own rendering.[unverified]
