feat(578): artist typeahead covers music-video and song credits; album_artist stops 404ing #676

Merged

timothy merged 8 commits from feat/578-artist-typeahead-source into main 2026-07-27 08:09:31 +02:00

Conversation 1 Commits 8 Files Changed 12

+1113 -31

timothy commented 2026-07-27 03:14:02 +02:00

Owner

Copy Link

Copy Source

Part of #578 — deliberately not fixes. #578 lists four items; this PR delivers the backend one (the artist/album_artist suggestion source). The other three (fetch-on-focus, the unmount guard, the LCG divisor) are on the still-unmerged feat/651-searchable-pickers branch. A fixes trailer here would close #578 with three of its four items undelivered — the #649 trap. I'll close #578 once both land.

What it does

GetSearchFieldValuesHandler sourced artist from ArtistMetadata.Title only — entity artists. Free-text credits on music videos (MusicVideoArtist.Name) and songs (SongMetadata.Artists) were searchable but never suggested, and album_artist 404'd.

• MusicVideoArtist is a real table → a plain Concat into the existing EF pipeline.
• SongMetadata.Artists/AlbumArtists are EF9 primitive collections (one JSON array per row in a single TEXT column). Neither provider can project the elements server-side — SQLite needs SQL APPLY, Pomelo hasn't implemented primitive collections — verified by probe, not assumed. So they're read by paging raw rows and matching in memory.
• album_artist 404 → 200, additive under api.versioning-v1.

The interesting part: five attempts at the word "bounded"

Each looked obviously correct and each bounded the wrong quantity. The decision record carries the table, which is worth more than the code:

Attempt	Bounded	Why wrong
1–2	the result (LIMIT 1000)	a widened prefix starves the budget with non-matching rows before the real match
3	candidates returned	LIMIT caps output; a no-match query must evaluate every eligible row first, so the counter recorded zero
4	keyspace width	keyspace ≠ rows — one row at Id=20001 behind a 20,000-Id gap returned [] having inspected nothing
5	actual rows	—

The fix is a deletion: the page query is now a bare primary-key range with no residual predicate at all — no LIKE, no LOWER, not even IS NOT NULL. A LIMIT only bounds work when there is nothing to discard; the cursor (Id > @AfterId) is a seekable ordering-key predicate that positions the scan without discarding, so LIMIT n yields n rows. A short page can finally only mean exhaustion.

Deleting the predicate deleted a whole bug family with it: the JSON-escape reasoning, the verbatim-ASCII narrowing rule, the exhaustive Unicode sweep proving it sound, the ESCAPE '/' workaround, and a "may over-match, never under-match" invariant that turned out to be conditional on something untrue. Five tests removed because their subject no longer exists.

Along the way it fixed the original defect properly: the old prefilter JSON-encoded the whole prefix, so q=é built %"é% against a stored Édith — LOWER() folds the ASCII escape text, not the codepoint. Every accented artist was unsuggestable.

What is and isn't guaranteed

At most 20,000 logical rows returned/materialized and ≤10 round trips (11 for artist). Explicitly not claimed — and struck through in the record, since I asserted it earlier: bounded physical work, bounded bytes, or exact index-entry counts. MySQL purge lag leaves deleted clustered-index records traversable, and TEXT spills to overflow pages, so a row count implies neither. The 392 KiB measurement is one data point on one library averaging ~20 B/row.

Accepted, pinned loss: a match past row 20,000 isn't found — now depending only on row position, no longer on prefix shape, deletion history, or Id distribution.

Review

Five cold cross-family rounds (read-only sandbox). Rounds 1–4 BLOCKED on correctness; round 5 verified the implementation and BLOCKED only on the claim written around it; round 6 on an incomplete sweep. Every new test was mutation-verified. Follow-ups filed: #668 (accented values on the EF-sourced fields — a provider-divergent defect), #669 (a normalized SongArtist join table, rejected here on blast radius).

Verification

dotnet test ErsatzTV.Tests 1928 passed / 0 failed / 2 skipped · Architecture.Tests 7 passed · decisions_validate.py OK · scripts/tests 325 passed · solution 0 errors · BOM-free on all 4 touched .cs · dotnet format whitespace --verify-no-changes exit 0 · no OpenAPI regeneration needed (the endpoint description already claimed only bounded song rows).

The record trips the 60-line prose ceiling at 195 lines. Per docs.corpus-size-signal that's a non-blocking invitation to check for redundancy, and this is a considered decline: it's the five-attempt history plus three struck-through retractions, all distinct findings, and the history is the reusable part.

**Part of #578** — deliberately *not* `fixes`. #578 lists four items; this PR delivers the backend one (the `artist`/`album_artist` suggestion source). The other three (fetch-on-focus, the unmount guard, the LCG divisor) are on the still-unmerged `feat/651-searchable-pickers` branch. A `fixes` trailer here would close #578 with three of its four items undelivered — the #649 trap. I'll close #578 once both land. ## What it does `GetSearchFieldValuesHandler` sourced `artist` from `ArtistMetadata.Title` only — entity artists. Free-text credits on music videos (`MusicVideoArtist.Name`) and songs (`SongMetadata.Artists`) were searchable but never suggested, and `album_artist` 404'd. - **`MusicVideoArtist`** is a real table → a plain `Concat` into the existing EF pipeline. - **`SongMetadata.Artists`/`AlbumArtists`** are EF9 **primitive collections** (one JSON array per row in a single `TEXT` column). Neither provider can project the elements server-side — SQLite needs SQL `APPLY`, Pomelo hasn't implemented primitive collections — verified by probe, not assumed. So they're read by paging raw rows and matching in memory. - **`album_artist` 404 → 200**, additive under `api.versioning-v1`. ## The interesting part: five attempts at the word "bounded" Each looked obviously correct and each bounded the wrong quantity. The decision record carries the table, which is worth more than the code: | Attempt | Bounded | Why wrong | |---|---|---| | 1–2 | the **result** (`LIMIT 1000`) | a widened prefix starves the budget with non-matching rows before the real match | | 3 | **candidates returned** | `LIMIT` caps output; a no-match query must evaluate every eligible row first, so the counter recorded **zero** | | 4 | **keyspace width** | keyspace ≠ rows — one row at `Id=20001` behind a 20,000-Id gap returned `[]` having inspected nothing | | 5 | **actual rows** | — | The fix is a **deletion**: the page query is now a bare primary-key range with **no residual predicate at all** — no `LIKE`, no `LOWER`, not even `IS NOT NULL`. A `LIMIT` only bounds work when there is nothing to discard; the cursor (`Id > @AfterId`) is a *seekable ordering-key* predicate that positions the scan without discarding, so `LIMIT n` yields n rows. A short page can finally only mean exhaustion. Deleting the predicate deleted a whole bug family with it: the JSON-escape reasoning, the verbatim-ASCII narrowing rule, the exhaustive Unicode sweep proving it sound, the `ESCAPE '/'` workaround, and a "may over-match, never under-match" invariant that turned out to be conditional on something untrue. Five tests removed because their subject no longer exists. **Along the way it fixed the original defect properly**: the old prefilter JSON-encoded the whole prefix, so `q=é` built `%"é%` against a stored `Édith` — `LOWER()` folds the ASCII *escape text*, not the codepoint. Every accented artist was unsuggestable. ## What is and isn't guaranteed **At most 20,000 logical rows** returned/materialized and **≤10 round trips** (11 for `artist`). Explicitly *not* claimed — and struck through in the record, since I asserted it earlier: bounded physical work, bounded bytes, or exact index-entry counts. MySQL purge lag leaves deleted clustered-index records traversable, and `TEXT` spills to overflow pages, so a row count implies neither. The 392 KiB measurement is one data point on one library averaging ~20 B/row. Accepted, pinned loss: a match past row 20,000 isn't found — now depending only on row position, no longer on prefix shape, deletion history, or `Id` distribution. ## Review Five cold cross-family rounds (read-only sandbox). Rounds 1–4 BLOCKED on correctness; round 5 verified the implementation and BLOCKED only on the *claim* written around it; round 6 on an incomplete sweep. Every new test was mutation-verified. Follow-ups filed: **#668** (accented values on the EF-sourced fields — a provider-divergent defect), **#669** (a normalized `SongArtist` join table, rejected here on blast radius). ## Verification `dotnet test ErsatzTV.Tests` 1928 passed / 0 failed / 2 skipped · Architecture.Tests 7 passed · `decisions_validate.py` OK · `scripts/tests` 325 passed · solution 0 errors · BOM-free on all 4 touched `.cs` · `dotnet format whitespace --verify-no-changes` exit 0 · no OpenAPI regeneration needed (the endpoint description already claimed only bounded song *rows*). > The record trips the 60-line prose ceiling at 195 lines. Per `docs.corpus-size-signal` that's a non-blocking invitation to check for redundancy, and this is a **considered decline**: it's the five-attempt history plus three struck-through retractions, all distinct findings, and the history is the reusable part.

timothy added 8 commits 2026-07-27 03:14:03 +02:00

feat(578): artist typeahead covers music-video and song credits; album_artist stops 404ing ... cd6f36185c

`GET /api/v1/search/fields/{name}/values` sourced `artist` from `ArtistMetadata.Title` only —
entity artists — so the free-text credits that `LuceneSearchIndex` also writes to the `artist`
field (`MusicVideoArtist.Name`, `SongMetadata.Artists`) produced no suggestions, and
`album_artist` 404'd outright.

`MusicVideoArtist` turned out to be a real entity table, so it just joins the existing server-side
pipeline as a `Concat` — one bounded `UNION ALL` + `LOWER(...) LIKE ... LIMIT` on both providers.

`SongMetadata.Artists`/`AlbumArtists` are the hard case: EF 9 maps them as primitive collections
(one JSON array per row in a single column), and neither provider can project the elements
server-side — SQLite needs the SQL APPLY operator it lacks, Pomelo MySQL 9.0.0 has no
primitive-collection support at all. Both failures are now pinned by a test, so a provider upgrade
that fixes them shows up as a red rather than as a stale workaround. For those columns the handler
pre-filters on the raw JSON (`LOWER(col) LIKE '%"<encoded-prefix>%' ESCAPE '/'` — a deliberate
superset, since it matches a row and not an element), caps the rows at 1000 with an `ORDER BY` that
makes the truncation deterministic, then splits and exact-filters in memory.

Provider portability is by construction rather than by trusting `LIKE`: the prefix is JSON-encoded
before matching (which is how it is stored, and which makes the pattern pure ASCII, so SQLite's
ASCII-only `lower()` and MySQL's Unicode-aware `LOWER()` agree) and lowercased in C#, so the match
is correct under a case-sensitive MySQL collation as well as a case-insensitive one. The escape
character is `/`, never `\`: `ESCAPE '\'` is not a portable SQL literal.

An empty `q` stays supported for these fields — the row cap already bounds it, and a non-empty-`q`
carve-out would make one group of fields behave differently for the same client code.

Docs: new `api.search-field-values-list-columns` record (additive sibling — the base record's rule
still reads true; only its body's "intentionally-uncovered gap" claim was stale), plus
`api-conventions.md`, `spa-conventions.md` §12 and the regenerated `v1.json`.

Decisions-Edit: yes

fix(578): the LIKE prefilter under-matched every accented artist; make the superset provable ... 1641ca8305

Review of 1b78dc9e found the pre-filter's correctness claim was false, and the claim was in the
decision record as well as the code.

F1 (high). The pattern JSON-encoded the whole query prefix on the reasoning that the stored text
escapes non-ASCII, so encoding the prefix the same way would line up. It does not: SQL LOWER()
lowercases the *escape text* (`É` -> `é`); it cannot case-fold the codepoint that escape
denotes. So `q=é` built `%"é%`, the stored `Édith Piaf` never matched, and the row was
discarded before the in-memory filter could accept it. Every accented artist — Beyoncé, Björk,
Sigur Rós, Édith Piaf — was silently unsuggestable, which in a music library is the common case.

The invariant that was missing, now stated in the code: the SQL pre-filter is an OPTIMIZATION. It
may over-match; it must never under-match. Correctness lives in the in-memory filter. So the pattern
now narrows only on the leading run of characters the JSON writer stores verbatim and stops at the
first character it cannot prove — `q=Beyoncé` still narrows on `beyonc`, `q=é` narrows on nothing
and leans on the row cap. Soundness rests on two facts now asserted by exhaustive computation rather
than argued: no non-ASCII codepoint in U+0080..U+10FFFF OrdinalIgnoreCase-equals a printable ASCII
character (false for InvariantCultureIgnoreCase, which folds ~190 — the choice of Ordinal is
load-bearing), and the exact set of ASCII the encoder escapes.

F1b. `UseRequestLocalization` honours Accept-Language, so the culture was caller-controlled and
`ToLower()` plus the default linguistic `StartsWith(string)` let a header change the answer.
Comparison is now OrdinalIgnoreCase and ordering StringComparer.Ordinal throughout — including the
shared FilterSortTake that state/video_dynamic_range/content_rating also use. Sets unchanged,
order now ordinal rather than culture-dependent.

F2. The merge comment asserted an exactness the code does not have: sources truncate by their own
ordering (DB collation / primary key), not the merge's, so a dropped value can outrank a survivor.
Comment and record now say best-effort, exact only below the truncation points.

F3/F4. The cap now rides `ORDER BY Id` rather than the JSON column: MySQL sorts TEXT by only
max_sort_length bytes, so the old ordering was not deterministic there, and sorting the whole
matching set was avoidable work. What the cap still does NOT bound is the scan — a leading-wildcard
LIKE cannot seek an index — so that cost is now documented as accepted, with a normalized
`SongArtist` table named as the follow-up candidate rather than left implicit.

Every clause above is covered by a test verified to FAIL when that clause is mutated (old pattern
builder: 5 red; culture chain: 3 red; cap=3 / cap=limit / ORDER BY json / no cap: red each).

F5. Converted to a proper supersession. The old record did not merely hold a stale fact — it
recorded song/music-video credits as an "intentionally-uncovered gap" and album_artist as
unsupported, and this reverses that call, which `docs.decision-lifecycle` says is never a
line-edit. `api.search-field-values` is archived with its original prose restored, and
`api.search-field-values-sources` replaces it carrying the whole endpoint contract.

fix(578): a fixed row budget turns an over-matching prefilter into a false negative ... a37847e509

BLOCKER 1. The invariant a883e5f0 established — "the pre-filter may over-match, it must never
under-match" — is only sound while the candidate set is not truncated. It was truncated. A non-ASCII
or JSON-escaped prefix collapses the pattern to the bare `%"%` anchor, so every row becomes a
candidate, and `ORDER BY Id LIMIT 1000` then spent the whole budget on rows that could not match.
Seed 1000 songs by "zzz", put the only "éclair" in row 1001, ask for album_artist?q=é: a883e5f0
returns [], while 1b78dc9e returned "éclair" because its (separately broken) tighter pattern kept
the candidate set small. Neither revision was correct — the old one under-matched at the pattern,
the new one under-matched at the cap. Widening a predicate under a fixed budget starves it.

So the budget is gone. Candidate rows are now walked keyset-paged on Id (`Id > @AfterId … ORDER BY
Id LIMIT @Batch`), continuing past non-matching candidates and stopping on the first of: enough
distinct exact matches for `limit`, a short page (source exhausted), or a 20,000-candidate-row
ceiling in 2,000-row batches. The bound is on effort; it no longer silently decides the result, and
the lossy case needs 20,000 rows that already passed the pre-filter before it bites.

BLOCKER 2. The endpoint description and the record's rule claimed ordinal matching/dedup/ordering
endpoint-wide. False for EF-backed fields: the database runs LOWER/DISTINCT/ORDER BY/LIMIT before
any ordinal code, so `genre?q=é` still misses a stored "Éclair" on SQLite. Both are now scoped to
the final in-memory stages, and the underlying gap is referenced as #668 rather than described as
fixed. #669 (normalized SongArtist table) is referenced as the follow-up for the scan cost.

Accuracy corrections to my own claims, all verified by re-running the mutations:
- Only THREE of the nine Unicode cases fail 1b78dc9e (é/édith/BJÖRK — where query and stored casing
  differ, so the escape texts diverge); the other six pass it. The comment said all nine. They stay
  as continuity coverage, now labelled as such rather than as regression guards.
- Ordering_Is_Best_Effort used "Zulu"/"Éclair", where DB and ordinal orderings BOTH pick "Zulu" — it
  could not demonstrate the divergence it claimed. Now "Zulu"/"apple", which actually diverges:
  ordinal ranks "Zulu" first, the DB ranks "apple" first, and limit=1 returns ["apple"]. The record
  sentence was false and is corrected.
- The record printed literal "Édith"/"é" where it needed to show the escape TEXT (Édith,
  é), contradicting the very explanation it was giving.
- Corrected the cost claim: the leading wildcard forces scan ACCESS, but each page stops once it has
  filled @Batch, so a dense query finishes early — it is not necessarily a full table scan.
- The Unicode sweep is labelled a PROOF OBLIGATION: it is revision-independent and passes every
  revision, which is correct for what it is but must not read as regression coverage.

One process note: the new record's frontmatter had a lone apostrophe inside a single-quoted YAML
scalar ("SQLite's"). decisions_validate.py's hand parser accepted it; scripts/tests caught it.

fix(578): bound the Id KEYSPACE — LIMIT cannot bound a query that matches nothing ... fbc7b2a1dd

BLOCKER. Round 3's ceiling counted LIKE-positive candidates, which is not the quantity that needed
bounding. To return an empty page the engine must first evaluate every eligible row, so a no-match
query came back with rows.Count == 0, ended the walk having counted zero against the ceiling, and had
already inspected the entire table. Round trips and materialized rows were bounded; database work was
not. Worse, a dense widened prefix could materialize 20x the candidates and make 10x the round trips
of round 1 — a regression dressed as a bound.

Third time bounding the wrong quantity: revisions 1-2 bounded the RESULT (a fixed LIMIT budget the
widened pattern starved), revision 3 bounded the CANDIDATES, and neither bounds what the database
LOOKS AT. Now the Id range is closed on both sides — `Id > @AfterId AND Id <= @AfterId + @Window` —
so each round trip is a primary-key range scan of known width. The LIKE still decides what comes
back; it no longer decides how much gets looked at. The walk advances by the WINDOW, never by what
returned (an empty page means "nothing matched in this stretch", not "exhausted"), and there is
deliberately no LIMIT in the SQL — the window caps the row count, and a LIMIT would only restore the
illusion that it is doing the bounding. One indexed SELECT MAX(Id) up front stops the walk burning
windows on empty keyspace. Result: at most 10 round trips and 20,000 rows inspected for any q,
matching or not.

New test `List_Valued_Walk_Cannot_Inspect_Past_The_Bound_When_Nothing_Matches` covers the case every
earlier bound test missed — they all gave the pre-filter something to return. With a narrow pattern
and a needle at row 30,000, revision 3 returns the needle (right answer, unbounded work) and this
revision returns []. Paired with a positive control at row 20,000 inside the bound.

The ceiling's remaining false negative (20,000 "zzz" rows then "éclair" at 20,001 -> []) is kept and
still pinned: it is the documented bounded-best-effort contract at a rarer threshold, not a defect.

Stale/false comments, several of them repeat offenders:
- The handler still used "Zulu"/"Éclair" as the truncation example, where both orderings pick "Zulu".
  I had corrected the test and left the comment. Now "Zulu"/"apple" in both.
- "leans entirely on the row cap" described a mechanism that no longer exists.
- The escape example rendered as literal Édith/É/é rather than the escape TEXT (Édith,
  É, é) in both the handler and the record, contradicting the explanation it was giving.
- "every accented artist was unsuggestable" overstated it: exactly three of the nine pinned cases
  fail 1b78dc9e (those where query and stored casing differ, so the escape texts diverge); the other
  six pass. The record now says what the test comment already said.
- The record claimed the ordinal switch left response SETS unchanged. False — ordering runs before
  Take(limit), so with "Zulu"/"apple", empty q and limit=1 the comparer changes which value survives.
- The_Scan_Ceiling_Is_A_Whole_Number_Of_Batches was a style preference wearing a correctness costume
  (a 20,500 ceiling is perfectly safe — the final window clamps). Replaced with the invariant that
  actually matters: the walk's own arithmetic terminates and lands exactly on the ceiling.

fix(578): delete the SQL predicate — LIMIT only bounds work when there is nothing to discard ... 373956fcee

BLOCKER. Attempt 4 bounded the Id KEYSPACE, and keyspace is not rows. Delete 20,000 historical rows,
put one song at Id 20001, query artist?q=que: the walk burned all ten windows on empty ranges and
returned [] for a table containing exactly one row. Capacity fell linearly with deletion ratio and no
ratio was safe — one placed gap hides the next match. My record called that "heavily fragmented" and
the endpoint description said loss happens "on a very large library"; the one-row example disproves
both.

Option A. The query now carries NO predicate at all — no LIKE, no LOWER, not even IS NOT NULL:

  SELECT Id, Artists AS Payload FROM SongMetadata WHERE Id > @AfterId ORDER BY Id LIMIT @Batch

That is the whole fix, and it is the point. LIMIT truncates what survives a predicate, so with any
predicate present it bounds the OUTPUT and says nothing about the WORK; the engine may evaluate and
discard arbitrarily many rows first. Stripped to a bare primary-key range, LIMIT n reads exactly n
index entries and n rows — independent of sparsity, deletion history or where the gaps fall. All
selectivity moves into memory. A short page can now only mean exhaustion, which is precisely what it
could not mean while a predicate was present.

Four attempts, four wrong quantities: the result (a fixed budget the over-matching pre-filter
starved), candidates returned (a no-match query must evaluate every eligible row before returning an
empty page), keyspace width (above), and finally actual rows. The record carries the table; it is
worth more than the code.

Deleting the predicate deletes a whole bug family with it: the JSON-escape reasoning, the
narrow-only-on-verbatim-ASCII rule, the exhaustive Unicode sweep that proved it sound, the ESCAPE '/'
portability workaround, and the may-over-match-never-under-match invariant that turned out to be
conditional on something untrue. SearchFieldValuesPrefilterSupersetTests is deleted entirely; the one
assertion worth keeping — that the SQL has no predicate — moved to the query-shape suite, which pins
the SQL string exactly so "just a cheap filter" fails a test instead of silently unbounding the walk.

Measured cost of no server-side narrowing, on a seeded 20,000-song library (in-memory SQLite):
worst case (no match, full walk) 20,000 rows / 10 round trips / 391.9 KiB / 119ms SQL, ~40ms warm
end-to-end. Empty q, dense and non-ASCII prefixes all stop on page 1 at ~39 KiB and ~40ms. Judged
acceptable for a debounced typeahead against a local file. If it ever is not, the answer is #669, not
reintroducing selectivity — the record says so explicitly.

Also fixed:
- Round-trip count was advertised as 10; it is at most 10 for album_artist and 11 for artist, which
  also runs its EF query. The MAX(Id) probe is gone with the keyspace scheme, so there is no extra
  scalar call.
- The duplicated-formula ceiling test is deleted rather than rewritten. It re-implemented the loop's
  arithmetic and would have passed through an off-by-one or a stall in the real loop; the dense
  integration tests carry that coverage. Its MaxVisited >= Window assertion was a style constraint in
  correctness clothing.
- Stale text swept by grepping the mechanism nouns rather than re-reading: candidate/keyspace/
  pre-filter/superset/row cap/LIKE/ESCAPE and the removed constant names, across handler, tests,
  record, api-conventions and the endpoint description. The two surviving "pre-filter" mentions are
  deliberate history. Test comments that rendered escaped non-ASCII as literal characters (which
  contradicted the raw-storage assertion in the same file) now show the escape text.

New test List_Valued_Walk_Reads_Live_Rows_Regardless_Of_Id_Density reproduces the one-row killer and
fails against attempt 4.

docs(578): record the update-openapi.sh incremental-skip trap that made my own check vacuous ... b93a7d33ff

Verifying the regenerated OpenAPI artifacts, I re-ran the pipeline against an already-built tree and
got a clean git diff — which I nearly reported as "artifacts confirmed". It was a no-op.

When the project is already built and unchanged, MSBuild skips the document-generation work but
still runs RenameOpenApiFiles (AfterTargets), whose Move then fails with MSB3680 "ErsatzTV.json does
not exist" — nothing produced it. The script exits non-zero correctly, but I had piped it
(`./scripts/update-openapi.sh 2>&1 | tail -2`), so the shell reported tail's 0 and the failure was
invisible. A clean diff after a regeneration that never regenerated proves nothing.

Caught it with a positive control: tamper all three artifacts, re-run, see which get restored.
v1.d.ts came back (npm run generate:api is unconditional) while v1.json and endpoint-index.md stayed
tampered. A `touch` on a compiled source then made the real regeneration run and restore all three
byte-exact, which is the verification that actually means something.

CI is unaffected — the api-docs job restores into a clean tree, so generation never skips. This is a
local-dev hazard only, and it is the same shape as the bug arc this branch is about: a check that
reports success without examining anything, exactly what LIMIT was doing to the row bound.

docs(578): scope the bound to LOGICAL ROWS — physical work is not bounded, and I claimed it was ... 5f73cd4482

Comment- and docs-only. Verified: the diff for both .cs files contains no non-comment line.

HIGH (claim). "LIMIT n reads exactly n index entries and n rows" is false, and the bounded-physical-
work reading it implied is false with it. Two mechanisms, both retracted explicitly in the record
rather than quietly reworded:

- MySQL purge lag. Deleted clustered-index records survive until purge runs and a range scan still
  traverses them, so returning 2,000 VISIBLE rows can touch far more index records. Deletion history
  therefore STILL affects physical work — precisely what attempt 4's failure was supposed to have
  made irrelevant. Attempt 5 removes the LOGICAL dependence on Id distribution; it does not make
  physical work independent of deletion history.
- Unbounded row width. Artists/AlbumArtists are unrestricted TEXT/longtext and both SQLite and InnoDB
  spill to overflow pages, so a row count implies neither a byte count nor a page-read count.

The guarantee is now stated as exactly what it is: at most 20,000 LOGICAL rows returned/materialized,
and at most 10 round trips (11 for artist). The 392 KiB measurement is labelled one data point on one
library whose credits average ~20 B/row, with an instruction to re-measure rather than extrapolate
for long credit lists or MySQL over a network. Also noted: the query-shape test pins the SQL STRING —
it cannot pin a plan, MVCC visibility work or payload I/O, and on MySQL using the index to satisfy
ORDER BY is an optimizer choice, not a semantic guarantee.

Precision fix, and the reviewer is right that the sloppy version misleads: "any predicate defeats
LIMIT" is wrong, since the query's own cursor is a predicate. The distinction is a SEEKABLE PREDICATE
ON THE ORDERING KEY (positions the scan, never discards a row) versus a RESIDUAL predicate (discards
rows the engine already produced, so LIMIT bounds survivors and says nothing about how many were
produced). Restated in the handler, the record and api-conventions.

MEDIUM. docs/decisions.md still advertised "a superset LIKE pre-filter that may over-match but never
under-match" for list-valued columns. That is the documented entry point for convention lookups, so a
maintainer starting there was told to preserve exactly what round 4 deleted, in direct contradiction
of the linked active record. Fixed.

LOW. Two test comments read as current: "however many non-matching rows" (false past the ceiling —
now bounded and cross-referenced to the test that pins that boundary) and the ampersand case
"widens to the bare anchor" (no prefix predicate exists; kept because it is the input shape that
broke the old scheme, now labelled as such).

Fifth consecutive round of stale text, so this sweep was done by grepping the subject across handler,
tests, record, decisions.md, api-conventions.md and the endpoint description: LIKE, ESCAPE,
pre-filter, superset, over-match, under-match, anchor, keyspace, window, candidate, row cap, index
entries, 392, and every deleted constant name. Every surviving hit is now either current-and-correct
or explicitly framed as history.

docs(578): the retracted claim survived in 9 places, including the record title and rule ... fc3ede09bc

Comment- and docs-only; verified no non-comment line changed in any .cs.

I reported last round that I had "classified every surviving hit". That was false, and the false
confidence is the expensive part: a confidently-stated "I checked everything" stops anyone else
checking. The retracted wording survived in nine places, two of them the record's title and rule: —
and the catalog copies rule: verbatim, so the generated entry point and the record disagreed
semantically while docs/decisions.md said the correct thing.

Root cause of the miss, because it will recur otherwise: I built the sweep term list from the
DELETED MECHANISM's vocabulary (LIKE, superset, keyspace, anchor, over-match) and never added the
RETRACTED CLAIM's own words. "no predicate", "bound on work", "index entries", "no gap" and
"holds in memory" were never grepped. After a retraction the subject list has to include the words
of the thing being retracted, not just the thing already deleted.

Second, worse: my first attempt at this round's sweep printed nothing for every term and I nearly
read that as "all clear". zsh does not word-split an unquoted $FILES, so grep received one giant
non-existent path — and the `|| echo "(none)"` never fired because the pipeline's exit status was
sed's. Same failure shape as the bug arc itself: a check reporting success while examining nothing.
Re-run with a proper array plus a control term ("SongMetadata" -> 42 hits) so an empty result is
distinguishable from a broken grep.

Fixed all nine, replacing "no predicate" with the seekable-cursor-vs-residual distinction already
written correctly elsewhere:
- handler: the "real bound on work" claim, the short-page rationale
- SearchFieldValuesQueryShapeTests: "ANY predicate" + "reads exactly n index entries", and added what
  the test can and cannot pin (a SQL string, not a plan / visibility work / payload I/O)
- GetSearchFieldValuesHandlerTests: "no gap between what the engine looks at and what it hands back",
  and the current-behaviour comment
- record title, rule:, attempt-5 table row; api-conventions
- regenerated docs/decisions/README.md so catalog and record agree again

Tenth item, the same overclaim one level down and it survived the first retraction: the row bound was
said to cap what the process holds in memory. It does not — payload width is unrestricted and one
JSON array can contain arbitrarily many strings, each of which may enter the in-memory distinct set.
It caps logical rows returned/materialized and round-trip count, nothing about bytes. Added as a
third struck-through bullet next to the other two retractions.

timothy commented 2026-07-27 04:03:32 +02:00

Author

Owner

Copy Link

Copy Source

Review-verdict: MERGEABLE @ fc3ede0

Five cold cross-family rounds on the implementation (rounds 1-4 correctness, 5 the claim, 6 an incomplete sweep). Final docs-only pass was SAME-FAMILY (cold, isolated, no implementation role) because Codex quota is exhausted - noted as a reduction in independence. That pass verified prose-vs-code across 6 artifacts, PyYAML-parsed every decisions record, and swept repo-wide with control terms. All substantive CI green.

Review-verdict: MERGEABLE @ fc3ede0 Five cold cross-family rounds on the implementation (rounds 1-4 correctness, 5 the claim, 6 an incomplete sweep). Final docs-only pass was SAME-FAMILY (cold, isolated, no implementation role) because Codex quota is exhausted - noted as a reduction in independence. That pass verified prose-vs-code across 6 artifacts, PyYAML-parsed every decisions record, and swept repo-wide with control terms. All substantive CI green.

timothy referenced this pull request 2026-07-27 04:27:30 +02:00

feat(651): library-browse pickers resolve by search, not a 100-row window #678

timothy referenced this pull request 2026-07-27 08:09:19 +02:00

#434 follow-ups: typeahead combobox polish + source limitations #578

timothy merged commit 1d76a088c6 into main 2026-07-27 08:09:31 +02:00

timothy deleted branch feat/578-artist-typeahead-source 2026-07-27 08:09:32 +02:00

timothy referenced this issue from a commit 2026-07-27 08:09:33 +02:00

Merge pull request 'feat(578): artist typeahead covers music-video and song credits; album_artist stops 404ing' (#676) from feat/578-artist-typeahead-source into main

timothy referenced this pull request 2026-07-27 08:11:01 +02:00

#434 follow-ups: typeahead combobox polish + source limitations #578

timothy referenced this issue from a commit 2026-07-27 19:08:39 +02:00

fix(684): key the pageSize guard registry on identity, not source position

timothy referenced this pull request 2026-07-27 19:09:01 +02:00

fix(684): key the pageSize guard registry on identity, not source position #686

timothy referenced this pull request 2026-07-27 19:24:41 +02:00

main is red: the #650 pageSize call-site guard keys its registry on line:column, so any merge that shifts a line breaks it #684

timothy referenced this pull request 2026-07-27 19:40:20 +02:00

main is red: the #650 pageSize call-site guard keys its registry on line:column, so any merge that shifts a line breaks it #684

timothy referenced this issue from a commit 2026-08-13 20:50:54 +02:00

docs(773): the round-count table sampled instead of enumerating — 33, not 18

timothy referenced this pull request 2026-09-03 21:03:03 +02:00

fix(876): sweep session narrative out of hooks, workflows, scripts, tests and code comments; grow the detector to the process corpus #906

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

ad-hoc

One-off / ad-hoc work not tracked by a dedicated issue

api

REST API / HTTP endpoints

bug

Something isn't working

ci-cd

Build, test, deploy pipeline

content

Channel content / schedules / playlists

dependencies

Dependency updates (Renovate)

enhancement

New feature or improvement

frontend

ChicoryTV React SPA frontend

in-progress

Claimed by an active session — do not pick up

jellyfin

Jellyfin tuner / IPTV integration

parked

Excluded from automatic queue pickup; work only when explicitly selected

priority: high

priority: low

priority: medium

review

Adversarial review finding

security

Security / vulnerability fix

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate timothy (Timothy)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: timothy/ersatztv#676