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
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>
This commit is contained in:
@@ -91,14 +91,21 @@ import { scanPageSizeSites } from './pageSizeScan';
|
||||
* NOT search-bounded (see 'deviation').
|
||||
* - 'class-b' — one bounded page at (or under) the cap, with the real truncation
|
||||
* (`totalCount` vs items shown) surfaced to the user. Post-#651 this no
|
||||
* longer covers media-library pickers (those are 'search-bounded'). It
|
||||
* remains correct for exactly three shapes, which is the whole list: a
|
||||
* list bounded by its PARENT (`ChannelBuilder`, seasons of one show); the
|
||||
* collection-family types §3b explicitly excludes from search
|
||||
* (`FillerPresetsScreen`), which keep the bounded page and its hint; and
|
||||
* a preview over an already-bounded set (`AutoTuneScreen`'s channel
|
||||
* members). Each of the three renders a real `totalCount`-backed hint —
|
||||
* that render is the entry requirement, not the intent.
|
||||
* longer covers media-library pickers (those are 'search-bounded').
|
||||
* **The RENDER is the entry requirement, not the intent** — that is the
|
||||
* operative rule, and the only one to apply to a new site. Today's
|
||||
* entries happen to take four shapes: a list bounded by its PARENT
|
||||
* (`ChannelBuilder`, seasons of one show); the collection-family types
|
||||
* §3b excludes from search (`FillerPresetsScreen`), which keep the
|
||||
* bounded page and its hint; a preview over an already-bounded set
|
||||
* (`AutoTuneScreen`'s channel members); and a preview over an UNBOUNDED
|
||||
* user-authored query that surfaces its match count
|
||||
* (`SmartCollectionDialog`). That list is illustrative and NOT
|
||||
* exhaustive: a site qualifies by rendering a real `totalCount`-backed
|
||||
* hint, not by resembling one of these four. (#684 review: an earlier
|
||||
* revision of this comment called it "the whole list" while the registry
|
||||
* below already held a fourth — the same false-exhaustiveness defect this
|
||||
* PR exists to remove.)
|
||||
* - 'paged-ui' — real paging UI (a page/"load more" control, or a user-adjustable
|
||||
* page-size selector, keyed to a genuine `totalCount`), so a `pageSize`
|
||||
* at or below the cap is correct as-is.
|
||||
@@ -106,9 +113,11 @@ import { scanPageSizeSites } from './pageSizeScan';
|
||||
* into a compliant-looking label. A registry exists to state what is
|
||||
* true; recording a defect as 'class-b' or 'search-bounded' would make
|
||||
* the guard assert a hint or a query gate that demonstrably does not
|
||||
* exist, and the next reader would trust it. Every such entry MUST name
|
||||
* its tracking issue in the note, and flips to a real class only when the
|
||||
* behaviour is fixed.
|
||||
* exist, and the next reader would trust it. Every such entry MUST carry
|
||||
* its tracking issue in the structural `issue` field — enforced below,
|
||||
* and deliberately NOT a `#\d+` scrape of the note, which passed with the
|
||||
* reference deleted because notes legitimately cite historical issues —
|
||||
* and flips to a real class only when the behaviour is fixed.
|
||||
*
|
||||
* **Known residual gap:** object SPREAD (`getFoo({ ...opts })` where `opts` was built elsewhere
|
||||
* with an at-cap `pageSize`) and a `pageSize` passed as a bare POSITIONAL argument rather than an
|
||||
@@ -419,7 +428,10 @@ function describeDiscovered(sites: DiscoveredSite[], ids: string[]): string[] {
|
||||
}
|
||||
return ids.map((id) => {
|
||||
const at = positions.get(id);
|
||||
return at && at.length > 0 ? `${id} (at ${at.join(', ')})` : id;
|
||||
// Every position sharing this identity, not just the excess one: a positionless key genuinely
|
||||
// cannot tell which occurrence is new, so the candidate set IS the honest answer. Labelled so a
|
||||
// reader does not take all of them as unregistered (#684 review L-a).
|
||||
return at && at.length > 0 ? `${id} (identity seen at: ${at.join(', ')})` : id;
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user