docs(adr): propose ADR-0010 and ADR-0011 — the two halves of offline browse - #53
Open
jeffcrouse wants to merge 4 commits into
Open
docs(adr): propose ADR-0010 and ADR-0011 — the two halves of offline browse#53jeffcrouse wants to merge 4 commits into
jeffcrouse wants to merge 4 commits into
Conversation
…pinned The engine already downloads every track whole before playing it and deletes the file when the next one loads. ADR-0009 recorded keeping those bytes as a follow-up rather than deciding it, because bytes retained because they happened to be fetched are a different promise from a track the listener asked for. This decides that follow-up, and can be rejected without disturbing ADR-0009. The mechanism is nearly free: `ManagedTempFile(url:owned:)` already means "do not delete this one", and what the engine lacks is a say in *where* the download lands. What is not free is the policy — a cache that is never evicted is a slow download of the entire library, while ADR-0009 point 10 promises nothing is evicted automatically. Both hold only if the store keeps two classes of file with two different promises, explicitly, with a test on the boundary. The web client's one recorded bug is exactly that boundary getting blurred: "metadata present, audio absent". Measured rather than assumed, and the measurement deliberately does not settle it. 63% of all bytes ever sent for playback were re-sends — 62 GB of 99 GB — but that is a 175-day aggregate with no ordering, and the only ordered data is two days of `play_events` drawn from the days the Swift client was being developed against this library. Simulated LRU over it gives 5.5% at 0.5 GB and saturates at 13.3%. The ADR claims no hit rate; a follow-up says to re-run the simulation on a month of real use and supersede this rather than implement it if the curve flattens too low. Line citations re-verified against `NativeAudioEngine.swift` today: the four `ManagedTempFile` sites and `cleanupTempFile` had all drifted 30-40 lines since drafting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014P9p2fvFnyiywBxGkv4gfW
…by delta ADR-0009 point 9 limited offline browse to the downloads list and deferred this, which leaves downloads as half a feature: with no network the Apple client can play what it downloaded but cannot navigate to it the way it was found. It also named this as the decision settling whether the `Codable` index becomes SQLite, so it wants deciding before that index acquires dependents. **The premise this was drafted from has since been fixed, and is kept as history rather than deleted.** `libraryCache.ts` passed `limit` to an endpoint that takes `page_size`, so "cache library for offline browsing" cached 50 tracks of 26,396 — and nothing caught it, because a 50-track offline library looks like a feature that works. That was fixed independently in #46 on 2026-07-30. The reasoning survives the fix: the ADR's argument was never "the web client has a bug" but that porting that design would port the shape of the defect, which is the same failure ADR-0012 point 3 rejects for favourites. What the fix removes is urgency, not the case. Re-measured against the live library today rather than trusting the draft: 26,396 active tracks, 132 requests at the `page_size` cap, and **13.2 MB** at 499 B/track — the draft said 12.5 MB, and its ten-field figure did not reconcile with its own per-track number (271 B × 26,396 is 7.1 MB, not 6.8). The `/library/stats` rejection holds exactly: 26,462 vs 26,396 tracks, 3,871 vs 3,925 albums, 3,662 vs 3,475 artists — three notions of "how many albums" and two of "how many tracks", none of them the set a client pages through. The three backend prerequisites are unchanged and still true: `TrackResponse` exposes no `updated_at`, `list_tracks` takes no `since`, and the scanner's upsert `set_` clause never mentions `updated_at` — so a rescan that rewrites a track's tags leaves the cursor untouched, which is a correctness bug on its own merits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014P9p2fvFnyiywBxGkv4gfW
… exists The follow-up explained at length that it was deliberately not linking ADR-0011 because the file was held on an unpushed branch and there was nothing to link to. There is now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014P9p2fvFnyiywBxGkv4gfW
The draft claimed a rescan that changes a track's tags leaves `updated_at` untouched, because the scanner's upsert omits it from an `on_conflict_do_update` `set_` clause SQLAlchemy emits verbatim. The omission was real. The conclusion was not: that upsert is in `_create_track`, the new-track path, whose conflict branch fires only when two scans race on the same `file_path`. An ordinary rescan goes through `_update_track`, which is plain ORM attribute assignment, so `onupdate` applies and the timestamp moves. Caught only by measuring it. The first two attempts reported a false positive, because `expire_on_commit=False` lets the in-memory attribute be older than the row it came from — reading back on a separate connection showed the timestamp advancing exactly when the rescan ran. This makes the ADR cheaper: two backend prerequisites rather than three. The race-path gap was fixed separately, with three tests pinning what a delta rests on — a retag moves the cursor, an unchanged rescan does not, and the conflict branch moves it. Recorded rather than quietly edited, per rule 5. A premise that made the work look *larger* is worth writing down for the same reason as one that made it look smaller: the next person to read `scanner.py` will see the same `set_` clause and reach the same wrong conclusion. Also corrected: the clause lists 22 columns, not 23. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014P9p2fvFnyiywBxGkv4gfW
This was referenced Jul 31, 2026
jeffcrouse
added a commit
that referenced
this pull request
Aug 1, 2026
…ay mark (#57) Every web play was landing with a completion ratio of almost exactly 0.5, whatever the listener actually heard. `usePlayTracking` delivered the play the moment listening crossed `min(duration / 2, 4 min)` and sent `completion_ratio` as measured *at that instant*. Nothing revised it afterwards, so the figure recorded when someone heard a track once through and when they heard half of it were the same number. Measured on the live database, 2026-08-01: **289 of 357 completed events sit in the 0.5–0.6 bucket**, while rows from the native client — which reports at the end — correctly read 0.95–1.00. Sample rows are exact: 166.0/331.4, 124.3/248.6, 120.6/241.0. This matters because of what the data is for. ADR-0004 exists so ADR-0005's recommender has a taste signal that can only accumulate in wall-clock time, and completion is that signal. A constant carries none of it, so the month of data `familiar` #53 is parked waiting for would have been a month of noise — and the error is invisible in aggregate, because 0.5 is a thoroughly plausible average. **Nothing changes about which plays count.** The early delivery only decided *when*: a play past 30s but short of the half mark was already reported on track change, by the same rule that now reports all of them. The Last.fm half/4-minute threshold turned out to gate nothing, which is why it is gone rather than moved. Two things that came with it: - **A page-hide flush**, because the early delivery was quietly buying durability: close the tab mid-track and the play still counted, since it had already been sent. Reporting at the end would have dropped that silently. Restored on `pagehide` and `visibilitychange`, with the ratio actually heard. Best effort and described as such — a browser tearing down a page owes the request nothing. - **The ratio is clamped.** Played time accumulates from the engine's clock while the duration is track metadata, so a looping or mis-tagged track could send 1.1. The server clamps as well, but agreeing with the native client — which has always clamped — beats leaving every stored figure a question about which end did it. Tests: the three that asserted mid-playback delivery now assert delivery on track change, and two were added — one pinning the ratio of a track heard nearly to the end, one covering the page-hide path and that it cannot double-report. 940 pass. Claude-Session: https://claude.ai/code/session_014P9p2fvFnyiywBxGkv4gfW Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two decisions ADR-0009 explicitly deferred, proposed together because they are a pair: 0010 decides
what happens to bytes the engine already fetched, 0011 decides whether the library can be browsed
without a server. ADR-0012's offline follow-up is blocked on 0011, which is what brought them up
now.
They are separately approvable. 0010 can be rejected without disturbing anything; 0011 has real
prerequisites.
ADR-0010 — played bytes are cached, downloads are pinned
The engine already downloads every track whole before playing it and deletes it when the next one
loads. Keeping those bytes is nearly free —
ManagedTempFile(url:owned:)already means "do notdelete this one"; what the engine lacks is a say in where the download lands.
The policy is not free, and that is why it is a decision rather than a patch. A cache that is never
evicted is a slow download of the entire library, while ADR-0009 point 10 promises nothing is
evicted automatically. Both hold only if the store keeps two classes of file with two different
promises, explicitly, with a test on the boundary — which is precisely the boundary the web client
already got wrong once ("metadata present, audio absent").
The measurement deliberately does not settle it. 63% of all bytes ever sent for playback were
re-sends (62 GB of 99 GB) — but that is a 175-day aggregate with no ordering, and the only ordered
data is two days of
play_eventsdrawn from the days the Swift client was being developedagainst this library. Simulated LRU gives 5.5% at 0.5 GB, saturating at 13.3%. The ADR claims no hit
rate, and a follow-up says to re-run the simulation on a month of real use and supersede rather
than implement if the curve flattens too low.
ADR-0011 — the library is cached whole, refreshed by delta
Caches all 26,396 tracks (13.2 MB, 132 requests), keeps albums and artists as the server returns them
rather than deriving them, stores files rather than SQLite — which is the answer ADR-0009 deferred —
and refreshes by delta behind a purpose-built fingerprint endpoint.
One premise died and is kept as history rather than deleted.
libraryCache.tspassedlimittoan endpoint that takes
page_size, so "cache library for offline browsing" cached 50 tracks of26,396. That was fixed independently in #46 the day after this was drafted. The reasoning survives:
the argument was never "the web client has a bug" but that porting that design would port the shape
of the defect — a cache that silently holds part of a collection is indistinguishable from a working
one, which is the same failure ADR-0012 point 3 rejects for favourites.
Verification before proposing
Re-measured against the live library and re-checked against current code today, not trusted from the
draft:
page_sizecap, 13.2 MB at 499 B/track — the draftsaid 12.5 MB, and its ten-field figure did not reconcile with its own per-track number
(271 B × 26,396 is 7.1 MB, not 6.8).
/library/statsrejection holds exactly: 26,462 vs 26,396 tracks, 3,871 vs 3,925 albums,3,662 vs 3,475 artists.
updated_atonTrackResponse, nosinceonlist_tracks, and noupdated_atanywhere in the scanner — so a rescan that rewrites a track'stags leaves the cursor untouched. That last one is a correctness bug worth fixing on its own
merits, whatever happens to this ADR.
🤖 Generated with Claude Code
https://claude.ai/code/session_014P9p2fvFnyiywBxGkv4gfW