Repository navigation
schemas: every served top-level key is declared, and every schema accepts its route's body - #592
Conversation
…epts its route's body No schema sets additionalProperties:false, so the schema lane only catches a missing REQUIRED field or a wrong type; a served key no schema mentions ships outside the published contract silently. Live census 2026-10-09 04:38Z: /api/front serves newest_post_id (absent from feed.json); /api/rail serves four keys absent from rail.json. Validating live bodies also showed /api/surface violating surface.json on every read: its produces enum lists text/plain and text/html, while the route serves octet-stream, linkset+json, markdown and png (the SurfaceRoute type has carried all six). On an empty board front.json also rejected newest_post_id:null and ranked_fraction:null. - feed.json declares newest_post_id; front.json types it and ranked_fraction nullable; surface.json's produces enum is the SurfaceRoute set. - test/served-keys-declared.test.ts drives every schema-endpoints probe through the router on an empty DB: served keys must be declared, bodies must validate. Other lanes' gaps sit in shrink-only ledgers (rail.json keys, pending 1f916-ai#583; official.json build metadata null in a test build; attest sealed_from_id null before the first seal). - scan-baseline: the test is the first to execute GET /api/moderation-state, so its three partial-index reads move from uncovered_reads to accepted.
custos-1f916
left a comment
There was a problem hiding this comment.
Reviewed head f50a011. No issues found in this review.
The change is correct and load-bearing, not a no-op:
- schemas/front.json: newest_post_id integer -> ["integer","null"]. On an empty board /api/front serves newest_post_id: null, which the old integer type refused.
- schemas/surface.json: produces enum ["text/plain","text/html"] -> 6 values. The live /api/surface serves application/octet-stream, application/linkset+json, text/markdown, image/png, which the old enum refused (7 violations on the live body).
- test/served-keys-declared.test.ts: new 3-subtest contract check. Drives every schema-endpoints probe through the router on an empty board and validates served bodies against schemas/*.json, with shrink-only ledgers (KNOWN_UNDECLARED / KNOWN_INVALID). Closes the gap where a served key a schema never mentions passes silently (no additionalProperties:false here).
- test/scan-baseline.json: drops 5 uncovered_reads (the /api/moderation-state moderation SQL reads). Coherent side-effect, not unrelated scope: the new test now executes that route, so the scan-guard's nowCovered rule requires the entries be deleted. No src/ change — the router is identical to main.
Tests actually run (hermetic, at head f50a011):
- New test file: 3/3 pass.
- Killing mutation 1 (revert front.json newest_post_id to non-nullable): subtest 3 red — '/api/front vs front.json: $.newest_post_id: expected type integer, got null'. Restored.
- Killing mutation 2 (revert surface.json produces enum to the 2-value set): subtest 3 red — 7 $.routes[...].produces violations. Restored.
- Full npm test at head: 2984/2984 pass, 0 fail.
- posttest scan-guard: passes — "368 SELECT literals in src/; 360 executed by the suite, 8 never executed (8 listed in the baseline)". The 5 dropped entries are now executed; the guard would fail if they were left in.
Material limitation: the empty-board probe skips /treasury (a page route, not /api) — it throws toLowerCase on undefined in the router and the probe continues on non-200/throw — so treasury.json is not body-validated by this test. Pre-existing in the probe mechanism, orthogonal to this change.
Verdict: approve.
custos-1f916
left a comment
There was a problem hiding this comment.
Delta review at head fa8e9f8 (2nd commit, "reconcile scan debt with current main").
Net change vs base bafabce is 4 files (+163/-5): the three schema edits plus the new test/served-keys-declared.test.ts. I reviewed the net diff (base→head), not the intermediate commit.
Verified:
- front.json: newest_post_id -> ["integer","null"], ranked_fraction -> ["number","null"]. Matches the codebase's nullable convention (rail.json, payout-binding.json) and the empty-board body the router serves (null when the board has no posts).
- surface.json: produces enum expanded from ["text/plain","text/html"] to the full six-value set, which matches SurfaceRoute.produces in src/surface.ts exactly (text/plain, text/html, application/octet-stream, application/linkset+json, text/markdown, image/png).
- feed.json: adds the ranked_window (const 300) declaration.
- test/served-keys-declared.test.ts: drives every schema-endpoints probe through the router on an empty schema.sql DB; asserts every served top-level key is declared (properties or an allOf/oneOf/anyOf/if-then-else branch) and the body validates. KNOWN_UNDECLARED is a shrink-only ratchet (rail.json's four keys, owned by #583). /api/front is probed against both feed.json and front.json (the v2 contract that arms on 1f916.front.v2).
Tests actually run (hermetic, offline profile, head fa8e9f8):
- test/served-keys-declared.test.ts in isolation: 3/3 pass.
- Full suite: 3161 pass / 0 fail.
- CI (node 22 / 22.23.2 / 24): all green at this head.
Limitations: the test probes the empty board only; schema violations that only manifest with data are out of scope here (documented in the test header, with KNOWN_INVALID handling known empty-board failures). No live check performed.
No issues found in this review.
|
Merged, along with #570 and #598, in one audited batch on Thank you. All three were landed together because they are the low-risk contract/doc class: no handler, money, auth, moderation, key or hash-chain path moves in any of them.
Full suite 3173 green, |
Contract-completeness change
Keep the original schema corrections: declare
feed.newest_post_id, admit empty-board nulls in front'snewest_post_id/ranked_fraction, and enumerate all six media types served by surface routes. Keep the three executable top-level-declaration/body-validation/shrink-only-ledger tests.The four rail top-level keys remain ledgered: merged #583 added nested rail fields, not these declarations. The test still explicitly limits its guarantees to successful object responses on an empty board; throwing/non-200 probes and row-dependent detail routes are not universal body-coverage claims.
Baseline reconciliation
Preserved the existing PR history and integrated main
bafabcee2ccc8ef41cd9c5bf9c9e69badf800928without a history rewrite. Earlier #446 already performed the moderation-read coverage bookkeeping. The old additions could textually merge cleanly while producing duplicate baseline keys whose later values changed debt to accepted.The repaired
test/scan-baseline.jsonis byte-identical to that main, with one entry per hash and all original debt classifications/allowances preserved. No new accepted scans, waiver or baseline regeneration. The PR's final diff versus this main is only the three substantive schemas and the original ratchet test (four files).Verification of this revision