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

Merged
timothy merged 2 commits from fix/pagesize-guard-line-churn into main 2026-07-27 19:39:39 +02:00
2 Commits
Author SHA1 Message Date
timothyandClaude Opus 5 689451161e fix(684): review round 2 -- drop a false exhaustiveness claim I introduced
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 14s
PR Gates / Docs update reminder (pull_request) Successful in 16s
Review verdict / Set review-verdict status (pull_request) Successful in 7s
PR Gates / decisions lifecycle (pull_request) Successful in 20s
PR Gates / Script tests (pytest) (pull_request) Successful in 51s
review-verdict/h10 Review-verdict: MERGEABLE @ 6894511 (base: main)
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 6m33s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 27s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 18s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 6m17s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 21m38s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
The L1 fix (name every class-b shape) collided with the M1 fix (move
SmartCollectionDialog into class-b) in the same commit: the comment claimed
"exactly three shapes, which is the whole list" while the registry 80 lines
below already held four. That is the same false-exhaustiveness defect this PR
exists to remove -- a reader adding a fifth class-b site would conclude theirs
cannot be class-b despite rendering a real totalCount hint, and reach for
search-bounded or deviation.

The operative rule is now stated first and the shape list is explicitly
illustrative: a site qualifies by RENDERING a totalCount-backed hint, not by
resembling a listed shape.

Also, both non-blocking review items:
- the deviation prose said the tracking issue goes "in the note", while
  enforcement is on the structural `issue` field -- it now points at the
  mechanism that actually binds, and says why the note scrape was rejected;
- the UNREGISTERED report prints every position sharing an identity, so it now
  says "identity seen at:" rather than implying all of them are unregistered. A
  positionless key cannot know which occurrence is excess; the candidate set is
  the honest answer.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 19:17:07 +02:00
timothyandClaude Opus 5 fc8353c75c fix(684): key the pageSize guard registry on identity, not source position
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 15s
PR Gates / Docs update reminder (pull_request) Successful in 21s
review-verdict/h10 Awaiting review verdict for fc8353c
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 25s
PR Gates / decisions lifecycle (pull_request) Successful in 34s
Review verdict / Set review-verdict status (pull_request) Successful in 31s
PR Gates / Script tests (pytest) (pull_request) Successful in 44s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 1m42s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 16m38s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 21m16s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 22m38s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
The #650 guard pinned every pageSize call site by absolute line:column, making
the registry a function of every OTHER file in the repo.

The guard was BORN RED. #651 moved AutoTuneScreen.tsx up ten lines and
FillerPresetsScreen.tsx down seventy-two, and merged to main BEFORE the guard's
own PR (#675) did -- so the registry, authored against a pre-#651 base, was
stale the instant it landed. Its own merge run was CANCELLED, so nothing
reported it; the red first surfaced on the next push (#676's merge, which
touches no web/src file and is not the cause). One ordering accident, not a
recurring pattern -- but the exposure is general, because every PR is green
against its own base and the breakage exists only in the merge result.

Identity is now (file, kind, value). New/removed/changed sites all still fail.
The MULTISET comparison is preserved, so a shared identity must be discovered
exactly as many times as it is registered. The scanner's positional
pageSizeSiteId is untouched: pageSizeScan.test.ts asserts real AST positions
against fixed fixtures, the opposite case, with no churn to remove.

The one case this costs is stated rather than implied: a same-identity
substitution within one file (delete a registered site, add a different
unreviewed one with the same kind and value token) now passes. Narrow, and
caught only incidentally before. Named in the guard and the record because
"costs no coverage" is a claim that outlives whoever made it.

Failure reports still print the discovered line:column -- identity and
diagnostics need not share a format, and a bare id was useless in a file
holding two such sites.

Registry reconciled with #651: Playlists/RerunCollections lost their bounded
windows to the shared searchLibraryPickerOptions, now registered in its place.
Adds a 'search-bounded' class for that shape.

Adds a 'deviation' class rather than laundering a live defect into a
compliant-looking label. Reconciling the registry surfaced a §3b violation
(#685, filed): CollectionsScreen's AddItemsDialog degrades to an unfiltered
whole-type window on an empty query and surfaces nothing. Both existing labels
would have been false, and either would have made the guard vouch for behaviour
that does not exist. Deviation entries must name a tracking issue, enforced by
a structural field -- a #\d+ scrape of the note passed with the reference
deleted, because notes legitimately cite historical issues.

Corrects SmartCollectionDialog to class-b: it does render a totalCount badge,
which is class-b's defining evidence.

fixes #684

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 19:08:26 +02:00