fix(807): SPA full-replace bodies are built as Complete<T>, with a derived guard over droppable request members (#827)
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 14s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 30s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m36s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m9s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 5m49s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Failing after 15s
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 14s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 30s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m36s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m9s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 5m49s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Failing after 15s
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
This commit was merged in pull request #827.
This commit is contained in:
+1
-1
@@ -26,7 +26,7 @@ doc below, or that changes which sections a task signal points to.**
|
||||
| CI / release pipeline work | `docs/ci-cd.md` + `docs/decisions/release-ci-governance.md` |
|
||||
| Proposing a new guard / CI check / regression test convention | `docs/defect-shapes-773.md` §4 (detector menu + the classes where no detector is plausible), then the three rules every guard must satisfy: `docs/decisions/records/testing/guard-derives-population-from-source.md`, `…/guard-ships-with-mutation-proof.md` and `…/mutation-claims-are-executed.md` (a `MUTATION` grade carries a DECLARED clause mutation that is re-run every suite) |
|
||||
| Testing a surface gated by config / an env var / a credential | `docs/decisions/records/testing/deny-path-at-production-config-value.md` — cover the setting absent, at its production value, and each opt-out, and assert the DENY branch |
|
||||
| Touching a full-replace write path or a hand-built request object | `docs/decisions/records/testing/full-replace-asserts-field-list.md` — derive the field list from the DTO and assert set equality; reconcile by id where child state exists |
|
||||
| Touching a full-replace write path or a hand-built request object | `docs/decisions/records/testing/full-replace-asserts-field-list.md` — derive the field list from the DTO and assert set equality; reconcile by id where child state exists. In the SPA the same rule is enforced by the type system: `docs/spa-conventions.md` §4b — build the body as `Complete<T>`, annotating BOTH the wrapper parameter and every construction site |
|
||||
| Writing or editing any doc, or answering a review finding in prose | `docs/decisions/records/docs/no-session-narrative.md` — the doc records the END STATE; the path to it goes in the commit message. Apply the who-benefits test, and read the carve-out before you cut (dated measurements, stated snapshot boundaries and tested-and-rejected results stay) |
|
||||
| Adding / changing / deleting a guard file | `docs/guard-inventory.md` — every guard's row is machine-checked by `scripts/tests/test_guard_inventory.py`, so a new guard must acquire a row before the suite goes green, and a row graded `MUTATION` must also acquire a declared clause in `scripts/tests/mutation_manifest.py` |
|
||||
| Writing code that reads live Gitea/remote state and then acts on it | `docs/decisions/records/process/check-and-use-pins-a-version.md`, then `docs/remote-state-inventory.md` — a new executable under `scripts/` (**excluding `scripts/tests/`**), `.claude/hooks/`, `.husky/` or `.gitea/workflows/` must acquire a row there before `scripts/tests/test_remote_state_inventory.py` goes green |
|
||||
|
||||
@@ -201,7 +201,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera
|
||||
| `testing.e2e-local-fresh-config-dir` | Always point `scripts/e2e-local.sh` at a fresh config dir — leftover channels/schedules/DB rows bleed state between runs and corrupt assertions. (The *readiness-probe hang* this record was originally written about was fixed in #533; the fresh-dir rule stands on state-bleed grounds alone.) | 2026-07-21 | [link](records/testing/e2e-local-fresh-config-dir.md) |
|
||||
| `testing.enumerating-guard-identity-not-position` | A guard that cross-checks a hand-reviewed registry against call sites discovered across the whole repo must key each entry on properties INTRINSIC to the site — file, kind, and the value source text — and never on its absolute line or column. A registry keyed on position is a function of every other file in the repo, so a branch that never touches the guard can invalidate it; and because each PR is green against its own base, that failure is structurally invisible pre-merge and lands on `main` after review and after the merge gate. Dropping the position keeps every mutation the guard exists for — a NEW site, a REMOVED site and a CHANGED value each still fail, since each changes the identity multiset — and costs exactly ONE case, which must be stated rather than implied: a SAME-IDENTITY SUBSTITUTION within one file (delete a registered site, add a different unreviewed one with the same kind and value token, net-zero count) now passes. A REPORTED failure still prints the discovered line:column, because identity and diagnostics need not share a format. Comparison stays a MULTISET count rather than set membership, so two sites in one file sharing an identity must be discovered exactly that many times and a third occurrence still fails. A SCANNER test that asserts real AST positions against FIXED inline fixtures is the opposite case and keeps its line/column identity — it has no churn, because its input does not move. | 2026-07-27 | [link](records/testing/enumerating-guard-identity-not-position.md) |
|
||||
| `testing.fix-ships-a-witnessed-red-test` | A commit claiming to fix something may carry a `Proves: <pytest selector>` trailer; when it does, `scripts/prove-fix.sh` must show that selector GREEN with the fix and RED with the code side reverted, and CI enforces it per-PR. The trailer is opt-in — an unproven commit is allowed — but a claimed proof that does not hold fails the build. | 2026-08-16 | [link](records/testing/fix-ships-a-witnessed-red-test.md) |
|
||||
| `testing.full-replace-asserts-field-list` | 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. | 2026-08-21 | [link](records/testing/full-replace-asserts-field-list.md) |
|
||||
| `testing.full-replace-asserts-field-list` | 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. | 2026-08-21 | [link](records/testing/full-replace-asserts-field-list.md) |
|
||||
| `testing.guard-derives-population-from-source` | A guard that asserts a COMPLETENESS property enumerates its population from a machine-readable authoritative source — the enum, the generated OpenAPI document, the parsed workflow YAML, the provider list — and asserts SET EQUALITY in BOTH directions against it. It may not narrow that population with a filter, a `Where`, a `grep` or an early `continue` before the assertion, because a filter cannot see the member that is MISSING: the member whose absence is the defect is precisely the one the predicate excludes. A hand-written literal list of members is the same defect in slower motion — a filter frozen at authoring time, correct on the day it was written and unable to report the day it stopped being. Two boundaries bound the rule rather than weaken it. FIRST, filtering to select the SUBJECT of a PER-MEMBER property is legitimate and is not this defect: the excluded members satisfy the property vacuously, so the filtered walk and the whole walk assert the same thing (`ToolCatalogTests.Every_Query_Parameter_Should_Be_A_Declared_Property` filters to tools that declare query parameters, and a tool declaring none has nothing to check). The defect is filtering the population before a COMPLETENESS claim, which is what makes an absent member unrepresentable (#757 filtered on `QueryParameters is {Count: > 0}` and so could not see a tool that should have declared one and did not). SECOND, a population of VALUES always has an external authoritative source and this rule applies directly; a population of SITES IN CODE has no such list, needs find-all-references tooling, and is tracked separately in #777 — do not stretch a set-equality assertion over it. Distinguish the guard SCOPE (which subsystems it covers — a reviewed policy choice, legitimately hand-written) from the guard POPULATION (the members inside that scope — always derived). When the scope itself MIRRORS an authoritative source, the mirror needs its own equality check or a dated staleness marker, or the guard is complete within a scope that has silently gone stale. The canonical worked example in this repo is `ToolCatalogTests.Every_Tool_Should_Declare_Exactly_Its_OpenApi_Query_Parameters`; the canonical residual gap is `MARKED_JOBS` in `scripts/tests/test_ci_dropped_step_guard.py`. WHEN THE POPULATION IS FILES (#806), the authoritative source is the GIT INDEX and never a filesystem walk. A walk is not merely a weaker enumerator, it answers a question about the MACHINE rather than about the repo: it reports build output, generated shims and editor droppings, and it differs between CI and every checkout, so the same guard asserts a different population in each place. Derive with `git ls-files`, take direct children only unless a nested population is stated and wanted, and assert existence rather than filtering on it, because filtering is what makes a missing member unrepresentable. This is an instantiation and not a blanket rewrite: the question per guard remains whether it makes a COMPLETENESS claim over TRACKED files, and a walk that assembles a fixture or selects the SUBJECT of a per-member property stays a walk with its reason written down. | 2026-08-13 | [link](records/testing/guard-derives-population-from-source.md) |
|
||||
| `testing.guard-ships-with-mutation-proof` | A guard is not considered tested because a test involving it passes. It ships with a MUTATION PROOF: remove or disarm THAT GUARD'S CLAUSE ALONE, and a NAMED test must go red. ONE NAMED EXCEPTION, with its limits, because the rule degenerates without it: where the guard IS a test (a checker enforcing a repo invariant, with no separate script behind it), disarming it makes it ABSENT rather than red, so the proof is the contrapositive — INTRODUCE THE DEFECT THE GUARD EXISTS TO CATCH into an isolated copy of the guarded artifact, and the named test must go red. That is a mutation of the guarded SYSTEM rather than of the assertion, and it is admissible ONLY for checker-guards and ONLY when the mutation was executed and witnessed. It is NOT a licence to grade an ordinary script-guard MUTATION for having a bad-input test: feeding a script an input its clause rejects is BEHAVIOUR-ONLY, which is what three rows were regraded for. A file-level grade under this exception covers the clause its cited case actually mutates, not every assertion that later lands in the same file. Three things this excludes, each of which has already shipped here as a green suite over a dead check. FIRST, a behavioural test — one that feeds the guard a good input and a bad input and checks it passes and fails — proves the guard REACTS, never that it is LOAD-BEARING; #685 had two guards on one condition where deleting either left the whole suite green while every behavioural test passed. SECOND, mutating the WHOLE FILE does not count (#510): a whole-file revert cannot show that a test reaches a particular clause, so the mutation must target the clause. THIRD, the guard being WIRED is not the guard RUNNING — #631's suite was invoked by no CI job, #751's step was dropped by the runner and the job reported success in 6s against a normal 14-17s, and #719's new logic was never connected to stdin. Every guard that DERIVES A POPULATION also carries an ANTI-VACUITY assertion, because the characteristic failure of a completeness check is reporting that it proved everything while its population was empty; a guard with no population has nothing for such an assertion to be about, and stating it universally reads as coverage the unproven rows do not have. Mechanical enforcement is possible for the BOOKKEEPING and not for the JUDGEMENT, and the split is the decision: `docs/guard-inventory.md` lists every guard file with its Kind, its Proof class (`MUTATION`/`BEHAVIOUR-ONLY`/`NONE`) and a `file::function` ref, and `scripts/tests/test_guard_inventory.py` derives the guard population from the GIT INDEX and the call sites (#806), asserts SET EQUALITY against the rows, and resolves every claimed ref to a real `def`. So a new guard cannot ship unclassified and a renamed test cannot leave a row silently claiming coverage. Whether a row claiming `MUTATION` is telling the truth is no longer left to review: `testing.mutation-claims-are-executed` (#790) requires each such row to carry a DECLARED clause mutation that is applied to an isolated copy of the repository on every run, with the row's own named test required to go red. | 2026-08-13 | [link](records/testing/guard-ships-with-mutation-proof.md) |
|
||||
| `testing.hook-reports-its-own-execution` | Every script in `.claude/hooks/` sources `scripts/hook-fire-log.sh` and calls `etv_hook_fire_begin <its-own-name> <label> <capture\|stream>` as its FIRST act, before anything reads stdin. Two records are appended per invocation — a `fire` record on entry and an `exit` record carrying the exit status and the decision — to a session-scoped JSONL log. THE DECISION IS READ FROM WHAT THE HOOK ACTUALLY EMITTED, never declared by the hook author: Claude Code hooks (`capture` mode) always exit 0 and communicate by PRINTING JSON, so their stdout is diverted and replayed, and the recorded decision is parsed from those bytes; git hooks (`stream` mode) decide by EXIT CODE and their stdout is live progress text a human is watching, so it is not diverted and the decision is the status. That split is not a tuning knob — capturing a slow pre-push hook's output would hold it back until the end and read as a hang, and inferring a git hook's decision from absent JSON would put the report back into the guessing business this record exists to end. The population is DERIVED from `.claude/hooks/*.sh` by `scripts/tests/test_hook_fire_log.py`, so a new hook is uninstrumented-and-red rather than silently unobserved, and the report lists every hook that EXISTS rather than every hook that appears in the log — a report built from the log alone can only show hooks that fired, which makes the never-fired hook, the one finding worth having, invisible. THE INSTRUMENTATION MUST BE INVISIBLE TO THE HARNESS, and this is the load-bearing half: it sits in the stdin and stdout path of the most authoritative guards in the repo, so a differential test drives EVERY hook with and without it over a payload matrix and demands byte-equal stdout and equal exit status. It fails OPEN in exactly one direction — if the log cannot be written the hook behaves exactly as before — because observability that breaks a guard is worse than the blindness it replaces. Two mechanical traps are pinned by tests rather than left to care: stdout must be replayed from the FILE, since `out=$(cat f)` strips trailing newlines and delivers a guard's JSON one byte short with no parser anywhere to complain; and stdin must never be slurped when it is a TTY, because an interactive `git commit` hands its hooks a terminal and `cat` would block forever, hanging the commit the instrumentation was added to observe. | 2026-08-14 | [link](records/testing/hook-reports-its-own-execution.md) |
|
||||
|
||||
@@ -5,8 +5,8 @@ status: active
|
||||
since: '2026-08-21'
|
||||
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.'
|
||||
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 · paths: `ErsatzTV.Tests/Application/ProgramSchedules/ScheduleItemResponseRoundTripTests.cs`, `ErsatzTV.Mcp.Tests/ToolCatalogTests.cs`, `ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItemsHandler.cs` · issues: #779, #773, #757, #754, #500, #252'
|
||||
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.'
|
||||
---
|
||||
|
||||
@@ -130,9 +130,128 @@ be a name matcher over handler classes ending `Replace…Handler`, which misses
|
||||
and flags the ones that do it correctly. Per-site derivation, applied when a full-replace path is
|
||||
touched, is the enforceable version.
|
||||
|
||||
**Residue, tracked rather than implied closed.** The SPA builds request objects field-by-field in six
|
||||
screens (`normalizeForSave` and its equivalents). A *required* field omitted there fails `tsc`; an
|
||||
*optional* field added to the DTO and not carried through compiles clean and drops silently — #754's
|
||||
mechanism narrowed to optional fields, with no equivalent of the MCP write-tool test on the SPA side.
|
||||
That is the highest-probability recurrence in the repo today and is filed separately rather than
|
||||
folded in here.
|
||||
**The SPA half.** The residue this record recorded was that the SPA builds request objects
|
||||
field-by-field in six screens (`normalizeForSave` and its equivalents), so an *optional* field added
|
||||
to a DTO and not carried through would compile clean and drop silently.
|
||||
|
||||
**The current statement.** A member is omittable exactly when it sits outside its schema's
|
||||
`required` array; nullable properties usually emit as required-and-nullable (`"name": null | string`),
|
||||
so most builders are checked and the gap reads as closed on inspection. It is not. Two full-replace
|
||||
PUTs carried a live optional member (verified by execution 2026-08-22, by deleting the field and
|
||||
watching `npm run typecheck` stay CLEAN):
|
||||
|
||||
| schema | optional member | wrapper | consequence of the drop |
|
||||
|---|---|---|---|
|
||||
| `MultiCollectionItemRequest` | `weight` | `updateMultiCollection` (PUT) | every weight resets to 1 on save |
|
||||
| `UpdateFFmpegProfileRequest` | `qsvPreferNativeDecoder` | `updateFFmpegProfile` (PUT) | the setting reverts to its default |
|
||||
|
||||
**The framing this replaced, recorded so it is not re-adopted**: that the mechanism was *latent* —
|
||||
true of the request schemas the six named builders target, which were the ones checked, and
|
||||
reported as a property of the SPA. (No count is given: the earlier drafts carried one, and no
|
||||
natural cut of "the schemas those builders target" reproduces it.) A
|
||||
conclusion verified across the cases examined and then stated about the whole is the shape this
|
||||
corpus keeps recording, and it read as checked. `MultiCollectionsScreen` carries a prose comment
|
||||
warning about precisely that weight reset; a comment is not a check, and neither is a boundary
|
||||
drawn around the sample.
|
||||
|
||||
**`Complete<T>`** (`web/src/api/completeRequest.ts`) maps a request type so every member is
|
||||
required, making an omission a hard error however the schema was modelled. THE RULE is to apply it
|
||||
in two places: at the full-replace API-wrapper boundary, so later callers inherit it without knowing
|
||||
it exists, and at each construction site, which is what keeps TypeScript's excess-property check
|
||||
(the *phantom* direction) alive.
|
||||
|
||||
That is the rule, not a claim about how much of the tree currently follows it. Nothing enforces the
|
||||
second half — #820 — so any sentence here asserting present coverage would be falsifiable by one
|
||||
`tsc` run and would go stale on the next screen anyone adds. Two wrappers annotated under #807
|
||||
initially shipped with unannotated construction sites for exactly that reason. To find out what is
|
||||
actually annotated, read the code; do not read a count here.
|
||||
|
||||
**Why the second half of the rule is the one that gets skipped.** The wrapper annotation catches a
|
||||
MISSING member anywhere. The PHANTOM direction — a field the schema does not accept — relies on
|
||||
TypeScript's excess-property check, and that fires only on a fresh object literal in a contextually
|
||||
typed position. A literal returned from a generic `.map` callback is not one, because `map<U>`
|
||||
infers `U` from the callback rather than from the target element type. Measured on the pre-#807 tree by injecting a
|
||||
phantom property at each construction site then present: some rejected it and some accepted it,
|
||||
and among those that accepted, three contained neither a spread nor an inferred local — so "it has
|
||||
no spread" is not a reason to think a site is checked. No denominator is given: what counts as a
|
||||
"construction site" is not derived from anything, the figure was already wrong once on this branch,
|
||||
and the transferable finding is the mechanism, not the tally.
|
||||
|
||||
Measured, not argued (2026-08-22): injecting an optional member into `ScheduleItemRequest` reddens
|
||||
`normalizeForSave` with `TS2741` under `Complete<T>` and compiles clean without it; deleting
|
||||
`weight` from `MultiCollectionsScreen.toItemRequest` now reddens and previously did not. The proof
|
||||
ships as `@ts-expect-error` cases in `web/src/api/completeRequest.guard.test.ts`, re-executed on
|
||||
every `npm run typecheck` (a marked CI step).
|
||||
|
||||
**The boundary is DERIVED, and the two attempts to write it by hand are why.** The rule this
|
||||
record states — derive the population, never enumerate it — took two rounds to apply to the record's
|
||||
own coverage boundary, each time failing the same way: a schema sorted on its NAME rather than on
|
||||
what its endpoint does.
|
||||
|
||||
| round | the hand-written form | what it missed |
|
||||
|---|---|---|
|
||||
| 1 | a prose sentence exempting "create/update" | `updateMultiCollection` and `updateFFmpegProfile` are full replaces — both were LIVE silent drops |
|
||||
| 2 | a table, written to replace that sentence | `ArtworkContentTypeModel`, because `…Model` reads as a response model. It is reachable from the full-replace `PUT /channels/{id}` |
|
||||
|
||||
Two misses from one mechanism, so the mechanism went instead of the list getting a third patch.
|
||||
`scripts/tests/test_optional_request_members.py` now DERIVES the population every run from
|
||||
`ErsatzTV/wwwroot/openapi/v1.json` — every schema carrying a property outside its `required` array
|
||||
that is **transitively** reachable from ANY operation's request body, plus request bodies declared
|
||||
inline rather than by `$ref` — and asserts set equality in both directions against a registry of
|
||||
per-schema dispositions. Its own reach is bounded by what its composition resolver handles, and the
|
||||
resolver's branches are pinned by constructed-schema tests rather than by the one shape today's
|
||||
document happens to contain; read that file for the current boundary rather than a summary here. Transitivity is load-bearing:
|
||||
`MultiCollectionItemRequest` and `ArtworkContentTypeModel` are both nested, so a check reading only
|
||||
top-level bodies would have reproduced both misses.
|
||||
|
||||
The split follows `testing.guard-derives-population-from-source`: the POPULATION is derived, the
|
||||
DISPOSITIONS are the SCOPE — a reviewed policy choice per schema, legitimately hand-written, and
|
||||
forced to exist by the equality assertion. A new optional member in a named component schema
|
||||
reachable from a request body, or in an inline request body, now fails that test until someone
|
||||
writes down what should happen about it. The dispositions themselves
|
||||
live in that file rather than here, so there is one copy.
|
||||
|
||||
**On this record's own pin-vs-floor argument, applied to that guard.** The `Id` discussion above
|
||||
argues that an anti-vacuity check must be a PIN and not a `>=` floor, because a floor lets members
|
||||
vanish silently. `test_optional_request_members.py` uses floors, and that is not a quiet exception:
|
||||
a pin on "how many schemas are reachable from a request body" would be a pin on the size of the
|
||||
whole API, red on every unrelated endpoint added. What replaces the pin's strength is the
|
||||
both-directions set equality — a schema that vanishes from the population reports as PHANTOM, which
|
||||
is exactly what the pin existed to catch — plus a planted-member test that is red whenever the walk
|
||||
stops seeing nested or `oneOf`-referenced schemas. Measured 2026-08-22: the floors alone do NOT
|
||||
catch a partially broken walk (deleting the transitive step leaves them satisfied); the planted
|
||||
test and the equality assertion do. The floors are the crude backstop against a parse that reached
|
||||
nothing at all, and the guard's docstring says so rather than letting them read as coverage.
|
||||
|
||||
**One disposition is worth stating here because it bounds this record's own rule.** `Complete<T>`
|
||||
must NOT be applied to a schema whose optional members are computed get-only properties:
|
||||
`ArtworkContentTypeModel`'s `IsExternalUrl` / `HasContentType` / `UrlWithContentType` are derived
|
||||
from `Path` and are never deserialized, so a client omitting them drops nothing — while annotating
|
||||
the site would force a caller to fabricate server-computed values in an outbound request. The test
|
||||
that decides the column is **does this write replace a whole entity or collection, and can the
|
||||
member actually carry a stored value**, not whether the wrapper is named `update…` or `replace…`.
|
||||
|
||||
**The wrapper-boundary rationale is a DIRECTION, not a claim about today's coverage.** Some
|
||||
full-replace PUT wrappers in `web/src/api/*.ts` carry `Complete<>` — the ones reachable from a
|
||||
schema that can drop a member, plus the six builders #807 set out to fix — and most do not. No
|
||||
figure is given here deliberately, and neither is a characterisation of WHICH ones: nothing
|
||||
derives either, and a hand-maintained description of a code population is the thing this record
|
||||
argues against. (An earlier draft said "the ones reachable from a schema that can drop a member,
|
||||
plus the six builders #807 set out to fix" — five annotated wrappers fall outside both sets. A set
|
||||
phrased in words rots exactly like a count.) No reproduction command is offered either: isolating
|
||||
"full-replace PUT wrappers" from a grep needs the judgement about intent this record already says a
|
||||
matcher cannot make. For the unannotated wrappers the protection is still the
|
||||
contingent kind this record complains about: it holds only while their properties stay inside
|
||||
`required`. What makes that survivable rather than a re-run of the same mistake is that the
|
||||
contingency is now MONITORED — the derived test above reddens the moment one of them acquires an
|
||||
optional member — instead of being assumed. Annotating the rest is cheap and should happen when
|
||||
each is next touched.
|
||||
|
||||
**Residue, named rather than implied closed.** `Complete<T>` proves its own semantics but NOT that
|
||||
it is applied: reverting one screen to its pre-#807 form leaves the guard green, because the
|
||||
population of construction sites is derived from nothing. The repo already owns the tool for that
|
||||
— `web/src/api/pageSizeScan.ts` (TypeScript compiler API) plus a registry cross-checked in both
|
||||
directions — and building it is tracked in #820. Until then this convention is enforced by review,
|
||||
which is the weaker thing this record exists to warn about. Separately: `Complete<T>` is shallow,
|
||||
so a new nested item request type needs its own annotation; and requiring a field to be *named* is
|
||||
not the same as requiring it to be *populated* correctly.
|
||||
|
||||
@@ -56,8 +56,11 @@ inventory shipped with were sitting in the gap:
|
||||
1. **Guards inline in workflow YAML** — most importantly `pr-checks.yml:ci-image-pin`. "Which jobs
|
||||
are guards" needs a judgement call per job the filesystem cannot supply. Two were audited under
|
||||
#774 and one fixed; extending the population is tracked in #786.
|
||||
2. **C# and TypeScript guards** — `ErsatzTV.Mcp.Tests/ToolCatalogTests.cs` and
|
||||
`web/src/api/pageSizeCallSites.guard.test.ts` are both structural guards and neither has a row.
|
||||
2. **C# and TypeScript guards** — `ErsatzTV.Mcp.Tests/ToolCatalogTests.cs`,
|
||||
`web/src/api/pageSizeCallSites.guard.test.ts` and `web/src/api/completeRequest.guard.test.ts`
|
||||
are all structural guards and none has a row. Note what that costs: this list is a HAND-WRITTEN
|
||||
mirror of a population nothing derives, so it goes stale silently and CI stays green — #807
|
||||
added the third entry, and only review caught that the second had become the only one named.
|
||||
3. **Mentions counted as call sites.** The `scripts/…` scrape matches any occurrence, including
|
||||
inside a comment or an `::error::` string. `scripts/update-openapi.sh` is named in a
|
||||
`pr-checks.yml` error message, so removing the step that runs it would leave its row intact.
|
||||
@@ -213,6 +216,7 @@ recorded as unexamined rather than as cleared.
|
||||
| `scripts/tests/test_prepush_unsets_git_env.py` | the `script-tests` job | PROOF | NONE | — |
|
||||
| `scripts/tests/test_pr_changed_files.py` | the `script-tests` job | PROOF | NONE | — |
|
||||
| `scripts/tests/test_prepush_rebase_check_tag_exemption.py` | the `script-tests` job | PROOF | NONE | — |
|
||||
| `scripts/tests/test_optional_request_members.py` | the `script-tests` job, on an OpenAPI request schema that can silently drop a member with no stated disposition | GUARD | MUTATION | `test_optional_request_members.py::test_every_droppable_request_schema_has_a_stated_disposition` |
|
||||
| `scripts/tests/test_prove_fix.py` | the `script-tests` job | PROOF | NONE | — |
|
||||
| `scripts/tests/test_remote_state_inventory.py` | the `script-tests` job, on an executable that talks to a remote service with no row in `docs/remote-state-inventory.md` | GUARD | MUTATION | `test_remote_state_inventory.py::test_every_in_scope_file_has_a_row_and_every_row_names_a_real_file` |
|
||||
| `scripts/tests/test_worktree_ownership_guard.py` | the `script-tests` job | PROOF | NONE | — |
|
||||
@@ -237,7 +241,7 @@ clause-provable and the entry is regraded.
|
||||
|
||||
## What the numbers say
|
||||
|
||||
37 guards, 6 tooling scripts, 20 proof files. **15 guards carry a mutation proof; 6 are
|
||||
38 guards, 6 tooling scripts, 20 proof files. **16 guards carry a mutation proof; 6 are
|
||||
behaviour-only; 16 have none.** These figures are asserted against the table by
|
||||
`test_the_summary_counts_match_the_table` — they were wrong in the first draft (28/4/6/3/19 against
|
||||
a table holding 27/5/6/3/18), because a hand-maintained summary of a table is a second copy of it,
|
||||
|
||||
@@ -446,6 +446,52 @@ fresher edit:
|
||||
(authoritative) — never let an out-of-range value reach the server and surface a raw 400. Mirror the
|
||||
server's bound as a shared const (e.g. multi-collection weight `WEIGHT_MIN`/`WEIGHT_MAX` = 1..1000, #404).
|
||||
|
||||
## 4b. Full-replace request bodies are built as `Complete<T>` (#807)
|
||||
|
||||
A full-replace endpoint writes the WHOLE entity, so a field the builder never sets is not left
|
||||
alone — it is written as its default. Build every full-replace body against `Complete<T>`
|
||||
(`web/src/api/completeRequest.ts`), which maps a generated request type so **every** member is
|
||||
required:
|
||||
|
||||
```ts
|
||||
function toReplaceRequest(draft: Draft): Complete<ReplaceBlockRequest> { … }
|
||||
items.map((item): Complete<DecoTemplateItemRequest> => ({ … }))
|
||||
```
|
||||
|
||||
Two rules, and the second is the one that gets skipped:
|
||||
|
||||
- **Annotate the wrapper parameter** (`body: Complete<ReplaceBlockRequest>`) so every caller —
|
||||
including ones written later by someone who never read this — inherits the check.
|
||||
- **Annotate each construction site too, including every `.map` callback's return type.** The
|
||||
wrapper annotation catches a *missing* field anywhere. The *phantom* direction (a field the
|
||||
schema does not accept) relies on TypeScript's excess-property check, which fires only on a
|
||||
**fresh object literal in a contextually typed position** — and a literal returned from a generic
|
||||
`.map` callback is not one, because `map<U>` infers `U` from the callback's return rather than
|
||||
from the target element type. A spread or an inferred local loses it too, but the `.map` callback
|
||||
is the common case and the easy one to misread as safe: several construction sites accepted a
|
||||
phantom field before #807, and three of them contained neither a spread nor an inferred local.
|
||||
|
||||
An optional member may be written `field: undefined`. The point is not to forbid omitting a value,
|
||||
it is to forbid omitting the *decision*: an unmentioned field is an oversight, an explicit
|
||||
`undefined` is a choice a reviewer can see.
|
||||
|
||||
**Do NOT apply `Complete<T>` to a schema whose optional members are computed server-side.**
|
||||
`ArtworkContentTypeModel` (reachable from `PUT /channels/{id}` via `UpdateChannelRequest.logo`) has
|
||||
`isExternalUrl` / `hasContentType` / `urlWithContentType` as get-only properties derived from
|
||||
`path`. Nothing deserializes them, so omitting them drops nothing — and annotating the site would
|
||||
force you to invent server-computed values in an outbound request. Check the schema's disposition
|
||||
in `scripts/tests/test_optional_request_members.py` before annotating a new site.
|
||||
|
||||
Why this is needed even though most builders already typecheck: a member is omittable exactly when
|
||||
the generated type marks it `?:`, which comes from the `required` array of the schema in
|
||||
`ErsatzTV/wwwroot/openapi/v1.json` — the generator script only passes it through. Most nullable
|
||||
properties emit as required-and-nullable, so the gap looks closed on inspection while a minority
|
||||
sits unchecked inside it. Do not trust a list of which schemas those are: two hand-written ones
|
||||
were wrong (#807). `scripts/tests/test_optional_request_members.py` derives the set on every run
|
||||
and fails until each has a stated disposition. See
|
||||
`docs/decisions/records/testing/full-replace-asserts-field-list.md`, and
|
||||
`web/src/api/completeRequest.guard.test.ts` for the executed proof.
|
||||
|
||||
## 5. Artwork rendering
|
||||
|
||||
Render `item.artwork` / `item.poster` (or whatever the DTO field is named) **directly as an `<img
|
||||
|
||||
@@ -146,6 +146,25 @@ MUTATIONS: tuple[Mutation, ...] = (
|
||||
"the index, which is what the proof does, separates them. That is why the proof has to "
|
||||
"remove EVERY member rather than sample one.",
|
||||
),
|
||||
Mutation(
|
||||
guard="scripts/tests/test_optional_request_members.py",
|
||||
target="scripts/tests/test_optional_request_members.py",
|
||||
clause='"ArtworkContentTypeModel": (',
|
||||
replacement='"ArtworkContentTypeModelRENAMED": (',
|
||||
proof="test_optional_request_members.py::test_every_droppable_request_schema_has_a_stated_disposition",
|
||||
granularity=CLAUSE,
|
||||
expect="no disposition written down",
|
||||
why="THE GUARD IS A TEST, so the mutation goes into the guarded ARTIFACT — here the "
|
||||
"DISPOSITIONS registry the checker maintains, the same shape as the deleted "
|
||||
"guard-inventory row below. Renaming the key rather than deleting the entry keeps the "
|
||||
"module importable, so the red is a real set-equality failure and not an ImportError "
|
||||
"reddening for the wrong reason. The rename fires BOTH directions — MISSING for the real "
|
||||
"schema and PHANTOM for the renamed key — which is the correct behaviour and worth stating, "
|
||||
"since `expect` names only the MISSING half. "
|
||||
"`ArtworkContentTypeModel` is the right key to name: it is the exact schema #807's "
|
||||
"hand-written table omitted, because `...Model` reads as a response model while it is in "
|
||||
"fact reachable from the full-replace PUT /channels/{id}.",
|
||||
),
|
||||
Mutation(
|
||||
guard="scripts/tests/test_guard_inventory.py",
|
||||
target="docs/guard-inventory.md",
|
||||
|
||||
@@ -0,0 +1,821 @@
|
||||
"""#807 guard: every schema that can SILENTLY DROP a member on a SPA write has a stated disposition.
|
||||
|
||||
WHAT THIS BLOCKS. A request-body property that is absent from its schema's `required` array emits
|
||||
into `web/src/api/generated/v1.d.ts` as an OPTIONAL member (`"weight"?: number`). A SPA builder may
|
||||
then omit it, `tsc` says nothing — optional means omittable, by design — and on a FULL-REPLACE write
|
||||
the server stores the field's default. That is #754's mechanism and it is what #807 found live in
|
||||
`MultiCollectionItemRequest.weight` and `UpdateFFmpegProfileRequest.qsvPreferNativeDecoder`.
|
||||
|
||||
WHY IT IS A DERIVED GUARD AND NOT A TABLE IN A DOC. #807 shipped the disposition list by hand
|
||||
TWICE and got it wrong BOTH times, each time by sorting a schema on its NAME rather than on what its
|
||||
endpoint does:
|
||||
|
||||
round 1 a prose sentence exempted "create/update" — `updateMultiCollection` and
|
||||
`updateFFmpegProfile` are full replaces, and both were live silent drops.
|
||||
round 2 a hand-written table replaced that sentence and omitted `ArtworkContentTypeModel`,
|
||||
because `…Model` reads as a response model. It is reachable from `PUT /channels/{id}`.
|
||||
|
||||
Two misses from one mechanism, so the mechanism goes rather than the list getting a third patch.
|
||||
`testing.guard-derives-population-from-source` is explicit that a hand-written list is "a filter
|
||||
frozen at authoring time, correct on the day it was written and unable to report the day it stopped
|
||||
being" — and unlike #820's population (sites in code, which needs compiler-API tooling), THIS
|
||||
population has an authoritative machine-readable source: the OpenAPI document.
|
||||
|
||||
SCOPE vs POPULATION, per that same record. The POPULATION — which schemas can drop a member on a
|
||||
write — is DERIVED here, every run, from `ErsatzTV/wwwroot/openapi/v1.json`. The DISPOSITIONS below
|
||||
are the SCOPE: a reviewed policy choice per schema, legitimately hand-written, and each one is
|
||||
FORCED to exist by the set-equality assertion. A new optional member in a named component schema
|
||||
reachable from a request body, or in an inline request body, fails this test until someone writes
|
||||
down what should happen about it — that is the reach, bounded by what `_resolve`
|
||||
resolves. That resolver walks `allOf`, `oneOf`, `anyOf`, `if`/`then`/`else`, `dependentSchemas`,
|
||||
`items`/`prefixItems` and inline objects under `properties`, and deliberately contributes nothing
|
||||
for `additionalProperties`/`patternProperties` (which name no fixed members) — each pinned by a
|
||||
case in `test_composition_is_resolved_the_way_JSON_Schema_means_it`. It does NOT follow `$ref`;
|
||||
that is the component walk's job. An earlier draft said "anywhere in the request graph"; inline bodies were invisible at
|
||||
the time, so the universal was false the day it was written.
|
||||
|
||||
Set equality is asserted in BOTH directions and reported separately, because they are opposite
|
||||
defects: `missing` is a schema the API can drop and nobody has ruled on (the #807 defect), `phantom`
|
||||
is a disposition for a schema that no longer has an optional member reachable from a request body
|
||||
(the registry claiming coverage of something gone).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
|
||||
OPENAPI = REPO_ROOT / "ErsatzTV" / "wwwroot" / "openapi" / "v1.json"
|
||||
|
||||
# --- the closed disposition vocabulary -------------------------------------------------------
|
||||
#
|
||||
# COVERED the SPA builds this body and the builder is annotated `Complete<T>`, so omitting a
|
||||
# member is a typecheck error (`web/src/api/completeRequest.ts`).
|
||||
# CREATE a POST that creates a new entity, where an omitted member correctly means "use the
|
||||
# default". Annotating it would be a BUG, not coverage — see the from-lineup note.
|
||||
# TRIGGER the body parameterises an ACTION and replaces no stored entity, so there is nothing to
|
||||
# drop.
|
||||
# COMPUTED the optional members are get-only computed properties on the C# record. System.Text.Json
|
||||
# never deserializes them, so the client cannot drop a stored value by omitting them —
|
||||
# and `Complete<T>` must NOT be applied here, because it would force a caller to invent
|
||||
# server-computed values in an outbound request.
|
||||
COVERED = "COVERED"
|
||||
CREATE = "CREATE"
|
||||
TRIGGER = "TRIGGER"
|
||||
COMPUTED = "COMPUTED"
|
||||
|
||||
DISPOSITIONS: dict[str, tuple[str, str]] = {
|
||||
"UpdateFFmpegProfileRequest": (
|
||||
COVERED,
|
||||
"PUT /ffmpeg/profiles/{id} is a full replace. `qsvPreferNativeDecoder` is a defaulted ctor "
|
||||
"param so ASP.NET drops it from `required`. Was a LIVE silent drop before #807.",
|
||||
),
|
||||
"CreateFFmpegProfileRequest": (
|
||||
COVERED,
|
||||
"FFmpegProfilesScreen's `Draft` feeds BOTH the POST and the full-replace PUT, so the draft "
|
||||
"type itself is `Complete<…>` rather than only the update wrapper.",
|
||||
),
|
||||
"MultiCollectionItemRequest": (
|
||||
COVERED,
|
||||
"Nested in the full-replace PUT /multi-collections/{id}. `weight` is a defaulted ctor param "
|
||||
"(`CreateMultiCollectionRequest.cs`). Was a LIVE silent drop before #807: the screen carried "
|
||||
"a prose comment warning that dropping it resets every weight to 1, and a comment is not a "
|
||||
"check.",
|
||||
),
|
||||
"ArtworkContentTypeModel": (
|
||||
COMPUTED,
|
||||
"Reachable from the full-replace PUT /channels/{id} (via `UpdateChannelRequest.logo`), so "
|
||||
"the endpoint test alone would put it in COVERED. It is not: `IsExternalUrl`, "
|
||||
"`HasContentType` and `UrlWithContentType` are computed get-only properties on the record "
|
||||
"`ArtworkContentTypeModel(string Path, string ContentType)`, never deserialized, so a "
|
||||
"client omitting them drops nothing. Annotating the SPA site `Complete<…>` would force it "
|
||||
"to fabricate server-computed values. This row exists because #807's hand-written table "
|
||||
"omitted this schema on the strength of its `…Model` name.",
|
||||
),
|
||||
"AutoTunedChannelRequest": (
|
||||
CREATE,
|
||||
"POST /channels/auto-tune. `CreateChannelFromLineupHandler` does `Channels.Add(...)` and "
|
||||
"rejects a duplicate number; it never overwrites an existing channel.",
|
||||
),
|
||||
"CreateChannelFromLineupAdvancedOptionsRequest": (
|
||||
CREATE,
|
||||
"POST /channels/from-lineup. Omission is LOAD-BEARING here: `CreateChannelFromLineupClearField` "
|
||||
"documents that for the template-inheritable fields a null/omitted override means INHERIT the "
|
||||
"template value, with a separate explicit `clear` list to force NONE. `Complete<T>` would "
|
||||
"collapse that third state into explicit-null.",
|
||||
),
|
||||
"AutoTuneSourceWeightRequest": (
|
||||
CREATE,
|
||||
"Nested in the auto-tune POST body; same create-time semantics as its parent.",
|
||||
),
|
||||
"POST /api/v1/artwork/uploads (multipart/form-data inline body)": (
|
||||
TRIGGER,
|
||||
"A multipart upload. Its body is declared INLINE rather than as a named schema, which is why "
|
||||
"it needs a key of this shape at all. `file` and `target` sit outside `required`, but the "
|
||||
"endpoint stores an uploaded blob and replaces no entity, so there is no stored value for an "
|
||||
"omitted member to overwrite; `web/src/api/artwork.ts` builds it as hand-rolled `FormData` "
|
||||
"with no generated type involved.",
|
||||
),
|
||||
"ScanShowRequest": (
|
||||
TRIGGER,
|
||||
"POST /libraries/{id}/scan-show starts a scan. `deepScan` parameterises the action; no entity is replaced.",
|
||||
),
|
||||
}
|
||||
|
||||
# Anti-vacuity floors. A completeness check whose population came back empty must not report that it
|
||||
# proved everything (`testing.guard-ships-with-mutation-proof`). These are LOWER bounds on the
|
||||
# derived intermediates, deliberately well under today's values (142 reachable, 13 with optional
|
||||
# members) so ordinary schema churn does not trip them — they exist to catch a broken parse or a
|
||||
# `$ref` walk that reached nothing, not to pin the corpus.
|
||||
MIN_REQUEST_REACHABLE_SCHEMAS = 40
|
||||
MIN_SCHEMAS_WITH_OPTIONAL_MEMBERS = 5
|
||||
|
||||
|
||||
def _load() -> dict:
|
||||
if not OPENAPI.is_file():
|
||||
pytest.fail(f"{OPENAPI.relative_to(REPO_ROOT)} is missing — run scripts/update-openapi.sh")
|
||||
return json.loads(OPENAPI.read_text())
|
||||
|
||||
|
||||
def _schema_refs(node: object, out: set[str]) -> None:
|
||||
"""Collect every `#/components/schemas/X` name anywhere under `node`."""
|
||||
if isinstance(node, dict):
|
||||
ref = node.get("$ref")
|
||||
if isinstance(ref, str) and ref.startswith("#/components/schemas/"):
|
||||
out.add(ref.rsplit("/", 1)[1])
|
||||
for value in node.values():
|
||||
_schema_refs(value, out)
|
||||
elif isinstance(node, list):
|
||||
for value in node:
|
||||
_schema_refs(value, out)
|
||||
|
||||
|
||||
def _request_reachable(doc: dict) -> set[str]:
|
||||
"""Every schema reachable from ANY operation's request body, transitively.
|
||||
|
||||
Two deliberate non-restrictions, both because this population has now been drawn by hand wrongly
|
||||
twice and every hand-drawn edge is a place to be wrong again:
|
||||
|
||||
NO VERB ALLOW-LIST. An earlier draft scanned POST/PUT/PATCH, which reads as obviously right and
|
||||
already had an exception: `DELETE /api/v1/media-items` carries a request body. Rather than argue
|
||||
that a DELETE body cannot cause a full-replace drop — probably true, and exactly the kind of
|
||||
"probably" that produced this record's two live misses — every operation carrying a request body
|
||||
seeds the walk, and anything it surfaces must acquire a stated disposition.
|
||||
|
||||
TRANSITIVE, and that is load-bearing rather than thorough: `MultiCollectionItemRequest` and
|
||||
`ArtworkContentTypeModel` are both NESTED, so a check reading only top-level request bodies
|
||||
would have missed both of the schemas this guard exists because of. `_schema_refs` walks every
|
||||
value of every dict and every list element, so `oneOf` (23 occurrences today), `items`,
|
||||
`additionalProperties` and any future composition keyword are covered without naming them.
|
||||
"""
|
||||
schemas = doc["components"]["schemas"]
|
||||
seeds: set[str] = set()
|
||||
for operations in doc["paths"].values():
|
||||
for operation in operations.values():
|
||||
if isinstance(operation, dict) and operation.get("requestBody"):
|
||||
_schema_refs(operation["requestBody"], seeds)
|
||||
|
||||
seen: set[str] = set()
|
||||
stack = list(seeds)
|
||||
while stack:
|
||||
name = stack.pop()
|
||||
if name in seen or name not in schemas:
|
||||
continue
|
||||
seen.add(name)
|
||||
nested: set[str] = set()
|
||||
_schema_refs(schemas[name], nested)
|
||||
stack.extend(nested - seen)
|
||||
return seen
|
||||
|
||||
|
||||
def _optional_of(schema: object) -> set[str]:
|
||||
"""The members of `schema` a client may omit, nested inline objects included.
|
||||
|
||||
This is the function to call. `_resolve` below is its recursive half and returns three sets;
|
||||
the split matters and is the fix for a real defect, so it is stated here rather than in a
|
||||
comment further down.
|
||||
|
||||
A nested inline object lives in its OWN namespace. Qualifying its members as `parent.child`
|
||||
BEFORE subtracting `required` merges the two namespaces, and a literal member named
|
||||
`parent.child` then collides with the nested one — masking it entirely when the literal is
|
||||
required, so a droppable member vanishes from the population with nothing failing (measured
|
||||
2026-08-23). Subtracting inside each namespace first and qualifying only the survivors means
|
||||
the two sets are never mixed, so the collision cannot arise and there is no separator to
|
||||
defend. An earlier version instead ASSERTED that no property name contains a dot, which is a
|
||||
guard where a restructure was available.
|
||||
|
||||
Residual, stated because it is real: if a literal `a.b` and a nested `a` -> `b` are BOTH
|
||||
optional they conflate into one reported string. That is a diagnostic ambiguity, not a miss —
|
||||
the schema still enters the population and still forces a disposition.
|
||||
"""
|
||||
properties, required, nested_optional = _resolve(schema)
|
||||
return (properties - required) | nested_optional
|
||||
|
||||
|
||||
def _resolve(schema: object) -> tuple[set[str], set[str], set[str]]:
|
||||
"""(properties, required, already-resolved nested optional members) for one schema node.
|
||||
|
||||
The third set is carried through every composition site rather than merged into the first two,
|
||||
for the namespace reason in `_optional_of`. Do NOT collapse this back to a 2-tuple to spare the
|
||||
callers: folding `nested_optional` into `properties` re-creates the mask at the merge boundary
|
||||
and the whole suite stays green while it does.
|
||||
|
||||
NESTING. Handling `allOf` one level deep misses an `allOf` inside an `allOf` — a property there
|
||||
reported as no properties at all. Composition nests, so the resolution has to recurse.
|
||||
|
||||
`$ref` IS DELIBERATELY NOT RESOLVED HERE, and that is not a hole: an arm that is a `$ref` names
|
||||
a component schema, which `_request_reachable` already seeds on (`_schema_refs` finds a `$ref`
|
||||
anywhere, arms included) and `_optional_members` already walks, so its optional members surface
|
||||
under their OWN key rather than being merged into the inline one. Verified by construction in
|
||||
`test_a_ref_ARM_is_covered_by_the_component_walk_not_by_this_one`. Resolving refs here as well
|
||||
would report the same member twice under two keys, which is worse than either.
|
||||
|
||||
CONJUNCTION vs DISJUNCTION. `allOf` arms ALL apply, so their `required` sets UNION. `oneOf` and
|
||||
`anyOf` arms are ALTERNATIVES, so a member is only genuinely required when EVERY alternative
|
||||
requires it — the `required` sets INTERSECT. Unioning them instead (the first version) marks a
|
||||
member required because one arm requires it, hiding the arm that lets a client omit it. That is
|
||||
the drop this whole guard exists to catch, so getting it backwards is not a detail.
|
||||
|
||||
No cycle guard, deliberately. This resolver never follows `$ref`, so the only way to recurse
|
||||
forever is a schema that contains ITSELF by object identity — which `json.load` cannot produce,
|
||||
since JSON has no back-references. Verified 2026-08-23: a hand-built self-referential dict does
|
||||
raise `RecursionError`, a LOUD red rather than a silent wrong answer. The `$ref` cycle a real
|
||||
document CAN express is handled by `_request_reachable`'s `seen` set, also verified.
|
||||
"""
|
||||
if not isinstance(schema, dict):
|
||||
return set(), set(), set()
|
||||
|
||||
properties = set(schema.get("properties") or {})
|
||||
required = set(schema.get("required") or [])
|
||||
nested_optional: set[str] = set()
|
||||
|
||||
# A property may itself be an INLINE OBJECT rather than a `$ref`, and the generator recurses
|
||||
# into it (`objectTypeFromSchema` -> `typeFromSchema`), so a member outside that nested
|
||||
# `required` really does emit `?:` and really is droppable. Resolved in its own namespace and
|
||||
# qualified afterwards — see `_optional_of`.
|
||||
for name, value in (schema.get("properties") or {}).items():
|
||||
if not isinstance(value, dict) or "$ref" in value:
|
||||
continue
|
||||
nested_optional |= {f"{name}.{child}" for child in _optional_of(value)}
|
||||
|
||||
# CONJUNCTIVE: every `allOf` arm applies, so both sets union.
|
||||
for branch in schema.get("allOf") or []:
|
||||
branch_properties, branch_required, branch_nested = _resolve(branch)
|
||||
properties |= branch_properties
|
||||
required |= branch_required
|
||||
nested_optional |= branch_nested
|
||||
|
||||
# CONDITIONAL keywords — `if`, `then`, `else`, `dependentSchemas`. All four are treated the same
|
||||
# way: collect their properties (a client may send them) and DISCARD their `required` (it binds
|
||||
# only on a branch that may not be taken, so the member is omittable).
|
||||
#
|
||||
# An earlier version put `then`/`else` in the conjunctive list above, unioning their `required`.
|
||||
# That is the polarity error this function warns about above, committed in the same block:
|
||||
# `then` and `else` are MUTUALLY EXCLUSIVE, so a member required only under `then` is omittable
|
||||
# whenever `if` does not match, and the guard reported it as required — the silent-miss
|
||||
# direction, which is the one this whole file exists to catch. `if`'s `required` is discarded
|
||||
# for a different reason (it selects a branch rather than obliging anyone), and
|
||||
# `dependentSchemas` for a third (it binds only when its trigger key is present), but the
|
||||
# resulting rule is identical, so they share one loop rather than three arguments.
|
||||
conditional: list[object] = []
|
||||
for keyword in ("if", "then", "else"):
|
||||
value = schema.get(keyword)
|
||||
conditional.extend(value if isinstance(value, list) else ([value] if isinstance(value, dict) else []))
|
||||
conditional.extend((schema.get("dependentSchemas") or {}).values())
|
||||
for branch in conditional:
|
||||
branch_properties, _, branch_nested = _resolve(branch)
|
||||
properties |= branch_properties
|
||||
nested_optional |= branch_nested
|
||||
|
||||
# DISJUNCTIVE keywords, handled SEPARATELY rather than concatenated: `oneOf` and `anyOf` are
|
||||
# conjunctive WITH EACH OTHER (a body satisfying both must satisfy one arm of each), so the
|
||||
# correct required set is the intersection within each keyword, unioned across them. Merging
|
||||
# the two lists first intersects across keywords and under-reports required.
|
||||
for keyword in ("oneOf", "anyOf"):
|
||||
alternatives = schema.get(keyword) or []
|
||||
shared_required: set[str] | None = None
|
||||
for branch in alternatives:
|
||||
branch_properties, branch_required, branch_nested = _resolve(branch)
|
||||
properties |= branch_properties
|
||||
nested_optional |= branch_nested
|
||||
shared_required = branch_required if shared_required is None else (shared_required & branch_required)
|
||||
if shared_required:
|
||||
required |= shared_required
|
||||
|
||||
# An inline body may be an ARRAY of inline objects; the members live on `items`, and a member
|
||||
# droppable there is droppable in the request. `items` is a SCHEMA in OpenAPI 3.1 / JSON Schema
|
||||
# 2020-12 and may be a LIST in the 3.0 tuple form, so both shapes are walked — and `prefixItems`
|
||||
# is the 2020-12 spelling of that tuple.
|
||||
for keyword in ("items", "prefixItems"):
|
||||
value = schema.get(keyword)
|
||||
branches = value if isinstance(value, list) else ([value] if isinstance(value, dict) else [])
|
||||
for branch in branches:
|
||||
branch_properties, branch_required, branch_nested = _resolve(branch)
|
||||
properties |= branch_properties
|
||||
required |= branch_required
|
||||
nested_optional |= branch_nested
|
||||
|
||||
return properties, required, nested_optional
|
||||
|
||||
|
||||
def _optional_members(doc: dict) -> dict[str, list[str]]:
|
||||
"""Schema -> its properties that sit OUTSIDE `required`, i.e. the ones that emit `?:`."""
|
||||
out: dict[str, list[str]] = {}
|
||||
for name, schema in doc["components"]["schemas"].items():
|
||||
optional = sorted(_optional_of(schema))
|
||||
if optional:
|
||||
out[name] = optional
|
||||
return out
|
||||
|
||||
|
||||
def _inline_body_members(doc: dict) -> dict[str, list[str]]:
|
||||
"""Optional members of request bodies declared INLINE, i.e. with no `$ref` to a named schema.
|
||||
|
||||
`_optional_members` iterates `components.schemas`, and `_request_reachable` seeds from `$ref`s,
|
||||
so between them an inline body is invisible in BOTH directions. That was not hypothetical: the
|
||||
document declares one today (`POST /api/v1/artwork/uploads`, multipart), whose `file` and
|
||||
`target` sit outside any `required` array — a member already present and outside the guard's
|
||||
reach while its docstring claimed to cover the whole request graph.
|
||||
|
||||
Keyed by `"<VERB> <path> (<media type> inline body)"` rather than by a schema name, because there is no name
|
||||
to use — which is exactly why the component-schema walk cannot see it.
|
||||
|
||||
Composition is resolved by `_optional_of`/`_resolve`, which recurse and treat `allOf` as
|
||||
conjunction and `oneOf`/`anyOf` as alternatives — this body splits its properties across `allOf`
|
||||
arms and would otherwise report none.
|
||||
"""
|
||||
out: dict[str, list[str]] = {}
|
||||
for path, operations in doc["paths"].items():
|
||||
for verb, operation in operations.items():
|
||||
if not isinstance(operation, dict) or not operation.get("requestBody"):
|
||||
continue
|
||||
for content_type, media in (operation["requestBody"].get("content") or {}).items():
|
||||
schema = media.get("schema") or {}
|
||||
if "$ref" in schema:
|
||||
# Belt-and-braces, not load-bearing: `_optional_of` returns an empty
|
||||
# sets for a bare `$ref` node anyway, so deleting this line changes no result
|
||||
# today. It stays because a body that names a component schema is that schema's
|
||||
# business — `_optional_members` already covers it — and skipping it here keeps
|
||||
# that division explicit rather than accidental.
|
||||
continue
|
||||
optional = sorted(_optional_of(schema))
|
||||
if optional:
|
||||
# Keyed by MEDIA TYPE as well as verb and path. An operation may declare more
|
||||
# than one inline body (a second `[Consumes]` is all it takes), and keying on
|
||||
# verb+path alone made the later one overwrite the earlier — a droppable member
|
||||
# silently disappearing from the population rather than failing.
|
||||
out[f"{verb.upper()} {path} ({content_type} inline body)"] = optional
|
||||
return out
|
||||
|
||||
|
||||
def _droppable(doc: dict) -> dict[str, list[str]]:
|
||||
reachable = _request_reachable(doc)
|
||||
droppable = {n: m for n, m in _optional_members(doc).items() if n in reachable}
|
||||
droppable.update(_inline_body_members(doc))
|
||||
return droppable
|
||||
|
||||
|
||||
def test_the_derivation_reached_a_real_population() -> None:
|
||||
"""Anti-vacuity: a broken `$ref` walk or parse must not read as 'nothing to rule on'.
|
||||
|
||||
What this test does NOT do, measured 2026-08-22 rather than assumed: it does not catch a
|
||||
PARTIALLY broken walk. Deleting the transitive step from `_request_reachable` — so only the
|
||||
schemas named directly on a request body resolve — leaves both floors satisfied and this test
|
||||
GREEN. What reddens is `test_MUTATION_a_planted_optional_member_is_reported_as_MISSING`, whose
|
||||
planted schema is reached through a nested `$ref` precisely so that it can, plus the disposition
|
||||
test (the nested rows vanish and report as PHANTOM). So the floors below are the crude backstop
|
||||
against a parse that reached nothing at all; the planted-member test is what actually holds the
|
||||
walk honest, and it should be the one kept working if these two ever conflict.
|
||||
"""
|
||||
doc = _load()
|
||||
reachable = _request_reachable(doc)
|
||||
optional = _optional_members(doc)
|
||||
assert len(reachable) >= MIN_REQUEST_REACHABLE_SCHEMAS, (
|
||||
f"only {len(reachable)} schemas reachable from a request body — the $ref walk is broken, not the API"
|
||||
)
|
||||
assert len(optional) >= MIN_SCHEMAS_WITH_OPTIONAL_MEMBERS, (
|
||||
f"only {len(optional)} schemas have a property outside `required` — suspect the parse"
|
||||
)
|
||||
|
||||
|
||||
def test_the_walks_ASSUMPTIONS_about_the_document_still_hold() -> None:
|
||||
"""The derivation's scope mirrors two properties of the OpenAPI document. Check them.
|
||||
|
||||
`testing.guard-derives-population-from-source`: "When the scope itself MIRRORS an authoritative
|
||||
source, the mirror needs its own equality check or a dated staleness marker, or the guard is
|
||||
complete within a scope that has silently gone stale." Two such assumptions are baked into
|
||||
`_request_reachable`, and both are true of the document today (2026-08-22) rather than
|
||||
guaranteed by anything:
|
||||
|
||||
1. Request bodies are declared INLINE on the operation. If ASP.NET ever emits a
|
||||
`components.requestBodies` bucket and operations `$ref` into it, the seed walk still finds
|
||||
the `$ref` — but only because `_schema_refs` collects `#/components/schemas/...` names, so a
|
||||
body referencing `#/components/requestBodies/X` would seed NOTHING and the schemas under it
|
||||
would drop out of the population silently.
|
||||
2. Every `$ref` in the document points into `#/components/schemas/`. `_schema_refs` matches on
|
||||
that prefix, so a ref into any other bucket is invisible to it.
|
||||
|
||||
Both are cheap to assert and neither is asserted anywhere else, so a change in the emitter
|
||||
would otherwise shrink this guard's population without failing anything.
|
||||
|
||||
A THIRD assumption used to sit here unstated and was already violated: that every request body
|
||||
`$ref`s a named component schema. `POST /api/v1/artwork/uploads` declares its body inline, so
|
||||
both `_optional_members` (which iterates `components.schemas`) and `_request_reachable` (which
|
||||
seeds from `$ref`s) were blind to it. That one is not an assumption any more —
|
||||
`_inline_body_members` handles it — which is why it is described here rather than asserted.
|
||||
"""
|
||||
doc = _load()
|
||||
buckets = set(doc.get("components", {}))
|
||||
assert "requestBodies" not in buckets, (
|
||||
"the OpenAPI document now declares components.requestBodies — `_request_reachable` seeds "
|
||||
"only from inline operation bodies and `_schema_refs` only follows #/components/schemas/, "
|
||||
"so schemas behind a shared request body are now INVISIBLE to this guard. Teach the walk "
|
||||
"to resolve that bucket before deleting this assertion."
|
||||
)
|
||||
ref_buckets = set(re.findall(r'"#/components/([^/"]+)/', json.dumps(doc)))
|
||||
assert ref_buckets <= {"schemas"}, (
|
||||
f"$refs now point into {sorted(ref_buckets - {'schemas'})} as well as schemas; "
|
||||
"`_schema_refs` matches only the schemas prefix and silently ignores the rest"
|
||||
)
|
||||
|
||||
|
||||
def test_every_droppable_request_schema_has_a_stated_disposition() -> None:
|
||||
"""Set equality, both directions, accumulated into ONE message.
|
||||
|
||||
Failing fast on the first mismatch hands back one schema at a time and invites fixing them one
|
||||
at a time, which is how #754's twin stayed hidden.
|
||||
"""
|
||||
droppable = _droppable(_load())
|
||||
|
||||
unruled = sorted(set(droppable) - set(DISPOSITIONS))
|
||||
phantom = sorted(set(DISPOSITIONS) - set(droppable))
|
||||
|
||||
problems: list[str] = []
|
||||
if unruled:
|
||||
problems.append(
|
||||
"MISSING — reachable from a request body with a member outside `required`, and "
|
||||
"no disposition written down. Decide what happens to each and add a row:\n"
|
||||
+ "\n".join(f" {n}: optional members {droppable[n]}" for n in unruled)
|
||||
)
|
||||
if phantom:
|
||||
problems.append(
|
||||
"PHANTOM — a disposition for a schema that is no longer request-reachable with an "
|
||||
"optional member. Delete the row rather than leaving it claiming coverage:\n"
|
||||
+ "\n".join(f" {n}" for n in phantom)
|
||||
)
|
||||
assert not problems, "\n\n".join(problems)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("case", "schema", "expected"),
|
||||
[
|
||||
(
|
||||
"a flat schema reports the properties outside `required`",
|
||||
{"type": "object", "properties": {"a": {}, "b": {}}, "required": ["a"]},
|
||||
["b"],
|
||||
),
|
||||
(
|
||||
"allOf arms are conjunctive: each arm's `required` applies",
|
||||
{"allOf": [{"properties": {"a": {}}}, {"properties": {"b": {}}, "required": ["b"]}]},
|
||||
["a"],
|
||||
),
|
||||
(
|
||||
"allOf NESTED inside allOf is reached — a one-level walk reported nothing here",
|
||||
{"allOf": [{"allOf": [{"properties": {"deep": {}}}]}]},
|
||||
["deep"],
|
||||
),
|
||||
(
|
||||
"oneOf arms are ALTERNATIVES: required in one arm only means a client may omit it",
|
||||
{"oneOf": [{"properties": {"x": {}}, "required": ["x"]}, {"properties": {"x": {}}}]},
|
||||
["x"],
|
||||
),
|
||||
(
|
||||
"oneOf where EVERY arm requires it is genuinely required",
|
||||
{
|
||||
"oneOf": [
|
||||
{"properties": {"x": {}}, "required": ["x"]},
|
||||
{"properties": {"x": {}}, "required": ["x"]},
|
||||
]
|
||||
},
|
||||
[],
|
||||
),
|
||||
(
|
||||
"anyOf arms are alternatives too — required in one arm only means droppable",
|
||||
{"anyOf": [{"properties": {"y": {}}, "required": ["y"]}, {"properties": {"y": {}}}]},
|
||||
["y"],
|
||||
),
|
||||
(
|
||||
"anyOf where EVERY arm requires it is genuinely required",
|
||||
{
|
||||
"anyOf": [
|
||||
{"properties": {"y": {}}, "required": ["y"]},
|
||||
{"properties": {"y": {}}, "required": ["y"]},
|
||||
]
|
||||
},
|
||||
[],
|
||||
),
|
||||
(
|
||||
"oneOf and anyOf on ONE node are conjunctive with EACH OTHER, not one alternative list",
|
||||
{
|
||||
"oneOf": [{"properties": {"x": {}}, "required": ["x"]}, {"properties": {"x": {}}, "required": ["x"]}],
|
||||
"anyOf": [{"properties": {"y": {}}, "required": ["y"]}, {"properties": {"y": {}}, "required": ["y"]}],
|
||||
},
|
||||
[],
|
||||
),
|
||||
(
|
||||
"composition NESTED inside an alternative arm is reached",
|
||||
{"oneOf": [{"allOf": [{"properties": {"nestedInArm": {}}}]}]},
|
||||
["nestedInArm"],
|
||||
),
|
||||
(
|
||||
"properties in arms 2..n are collected, not just the first arm's",
|
||||
{"oneOf": [{"properties": {"first": {}}}, {"properties": {"second": {}}}]},
|
||||
["first", "second"],
|
||||
),
|
||||
(
|
||||
"a top-level `required` still applies when alternatives are present",
|
||||
{"properties": {"top": {}, "other": {}}, "required": ["top"], "oneOf": [{"properties": {"arm": {}}}]},
|
||||
["arm", "other"],
|
||||
),
|
||||
(
|
||||
"an inline body that is an ARRAY of objects exposes its item members",
|
||||
{"type": "array", "items": {"properties": {"itemReq": {}, "itemOpt": {}}, "required": ["itemReq"]}},
|
||||
["itemOpt"],
|
||||
),
|
||||
(
|
||||
"`items` in the 3.0 TUPLE form (a list) is walked, not just the schema form",
|
||||
{"type": "array", "items": [{"properties": {"tupleReq": {}, "tupleOpt": {}}, "required": ["tupleReq"]}]},
|
||||
["tupleOpt"],
|
||||
),
|
||||
(
|
||||
"`prefixItems`, the 2020-12 spelling of a tuple, is walked",
|
||||
{"type": "array", "prefixItems": [{"properties": {"prefixOpt": {}}}]},
|
||||
["prefixOpt"],
|
||||
),
|
||||
(
|
||||
"nested arrays are followed to the object at the bottom",
|
||||
{"type": "array", "items": {"type": "array", "items": {"properties": {"deepOpt": {}}}}},
|
||||
["deepOpt"],
|
||||
),
|
||||
(
|
||||
"`dependentSchemas` members are collectable and its conditional `required` is discarded",
|
||||
{"dependentSchemas": {"trigger": {"properties": {"depOpt": {}}, "required": ["depOpt"]}}},
|
||||
["depOpt"],
|
||||
),
|
||||
(
|
||||
"a `required` under `then` does NOT make a member required — the branch may not be taken",
|
||||
{
|
||||
"if": {"properties": {"k": {}}, "required": ["k"]},
|
||||
"then": {"properties": {"m": {}}, "required": ["m"]},
|
||||
},
|
||||
["k", "m"],
|
||||
),
|
||||
(
|
||||
"a `required` under `else` does not either, and `then`/`else` are mutually exclusive",
|
||||
{
|
||||
"if": {"properties": {"k": {}}},
|
||||
"then": {"properties": {"m": {}}, "required": ["m"]},
|
||||
"else": {"properties": {"m": {}}},
|
||||
},
|
||||
["k", "m"],
|
||||
),
|
||||
(
|
||||
"a `required` under `if` selects a branch, it does not oblige the client",
|
||||
{"if": {"properties": {"k": {}}, "required": ["k"]}, "then": {"properties": {"m": {}}}},
|
||||
["k", "m"],
|
||||
),
|
||||
(
|
||||
"an INLINE OBJECT under `properties` is recursed into, reported with a dotted path",
|
||||
{
|
||||
"properties": {
|
||||
"top": {},
|
||||
"nested": {"properties": {"a": {}, "b": {}}, "required": ["a"]},
|
||||
},
|
||||
"required": ["top", "nested"],
|
||||
},
|
||||
["nested.b"],
|
||||
),
|
||||
(
|
||||
"a REQUIRED literal `a.b` cannot mask a nested `a` -> `b` — the namespaces never merge",
|
||||
{"properties": {"a.b": {}, "a": {"properties": {"b": {}}}}, "required": ["a.b", "a"]},
|
||||
["a.b"],
|
||||
),
|
||||
(
|
||||
"`prefixItems` positions are conjunctive, so a `required` there really does bind",
|
||||
{
|
||||
"type": "array",
|
||||
"prefixItems": [{"properties": {"pReq": {}, "pOpt": {}}, "required": ["pReq"]}],
|
||||
},
|
||||
["pOpt"],
|
||||
),
|
||||
(
|
||||
"a dotted nested member cannot be confused with a top-level member of the same name",
|
||||
{
|
||||
"properties": {"b": {}, "nested": {"properties": {"b": {}}}},
|
||||
"required": ["b", "nested"],
|
||||
},
|
||||
["nested.b"],
|
||||
),
|
||||
(
|
||||
"`patternProperties` names no FIXED members, so it contributes none — same as additionalProperties",
|
||||
{"properties": {"named": {}}, "patternProperties": {"^x-": {"properties": {"notAMember": {}}}}},
|
||||
["named"],
|
||||
),
|
||||
(
|
||||
"`additionalProperties` names no members, so it contributes none",
|
||||
{"properties": {"named": {}}, "additionalProperties": {"properties": {"notAMember": {}}}},
|
||||
["named"],
|
||||
),
|
||||
(
|
||||
"`not` cannot make a member required",
|
||||
{"properties": {"a": {}}, "not": {"required": ["a"]}},
|
||||
["a"],
|
||||
),
|
||||
(
|
||||
"properties declared under `else` count for the same reason `then` does",
|
||||
{"if": {}, "else": {"properties": {"elseOpt": {}}}},
|
||||
["elseOpt"],
|
||||
),
|
||||
(
|
||||
"properties declared under `then` are reachable on some branch, so they count",
|
||||
{"if": {"properties": {"kind": {}}}, "then": {"properties": {"conditional": {}}}},
|
||||
["conditional", "kind"],
|
||||
),
|
||||
(
|
||||
"top-level properties and an allOf arm are merged, not either/or",
|
||||
{"properties": {"top": {}}, "required": ["top"], "allOf": [{"properties": {"inner": {}}}]},
|
||||
["inner"],
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_composition_is_resolved_the_way_JSON_Schema_means_it(case: str, schema: dict, expected: list[str]) -> None:
|
||||
"""Pin `_optional_of`/`_resolve` against constructed schemas, not against today's document.
|
||||
|
||||
The document exercises exactly one shape (a two-arm `allOf` in the artwork upload body), so
|
||||
every other branch of this resolver would otherwise be unexercised prose. Both of the first
|
||||
version's defects are here as cases: the nested `allOf` it could not reach, and the `oneOf`
|
||||
whose `required` it unioned instead of intersecting — which marked a member required because
|
||||
ONE arm required it, hiding the arm that lets a client drop it.
|
||||
"""
|
||||
assert sorted(_optional_of(schema)) == expected, case
|
||||
|
||||
|
||||
_NESTED_PROBE = {
|
||||
"properties": {"outer": {"properties": {"req": {}, "opt": {}}, "required": ["req"]}},
|
||||
"required": ["outer"],
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("site", "schema"),
|
||||
[
|
||||
("top level", _NESTED_PROBE),
|
||||
("allOf arm", {"allOf": [_NESTED_PROBE]}),
|
||||
("if", {"if": _NESTED_PROBE}),
|
||||
("then", {"then": _NESTED_PROBE}),
|
||||
("else", {"else": _NESTED_PROBE}),
|
||||
("dependentSchemas", {"dependentSchemas": {"trigger": _NESTED_PROBE}}),
|
||||
("oneOf arm", {"oneOf": [_NESTED_PROBE]}),
|
||||
("anyOf arm", {"anyOf": [_NESTED_PROBE]}),
|
||||
("items", {"type": "array", "items": _NESTED_PROBE}),
|
||||
("prefixItems", {"type": "array", "prefixItems": [_NESTED_PROBE]}),
|
||||
],
|
||||
)
|
||||
def test_a_nested_inline_object_survives_EVERY_composition_site(site: str, schema: dict) -> None:
|
||||
"""`_resolve` returns three sets, and the third has to be threaded through every branch.
|
||||
|
||||
A composition site that unions `properties` and `required` but forgets `nested_optional` loses
|
||||
the nested member silently — no error, just a smaller population — and each site is a separate
|
||||
opportunity to forget. Enumerating the sites here is what makes "threaded through every branch"
|
||||
a checked property instead of a claim in a docstring; a NEW composition keyword must be added
|
||||
to this list, and if it is not, the omission is at least visible in one place rather than
|
||||
spread across the resolver.
|
||||
"""
|
||||
assert "outer.opt" in _optional_of(schema), (
|
||||
f"a nested inline object under `{site}` lost its optional member — `nested_optional` is "
|
||||
"not threaded through that branch of `_resolve`"
|
||||
)
|
||||
|
||||
|
||||
def test_a_ref_ARM_is_covered_by_the_component_walk_not_by_this_one() -> None:
|
||||
"""The two walks COMPOSE; neither alone covers an inline body with a `$ref` arm.
|
||||
|
||||
`_resolve` does not resolve `$ref`, so an `allOf` arm that is a `$ref`
|
||||
contributes nothing to the inline key. That looks like a gap and is not: the referenced schema
|
||||
is a named component, so it is seeded by `_request_reachable` and walked by `_optional_members`,
|
||||
and its optional members surface under their own key. Pinned here because the obvious "fix" —
|
||||
resolving refs in this resolver too — would report the same member under two keys, and because
|
||||
a reader checking only one of the two walks would reasonably conclude the case is uncovered.
|
||||
"""
|
||||
doc = _load()
|
||||
doc["components"]["schemas"]["RefArmProbeSchema"] = {
|
||||
"type": "object",
|
||||
"properties": {"probeRequired": {"type": "string"}, "probeOptional": {"type": "string"}},
|
||||
"required": ["probeRequired"],
|
||||
}
|
||||
doc["paths"]["/probe-ref-arm"] = {
|
||||
"post": {
|
||||
"requestBody": {
|
||||
"content": {
|
||||
"application/json": {
|
||||
"schema": {
|
||||
"allOf": [
|
||||
{"$ref": "#/components/schemas/RefArmProbeSchema"},
|
||||
{"properties": {"probeInline": {}}},
|
||||
]
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
droppable = _droppable(doc)
|
||||
assert droppable.get("RefArmProbeSchema") == ["probeOptional"], (
|
||||
"a $ref arm's target must still surface via the component walk — if this is empty, an "
|
||||
"inline body can hide a droppable member behind a $ref"
|
||||
)
|
||||
assert droppable.get("POST /probe-ref-arm (application/json inline body)") == ["probeInline"]
|
||||
|
||||
|
||||
def test_every_disposition_uses_the_closed_vocabulary() -> None:
|
||||
"""A free-text disposition would let a row read as considered while saying nothing."""
|
||||
allowed = {COVERED, CREATE, TRIGGER, COMPUTED}
|
||||
for name, (disposition, why) in DISPOSITIONS.items():
|
||||
assert disposition in allowed, f"{name}: {disposition!r} is not one of {sorted(allowed)}"
|
||||
assert len(why.strip()) >= 40, f"{name}: the reason is too thin to be a decision"
|
||||
|
||||
|
||||
def test_MUTATION_a_planted_optional_member_is_reported_as_MISSING() -> None:
|
||||
"""The checker-guard mutation proof (`testing.guard-ships-with-mutation-proof`).
|
||||
|
||||
Disarming a checker makes it ABSENT rather than red, so the mutation goes into the guarded
|
||||
ARTIFACT: plant a request-reachable schema carrying a member outside `required` and require the
|
||||
derivation to surface it. Mutating the checker's own population instead would be the trap that
|
||||
record names — a shrunken population makes every real row report PHANTOM, so the proof would go
|
||||
red on a false positive while saying nothing about the MISSING detection this row claims.
|
||||
|
||||
The planted schema is reached through a real request body, so this also exercises the
|
||||
transitive `$ref` walk that the hand-written list twice failed to do by eye.
|
||||
"""
|
||||
doc = _load()
|
||||
schemas = doc["components"]["schemas"]
|
||||
|
||||
# Deterministic, because `set` iteration order over strings varies per process: picking the
|
||||
# host with `next(iter(...))` would silently vary WHICH walk depth the proof exercises from run
|
||||
# to run, so a green run would not mean the same thing twice.
|
||||
candidates = sorted(
|
||||
n for n in _request_reachable(doc) if isinstance(schemas.get(n), dict) and schemas[n].get("properties")
|
||||
)
|
||||
assert candidates, "no request-reachable schema with properties — the walk is broken"
|
||||
host = candidates[0]
|
||||
|
||||
schemas["PlantedDroppableRequest"] = {
|
||||
"type": "object",
|
||||
"properties": {"plantedRequired": {"type": "string"}, "plantedOptional": {"type": "string"}},
|
||||
"required": ["plantedRequired"],
|
||||
}
|
||||
# Planted through a `oneOf` LIST rather than a bare dict `$ref`, because `_schema_refs` has two
|
||||
# descent branches and only the dict one was exercised: deleting its list descent — which makes
|
||||
# every `oneOf` reference invisible, 23 of them in the document today — left the whole file
|
||||
# GREEN when measured 2026-08-22. A proof that cannot see half its own walk is the "green for
|
||||
# the wrong reason" shape this repo keeps recording.
|
||||
schemas[host]["properties"]["plantedLink"] = {
|
||||
"oneOf": [{"type": "null"}, {"$ref": "#/components/schemas/PlantedDroppableRequest"}]
|
||||
}
|
||||
|
||||
# Plant into the INLINE request body too, so `_inline_body_members` is exercised rather than
|
||||
# merely present. Without this the whole inline branch could be deleted and every test here
|
||||
# would stay green — the exact shape the `oneOf` list branch was in before it was planted
|
||||
# through.
|
||||
inline_host = next(
|
||||
(
|
||||
media["schema"]
|
||||
for operations in doc["paths"].values()
|
||||
for operation in operations.values()
|
||||
if isinstance(operation, dict) and operation.get("requestBody")
|
||||
for media in (operation["requestBody"].get("content") or {}).values()
|
||||
if isinstance(media.get("schema"), dict) and "$ref" not in media["schema"]
|
||||
),
|
||||
None,
|
||||
)
|
||||
assert inline_host is not None, "no inline request body in the document — re-target this plant"
|
||||
inline_host.setdefault("properties", {})["plantedInlineOptional"] = {"type": "string"}
|
||||
|
||||
droppable = _droppable(doc)
|
||||
planted_inline = [k for k, v in droppable.items() if "plantedInlineOptional" in v]
|
||||
assert planted_inline, (
|
||||
"the member planted in an INLINE request body was NOT surfaced — `_inline_body_members` is "
|
||||
"not reaching inline bodies, so a body declared without a $ref is invisible to this guard"
|
||||
)
|
||||
assert "PlantedDroppableRequest" in droppable, (
|
||||
"the planted schema was NOT surfaced — the derivation cannot see a droppable member, so a "
|
||||
"green run of this file proves nothing"
|
||||
)
|
||||
assert droppable["PlantedDroppableRequest"] == ["plantedOptional"]
|
||||
assert "PlantedDroppableRequest" not in DISPOSITIONS
|
||||
@@ -1,3 +1,4 @@
|
||||
import type { Complete } from './completeRequest';
|
||||
import { ApiError, request, requestWithMeta, type ResponseWithMeta } from './client';
|
||||
import type { components } from './generated/v1';
|
||||
|
||||
@@ -60,7 +61,7 @@ export function getBlockItemsWithMeta(id: number): Promise<ResponseWithMeta<Bloc
|
||||
*/
|
||||
export function replaceBlock(
|
||||
id: number,
|
||||
body: ReplaceBlockRequest,
|
||||
body: Complete<ReplaceBlockRequest>,
|
||||
ifMatch?: string | null
|
||||
): Promise<ResponseWithMeta<BlockWithItems>> {
|
||||
return requestWithMeta<BlockWithItems>(`/api/v1/blocks/${id}`, {
|
||||
@@ -70,7 +71,7 @@ export function replaceBlock(
|
||||
});
|
||||
}
|
||||
|
||||
export function previewBlock(id: number, body: ReplaceBlockRequest): Promise<BlockPreviewItem[]> {
|
||||
export function previewBlock(id: number, body: Complete<ReplaceBlockRequest>): Promise<BlockPreviewItem[]> {
|
||||
return request<BlockPreviewItem[]>(`/api/v1/blocks/${id}/preview`, { body, method: 'POST' });
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,146 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import type { Complete } from './completeRequest';
|
||||
import type { components } from './generated/v1';
|
||||
|
||||
/**
|
||||
* #807 guard: the mutation proof for `Complete<T>` (`./completeRequest.ts`).
|
||||
*
|
||||
* `Complete<T>` is enforced by the COMPILER, not at runtime, so a vitest assertion cannot prove it
|
||||
* works — a passing `expect` here would say nothing about whether the type still rejects an
|
||||
* incomplete request. The proof is therefore the `@ts-expect-error` directives below, and it is a
|
||||
* real one in the sense `testing.guard-ships-with-mutation-proof` requires: each directive
|
||||
* INTRODUCES THE DEFECT the guard exists to catch, and `tsc` fails the build if the defect is NOT
|
||||
* reported. A `@ts-expect-error` on a line that compiles cleanly is itself an error
|
||||
* (`TS2578: Unused '@ts-expect-error' directive`). That mutation runs on every
|
||||
* `npm run typecheck`, which is a marked CI step (`docker-build.yml` → `ci-step-ran.sh mark
|
||||
* typecheck`), rather than being executed once by hand and asserted in prose.
|
||||
*
|
||||
* Which mutation reddens which case, measured 2026-08-22 against THIS file and not generalised:
|
||||
*
|
||||
* | mutation of `Complete<T>` | cases that go red |
|
||||
* |---|---|
|
||||
* | `{ [K in keyof T]: T[K] }` (drop `Required` — the realistic weakening) | 2 and 4 |
|
||||
* | `T` (delete the mapped type) | 2 and 4 |
|
||||
* | `Partial<T>` | 2 and 4 |
|
||||
*
|
||||
* Cases 2 and 4 are the load-bearing pair — one synthetic, one a real generated schema. Cases 1, 3
|
||||
* and 5 stay green under all three BY DESIGN: 1 and 3 are controls that must stay green, and 5's
|
||||
* excess-property check does not depend on `Required`. So do not read "5 cases, 2 red" as thin
|
||||
* coverage, and equally do not restate this as "every case reddens under any weakening" — that is
|
||||
* false, and a proof that overstates its own reach stops being re-examined.
|
||||
*
|
||||
* Case 4 reddened under `Partial<T>` ONLY until it was re-pinned from a required member to a
|
||||
* genuinely optional one; see its own comment for why that distinction is the whole game.
|
||||
*
|
||||
* What this file does NOT prove is that `Complete<T>` is APPLIED at every site that needs it.
|
||||
* Reverting one screen to its pre-#807 form leaves this guard green — the population of
|
||||
* construction sites is not derived from anything. That residue is tracked, not implied closed.
|
||||
*
|
||||
* Both directions are covered, because a MISSING field and a PHANTOM field are opposite defects:
|
||||
* cases 2 and 4 are the missing direction (the #807 defect proper — a silently dropped field on a
|
||||
* full-replace write), case 5 is the phantom direction.
|
||||
*
|
||||
* Cases 1-3 use a SYNTHETIC type so the proof cannot be invalidated by an unrelated schema change,
|
||||
* and cases 4-5 use a REAL generated request type so the proof is demonstrably wired to
|
||||
* `./generated/v1.d.ts` rather than only to a local fixture. Both are needed: the synthetic case
|
||||
* alone would pass even if the generated types stopped being importable here, and the real case
|
||||
* alone would break for reasons that have nothing to do with `Complete<T>`.
|
||||
*
|
||||
* Which real schemas carry a genuinely optional member is DERIVED, not listed here or anywhere
|
||||
* else in prose: `scripts/tests/test_optional_request_members.py` computes it from the OpenAPI
|
||||
* document every run and fails until each has a stated disposition. Three hand-written versions of
|
||||
* that list were wrong (#807), so do not add a fourth to this comment.
|
||||
*
|
||||
* Note the deliberate shape of the synthetic type: `optionalMember?` is what a schema property
|
||||
* emits as when it is absent from its `required` array in the OpenAPI document. Most request
|
||||
* properties are not, which is why most builders already typecheck and the gap reads as closed on
|
||||
* inspection. Case 2 is the whole point of this guard: it is the case that compiles clean WITHOUT
|
||||
* `Complete<T>`, which is why case 3 asserts exactly that. See `./completeRequest.ts`.
|
||||
*/
|
||||
|
||||
type SyntheticRequest = {
|
||||
requiredMember: string;
|
||||
optionalMember?: number;
|
||||
nullableMember: string | null;
|
||||
};
|
||||
|
||||
// Case 1 — every member named, the optional one explicitly `undefined`: MUST compile.
|
||||
// This is not filler. If `Complete<T>` were written so that an optional member had to carry a
|
||||
// real value, every builder would be forced to invent one, and the guard would be abandoned.
|
||||
const case1: Complete<SyntheticRequest> = {
|
||||
requiredMember: 'x',
|
||||
optionalMember: undefined,
|
||||
nullableMember: null
|
||||
};
|
||||
|
||||
// Case 2 — the OPTIONAL member omitted: MUST be an error. This is #807's defect exactly.
|
||||
// @ts-expect-error omitting an optional member of a Complete<T> request must not compile
|
||||
const case2: Complete<SyntheticRequest> = {
|
||||
requiredMember: 'x',
|
||||
nullableMember: null
|
||||
};
|
||||
|
||||
// Case 3 — the SAME omission against the bare type: MUST compile.
|
||||
// This is the negative control, and without it cases 2 and 4 prove nothing: it demonstrates that
|
||||
// the error in case 2 comes from `Complete<T>` and not from some other property of the object.
|
||||
// It is the "compiles clean and is silently dropped" state #807 was filed about.
|
||||
const case3: SyntheticRequest = {
|
||||
requiredMember: 'x',
|
||||
nullableMember: null
|
||||
};
|
||||
|
||||
// Case 4 — a REAL generated request type with a genuinely OPTIONAL member omitted: MUST error.
|
||||
//
|
||||
// `MultiCollectionItemRequest.weight` sits outside its schema's `required` array (it is a defaulted
|
||||
// ctor param), so it emits as `weight?: number` and omitting it is legal against the bare type.
|
||||
// That makes this case DISCRIMINATING: it goes red under the realistic weakening
|
||||
// `{ [K in keyof T]: T[K] }`, not only under `Partial<T>`. It is also the exact field whose loss
|
||||
// was live before #807 — dropping it from `MultiCollectionsScreen.toItemRequest` typechecked clean
|
||||
// and would have reset every weight to 1 on the next full-replace save.
|
||||
//
|
||||
// An earlier version of this case pinned `ReplaceDecoTemplateRequest.name`, a REQUIRED member, and
|
||||
// then reasoned in a comment that no real request type could discriminate "because none has an
|
||||
// optional member". That was false when written — `scripts/tests/test_optional_request_members.py`
|
||||
// derives the schemas that do, and there are several — and it cost the proof its most valuable
|
||||
// case. No count is given here on purpose: that population is derived, and every hand-written
|
||||
// version of it on this issue has been wrong. Do not re-pin this to a required member: check
|
||||
// the schema's `required` array
|
||||
// first, and prefer a member that a builder could actually drop.
|
||||
//
|
||||
// If `weight` ever becomes required or disappears, this directive goes unused and tsc reports it.
|
||||
// That is the correct outcome — re-pin to another genuinely optional member rather than deleting
|
||||
// the case.
|
||||
// @ts-expect-error omitting the optional `weight` from a real
|
||||
// Complete<MultiCollectionItemRequest> must not compile
|
||||
const case4: Complete<components['schemas']['MultiCollectionItemRequest']> = {
|
||||
collectionId: 1,
|
||||
smartCollectionId: null,
|
||||
scheduleAsGroup: false,
|
||||
playbackOrder: 'Chronological'
|
||||
};
|
||||
|
||||
// Case 5 — the PHANTOM direction: a field the schema does not accept: MUST be an error.
|
||||
// The directive sits on the OFFENDING PROPERTY, not on the declaration: an excess-property error
|
||||
// (TS2353) is reported at the property, whereas a missing-property error (TS2741, case 4) is
|
||||
// reported at the declaration. Placing it on the declaration instead made tsc report the directive
|
||||
// as unused AND the excess property as an error — two reds, not a proof.
|
||||
const case5: Complete<components['schemas']['ReplaceDecoTemplateRequest']> = {
|
||||
name: 'x',
|
||||
items: [],
|
||||
// @ts-expect-error a field absent from the schema must not compile
|
||||
fieldTheSchemaDoesNotHave: 1
|
||||
};
|
||||
|
||||
describe('#807 Complete<T> request guard', () => {
|
||||
it('is proven by the @ts-expect-error directives above, which npm run typecheck executes', () => {
|
||||
// These runtime assertions exist only so the compile-time cases are REFERENCED. Without a use,
|
||||
// `noUnusedLocals`/lint could remove them and the proof would vanish silently — the exact
|
||||
// "a guard that never executed proves nothing" failure this repo has shipped before (#751).
|
||||
// They deliberately assert almost nothing about behaviour: the guard is the compiler.
|
||||
expect(case1.requiredMember).toBe('x');
|
||||
expect(case3.requiredMember).toBe('x');
|
||||
expect(case2).toBeDefined();
|
||||
expect(case4).toBeDefined();
|
||||
expect(case5).toBeDefined();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,70 @@
|
||||
/**
|
||||
* `Complete<T>` — a request type with every member REQUIRED, so a builder that forgets one
|
||||
* fails `npm run typecheck` instead of silently dropping the field (issue #807).
|
||||
*
|
||||
* ## The defect this closes
|
||||
*
|
||||
* The SPA builds write-request bodies field-by-field as object literals against the generated
|
||||
* schema types in `./generated/v1.d.ts`. Those bodies are sent to full-replace endpoints, so a
|
||||
* field the builder never sets is not "left alone" — it is written as its default. That is #754's
|
||||
* mechanism: a hand-maintained mirror drifts from a DTO by one field, the write returns HTTP 200,
|
||||
* and the loss surfaces hours later. See `testing.full-replace-asserts-field-list`.
|
||||
*
|
||||
* This is not hypothetical. Measured 2026-08-22, before this change: deleting
|
||||
* `weight: clampWeight(item.weight)` from `MultiCollectionsScreen.toItemRequest` typechecked
|
||||
* CLEAN, and that PUT replaces the item list — so every weight would have reset to 1 on the next
|
||||
* save. The screen carries a prose comment warning about exactly that; a comment is not a check.
|
||||
* `FFmpegProfilesScreen` had the same exposure on `qsvPreferNativeDecoder`.
|
||||
*
|
||||
* ## What determines whether the compiler can see it
|
||||
*
|
||||
* A member is omittable exactly when the generated type marks it `?:`, and that comes from the
|
||||
* `required` array of the schema in `ErsatzTV/wwwroot/openapi/v1.json`.
|
||||
* `web/scripts/generate-openapi-types.mjs` is a pure pass-through — its whole contribution is
|
||||
* `const optional = required.has(name) ? '' : '?'` — so the determinant is the ASP.NET-produced
|
||||
* OpenAPI document, NOT the generator script. A property lands outside `required` because of how
|
||||
* its DTO is modelled — typically a defaulted constructor parameter, as `weight` and
|
||||
* `qsvPreferNativeDecoder` are. Do not maintain a count of such properties here: two hand-written
|
||||
* lists of them were wrong (#807), and `scripts/tests/test_optional_request_members.py` derives
|
||||
* the current set from the OpenAPI document on every run.
|
||||
*
|
||||
* NOT EVERY optional member is a droppable field. Where the optional members are computed get-only
|
||||
* properties on the C# record — `ArtworkContentTypeModel`'s `IsExternalUrl`, `HasContentType`,
|
||||
* `UrlWithContentType` — nothing deserializes them, so omitting them drops nothing, and applying
|
||||
* `Complete<T>` there would be a BUG: it would force a caller to fabricate server-computed values
|
||||
* in an outbound request. That test file records the disposition per schema.
|
||||
*
|
||||
* Note the direction of the trap: a nullable property usually emits as REQUIRED-and-nullable
|
||||
* (`"name": null | string`), so most builders are checked and the gap looks closed on inspection.
|
||||
* The unchecked ones are a small minority hiding inside a large majority of checked ones.
|
||||
*
|
||||
* ## Both directions
|
||||
*
|
||||
* - MISSING (a schema field the builder never sets) — `Complete<T>` makes it a hard error, and it
|
||||
* does so wherever the value is assigned, including through a spread or an inferred local.
|
||||
* - PHANTOM (a field the builder sets that the schema does not accept) — TypeScript's excess
|
||||
* property check already reports this, but ONLY for a "fresh" object literal in a typed
|
||||
* position. Returning a literal from a generic `.map` callback is NOT such a position: `map<U>`
|
||||
* infers `U` FROM the callback's return, so the target element type never contextually types the
|
||||
* literal, and the check does not fire — with or without a spread in it. Measured on the
|
||||
* pre-#807 tree: several construction sites accepted a phantom field, three of them with no
|
||||
* spread and no inferred local. Annotating each site's return type is what restores this
|
||||
* direction. (No tally: "construction site" is not a derived population, and the count in an
|
||||
* earlier draft was wrong.)
|
||||
*
|
||||
* ## Explicit `undefined` is still allowed, deliberately
|
||||
*
|
||||
* `T[K]` is preserved, so an optional member may be written as `field: undefined`. The point is
|
||||
* not to forbid omitting a value, it is to forbid omitting the DECISION — an unmentioned field is
|
||||
* an oversight, `field: undefined` is a choice a reviewer can see.
|
||||
*
|
||||
* ## Shallow, and what that costs
|
||||
*
|
||||
* `Complete<T>` does not recurse into `items: Array<ItemRequest>`; each nested item builder is
|
||||
* annotated `Complete<ItemRequest>` directly instead. This is a scope choice, not a claim that a
|
||||
* deep variant is infeasible — probed 2026-08-22, the shallow mapped form distributes correctly
|
||||
* over unions, leaves `number[]` an array, preserves `readonly`, and passes an index signature
|
||||
* through, so a deep variant is not obviously blocked. A NEW nested request type therefore needs
|
||||
* its own annotation and nothing forces that; see the residue noted on the decision record.
|
||||
*/
|
||||
export type Complete<T> = { [K in keyof Required<T>]: T[K] };
|
||||
@@ -1,3 +1,4 @@
|
||||
import type { Complete } from './completeRequest';
|
||||
import { ApiError, request, requestWithMeta, type ResponseWithMeta } from './client';
|
||||
import type { components } from './generated/v1';
|
||||
|
||||
@@ -57,7 +58,7 @@ export function getDecoTemplateItemsWithMeta(id: number): Promise<ResponseWithMe
|
||||
*/
|
||||
export function replaceDecoTemplate(
|
||||
id: number,
|
||||
body: ReplaceDecoTemplateRequest,
|
||||
body: Complete<ReplaceDecoTemplateRequest>,
|
||||
ifMatch?: string | null
|
||||
): Promise<ResponseWithMeta<DecoTemplateWithItems>> {
|
||||
return requestWithMeta<DecoTemplateWithItems>(`/api/v1/deco-templates/${id}`, {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import type { Complete } from './completeRequest';
|
||||
import { ApiError, request } from './client';
|
||||
import type { components } from './generated/v1';
|
||||
|
||||
@@ -42,7 +43,7 @@ export function deleteDeco(id: number): Promise<void> {
|
||||
return request<void>(`/api/v1/decos/${id}`, { method: 'DELETE' });
|
||||
}
|
||||
|
||||
export function replaceDeco(id: number, body: ReplaceDecoRequest): Promise<Deco> {
|
||||
export function replaceDeco(id: number, body: Complete<ReplaceDecoRequest>): Promise<Deco> {
|
||||
return request<Deco>(`/api/v1/decos/${id}`, { body, method: 'PUT' });
|
||||
}
|
||||
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import type { Complete } from './completeRequest';
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import {
|
||||
createFFmpegProfile,
|
||||
@@ -19,7 +20,7 @@ function noContent(): Response {
|
||||
return new Response(null, { status: 204 });
|
||||
}
|
||||
|
||||
const sampleRequest: CreateFFmpegProfileRequest = {
|
||||
const sampleRequest: Complete<CreateFFmpegProfileRequest> = {
|
||||
allowBFrames: false,
|
||||
audioBitrate: 192,
|
||||
audioBufferSize: 384,
|
||||
@@ -37,6 +38,7 @@ const sampleRequest: CreateFFmpegProfileRequest = {
|
||||
normalizeVideo: true,
|
||||
padMode: 'Software',
|
||||
qsvExtraHardwareFrames: null,
|
||||
qsvPreferNativeDecoder: true,
|
||||
resolutionId: 1,
|
||||
scalingBehavior: 'ScaleAndPad',
|
||||
tonemapAlgorithm: 'Linear',
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import type { Complete } from './completeRequest';
|
||||
import { ApiError, request } from './client';
|
||||
import type { components } from './generated/v1';
|
||||
|
||||
@@ -14,11 +15,11 @@ export function getFFmpegProfile(id: number): Promise<FFmpegProfileDetail> {
|
||||
return request<FFmpegProfileDetail>(`/api/v1/ffmpeg/profiles/${id}`);
|
||||
}
|
||||
|
||||
export function createFFmpegProfile(body: CreateFFmpegProfileRequest): Promise<FFmpegProfileDetail> {
|
||||
export function createFFmpegProfile(body: Complete<CreateFFmpegProfileRequest>): Promise<FFmpegProfileDetail> {
|
||||
return request<FFmpegProfileDetail>('/api/v1/ffmpeg/profiles', { body, method: 'POST' });
|
||||
}
|
||||
|
||||
export function updateFFmpegProfile(id: number, body: UpdateFFmpegProfileRequest): Promise<FFmpegProfileDetail> {
|
||||
export function updateFFmpegProfile(id: number, body: Complete<UpdateFFmpegProfileRequest>): Promise<FFmpegProfileDetail> {
|
||||
return request<FFmpegProfileDetail>(`/api/v1/ffmpeg/profiles/${id}`, { body, method: 'PUT' });
|
||||
}
|
||||
|
||||
|
||||
@@ -6,6 +6,7 @@ export * from './channels';
|
||||
export * from './channelTemplates';
|
||||
export * from './client';
|
||||
export * from './collections';
|
||||
export * from './completeRequest';
|
||||
export * from './dashboard';
|
||||
export * from './decos';
|
||||
export * from './decoTemplates';
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import type { Complete } from './completeRequest';
|
||||
import { ApiError, request } from './client';
|
||||
import type { components } from './generated/v1';
|
||||
import type { RemoteFamily } from '../mediaSources/familyMeta';
|
||||
@@ -101,7 +102,7 @@ export function getRemoteLibraries(family: RemoteFamily, sourceId: number): Prom
|
||||
export function replaceRemoteLibraryPreferences(
|
||||
family: RemoteFamily,
|
||||
sourceId: number,
|
||||
body: ReplaceRemoteLibraryPreferencesRequest
|
||||
body: Complete<ReplaceRemoteLibraryPreferencesRequest>
|
||||
): Promise<RemoteLibrary[]> {
|
||||
return request<RemoteLibrary[]>(`/api/v1/media-sources/${family}/${sourceId}/libraries`, { body, method: 'PUT' });
|
||||
}
|
||||
@@ -113,7 +114,7 @@ export function getPathReplacements(family: RemoteFamily, sourceId: number): Pro
|
||||
export function replacePathReplacements(
|
||||
family: RemoteFamily,
|
||||
sourceId: number,
|
||||
body: ReplacePathReplacementsRequest
|
||||
body: Complete<ReplacePathReplacementsRequest>
|
||||
): Promise<PathReplacement[]> {
|
||||
return request<PathReplacement[]>(`/api/v1/media-sources/${family}/${sourceId}/path-replacements`, {
|
||||
body,
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import type { Complete } from './completeRequest';
|
||||
import { ApiError, request, requestWithMeta, type ResponseWithMeta } from './client';
|
||||
import type { components } from './generated/v1';
|
||||
|
||||
@@ -53,7 +54,7 @@ export function createMultiCollection(body: CreateMultiCollectionRequest): Promi
|
||||
*/
|
||||
export function updateMultiCollection(
|
||||
id: number,
|
||||
body: UpdateMultiCollectionRequest,
|
||||
body: Complete<UpdateMultiCollectionRequest>,
|
||||
ifMatch?: string | null
|
||||
): Promise<ResponseWithMeta<MultiCollection>> {
|
||||
return requestWithMeta<MultiCollection>(`/api/v1/multi-collections/${id}`, {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import type { Complete } from './completeRequest';
|
||||
import { ApiError, request, requestWithMeta, type ResponseWithMeta } from './client';
|
||||
import type { components } from './generated/v1';
|
||||
|
||||
@@ -55,7 +56,7 @@ export function createPlaylist(body: CreatePlaylistRequest): Promise<Playlist> {
|
||||
// the new ETag for a subsequent save (issue #253).
|
||||
export function updatePlaylist(
|
||||
id: number,
|
||||
body: ReplacePlaylistRequest,
|
||||
body: Complete<ReplacePlaylistRequest>,
|
||||
ifMatch?: string | null
|
||||
): Promise<ResponseWithMeta<PlaylistItem[]>> {
|
||||
return requestWithMeta<PlaylistItem[]>(`/api/v1/playlists/${id}`, {
|
||||
@@ -69,7 +70,7 @@ export function deletePlaylist(id: number): Promise<void> {
|
||||
return request<void>(`/api/v1/playlists/${id}`, { method: 'DELETE' });
|
||||
}
|
||||
|
||||
export function previewPlaylist(body: ReplacePlaylistRequest): Promise<PlaylistPreviewItem[]> {
|
||||
export function previewPlaylist(body: Complete<ReplacePlaylistRequest>): Promise<PlaylistPreviewItem[]> {
|
||||
return request<PlaylistPreviewItem[]>('/api/v1/playlists/preview', { body, method: 'POST' });
|
||||
}
|
||||
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import type { Complete } from './completeRequest';
|
||||
import { useCallback, useEffect, useRef, useState } from 'react';
|
||||
import { ApiError, request, requestWithMeta, type ResponseWithMeta } from './client';
|
||||
import type { components } from './generated/v1';
|
||||
@@ -179,7 +180,7 @@ export function getAlternateSchedulesWithMeta(
|
||||
*/
|
||||
export function replaceAlternateSchedules(
|
||||
playoutId: number,
|
||||
body: ReplacePlayoutAlternateSchedulesRequest,
|
||||
body: Complete<ReplacePlayoutAlternateSchedulesRequest>,
|
||||
ifMatch?: string | null
|
||||
): Promise<ResponseWithMeta<PlayoutAlternateSchedule[]>> {
|
||||
return requestWithMeta<PlayoutAlternateSchedule[]>(`/api/v1/playouts/${playoutId}/alternate-schedules`, {
|
||||
@@ -204,7 +205,7 @@ export function getPlayoutTemplatesWithMeta(playoutId: number): Promise<Response
|
||||
*/
|
||||
export function replacePlayoutTemplates(
|
||||
playoutId: number,
|
||||
body: ReplacePlayoutTemplatesRequest,
|
||||
body: Complete<ReplacePlayoutTemplatesRequest>,
|
||||
ifMatch?: string | null
|
||||
): Promise<ResponseWithMeta<PlayoutTemplate[]>> {
|
||||
return requestWithMeta<PlayoutTemplate[]>(`/api/v1/playouts/${playoutId}/templates`, {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import type { Complete } from './completeRequest';
|
||||
import { ApiError, request, requestWithMeta, type ResponseWithMeta } from './client';
|
||||
import type { components } from './generated/v1';
|
||||
// FillerPreset is exported by ./pickers — import (don't re-export) to avoid a duplicate `export *`
|
||||
@@ -67,7 +68,7 @@ export function addScheduleItem(scheduleId: number, body: ScheduleItemRequest):
|
||||
// response (see SchedulesScreen.tsx `save()`), not reuse the ids it submitted.
|
||||
export function replaceScheduleItems(
|
||||
scheduleId: number,
|
||||
body: ReplaceScheduleItemsRequest,
|
||||
body: Complete<ReplaceScheduleItemsRequest>,
|
||||
ifMatch?: string | null
|
||||
): Promise<ResponseWithMeta<ScheduleItem[]>> {
|
||||
return requestWithMeta<ScheduleItem[]>(`/api/v1/schedules/${scheduleId}/items`, {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import type { Complete } from './completeRequest';
|
||||
import { ApiError, request, requestWithMeta, type ResponseWithMeta } from './client';
|
||||
import type { components } from './generated/v1';
|
||||
|
||||
@@ -58,7 +59,7 @@ export function getTemplateItemsWithMeta(id: number): Promise<ResponseWithMeta<T
|
||||
*/
|
||||
export function replaceTemplate(
|
||||
id: number,
|
||||
body: ReplaceTemplateRequest,
|
||||
body: Complete<ReplaceTemplateRequest>,
|
||||
ifMatch?: string | null
|
||||
): Promise<ResponseWithMeta<TemplateWithItems>> {
|
||||
return requestWithMeta<TemplateWithItems>(`/api/v1/templates/${id}`, {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// Pure, exhaustively-tested rules for the schedule-item editor. Encodes the Blazor
|
||||
// ProgramScheduleItemEditViewModel gating + forced-reset behavior (the parity standard for #207).
|
||||
// No React, no fetch — every function is a deterministic transform so itemRules.test.ts can pin it.
|
||||
import type { Complete } from '../api/completeRequest';
|
||||
import type { components } from '../api/generated/v1';
|
||||
|
||||
export type CollectionType = components['schemas']['CollectionType'];
|
||||
@@ -389,7 +390,7 @@ export function fromResponse(model: ScheduleItem): DraftItem {
|
||||
|
||||
// Projects a draft to the request body, applying Blazor's getter-gating: gated-off values are
|
||||
// nulled (or defaulted to the enum's None) so the payload matches what a non-editing Blazor VM emits.
|
||||
export function normalizeForSave(item: DraftItem): ScheduleItemRequest {
|
||||
export function normalizeForSave(item: DraftItem): Complete<ScheduleItemRequest> {
|
||||
const isFixed = item.startType === 'Fixed';
|
||||
const isMultipleCount = item.playoutMode === 'Multiple' && item.multipleMode === 'Count';
|
||||
const isDuration = item.playoutMode === 'Duration';
|
||||
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
Trash2,
|
||||
TriangleAlert
|
||||
} from 'lucide-react';
|
||||
import type { Complete } from '../api/completeRequest';
|
||||
import { navigateToPath } from '../routing';
|
||||
import {
|
||||
Badge,
|
||||
@@ -206,7 +207,7 @@ function itemFromResponse(item: BlockItem): DraftItem {
|
||||
};
|
||||
}
|
||||
|
||||
function toRequestItem(item: DraftItem): BlockItemRequest {
|
||||
function toRequestItem(item: DraftItem): Complete<BlockItemRequest> {
|
||||
return {
|
||||
collectionType: item.collectionType,
|
||||
collectionId: item.collectionType === 'Collection' ? item.collectionId : null,
|
||||
@@ -269,7 +270,7 @@ function validate(draft: Draft): null | string {
|
||||
return null;
|
||||
}
|
||||
|
||||
function toReplaceRequest(draft: Draft): ReplaceBlockRequest {
|
||||
function toReplaceRequest(draft: Draft): Complete<ReplaceBlockRequest> {
|
||||
return {
|
||||
name: draft.name.trim(),
|
||||
minutes: draft.hours * 60 + draft.minutes,
|
||||
|
||||
@@ -20,8 +20,10 @@ import {
|
||||
type DecoListItem,
|
||||
type DecoTemplate,
|
||||
type DecoTemplateGroup,
|
||||
type DecoTemplateItem
|
||||
type DecoTemplateItem,
|
||||
type DecoTemplateItemRequest
|
||||
} from '../api';
|
||||
import type { Complete } from '../api/completeRequest';
|
||||
|
||||
const BASE_PATH = '/app/deco-templates';
|
||||
const MINUTES_PER_DAY = 24 * 60;
|
||||
@@ -604,7 +606,7 @@ function DecoTemplateEditor({ decoTemplateId }: { decoTemplateId: number }) {
|
||||
decoTemplateId,
|
||||
{
|
||||
name: draft.name.trim(),
|
||||
items: draft.items.map((item) => ({
|
||||
items: draft.items.map((item): Complete<DecoTemplateItemRequest> => ({
|
||||
decoId: item.decoId,
|
||||
startTime: item.startTime,
|
||||
endTime: item.endTime
|
||||
|
||||
@@ -36,6 +36,7 @@ import {
|
||||
type ReplaceDecoRequest,
|
||||
type SchedulingPickerOption
|
||||
} from '../api';
|
||||
import type { Complete } from '../api/completeRequest';
|
||||
import { getGraphicsElements, getWatermarks, type GraphicsElement, type Watermark } from '../api/pickers';
|
||||
import { getPlaylistGroups, getPlaylists, type Playlist, type PlaylistGroup } from '../api/playlists';
|
||||
|
||||
@@ -206,7 +207,7 @@ function fillerIdFields(type: CollectionType, id: number | null) {
|
||||
};
|
||||
}
|
||||
|
||||
function breakToRequest(item: BreakDraft): DecoBreakContentRequest {
|
||||
function breakToRequest(item: BreakDraft): Complete<DecoBreakContentRequest> {
|
||||
if (item.collectionType === 'Playlist') {
|
||||
return {
|
||||
id: item.id,
|
||||
@@ -233,7 +234,7 @@ function breakToRequest(item: BreakDraft): DecoBreakContentRequest {
|
||||
};
|
||||
}
|
||||
|
||||
function toReplaceRequest(draft: Draft): ReplaceDecoRequest {
|
||||
function toReplaceRequest(draft: Draft): Complete<ReplaceDecoRequest> {
|
||||
const defaultFiller = fillerIdFields(draft.defaultFillerCollectionType, draft.defaultFillerId);
|
||||
const deadAir = fillerIdFields(draft.deadAirFallbackCollectionType, draft.deadAirFallbackId);
|
||||
return {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import type { Complete } from '../api/completeRequest';
|
||||
import { useCallback, useEffect, useRef, useState, type ReactNode } from 'react';
|
||||
import { ArrowLeft, Check, Copy, Plus, SlidersHorizontal, Trash2, TriangleAlert } from 'lucide-react';
|
||||
import { navigateToPath } from '../routing';
|
||||
@@ -24,7 +25,11 @@ const BASE_PATH = '/app/ffmpeg-profiles';
|
||||
// form must not offer one it would silently override (ersatztv#529)
|
||||
const MINIMUM_QSV_EXTRA_HARDWARE_FRAMES = 64;
|
||||
|
||||
type Draft = CreateFFmpegProfileRequest;
|
||||
// `Complete<…>` so the draft must name every request member: the edit path PUTs this whole
|
||||
// object to a full-replace endpoint, where an unset member is written as its default rather
|
||||
// than left alone. `qsvPreferNativeDecoder` is optional in the schema and both draft builders
|
||||
// happened to set it; nothing required them to (#807).
|
||||
type Draft = Complete<CreateFFmpegProfileRequest>;
|
||||
|
||||
/* ---------- enum option lists (mirror ErsatzTV/Pages/FFmpegEditor.razor) ---------- */
|
||||
|
||||
|
||||
@@ -16,8 +16,10 @@ import {
|
||||
type MediaCollection,
|
||||
type MultiCollection,
|
||||
type MultiCollectionItemRequest,
|
||||
type SmartCollection
|
||||
type SmartCollection,
|
||||
type UpdateMultiCollectionRequest
|
||||
} from '../api';
|
||||
import type { Complete } from '../api/completeRequest';
|
||||
|
||||
/* ---------- data hook ---------- */
|
||||
|
||||
@@ -137,7 +139,7 @@ function itemsFromMultiCollection(mc: MultiCollection): DraftItem[] {
|
||||
);
|
||||
}
|
||||
|
||||
function toItemRequest(item: DraftItem): MultiCollectionItemRequest {
|
||||
function toItemRequest(item: DraftItem): Complete<MultiCollectionItemRequest> {
|
||||
return {
|
||||
collectionId: item.kind === 'manual' ? item.id : null,
|
||||
playbackOrder: 'Chronological',
|
||||
@@ -277,7 +279,10 @@ function MultiCollectionEditor({
|
||||
setSaveError(null);
|
||||
|
||||
try {
|
||||
const body = { items: items.map(toItemRequest), name: trimmedName };
|
||||
const body: Complete<UpdateMultiCollectionRequest> = {
|
||||
items: items.map(toItemRequest),
|
||||
name: trimmedName
|
||||
};
|
||||
if (initial) {
|
||||
await updateMultiCollection(initial.id, body, etagRef.current);
|
||||
} else {
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import type { Complete } from '../api/completeRequest';
|
||||
import type { PathReplacementItemRequest } from '../api/mediaSources';
|
||||
import { useCallback, useEffect, useRef, useState } from 'react';
|
||||
import { ArrowLeft, Check, Plus, Trash2, TriangleAlert } from 'lucide-react';
|
||||
import { Button, Card, Input, Spinner } from '../components';
|
||||
@@ -172,7 +174,13 @@ export function PathReplacementsEditScreen({ family, sourceId }: { family: Remot
|
||||
setSaving(true);
|
||||
setSaveError(null);
|
||||
replacePathReplacements(family, sourceId, {
|
||||
items: draft.map((row) => ({ id: row.id, remotePath: row.remotePath.trim(), localPath: row.localPath.trim() }))
|
||||
items: draft.map(
|
||||
(row): Complete<PathReplacementItemRequest> => ({
|
||||
id: row.id,
|
||||
remotePath: row.remotePath.trim(),
|
||||
localPath: row.localPath.trim()
|
||||
})
|
||||
)
|
||||
})
|
||||
.then((rows) => {
|
||||
if (!activeRef.current) {
|
||||
|
||||
@@ -44,8 +44,10 @@ import {
|
||||
type PlaylistGroup,
|
||||
type PlaylistItem,
|
||||
type PlaylistItemRequest,
|
||||
type PlaylistPreviewItem
|
||||
type PlaylistPreviewItem,
|
||||
type ReplacePlaylistRequest
|
||||
} from '../api';
|
||||
import type { Complete } from '../api/completeRequest';
|
||||
import { SearchPicker } from '../schedules/pickers';
|
||||
|
||||
type CollectionType = PlaylistItemRequest['collectionType'];
|
||||
@@ -273,7 +275,7 @@ function draftFromItem(item: PlaylistItem): DraftItem {
|
||||
};
|
||||
}
|
||||
|
||||
function toItemRequest(item: DraftItem, index: number): PlaylistItemRequest {
|
||||
function toItemRequest(item: DraftItem, index: number): Complete<PlaylistItemRequest> {
|
||||
const source = configFor(item.collectionType)?.source ?? 'browse';
|
||||
const trimmedCount = item.count.trim();
|
||||
const parsedCount = trimmedCount === '' ? null : Number(trimmedCount);
|
||||
@@ -582,7 +584,10 @@ function PlaylistEditor({ playlistId, onBack, onSaved }: { playlistId: number; o
|
||||
});
|
||||
};
|
||||
|
||||
const buildRequest = () => ({ items: items.map(toItemRequest), name: name.trim() });
|
||||
const buildRequest = (): Complete<ReplacePlaylistRequest> => ({
|
||||
items: items.map(toItemRequest),
|
||||
name: name.trim()
|
||||
});
|
||||
|
||||
// Every item must carry a selection the API can bind. Without this the screen happily PUT an item
|
||||
// with a null id — the server 422s it (`ReplacePlaylistItemsHandler.CollectionTypeMustBeValid`),
|
||||
|
||||
@@ -20,8 +20,11 @@ import {
|
||||
type PlayoutAlternateSchedule,
|
||||
type PlayoutTemplate,
|
||||
type ProgramSchedule,
|
||||
type Template
|
||||
type Template,
|
||||
type PlayoutAlternateScheduleItemRequest,
|
||||
type PlayoutTemplateItemRequest
|
||||
} from '../api';
|
||||
import type { Complete } from '../api/completeRequest';
|
||||
|
||||
const PLAYOUTS_PATH = '/app/playouts';
|
||||
|
||||
@@ -526,11 +529,13 @@ export function PlayoutAlternateSchedulesScreen({ playoutId }: { playoutId: numb
|
||||
replaceAlternateSchedules(
|
||||
playoutId,
|
||||
{
|
||||
items: items.map((item) => ({
|
||||
id: item.id,
|
||||
programScheduleId: item.programScheduleId,
|
||||
...toRequestRecurrence(item)
|
||||
}))
|
||||
items: items.map(
|
||||
(item): Complete<PlayoutAlternateScheduleItemRequest> => ({
|
||||
id: item.id,
|
||||
programScheduleId: item.programScheduleId,
|
||||
...toRequestRecurrence(item)
|
||||
})
|
||||
)
|
||||
},
|
||||
etagRef.current
|
||||
)
|
||||
@@ -830,12 +835,14 @@ export function PlayoutTemplatesEditorScreen({ playoutId }: { playoutId: number
|
||||
replacePlayoutTemplates(
|
||||
playoutId,
|
||||
{
|
||||
items: items.map((item) => ({
|
||||
id: item.id,
|
||||
templateId: item.templateId,
|
||||
decoTemplateId: item.decoTemplateId,
|
||||
...toRequestRecurrence(item)
|
||||
}))
|
||||
items: items.map(
|
||||
(item): Complete<PlayoutTemplateItemRequest> => ({
|
||||
id: item.id,
|
||||
templateId: item.templateId,
|
||||
decoTemplateId: item.decoTemplateId,
|
||||
...toRequestRecurrence(item)
|
||||
})
|
||||
)
|
||||
},
|
||||
etagRef.current
|
||||
)
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import type { Complete } from '../api/completeRequest';
|
||||
import type { RemoteLibraryPreferenceRequest } from '../api/mediaSources';
|
||||
import { useCallback, useEffect, useRef, useState } from 'react';
|
||||
import { ArrowLeft, ArrowUpDown, Check, TriangleAlert } from 'lucide-react';
|
||||
import { Button, Card, Spinner, Switch } from '../components';
|
||||
@@ -171,7 +173,12 @@ export function RemoteLibrariesEditScreen({ family, sourceId }: { family: Remote
|
||||
setSaving(true);
|
||||
setSaveError(null);
|
||||
replaceRemoteLibraryPreferences(family, sourceId, {
|
||||
libraries: draft.map((library) => ({ id: library.id, shouldSyncItems: library.shouldSyncItems }))
|
||||
libraries: draft.map(
|
||||
(library): Complete<RemoteLibraryPreferenceRequest> => ({
|
||||
id: library.id,
|
||||
shouldSyncItems: library.shouldSyncItems
|
||||
})
|
||||
)
|
||||
})
|
||||
.then((libraries) => {
|
||||
if (!activeRef.current) {
|
||||
|
||||
@@ -21,8 +21,10 @@ import {
|
||||
type BlockGroup,
|
||||
type Template,
|
||||
type TemplateGroup,
|
||||
type TemplateItem
|
||||
type TemplateItem,
|
||||
type TemplateItemRequest
|
||||
} from '../api';
|
||||
import type { Complete } from '../api/completeRequest';
|
||||
|
||||
const BASE_PATH = '/app/templates';
|
||||
|
||||
@@ -678,7 +680,9 @@ function TemplateEditor({ templateId }: { templateId: number }) {
|
||||
templateId,
|
||||
{
|
||||
name: draft.name.trim(),
|
||||
items: draft.items.map((item) => ({ blockId: item.blockId, startTime: item.startTime }))
|
||||
items: draft.items.map(
|
||||
(item): Complete<TemplateItemRequest> => ({ blockId: item.blockId, startTime: item.startTime })
|
||||
)
|
||||
},
|
||||
etagRef.current
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user