fix(820): derive where Complete<T> is APPLIED, not just what it means #883

2026-08-29T22:42:50+02:00

timothy commented

2026-08-29 22:42:50 +02:00

fixes #820

Complete<T> (#807) makes SPA full-replace request bodies fail typecheck when a
builder omits a schema member. Two guards existed and neither checked that it was
APPLIED anywhere: completeRequest.guard.test.ts proves the type's semantics and
would stay green with every annotation deleted, and
scripts/tests/test_optional_request_members.py's COVERED disposition — worded
"the builder is annotated Complete<T>" — was a claim about another language's
source that nothing verified. A COVERED row and a SPA with the annotation gone
were indistinguishable to the whole suite.

What this adds

web/src/api/completeAnnotationScan.ts (compiler-API scanners) and
web/src/api/completeAnnotations.guard.test.ts (the guard), with a fixture suite
over synthetic sources so the scanner's correctness does not depend on what the
repo happens to contain today.

Two derived populations, neither hand-listed:

Complete<…> annotations across the SPA, from the AST, intersected with the
git index
droppable schemas — those with an optional member — parsed out of the
generated v1.d.ts, which is a pure pass-through of the OpenAPI required
array, so this is one fact read at two points rather than a second derivation

What it asserts: every schema dispositioned as needing one carries a production
Complete<…> naming it (a *.test.*/*.spec.* file or the setup file does not
count); the server-computed and load-bearing-omission schemas carry none; every
Complete<X> resolves to a generated schema rather than a hand-written mirror; and
the droppable schemas are set-equal to a reviewed disposition table, so a new
optional member fails until someone rules on it. scripts/tests/test_complete_annotation_dispositions.py
cross-checks that table against the authoritative Python one and ships a declared,
harness-executed mutation.

What it found

playouts.ts declared ReplacePlayoutTemplatesRequest and
ReplacePlayoutAlternateSchedulesRequest as hand-written object literals that
SHADOWED generated schemas of the same name — so Complete<> on those wrappers
was checking a local copy, not the contract. The #754 mechanism wearing the
annotation meant to prevent it. They narrowed null | Array<T> to T[] and
dropped no field, which is why nothing else saw them.

Review history — eight rounds, seven BLOCKED

Worth reading, because two of them were defects in my reasoning rather than typos:

The obligation was on the wrong population. Stated per-SCHEMA, it was
dischargeable by moving an annotation into a test fixture while the production
wrapper went unprotected. Now anchored at the wrapper.
Reachability is not protection. The wrapper rule asked whether a body
reached a droppable schema transitively. Complete<T> is shallow, so that
justification was false — confirmed by execution: with
Complete<UpdateMultiCollectionRequest>, { items: [{}] } typechecks clean.
The rule is now shallow, matching what the type actually does.
Discovery keyed on the wrong thing. The body was found by a parameter
literally named body, so renaming it to payload made a wrapper vanish from
the population. Now resolved through the request options; a body built as a
typed local counts too.

Nine mutations witnessed, each reddening its own assertion and only that one.

Deliberately NOT done

A deep Complete<T> was probed against the real generated types and works —
it catches a missing nested member, preserves the null arm of null | Array<X>,
keeps explicit undefined legal, still rejects phantoms — and flipping it
repo-wide surfaced one real gap. It is not shipped: it cannot be applied
blanket-wise (UpdateChannelRequest.logo reaches ArtworkContentTypeModel, where
annotating is a bug by §4b), and it closes only the missing direction, not the
phantom one. Recorded in the decision record so it is not re-proposed on
plausibility.

Residuals, stated rather than implied closed

Per-SCHEMA, not per-construction-site. The phantom direction still rests on
§4b's site rule and on review — that population is sites-in-code, #777.
The disposition table here (13 rows) and the Python one (9) overlap rather than
coincide, so re-dispositioning one does not redden the other. Neither can miss a
schema entirely, since both derive from the same required arrays.
Complete is matched by NAME; a locally-declared one would make annotations
identity types, so that is asserted against directly rather than resolved
through a full type checker.
An untyped local body (artwork.ts's new FormData()) is not classified.

Docs

docs/spa-conventions.md §4b, docs/guard-inventory.md (row + the out-of-globbed-
population note), and docs/decisions/records/testing/full-replace-asserts-field-list.md
all updated, including reconciling the record body, which still described #820 as
unbuilt while its own frontmatter said otherwise.

Verification

web 1324 tests green · scripts/tests 1228 green (2 skipped) · typecheck + eslint +
ruff clean · decisions-validate OK.

fixes #820 `Complete<T>` (#807) makes SPA full-replace request bodies fail typecheck when a builder omits a schema member. Two guards existed and neither checked that it was APPLIED anywhere: `completeRequest.guard.test.ts` proves the type's semantics and would stay green with every annotation deleted, and `scripts/tests/test_optional_request_members.py`'s `COVERED` disposition — worded "the builder is annotated `Complete<T>`" — was a claim about another language's source that nothing verified. A `COVERED` row and a SPA with the annotation gone were indistinguishable to the whole suite. ## What this adds `web/src/api/completeAnnotationScan.ts` (compiler-API scanners) and `web/src/api/completeAnnotations.guard.test.ts` (the guard), with a fixture suite over synthetic sources so the scanner's correctness does not depend on what the repo happens to contain today. Two derived populations, neither hand-listed: - **`Complete<…>` annotations** across the SPA, from the AST, intersected with the git index - **droppable schemas** — those with an optional member — parsed out of the generated `v1.d.ts`, which is a pure pass-through of the OpenAPI `required` array, so this is one fact read at two points rather than a second derivation What it asserts: every schema dispositioned as needing one carries a **production** `Complete<…>` naming it (a `*.test.*`/`*.spec.*` file or the setup file does not count); the server-computed and load-bearing-omission schemas carry **none**; every `Complete<X>` resolves to a generated schema rather than a hand-written mirror; and the droppable schemas are set-equal to a reviewed disposition table, so a new optional member fails until someone rules on it. `scripts/tests/test_complete_annotation_dispositions.py` cross-checks that table against the authoritative Python one and ships a declared, harness-executed mutation. ## What it found `playouts.ts` declared `ReplacePlayoutTemplatesRequest` and `ReplacePlayoutAlternateSchedulesRequest` as hand-written object literals that SHADOWED generated schemas of the same name — so `Complete<>` on those wrappers was checking a local copy, not the contract. The #754 mechanism wearing the annotation meant to prevent it. They narrowed `null | Array<T>` to `T[]` and dropped no field, which is why nothing else saw them. ## Review history — eight rounds, seven BLOCKED Worth reading, because two of them were defects in my reasoning rather than typos: 1. **The obligation was on the wrong population.** Stated per-SCHEMA, it was dischargeable by moving an annotation into a test fixture while the production wrapper went unprotected. Now anchored at the wrapper. 2. **Reachability is not protection.** The wrapper rule asked whether a body *reached* a droppable schema transitively. `Complete<T>` is shallow, so that justification was false — confirmed by execution: with `Complete<UpdateMultiCollectionRequest>`, `{ items: [{}] }` typechecks clean. The rule is now shallow, matching what the type actually does. 3. **Discovery keyed on the wrong thing.** The body was found by a parameter literally named `body`, so renaming it to `payload` made a wrapper vanish from the population. Now resolved through the request options; a body built as a typed local counts too. Nine mutations witnessed, each reddening its own assertion and only that one. ## Deliberately NOT done A **deep `Complete<T>`** was probed against the real generated types and works — it catches a missing nested member, preserves the `null` arm of `null | Array<X>`, keeps explicit `undefined` legal, still rejects phantoms — and flipping it repo-wide surfaced one real gap. It is not shipped: it cannot be applied blanket-wise (`UpdateChannelRequest.logo` reaches `ArtworkContentTypeModel`, where annotating is a bug by §4b), and it closes only the missing direction, not the phantom one. Recorded in the decision record so it is not re-proposed on plausibility. ## Residuals, stated rather than implied closed - Per-SCHEMA, not per-construction-site. The **phantom** direction still rests on §4b's site rule and on review — that population is sites-in-code, #777. - The disposition table here (13 rows) and the Python one (9) overlap rather than coincide, so re-dispositioning one does not redden the other. Neither can miss a schema entirely, since both derive from the same `required` arrays. - `Complete` is matched by NAME; a locally-declared one would make annotations identity types, so that is asserted against directly rather than resolved through a full type checker. - An untyped local body (`artwork.ts`'s `new FormData()`) is not classified. ## Docs `docs/spa-conventions.md` §4b, `docs/guard-inventory.md` (row + the out-of-globbed- population note), and `docs/decisions/records/testing/full-replace-asserts-field-list.md` all updated, including reconciling the record body, which still described #820 as unbuilt while its own frontmatter said otherwise. ## Verification web 1324 tests green · scripts/tests 1228 green (2 skipped) · typecheck + eslint + ruff clean · decisions-validate OK.

timothy added 12 commits 2026-08-29 22:42:51 +02:00

fix(820): derive the population of Complete<T> APPLICATIONS, not just its semantics 66b965a4b5

member fails typecheck. Two guards then existed and neither covered application.
`completeRequest.guard.test.ts` proves the TYPE's semantics and would stay green
with every annotation in the SPA deleted. `test_optional_request_members.py`
derives which schemas can silently drop a member and files each under a
disposition — one of which, COVERED, is worded "the builder is annotated
`Complete<T>`".

That wording was a claim about another language's source that nothing checked. A
COVERED row and a SPA with the annotation deleted were indistinguishable to the
whole suite, and §4b's opposite rule — do NOT annotate a schema whose optional
members are computed server-side — was prose with no executable form.

`completeAnnotations.guard.test.ts` closes both. The droppable-schema population
is read every run off the generated `v1.d.ts` with the compiler API; the
annotation population is read off the SPA AST, intersected with the git index.
Neither is a list. Set equality against a reviewed disposition table means a new
optional member fails until someone rules on it.

Why the compiler API and not a regex: `Complete<Foo>` also appears in doc
comments, string literals and the `import type` specifier. Those are three
different AST node kinds, so they are excluded structurally rather than by a
string predicate — the class this repo has a withdrawn guard (six rounds, then
deleted) recording the cost of.

It found a real defect on landing. `playouts.ts` declared
ReplacePlayoutTemplatesRequest and ReplacePlayoutAlternateSchedulesRequest as
hand-written object literals that SHADOWED generated schemas of the same name,
so `Complete<>` on those wrappers was checking a local mirror rather than the
contract — the #754 mechanism wearing the annotation meant to prevent it. They
narrowed `null | Array<T>` to `T[]` and dropped no field, which is why nothing
else saw them. Now aliased to the generated schemas; the stated reason for the
hand-written form (satisfying the `Record<string, unknown>` RequestBody bound)
never required re-declaring the shape, since an alias is already a `type`.

Four mutations witnessed, each reddening one assertion and only that one:
stripping `Complete<>` from a covered site (Done-when's witnessed red),
annotating the forbidden schema, reverting a wrapper to a hand-written mirror,
and deleting a disposition row. The scanner's own fixture suite is mutation-
proven separately, over synthetic sources, so its correctness does not depend on
what the repo happens to contain today.

A deep `Complete<T>` was probed and NOT taken. It works — measured against the
real generated types, it catches a missing NESTED member, preserves the `null`
arm of `null | Array<X>`, keeps explicit `undefined` legal and still rejects
phantoms — and flipping it repo-wide left one real gap, a test fixture omitting
`weight`. It is not shipped because it cannot be applied blanket-wise
(`UpdateChannelRequest.logo` reaches ArtworkContentTypeModel, where annotating
is a bug by §4b) and because it closes only the missing direction: the phantom
direction needs a fresh literal in a contextually typed position, which a
generic `.map` callback's return is not. Recorded so it is not re-proposed on
plausibility.

The guard passed in isolation and timed out at vitest's 5s default under
full-suite load, because the population was walked twice per call across four
tests. Memoised (module-level, immutable inputs, fresh process per run, so it
cannot mask a mutation) and given explicit per-test timeouts per
ci.web-test-per-test-timeouts. Whole-suite time went 60s to 27s.

Residuals stated rather than implied closed: this is per-SCHEMA, not
per-construction-site, so the phantom direction still rests on the §4b site rule
and review (sites-in-code is #777); and the two disposition tables overlap
rather than coincide (13 rows vs 9 — this side also sees response models), so
re-dispositioning one does not redden the other. What neither can miss is a
schema unnoticed by BOTH, since both derive from the same `required` arrays.

Verified: web 1297 tests green, scripts/tests 1228 green, typecheck + eslint +
ruff clean, decisions-validate OK.

fixes #820

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UYNbVwgVszv6Pum7ZuGd75

fix(820): anchor the obligation at the WRAPPER, and correct what the first commit overclaimed 9691f93056

Cross-family cold review (Codex) returned BLOCKED on bafc8f65d with a working
bypass. It was right, and the fix is a different population, not a patch.

THE BLOCKER. Stating the obligation per-SCHEMA made it dischargeable from
anywhere. Demonstrated: strip `Complete<>` off the `updateFFmpegProfile` wrapper
and retype `ffmpegProfiles.test.ts`'s fixture to name `UpdateFFmpegProfileRequest`
— the Create/Update schemas are structurally identical and that fixture already
goes to both endpoints — and every assertion stayed green while the production
wrapper stopped protecting its callers. #820's Done-when asks for "construction
sites (or WRAPPER SIGNATURES)"; schemas are the weaker population, and that is
exactly the false green it permits.

So the wrappers are now derived too. `scanWriteWrappers` finds every exported
`web/src/api/*.ts` function with a `body` parameter and a mutating `method:`, and
`scanSchemaReferences` builds the schema graph so a body reaching a droppable
schema NESTED inside it counts — `MultiCollectionItemRequest` is reachable from
`UpdateMultiCollectionRequest`, not equal to it, and hand-listing that is the
thing this guard exists to avoid. A test-file annotation no longer discharges a
production obligation.

That found a SECOND live hole immediately: `createMultiCollection` took a bare
`CreateMultiCollectionRequest` whose `items` reach `MultiCollectionItemRequest.weight`.
`MultiCollectionsScreen` builds ONE body and sends it to both the create and the
full-replace update wrapper, so nothing was dropped in practice — the create
wrapper simply protected no future caller. One helper, two callers, opposite
widths. Now annotated.

Also from the review, each verified before acting on it:
- A one-line scope edit (`&& path !== 'src/api/playouts.ts'`) removed this
  guard's reach over the file whose defect motivated it while staying above every
  floor. Floors cannot see selective narrowing, so the load-bearing paths are
  pinned by name, and every tracked in-scope path must now be supplied by the
  glob — the inner join was silent before.
- Schema discovery was any property named `schemas` anywhere, so a `declare
  module` block could overwrite a real schema with an empty optional list.
  Constrained to `interface components`, and duplicate or computed-key schemas
  now THROW rather than silently shrinking the population.
- A local `type Complete<T> = T` would make annotations identity types while
  reading as covered; asserted against. An alias-name collision across files
  would credit the wrong schema; rejected rather than resolved last-write-wins.
- `CreateChannelFromLineupAdvancedOptionsRequest` was CREATE (permissive) while
  its own note said annotating would collapse a load-bearing third state. It is
  MUST-NOT-ANNOTATE, so the prohibition is asserted rather than described.

CORRECTIONS to bafc8f65d, which overstated in four places:
- "set equality against a reviewed disposition table" was true of the SCHEMA
  discovery only, never of where `Complete<>` is applied. Reworded everywhere.
- spa-conventions §4b said deleting an annotation reddens the suite. That was
  false for the per-site half and is now scoped to the wrapper half, with the
  construction-site half stated plainly as review-only.
- the guard-inventory row claimed tracked-file completeness "yes"; it is partial,
  and the row now says which half holds and which is only path-pinned.
- the decision record's BODY still described #820 as unbuilt while its own
  frontmatter said it was closed. Reconciled; both now name the wrapper/site split.
- "whole-suite time went 60s to 27s" was a single-run comparison whose slower arm
  contained a timing-out test. The memoisation is still right (the population was
  walked twice per call), but the number was not a controlled measurement and is
  dropped rather than defended.

Six mutations witnessed, each reddening its own assertion and only that one:
the two bypasses above, plus stripping a covered site, annotating a forbidden
schema, reverting a wrapper to a hand-written mirror, and deleting a disposition
row. They are hand-run, NOT re-executed per suite — this guard claims no standing
MUTATION grade, the same footing as pageSizeCallSites.guard.test.ts.

Residuals unchanged and still stated: the per-construction-site population is not
derived (#777), so the phantom direction rests on review; the two disposition
tables overlap rather than coincide; and a MUST-NOT-ANNOTATE violation written
inside a `*.guard.test.ts` is not caught, which changes no production behaviour.

Verified: web 1315 tests green, scripts/tests 1228 green (2 skipped), typecheck +
eslint + ruff clean, decisions-validate OK.

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UYNbVwgVszv6Pum7ZuGd75

fix(820): cross-check wrapper discovery against a second mechanism, and tighten the floors 7eb227395a

Self-audit of the fix commit, prompted by the re-review brief asking what the
FIX had introduced. Two gaps, both in the new wrapper population.

The floors and the path pins both assume the scanner SEES the wrappers in a file
it was handed. Nothing tested that. A `scanWriteWrappers` that silently dropped a
signature shape — every multi-line one, say — would keep every count plausible
and every pinned path present, and the guard would report a clean population it
had never fully read. Measured first: the AST scan finds 71 wrappers across
`src/api/*.ts`, and an independent regex count agrees per file, in every file,
with zero divergence. That agreement is now asserted rather than left as a
one-off, because the two mechanisms fail in unrelated ways — a regex cannot tell
a type position from a comment, which is exactly why it is the wrong PRIMARY
scan and a fine second opinion. Witnessed: making the scanner skip
three-parameter wrappers (`replaceBlock(id, body, ifMatch)`) reddens that
assertion and only that one.

Floors were loose against the real numbers — 150/100/20 against 257 files, 220
schemas, 33 annotations — so they were raised to 200/25 and a wrapper floor of 50
added. They remain crude by design: a floor cannot see SELECTIVE narrowing, which
is what the named path pins and the tracked-supplied cross-check are for. Kept
well clear of ordinary variation so the guard is not red on a normal working
state, which is the #806 failure that teaches readers to ignore a guard.

Verified: web 1316 tests green, typecheck + eslint clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UYNbVwgVszv6Pum7ZuGd75

fix(820): the wrapper rule is SHALLOW, because Complete<T> is e9b114373e

Second cross-family review round returned BLOCKED, and it was right about
something I had asserted rather than measured.

THE BLOCKER — reachability is not protection. The wrapper rule asked whether a
body REACHED a droppable schema transitively, and justified itself with a
sentence about protecting "the next caller". `Complete<T>` is shallow; that
sentence was false. Confirmed by execution, not argument: with
`send(body: Complete<UpdateMultiCollectionRequest>)`, the call
`send({ name: 'x', items: [{ …no weight }] })` typechecks with zero diagnostics.
The wrapper annotation requires `items` to be PRESENT and requires nothing of
each item.

So the rule now asks whether the body schema can drop one of its OWN members.
That keeps the original blocker closed — `UpdateFFmpegProfileRequest` has
`qsvPreferNativeDecoder` directly, so the test-fixture bypass still reddens — and
stops the guard demanding a control that does not control the thing named. What
protects a NESTED droppable schema is its own builder's annotation, which is the
per-schema assertion that was already here. `scanSchemaReferences` existed only
to serve the wrong rule and is deleted rather than left as unused machinery.

The comment on `createMultiCollection` claimed it protected
`MultiCollectionItemRequest.weight` for a future caller. It does not, for the same
reason. Corrected to say what it actually buys: `name` and `items` must be named,
the schema has no optional members of its own today, and `weight` is protected by
`toItemRequest` in the screen. The annotation stays — it is where the obligation
would land the day that schema gains an optional member — but it is described as
the no-op it currently is.

ALSO FROM THE REVIEW:
- Wrapper discovery keyed on a parameter literally named `body`, so renaming it
  to `payload` and sending `{ body: payload }` made the whole wrapper vanish from
  the derived population with every floor and pin green. The body is now resolved
  through the request OPTIONS — what makes a parameter the body is that it is
  SENT as one. Witnessed: the rename bypass now reddens.
- The AST-versus-regex cross-check became a lower bound rather than equality,
  since the options-based resolution legitimately finds wrappers the regex cannot.
  It still catches the dangerous direction, a scanner that stops seeing a shape.
- The `Complete`-shadow assertion recognised only `type Complete<T>`. Broadening
  it textually to cover `interface` and `as Complete` immediately false-positived
  on this repo's own fixture file, which contains `the body as Complete<…>` inside
  a quoted synthetic source — the exact string-predicate failure this module is
  built to avoid. It is now `declaresLocalComplete`, an AST check, with the
  quoted-example case pinned as a regression.

Docs corrected in the same three places that carried the reachability wording:
§4b now states "its own members" and spells out that a wrapper annotation does
not reach a nested request type; the guard-inventory row and the decision record
say the same.

Nine mutations witnessed across the three rounds, each reddening its own
assertion: the two original bypasses, the parameter-rename bypass, a scanner that
drops three-parameter wrappers, stripping a covered site, annotating a forbidden
schema, reverting a wrapper to a hand-written mirror, deleting a disposition row,
and narrowing the scope predicate.

Verified: web 1321 tests green, typecheck + eslint clean, decisions-validate OK.

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UYNbVwgVszv6Pum7ZuGd75

fix(820): a wrapper whose body is a typed LOCAL is part of the population too 559fa347d4

Self-audit against the round-3 review brief's question — what did the fix
introduce — found a discovery gap I had created by keying the body on a
parameter.

`updateCollectionCustomOrder` builds `const body: UpdateCollectionCustomOrderRequest
= { mediaItemIds }` from its arguments rather than taking a body parameter, so a
parameter-only lookup left it out of the derived population entirely. No live
defect: that schema has no optional members, so no obligation attaches to it
today. But "the population cannot see this shape" is the state that matters on
the day it gains one, and it is the same failure as the `payload` rename one step
earlier — the body identified by where it is DECLARED rather than by being sent.

A typed local now counts. An UNTYPED one does not and is stated as the residual:
`artwork.ts` posts a hand-rolled `new FormData()`, which is not a schema body and
has no annotation to read.

This one cannot be witnessed by a red on the current tree — with no obligation on
that schema, stripping the annotation changes nothing either way, and saying
"witnessed" would be false. The fixture test IS the proof: it pins both the typed
local (discovered) and the untyped local (deliberately not) against synthetic
sources, which is what that file exists for.

Also covers `import Complete from './x'` in `declaresLocalComplete`, the one
name-binding form the AST check still missed.

Verified: web 1324 tests green, typecheck + eslint clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UYNbVwgVszv6Pum7ZuGd75

docs(820): the inventory row claimed a hole the retracted rule invented 65c0059fc8

`createMultiCollection` was listed as one of two live holes this guard found.
That claim was produced by the reachability rule, which has since been retracted:
the schema has no optional members of its own, so the annotation there is a no-op
kept for the day it acquires one. One live hole, not two.

Mutation count was six and is nine.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UYNbVwgVszv6Pum7ZuGd75

fix(820): remove the wrapper scanner — five defects from one mechanism 7291a50740

Two independent cold reviews (Codex cross-family, and a fresh Claude reviewer on
the head) both returned BLOCKED, and between them demonstrated three more ways
past the wrapper machinery. Counting the whole branch, that mechanism produced:

1. the obligation attached to the wrong population (schemas, not wrappers)
2. reachability mistaken for protection — `Complete<T>` is shallow
3. body discovery keyed on a parameter NAME (`payload` made a wrapper vanish)
4. bodies built as typed LOCALS invisible to the same lookup
5. `export function` -> `export const` removing real protection while the AST
scan AND its "independent" regex cross-check went blind TOGETHER, because
both keyed on the token `export function`

Five defects from one mechanism, each found only by review, and #5 falsifies the
claim I had written that "the two fail in unrelated ways". This repo's answer to
that shape is not a sixth patch — `process.enumerate-workaround-behaviors-before-deleting`
and the withdrawn `test_review_verdict_vocabulary_parity.py` (six rounds, then
deleted) both say to remove the mechanism. The head reviewer independently
recommended narrowing rather than withdrawing, and named the parts worth keeping.

REMOVED: `scanWriteWrappers`, `bodyIdentifierOf`, `localDeclarationType`,
`methodOf`, `scanSchemaReferences`, the wrapper assertions, the wrapper floor and
the AST-versus-regex cross-check. About 250 lines.

KEPT, and each earns its place: every `Complete<X>` must resolve to a GENERATED
schema rather than a hand-written mirror — this is what found the real
`playouts.ts` shadowing defect; droppable schemas set-equal to the reviewed
dispositions, so a new optional member fails until ruled on; MUST-NOT-ANNOTATE as
an asserted absence, which makes §4b executable; a PRODUCTION annotation required
per droppable schema, so a test fixture cannot discharge it.

ADDED, closing the second reviewer's other blocker: the disposition VALUE was an
unchecked kill switch — flipping `MultiCollectionItemRequest` from ANNOTATED to
CREATE and deleting the annotation left every suite green with #807's silent
weight reset live again. `scripts/tests/test_complete_annotation_dispositions.py`
now cross-checks the SPA table against `test_optional_request_members.py`, the
authoritative source that already existed and was not being consulted. It ships a
DECLARED clause mutation in `mutation_manifest.py`, executed by the harness every
run — a real MUTATION grade, not a hand-run claim.

Also closed: a scope edit dropping 65 of 257 files passed every floor and pin.
There is now a ratio check whose denominator is computed by a DIFFERENT expression
than the scope predicate, so the two can actually disagree — a shared helper would
cancel, which is the #819 finding. Witnessed: that exact bypass reddens twice over.

DOCS REWRITTEN, because both reviewers found them asserting coverage the code did
not have. §4b, the inventory row and the decision record now lead with what is NOT
checked: the obligation is per-SCHEMA, not per-site and not per-wrapper, so moving
an annotation off an API wrapper stays green; it is token PRESENCE, so a dead
`export type X = Complete<Y>` discharges it; and the phantom direction is not
checked at all. The wrapper rule's removal is recorded there too, so the cheap
version of that idea is not rebuilt without reading why it failed.

Verified: web 1309 tests green, scripts/tests 1240 green (2 skipped), typecheck +
eslint + ruff clean, decisions-validate OK, guard-inventory counts machine-checked.

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UYNbVwgVszv6Pum7ZuGd75

docs(820): one residue paragraph still described the removed wrapper derivation a63223d100

Self-audit after the narrowing. The decision record's residue paragraph still said
the guard derives "the write wrappers from the SPA AST", and still counted
`createMultiCollection` as a second live hole — both artefacts of the wrapper rule
that was removed and the reachability framing that was retracted with it.

One live hole (the `playouts.ts` shadowing mirrors), and the guard reddens on the
deletion of every production annotation for a droppable schema, not on reverting a
wrapper.

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UYNbVwgVszv6Pum7ZuGd75

fix(820): .spec.ts and setupTests.ts are TESTS, and three doc claims outlived their code c3bde0bf77

Fifth review round. It recommends KEEPING the guard as narrowed, and blocks on two
things, both one-line-shaped and both real.

THE FALSE GREEN. `isProductionSource` excluded `*.test.ts` and nothing else, while
vitest's own include is `**/*.{test,spec}.*` (`web/vite.config.ts`) — so a
`*.spec.ts` file was a TEST to the runner and PRODUCTION to this predicate.
Measured by the reviewer at the previous head: strip `Complete<>` from
`MultiCollectionsScreen.toItemRequest` (the only production site for
`MultiCollectionItemRequest`, and the exact pre-#807 form), add a `.spec.ts`
holding a `Complete<MultiCollectionItemRequest>` fixture, and typecheck plus all
1309 tests stayed green with the real builder unprotected. `src/setupTests.ts` did
the same. This is the bypass an earlier round blocked on, closed only for the
`.test.` spelling — and the guarantee is stated in four places, so it had to be
true in every spelling vitest accepts. Both bypasses now redden; witnessed.

THE DOC CLAIMS. The record's residue paragraph still said the guard "derives the
write wrappers from the SPA AST, so reverting a wrapper now reddens" — machinery
deleted last commit, and the claim measured false. Already fixed in 2cb7bd0d6,
which the review predates; what survived it was the paragraph five lines above,
opening "Since #820 the WRAPPER half is checked". It is the SCHEMA half. Also a
duplicated clause in the frontmatter `mechanics:` left by the narrowing — the copy
that gets mirrored out of context — and a stale ordinal in the guard-inventory
prose.

ALSO CLOSED, from the same review:
- An `allOf` schema emits as an intersection, which `scanOptionalSchemaMembers`
  skipped silently. That is the one non-object shape that HIDES members rather
  than having none, so it now throws, matching what the surrounding code already
  did for computed and duplicate keys. The 54 enum-shaped schemas it legitimately
  skips have nothing to drop; a fixture pins both halves.
- The Python cross-check floored the TS parse but not its own table, so an empty
  Python table would collect zero parametrised cases and pass having compared
  nothing. It was covered only by the neighbour file's floor — borrowing the
  property rather than holding it.
- The population ratio floor was 0.9 against an actual 0.985, admitting a silent
  ~22-file narrowing. Now 0.95.
- Recorded the residual this guard shares with its sibling: every population here
  derives from `trackedSources.tracked`, so a narrowing inside the Vite plugin
  cancels out of every comparison, and it is the sibling guard that polices that.

Verified: web 1311 tests green, scripts/tests 1241 green (2 skipped), typecheck +
eslint + ruff clean, decisions-validate OK, inventory counts machine-checked.

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UYNbVwgVszv6Pum7ZuGd75

fix(820): stop asserting a universal negative about schema shapes, and pin the setup-file path 49f58e9c90

Sixth review round. Every finding from round five verified FIXED — including a
negative control on the `.spec.ts` bypass, which is the part that matters: with the
new clause reverted and the fixture present, the guard goes green with the real
builder unprotected, so the clause is load-bearing rather than decorative. Two new
MEDIUMs, both introduced by that fix, both measured by execution.

N1. The intersection throw carried a comment saying `allOf` was "the ONE non-object
shape that can HIDE members rather than having none". Review measured that false
against the real scanner: a union of refs, a union of object literals, a `$ref`
alias and an array each hid members too. None is hypothetical —
`generateTypes` emits a union for `oneOf`/`anyOf` AND for a plain nullable object
(`type: ["object","null"]`), so a request schema modelled that way would leave the
droppable population silently.

A universal negative in a comment is the exact shape this record is about, so the
fix is not a longer list of shapes to reject. The test is now POSITIVE: a schema
type is either something the scanner can enumerate (an object literal) or something
it has SHOWN carries nothing (`isEnumShaped` — a literal, a union of literals, a
bare keyword — a property checked rather than asserted). Everything else throws.
All five shapes are pinned as fixtures, the four enum-shaped ones too. Confirmed
against the real `v1.d.ts`: 220 object literals, 54 enum-shaped, zero throws.

N2. `isProductionSource` hardcodes `'src/setupTests.ts'`, mirroring `vite.config.ts`'s
`setupFiles` with nothing coupling them — a hand-written mirror of an authoritative
source, reintroduced inside the fix for a hand-written mirror. Renaming the setup
file would leave the exclusion stale while vitest kept loading the new one, and the
round-5 bypass would reopen with the suite green. The path is now pinned, so that
staleness is a red rather than a silence.

Two doc corrections from the same round: the guard-inventory splice had attached
"claims no standing MUTATION grade" to the plugin-residual sentence instead of to
the hand-run-mutations one that explains it, and the row omitted the 0.95 ratio
floor when describing what catches a scope edit. The record's frontmatter said the
guard requires an annotation for "each schema that can drop a member" — it requires
one for each schema DISPOSITIONED as needing it, three of the thirteen. §4b had it
right; the frontmatter is the copy that gets mirrored out of context, so it is the
one that had to be exact.

Verified: web 1318 tests green, scripts/tests 1241 green (2 skipped), typecheck +
eslint + ruff clean, decisions-validate OK, inventory counts machine-checked.

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UYNbVwgVszv6Pum7ZuGd75

fix(820): bound the test-file exclusion claim, which was a universal positive 62c48ae24d

Seventh review round. N1 verified fixed and checked harder than I checked it —
the reviewer cross-referenced all 54 enum-shaped schemas back to `v1.json` and
confirmed none carries `properties`, `allOf`, `oneOf`, `anyOf` or `$ref`, so
`isEnumShaped`'s "shown to carry nothing" holds against the generator's INPUT and
not just its output. Twenty-one adversarial shapes all sort correctly.

The blocker is mine and it is the previous commit's own subject, inverted. That
commit removed a universal NEGATIVE from a comment ("the one non-object shape that
can hide members") and shipped a universal POSITIVE in the decision record: "a
test-file annotation does not discharge it, IN ANY SPELLING VITEST ACCEPTS".
Measured false — a SECOND `setupFiles` entry is a spelling vitest accepts, it is
loaded every run, and an annotation there discharges the obligation with the real
`updateFFmpegProfile` wrapper unprotected and this guard green at 13/13.

Both claims are now bounded to what is actually checked — `*.test.*`, `*.spec.*`,
and the ONE setup file named in `vite.config.ts`, whose path is pinned so a rename
reddens — with the uncovered direction stated rather than implied. Deriving
`setupFiles` would need another virtual-module plugin, which is not proportionate
to a three-site coordinated edit; that is the same tolerance
`pageSizeCallSites.guard.test.ts` takes and says so.

Note what the pin does and does not do, since the previous commit blurred it: it
covers the RENAME direction (measured red) and not the ADDITION direction.

Nits from the same round: `NullKeyword` removed from `isEnumShaped`'s keyword list
as unreachable (`null` in type position parses as a `LiteralType`, so the earlier
branch always wins); the union recursion's one-level-in-practice behaviour written
down; the array fixture now also pins `Array<T>`, the shape `typeFromSchema`
actually emits, alongside the `T[]` shorthand that merely reads like one; the
record's "creates, triggers and read models" corrected to name the two
MUST-NOT-ANNOTATE rows it was folding in; and the inventory row's middle column
reconciled with its own notes column, which had been updated alone.

Verified: web 1319 tests green, scripts/tests 1241 green (2 skipped), typecheck +
eslint + ruff clean, decisions-validate OK, inventory counts machine-checked.

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UYNbVwgVszv6Pum7ZuGd75

docs(820): take the minors from the MERGEABLE verdict

Build ErsatzTV Image / CI toolchain image resolves (pull_request) Successful in 11s

Details

PR Gates / CI image pin matches docker/ci (pull_request) Successful in 13s

Details

PR Gates / Docs update reminder (pull_request) Successful in 14s

Details

PR Gates / Fix proofs (Proves trailers) (pull_request) Successful in 11s

Details

Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 42s

Details

PR Gates / decisions lifecycle (pull_request) Successful in 26s

Details

Review verdict / Set review-verdict status (pull_request_target) Successful in 17s

Details

PR Gates / Script lint and tests (ruff + pytest) (pull_request) Successful in 8m24s

Details

review-verdict/h10 Review-verdict: MERGEABLE @ 68caecb (base: main)

Details

Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 14m29s

Details

Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 9m8s

Details

Build ErsatzTV Image / Build & push image (amd64) (pull_request) Skipped

Details

Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 8m58s

Details

Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 10s

Details

Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 7s

Details

68caecbc83

Round eight returned MERGEABLE and named six minors, saying it would take the
closest one if cheap. All six are cheap and all six are the doc-accuracy class
that blocked this branch five times, so all six are taken.

The one that mattered: the decision record's BODY still said "a test-file
annotation cannot discharge the obligation — that bypass is asserted against",
while the same record's frontmatter, corrected one commit earlier, discloses that a
second `setupFiles` entry does discharge it. A record asserting closure of what it
elsewhere discloses as open is worse than either statement alone. Both now carry
the same bound. So does §4b, whose parenthetical "(a test fixture does not count)"
had the same unbounded form.

The guard-inventory row is the canonical residual register and was missing this
residual entirely — it had landed in two of the three artifacts that enumerate
residuals. Now listed as (5), with the plugin-level one renumbered to SIXTH.

Three nits: the `mechanics:` verb series, broken when the residual clause was
inserted mid-sentence; the comment citing vitest's include as `**/*.{test,spec}.*`
when 4.1.9's default is `**/*.{test,spec}.?(c|m)[jt]s?(x)` — the imprecision ran
conservative, but a stale fact in the one comment whose whole job is precision
about that boundary is not something to leave; and `isEnumShaped`'s comment
crediting the recursion for rejecting a parenthesised group, which the keyword
check does.

Verified: guard + scanner suites 47 green, guard-inventory counts machine-checked,
decisions-validate OK.

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UYNbVwgVszv6Pum7ZuGd75

timothy commented

2026-08-29 23:03:55 +02:00

Review-verdict: MERGEABLE @ 68caecb

Eight review rounds, seven BLOCKED. Two independent cold reviewers (Codex cross-family + Claude, isolated worktrees). Every blocker traced to one mechanism - the wrapper scanner - which was REMOVED rather than patched a sixth time. Final pass confirmed on this head: rebase is a clean superset on the one conflicted file, minors commit is docs+comments only, and gates re-verified including catalog --check, check-doc-narrative --diff, npm run check:api and npm run build.

Review-verdict: MERGEABLE @ 68caecb Eight review rounds, seven BLOCKED. Two independent cold reviewers (Codex cross-family + Claude, isolated worktrees). Every blocker traced to one mechanism - the wrapper scanner - which was REMOVED rather than patched a sixth time. Final pass confirmed on this head: rebase is a clean superset on the one conflicted file, minors commit is docs+comments only, and gates re-verified including catalog --check, check-doc-narrative --diff, npm run check:api and npm run build.

timothy merged commit e8f80c42ce into main

2026-08-30 00:06:39 +02:00

timothy deleted branch fix/820-complete-application-guard

2026-08-30 00:06:41 +02:00

timothy referenced this pull request

2026-08-30 00:07:31 +02:00

`Complete<T>` proves its semantics but not its APPLICATION — nothing derives the set of SPA full-replace construction sites #820

timothy referenced this pull request

2026-08-30 00:56:33 +02:00

fix(836): never pass --depth to a checkout that may already be complete #884

timothy referenced this pull request

2026-08-30 02:35:21 +02:00