docs(820): take the minors from the MERGEABLE verdict
Build ErsatzTV Image / CI toolchain image resolves (pull_request) Successful in 11s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 13s
PR Gates / Docs update reminder (pull_request) Successful in 14s
PR Gates / Fix proofs (Proves trailers) (pull_request) Successful in 11s
Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 42s
PR Gates / decisions lifecycle (pull_request) Successful in 26s
Review verdict / Set review-verdict status (pull_request_target) Successful in 17s
PR Gates / Script lint and tests (ruff + pytest) (pull_request) Successful in 8m24s
review-verdict/h10 Review-verdict: MERGEABLE @ 68caecb (base: main)
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 14m29s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 9m8s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Skipped
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 8m58s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 10s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 7s
Build ErsatzTV Image / CI toolchain image resolves (pull_request) Successful in 11s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 13s
PR Gates / Docs update reminder (pull_request) Successful in 14s
PR Gates / Fix proofs (Proves trailers) (pull_request) Successful in 11s
Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 42s
PR Gates / decisions lifecycle (pull_request) Successful in 26s
Review verdict / Set review-verdict status (pull_request_target) Successful in 17s
PR Gates / Script lint and tests (ruff + pytest) (pull_request) Successful in 8m24s
review-verdict/h10 Review-verdict: MERGEABLE @ 68caecb (base: main)
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 14m29s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 9m8s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Skipped
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 8m58s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 10s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 7s
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
This commit is contained in:
@@ -7,7 +7,7 @@ supersedes: none
|
||||
superseded-by: none
|
||||
rule: 'Any path that writes a WHOLE entity or a WHOLE child collection — a PUT-replace handler, a hand-built request object, a test comparer standing in for one — derives its field list from the authoritative type and asserts SET EQUALITY against it, rather than enumerating the fields by hand. A hand-written list is correct on the day it is written and structurally unable to report the day it stops being: the field that drifts is the one nobody wrote a line for, so no amount of care in the existing lines can reach it. The failure is silent by construction — a full replace with a field omitted returns HTTP 200 and destroys that field''s value (#754 drifted from a 28-property DTO by one and cleared it; the symptom arrived hours later as missing pixels). SECOND CLAUSE, separable from the first: where a replaced child row carries state keyed to its identity — progression, ordering, an enumerator position — the handler RECONCILES BY ID rather than delete-and-reinsert, because reinsertion silently resets state a client never asked to touch (#252: a schedule PUT reset fill-group progression; #500: a dedup fix became permanent data loss because the add filter and the remove filter used different keys, so the two halves must agree on the key). Delete-and-reinsert is acceptable ONLY where no such state exists, and that emptiness is a fact about today''s schema that a later feature can silently invalidate — so record it where the handler is, dated, rather than leaving it to be re-derived. The canonical worked example is `ToolCatalogTests.Every_Write_Tool_Should_Declare_Exactly_Its_OpenApi_Request_Body_Fields`, which reads the accepted fields from the generated OpenAPI document and compares both directions. ON THE SPA SIDE the same rule is enforced by the TYPE SYSTEM rather than by a test: a full-replace body is built as `Complete<T>` (`web/src/api/completeRequest.ts`), a mapped type that makes every member of a generated request type required, so a builder that omits one fails `npm run typecheck`. Annotate BOTH the API-wrapper parameter (so later callers inherit it) AND each construction site including every `.map` callback return type, because the excess-property check that catches a PHANTOM field fires only on a fresh literal in a contextually typed position and a generic `.map` callback is not one. Do not infer from "most builders already typecheck" that the gap is closed: a member is omittable exactly when it is absent from the schema `required` array in the ASP.NET-produced OpenAPI document, and two live cases (#807) sat unchecked inside a large majority of checked ones.'
|
||||
signals: 'full replace asserts its field list · hand-maintained mirror drifts by one field · reconcile by id not delete and reinsert · 200 and the field is gone · add filter and remove filter must share a key · fill-group progression reset by a PUT · derive the comparer from the DTO · a lossless round-trip test that is itself a hand-copied list · anti-vacuity PIN not a floor on a reflective walk · stale exemption must still name a real property · a complete comparer over a hand-written fixture · universal negative in a record is a trap · the SPA builds a full-replace body as `Complete<T>` · a member is omittable only when it is absent from the OpenAPI `required` array · excess-property checking does not fire inside a generic `.map` callback · a comment warning about a silent reset is not a check · `weight` · `qsvPreferNativeDecoder` · paths: `ErsatzTV.Tests/Application/ProgramSchedules/ScheduleItemResponseRoundTripTests.cs`, `ErsatzTV.Mcp.Tests/ToolCatalogTests.cs`, `ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItemsHandler.cs`, `web/src/api/completeRequest.ts`, `web/src/api/completeRequest.guard.test.ts` · issues: #807, #779, #773, #757, #754, #500, #252'
|
||||
mechanics: 'Detector G of `docs/defect-shapes-773.md` §4. Enforced per-site by a reflective comparison over the DTO plus a written-down count pin, not by a repo-wide check — see the record body for why a global one is not proposed. The exemption set is empty today; the machinery guarding exemptions remains for the first one that earns its place. ON THE SPA SIDE the same rule is carried by the type system (`Complete<T>`, `spa.full-replace-complete` in `docs/spa-conventions.md` §4b), and since ersatztv#820 its APPLICATION is checked rather than assumed: `web/src/api/completeAnnotations.guard.test.ts` derives the droppable schemas from the generated `v1.d.ts` and the annotations from the SPA AST, asserts set equality between the droppable SCHEMAS and a reviewed disposition table, requires a PRODUCTION annotation naming each schema DISPOSITIONED as needing one — three of the thirteen that can drop a member, the other ten being creates, triggers, read models and two MUST-NOT-ANNOTATE rows, each ruled on individually. A test-file annotation does not discharge that: `*.test.*`, `*.spec.*` and the one setup file named in `vite.config.ts` are excluded, and the setup path is pinned so a rename reddens. NOT covered, stated rather than implied — a SECOND `setupFiles` entry, which vitest would load every run; deriving that list would need another virtual-module plugin and is not proportionate to a three-site coordinated edit, forbids one on the schemas where annotating is a bug, cross-checks its disposition values against this file''s table from the Python side, and is per-SCHEMA rather than per-site or per-wrapper — a wrapper-level rule was built and REMOVED before merge after four review rounds each defeated it. It also rejects a `Complete<X>` over a hand-written mirror rather than a generated alias. Before it, `scripts/tests/test_optional_request_members.py`''s COVERED disposition ASSERTED the annotation existed and nothing verified it. Residual: the per-CONSTRUCTION-SITE population is not derived (sites in code, #777), so the phantom direction still rests on the §4b site rule and on review.'
|
||||
mechanics: 'Detector G of `docs/defect-shapes-773.md` §4. Enforced per-site by a reflective comparison over the DTO plus a written-down count pin, not by a repo-wide check — see the record body for why a global one is not proposed. The exemption set is empty today; the machinery guarding exemptions remains for the first one that earns its place. ON THE SPA SIDE the same rule is carried by the type system (`Complete<T>`, `spa.full-replace-complete` in `docs/spa-conventions.md` §4b), and since ersatztv#820 its APPLICATION is checked rather than assumed: `web/src/api/completeAnnotations.guard.test.ts` derives the droppable schemas from the generated `v1.d.ts` and the annotations from the SPA AST, asserts set equality between the droppable SCHEMAS and a reviewed disposition table, requires a PRODUCTION annotation naming each schema DISPOSITIONED as needing one — three of the thirteen that can drop a member, the other ten being creates, triggers, read models and two MUST-NOT-ANNOTATE rows, each ruled on individually. A test-file annotation does not discharge that: `*.test.*`, `*.spec.*` and the one setup file named in `vite.config.ts` are excluded, and the setup path is pinned so a rename reddens. NOT covered, stated rather than implied — a SECOND `setupFiles` entry, which vitest would load every run; deriving that list would need another virtual-module plugin and is not proportionate to a three-site coordinated edit. The guard also forbids an annotation on the schemas where annotating is a bug, cross-checks its disposition values against this file''s table from the Python side, and is per-SCHEMA rather than per-site or per-wrapper — a wrapper-level rule was built and REMOVED before merge after four review rounds each defeated it. It also rejects a `Complete<X>` over a hand-written mirror rather than a generated alias. Before it, `scripts/tests/test_optional_request_members.py`''s COVERED disposition ASSERTED the annotation existed and nothing verified it. Residual: the per-CONSTRUCTION-SITE population is not derived (sites in code, #777), so the phantom direction still rests on the §4b site rule and on review.'
|
||||
---
|
||||
|
||||
**This is `testing.guard-derives-population-from-source` applied to a write path, and it is worth its
|
||||
@@ -261,7 +261,9 @@ so their `Complete<>` was checking a local copy rather than the contract.
|
||||
|
||||
What is NOT closed: the per-CONSTRUCTION-SITE population, which is sites in code (#777), so the
|
||||
PHANTOM direction still rests on review, as does the wrapper half of the rule. A test-file annotation
|
||||
cannot discharge the obligation — that bypass was demonstrated in review and is asserted against —
|
||||
cannot discharge the obligation in any spelling vitest COLLECTS (`*.test.*`, `*.spec.*`) nor from the
|
||||
one setup file `vite.config.ts` names, whose path is pinned — but a SECOND `setupFiles` entry would,
|
||||
and that direction is open —
|
||||
but a screen that builds a body inline without annotating its `.map` callback is still only caught by
|
||||
a reader, and so is an annotation moved off a wrapper. A guard that DID enforce the wrapper half was
|
||||
built for #820 and removed before merge: four review rounds each found a fresh way past it, the last
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -517,8 +517,9 @@ and fails until each has a stated disposition. See
|
||||
**What is machine-checked, exactly (#820).**
|
||||
`web/src/api/completeAnnotations.guard.test.ts` scans the SPA with the TypeScript compiler API and
|
||||
asserts, per SCHEMA: every schema that can silently drop a member and is dispositioned as needing an
|
||||
annotation carries a `Complete<…>` naming it in some **production** file (a test fixture does not
|
||||
count); the schemas where annotating would be a bug carry none; and every `Complete<X>` resolves to
|
||||
annotation carries a `Complete<…>` naming it in some **production** file — a `*.test.*` or
|
||||
`*.spec.*` file does not count, nor does the setup file `vite.config.ts` names (a SECOND
|
||||
`setupFiles` entry would, and is a stated residual); the schemas where annotating would be a bug carry none; and every `Complete<X>` resolves to
|
||||
an alias of `components['schemas'][…]` rather than a hand-written mirror — annotating a mirror proves
|
||||
only that the caller filled in the mirror, which is the #754 mechanism wearing the annotation meant to
|
||||
prevent it. Its disposition table is cross-checked against
|
||||
|
||||
@@ -184,8 +184,9 @@ function isEnumShaped(type: ts.TypeNode): boolean {
|
||||
}
|
||||
if (ts.isUnionTypeNode(type)) {
|
||||
// One level deep in practice: TS flattens `A | B | C` into a single node, and a parenthesised
|
||||
// group parses as a `ParenthesizedTypeNode`, which falls through to the keyword check and is
|
||||
// rejected. The recursion is what makes that rejection correct rather than accidental.
|
||||
// group parses as a `ParenthesizedTypeNode`, which the keyword check below rejects — so nesting
|
||||
// is refused rather than mis-admitted. The recursion is what lets each MEMBER be judged on its
|
||||
// own kind instead of the union being waved through as a whole.
|
||||
return type.types.every((member) => isEnumShaped(member));
|
||||
}
|
||||
return (
|
||||
|
||||
@@ -243,7 +243,7 @@ function discoverAnnotations(): DiscoveredAnnotation[] {
|
||||
*/
|
||||
function isProductionSource(rootRelativePath: string): boolean {
|
||||
// `.spec.` and `setupTests.ts` are here because cold review measured the `.test.`-only form false:
|
||||
// vitest's own include is `**/*.{test,spec}.*` (`web/vite.config.ts`), so a `*.spec.ts` fixture was
|
||||
// vitest's own default include is `**/*.{test,spec}.?(c|m)[jt]s?(x)` (measured on 4.1.9), so a `*.spec.ts` fixture was
|
||||
// a TEST to the runner and PRODUCTION to this predicate, and could discharge the obligation while
|
||||
// the real builder lost its annotation. Same for the setup file, which vitest loads for every run.
|
||||
//
|
||||
|
||||
Reference in New Issue
Block a user