feat(taste): the pick's half of the digest is chosen for the moment it happens in [spec 14] - #283
Merged
Merged
Conversation
…t happens in [spec 14] The flexible half rendered the same rows on every pick of the day: the newest 40 kept songs, whatever the hour, whatever had just played, whatever the listener had said a minute ago. Now those rows are chosen against the ledger for the pick that is happening. Four signals, all already in the Director's hand: the local hour as a bucket word, the persona's own words, the last talk beat, and the avoid-list reduced to artists. Query terms come from recall's exported tokenising, so the CJK bigram problem is solved once and not twice. A row scores 3 for an exact artist or playlist-name match, 2 for a prefix either way, 1 for a term in its title/artist/album, +0.5 for a music sub-zone, -1 when the last read of its own list no longer saw it -- which is how an unliked song fades without the ledger ever deleting. An artist that just played is dropped, by CREDIT rather than by string: a collaboration IS the band they just heard. Relevance decides the order; the budget still decides the length. With no terms every row scores 0, the order is newest first, and the render is exactly what it was -- so an unmatched pick, a source with no ledger and a silent moment all degrade by the same path rather than by a special case. The red lines hold: it runs in code before the situation string is assembled, the brain gets no new tool and no extra call, and the pack keeps the static memoised render -- only `buildMusicSituation` passes a moment. `TasteReader` now splits parsing from rendering, so a moment costs the render alone and never a re-read. Budget: the median of fifteen warmed runs over a 4000-row ledger, under 5 ms. Tokenising every row on every pick was 4.3 ms of that on its own, so rows are scanned rather than tokenised -- same match, no per-row allocation, 2.5 ms. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All from the closing review, all reproduced, a test each. The first one is the whole feature. `runApp` spelled the Director's taste adapter out at the call site and wrote `digest: () => taste.reader.digest()` -- no moment parameter, so every real pick got the static render while the Director tests, which inject a fake that takes the argument, stayed green. Exactly the seam CLAUDE.md warns about: green tests are not a delivered engine. The adapter is built in `buildTaste` beside the reader now, and `buildTaste` is already covered, so the forwarding has a test. - The pick's avoid-list is up to 256 songs over seven days, not three. Handed over whole it deleted a week of artists from the selection and cost 10 ms of the 5 ms budget. The moment takes the last three; the song-level avoid-list keeps its own window. - The candidate pool came from the ledgers alone, so a source whose ledger was missing, unreadable or empty lost its songs from the pick while the Sources line went on counting them. It is per source now. - With no term matching anything, the pool was still reordered by `lastSeen` -- a READ time every row of one refresh shares, so ties fell to insertion order and the block came out in an order the static render would never produce. No match now answers nothing, and the render is byte-identical to the one without a moment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… 14] The budget is 5 ms on the machine murmur runs on, and the test measured it as a flat number everywhere. A shared CI runner is about three times slower -- 2.5 ms here, 8.1 ms there -- so the first CI run failed on hardware, not on a regression. The bound is scaled (25 ms under CI) rather than raised for everyone: what the test is for is a blow-up, a per-row tokenise or an index build, which is an order of magnitude and not a factor of three. A flat wall-clock number that only holds on one class of machine is the flake #269 already costs us, and adding a second one while recording the first would be a poor trade. Both numbers are in the spec. Co-Authored-By: Claude Opus 5 <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
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.
Implements
specs/spec14/14-listening-taste.md§2.12, frozen in #278. PR 3 of 3 — the last of the taste amendment (#278 the budget, #280 the ledger and the per-kind clock).What changes
The flexible half rendered the same rows on every pick of the day: the newest 40 kept songs, whatever the hour, whatever had just played, whatever the listener had said a minute ago. Those rows are now chosen against the ledger for the pick that is actually happening.
Four signals, all already in the Director's hand — no tool, no extra call, no extra read:
morning…late night) joined to the queryScore (highest wins): 3 an exact artist or playlist-name match · 2 a prefix either way · 1 a term in the title/artist/album · +0.5 a music sub-zone · −1 when the last read of that row's own list no longer saw it. That last one is how an unliked song fades without the ledger ever deleting. Ties break by
lastSeen, then the ledger's own order.Relevance decides the order; the budget still decides the length. The rows the moment matched lead, and the rest of the line fills behind them.
The red lines
TasteReader.digest()stays the static, mtime-memoised render the context pack gives talk and steer; onlybuildMusicSituationpasses a moment. The reader splits parsing from rendering, so a moment costs the render alone and never a re-read.Performance
Tokenising every ledger row on every pick cost 4.3 ms of the 5 ms budget on a 4000-row ledger. Rows are scanned instead: title/artist/album lowercased once, then each term tested against it — a latin term at a word boundary, a CJK bigram as a plain substring, which is the same match shingling both sides produces. Same answer, no per-row allocation: 2.5 ms. The query side still uses
recall.ts's exportedqueryTokens(), so the CJK bigram problem is solved once and not twice. Notaste.db, no second FTS index, nonode:sqliteon the pick path — §2.12 records the upgrade path if the assertion ever fails.Tests
pnpm test2045 passed (run twice);tscclean;oxlintclean.New
test/sources-moment.test.ts(the bucket words, the stop list, the 24-term cap, CJK bigrams, the score order, the exclusion, the gone-quiet penalty, determinism), plus the render-level and reader-level tests, the §5.16 acceptance on the full-size fixture, and the 5 ms budget.Peer review (codex gpt-6-astra): 4 findings, 4 applied, 0 dismissed.
runAppspelled the Director's taste adapter out at the call site asdigest: () => taste.reader.digest()— no moment parameter, so every real pick got the static render, while the Director tests injected a fake that does take the argument and stayed green. Exactly the seam CLAUDE.md warns about: green tests are not a delivered engine. The adapter is built inbuildTastebeside the reader now, where the existing coverage reaches it, and a test asserts the forwarding.Sourcesline went on counting them. Per source now.lastSeen— a read time every row of one refresh shares, so ties fell to insertion order and the block came out in an order the static render would never produce. No match now answers nothing.A fifth, found by the acceptance test rather than the review: the exclusion compared the whole artist string, so
Corin Vanterpool & Static Meadowsurvived after Static Meadow had just played. It is matched inside the credit at a word boundary now — a collaboration and afeat.go, a band whose name merely starts the same stays.No listener data
Nothing from
~/.murmuris in this branch: no song, artist, playlist or channel name, no account name, no urls.test/fixtures/tasteis untouched and was already invented. Checked before pushing —git log -p --allover the branch's whole history greps zero for the real titles, names and refs, and the diff adds no non-ASCII beyond typography.AI coding brief
Original request. Make the taste data feeding the music pick accumulate and pick for the moment, as three sequential PRs on a settled design. This is PR 3: the moment-matched half, on decisions fixed in the grilling round before #278 froze the spec — the dynamic half serves the pick and not the pack; an in-memory linear scan reusing
recall.ts's tokenising instead of a second FTS index; the four input signals; name matches above body hits; an unliked song fading bylastSeen; the artist just played excluded; no ledger meaning today's behaviour; 1500 characters; 5 ms.Manual interventions. Two standing rules from the user shaped this PR without being restated for it: no content from
~/.murmurenters git, and a pre-existing flaky test is recorded rather than fixed (#269 — a fourth occurrence was added as a comment there, and its pointer line rides this PR). The user also reviewed #280 personally and sent back one defect, which is why this PR's own review was read for seam bugs rather than logic bugs — and the P1 above was exactly that.Retro. The P1 is the lesson worth keeping: a new optional parameter on an interface is invisible to every test that injects a fake implementing that interface, because the fake is written against the new signature while production is not. When a change adds a parameter, the test that matters is the one over the production wiring, not the behaviour. Cheapest general fix, and the one applied here: build the adapter object next to the thing it adapts, inside an already-covered factory, instead of assembling it at the call site.
🤖 Generated with Claude Code