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

Merged

timothy merged 12 commits from fix/820-complete-application-guard into main 2026-08-30 00:06:39 +02:00

Conversation 1 Commits 12 Files Changed 11

+1283 -30

12 Commits

Author	SHA1	Message	Date
timothyandClaude Opus 5	68caecbc83	docs(820): take the minors from the MERGEABLE verdict ... 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	2026-08-29 22:34:31 +02:00
timothyandClaude Opus 5	62c48ae24d	fix(820): bound the test-file exclusion claim, which was a universal positive ... 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	2026-08-29 22:34:31 +02:00
timothyandClaude Opus 5	49f58e9c90	fix(820): stop asserting a universal negative about schema shapes, and pin the setup-file path ... 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	2026-08-29 22:34:31 +02:00
timothyandClaude Opus 5	c3bde0bf77	fix(820): .spec.ts and setupTests.ts are TESTS, and three doc claims outlived their code ... 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	2026-08-29 22:34:31 +02:00
timothyandClaude Opus 5	a63223d100	docs(820): one residue paragraph still described the removed wrapper derivation ... 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	2026-08-29 22:34:31 +02:00
timothyandClaude Opus 5	7291a50740	fix(820): remove the wrapper scanner — five defects from one mechanism ... 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	2026-08-29 22:34:31 +02:00
timothyandClaude Opus 5	65c0059fc8	docs(820): the inventory row claimed a hole the retracted rule invented ... `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	2026-08-29 22:34:31 +02:00
timothyandClaude Opus 5	559fa347d4	fix(820): a wrapper whose body is a typed LOCAL is part of the population too ... 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	2026-08-29 22:34:31 +02:00
timothyandClaude Opus 5	e9b114373e	fix(820): the wrapper rule is SHALLOW, because Complete<T> is ... 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	2026-08-29 22:34:30 +02:00
timothyandClaude Opus 5	7eb227395a	fix(820): cross-check wrapper discovery against a second mechanism, and tighten the floors ... 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	2026-08-29 22:34:30 +02:00
timothyandClaude Opus 5	9691f93056	fix(820): anchor the obligation at the WRAPPER, and correct what the first commit overclaimed ... 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	2026-08-29 22:34:30 +02:00
timothyandClaude Opus 5	66b965a4b5	fix(820): derive the population of Complete<T> APPLICATIONS, not just its semantics ... 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	2026-08-29 22:34:30 +02:00