docs(contracts): document schema-parity coverage boundary and deferred gaps - #17
Merged
Conversation
…ump ignores Constrained fields owe their own dedicated test. That convention is already well followed - `lease_seconds_is_bounded_like_the_schema` and its siblings exist because the schema parity test cannot cover them - but it was written down nowhere, so the next person to add a constrained field would reasonably assume parity covers it. `generated_schemas_match_checked_in_schemas` compares only property names, `required`, and `additionalProperties` via `assert_object_parity`. Measured: setting `$defs.change_request.properties.capability_id.type` to `"integer"` in `schemas/experiment.schema.json`, against `capability_id: String` in Rust, leaves all 29 contract tests green. Also replace the unanchored `core` / `core.*` crash-dump ignores. A slash-less pattern matches at any depth and matches directories, so bare `core` would silently untrack a future `src/core/` subsystem and bare `core.*` would swallow a source file such as `api/core.ts`. The replacement anchors to the repo root, negates a root-level `core/` package directory, and covers both per-host dump shapes. Verified: `git check-ignore` reports `core` and `core.node.12345` ignored and `api/core.ts` not ignored; `cargo test --workspace` and `cargo fmt --check` pass. No source or schema file is modified.
…hand-rolled sites
…way ceiling design
…iling drift entry
…e-ceiling precondition
…g doc-comment residual
… complete binders
…loss per constraint
…se-ceiling parity
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Two independent small documentation changes on one PR, deliberately not expanded: (1) document the contract-parity coverage boundary in AGENTS.md - the parity helpers compare property names, the required set, and additionalProperties only, never a property type or its constraints, and the repo compensates with hand-written per-field tests, a convention previously written down nowhere; (2) replace the core-dump .gitignore patterns with a supplied fleet v6 block, verbatim and frozen.
This branch is governed by a ruling ledger the captain owns:
/home/fleet/.treehouse/code-5eb5c1/4/code/data/fpsm-parity-gap-doc/ruling-ledger.md
Rows R1-R60a. Read it before proposing edits; most apparent defects are already ruled there, including several the captain ruled DECLINE. Scope is documentation and .gitignore only (R6b): no schema file, no executable code, no test assertion. Doc-comment /// lines are in scope. The .gitignore core-dump block and its comment are frozen byte-identical (R2, R18).
Four rulings are applied and pending as of head f6b33b7: R57/R57a (parameterize the bind definiendum; bind takes a schema as OBJECT and the constraint rides in an explicit "for" slot on the predicate, never as the object, and a participle on the subject does not supply that slot), R58, R59/R59a, R60.
What Changed
AGENTS.mdand in the doc comments onassert_object_parity(crates/contracts/src/lib.rs) andassert_same_shape(crates/contracts/src/test_support.rs): the parity helpers compare property names,required, andadditionalPropertiesonly, so a mismatchedtypeor a dropped bound stays green, and any field constrained beyondtypeor carrying adeserialize_withvalidator needs its own dedicated test that binds every checked-in schema carrying that constraint. Named the hand-rolled sites (response_variants_match_schema,capability_fields_match_capability_schema,manifest_fields_match_sidecar_schema) where the comparison takes no generated schema in. Also corrected thetest_supportmodule doc, which claimed all schema-sync modules share one comparison.docs/ARCHITECTURE.mdregistering three gaps as classes with derivable membership:fpsm-lease-ceiling-parity(missingmaximumon$defs.ChangeRequest.lease_secondsin the broker request schema plus the gateway's independently stated ceiling),fpsm-unbound-carrier-parity(constraints whose test binds fewer schemas than carry them), andfpsm-capid-guard(checked-in constraints with no Rust-side counterpart to compare against). Scoped the broker schema-sync claim in the same file to what the contract tests actually reach.core/core.*crash-dump ignores in.gitignorewith a depth-matching block (**/core,**/core.[0-9]*,**/core.*.[0-9]*) plus a!**/core/negation so directories namedcoreare no longer silently untracked; dumps land in the crashing process's cwd, which forcargo testis the package root, not the repo root.Risk Assessment
✅ Low: Documentation and .gitignore only - no executable code, schema, or test assertion changed - and every factual claim in the added prose was verified against the cited tests, Rust types, and checked-in schemas.
Testing
Baseline and final
cargo test --workspaceare green. The AGENTS.md coverage-boundary claim was exercised by mutating checked-in schemas rather than by reading the helpers: a 15-row matrix and an exhaustive 31-keyword sweep confirm property names,required, andadditionalPropertiesare caught while a driftedtypeor a dropped bound is not, except at the specific per-field tests the doc names; 17 constraint carriers turn out to be bound by no test, and every worked example in the new ARCHITECTURE.md section is in that list. Thefpsm-lease-ceiling-parityentry reproduces exactly, ending with a green workspace and a gateway still publishing the old ceiling. The .gitignore block was exercised with real files at real repo paths, before and after, showing four dump shapes ignored and three previously-swallowed source paths restored. No screenshot was possible - no browser runs in this sandbox (chromium missing libnspr4/libnss3, browser-harness symlinked to a macOS path) - so the markdown surface is captured as a rendered HTML file instead; separately, the "byte-identical to the supplied fleet v6 block" requirement is a provenance claim with no source to diff against, though the block's behavior is fully verified.Evidence: Parity mutation matrix: what the contract suite does and does not catch
--- A. keywords the parity helpers DO compare (names / required / additionalProperties) --- A1 property name renamed broker-request ChangeRequest.capability_id -> cap_id RED as documented caught by: ipc::tests::change_request_fields_match_request_schema_defs A2 required entry dropped broker-request ChangeRequest drops lease_seconds RED as documented A3 additionalProperties flipped broker-request ChangeRequest false -> true RED as documented --- B. property type: NOT compared by any parity helper (documented gap) --- B1 type drift, unasserted field broker-request lease_seconds integer -> string GREEN as documented B2 type drift, unbound carrier experiment change_request.parameters object -> string GREEN as documented B3 type drift, bound carrier broker-request ChangeRequest.parameters object -> string RED as documented caught by: ipc::tests::change_request_parameters_are_an_object_in_both --- C. constraints beyond type: caught only where a dedicated test binds that schema --- C1 protocol_version minimum sidecar.schema.json drops minimum RED as documented C2 hypothesis maxLength experiment.schema.json 500 -> 5000 RED as documented C3 lease ceiling (bound) experiment change_request.lease_seconds drops maximum RED as documented C4 lease floor (unbound) experiment change_request.lease_seconds drops minimum GREEN as documented C5 lease floor (unbound) broker-request ChangeRequest.lease_seconds drops minimum GREEN as documented C6 min_samples minimum experiment decision_bounds lone carrier drops minimum GREEN as documented C7 targets minItems sidecar.schema.json ProviderManifest.targets drops minItems GREEN as documented C8 capability_id pattern broker-request drops pattern GREEN as documented C9 capability_id minLength experiment drops minLength GREEN as documented worktree after matrix: 0 modified schema filesEvidence: Constraint carrier sweep: all 31 constraining keywords in schemas/, bound vs unbound
sidecar.schema.json protocol_version minimum BOUND tests::protocol_version_zero_is_rejected_like_the_schema sidecar.schema.json targets minItems unbound - experiment.schema.json change_request/lease_seconds maximum BOUND tests::lease_seconds_is_bounded_like_the_schema experiment.schema.json change_request/lease_seconds minimum unbound - experiment.schema.json decision_bounds/min_samples minimum unbound - unbound carriers: 17 of the swept constraints have no test asserting them - broker-request.schema.json ChangeRequest/capability_id.pattern - broker-request.schema.json ChangeRequest/lease_seconds.minimum - experiment.schema.json change_request/capability_id.minLength - experiment.schema.json change_request/lease_seconds.minimum - experiment.schema.json decision_bounds/min_samples.minimum - sidecar.schema.json targets.minItems (full 17-row list and all 31 sweep rows in the artifact file)Evidence: fpsm-lease-ceiling-parity: stale published ceiling survives a green suite
step 0 baseline MAX_LEASE_SECONDS = 300 experiment.schema maximum = 300 broker-request maximum = None gateway tools/list advertises= 300 suite = GREEN step 1 raise the constant alone (300 -> 600) suite = RED failing: tests::lease_seconds_is_bounded_like_the_schema step 2 bring the experiment schema along, as the failure asks suite = GREEN gateway tools/list advertises= 300 <-- stale, nothing failed reverted; worktree entries dirty: 0Evidence: Core-dump ignore rules exercised with real files, new block vs base block
### checked-in .gitignore at HEAD (fleet v6 block) PATH EXPECTED ACTUAL RESULT core ignored ignored ok (bare dump at repo root) crates/control-plane/core ignored ignored ok (dump from 'cargo test' cwd (package root)) sidecars/mock-provider/core.4711 ignored ignored ok (core.<pid> shape) apps/broker/core.broker.4711 ignored ignored ok (core.%e.%p shape) crates/control-plane/src/core/mod.rs tracked tracked ok (source dir named core/ must stay visible) crates/control-plane/src/core.rs tracked tracked ok (source file core.rs must stay visible) apps/broker/core.tuning.md tracked tracked ok (doc named core.<word> must stay visible) ### previous block at base 3f9a580 ("core" + "core.*"), for contrast crates/control-plane/src/core/mod.rs tracked ignored !! MISMATCH crates/control-plane/src/core.rs tracked ignored !! MISMATCH apps/broker/core.tuning.md tracked ignored !! MISMATCH worktree left dirty by this check: 0 entriesEvidence: AGENTS.md Repository rules rendered as GitHub-flavored Markdown, new rule highlighted
Evidence: Mutation matrix runner (reproducible)
Evidence: Constraint sweep script (reproducible)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 4 issues found → auto-fixed (5) ✅
.gitignore:41- Oldcore.*ignored every dotted dump name; the replacement requires a numeric field (**/core.[0-9]*,**/core.*.[0-9]*). A host whosekernel.core_patternhas no numeric component -core.%e(exe only) orcore.%h(hostname), both legal patterns - now produces a dump that is no longer ignored. By the block's own stated rationale that is a credential-exposure path, not bloat. Adding**/core.*back after the negation would restore the old breadth without weakening the!**/core/fix. Flagging rather than fixing because the block is supplied verbatim and frozen.AGENTS.md:18- The same normative rule now exists in three places:AGENTS.md:18-20, theassert_object_paritydoc comment (crates/contracts/src/lib.rs:492-503), and theassert_same_shapedoc comment (crates/contracts/src/test_support.rs:59-71). The two Rust copies are near word-for-word identical, and nothing fails if one drifts from the other or from AGENTS.md - a future edit to the rule will silently leave two stale copies asserting the old contract. Compounding it, the AGENTS.md bullet is ~450 words in three sentences averaging ~90 words each, against that file's own bar ("keep entries concise", "point to the authoritative file"); the threedocs/ARCHITECTURE.mdentries have the same density. Suggest: one authoritative statement plus the pointer todocs/ARCHITECTURE.md, and doc comments that reference it instead of restating it..gitignore:29- The comment credits**/with matching at any depth, but a gitignore pattern with no slash already matches its basename at every level - the previous barecoreline ignoredcrates/<pkg>/coretoo. The genuinely load-bearing addition is!**/core/on line 40, which un-ignores directories namedcore(src/core/, acrates/corepackage) that the old bare pattern silently swallowed. Noting only; the block is frozen byte-identical by ruling.crates/contracts/src/lib.rs:504-assert_object_parityandtest_support::assert_same_shapeimplement the same three-keyword comparison, differing only in taking&Valueplus a label vs&schemars::Schema. Duplicating the twelve-line doc comment onto both is what made the drift surface twice. Havingassert_object_paritydelegate to thetest_supporthelper (or exposing a&Valueentry point there) would leave one implementation and one place for the caveat.🔧 Fix: parameterize bind definition, restore per-constraint quantifier
1 warning still open:
AGENTS.md:20- The closing clause reads "blocked as it is in the first entry and merely pending in the second", immediately after naming "the deferred contract work list indocs/ARCHITECTURE.md". "Entry" is the word that list's items go by, and that list orders themfpsm-lease-ceiling-parity,fpsm-unbound-carrier-parity,fpsm-capid-guard(docs/ARCHITECTURE.md:105-107). Under that reading "the first entry" isfpsm-lease-ceiling-parity, which is not blocked - its deferred work is a binding test, writable today - so the sentence is false. The intended antecedent is the inline pair "bothfpsm-capid-guardandfpsm-unbound-carrier-parity" earlier in the same clause, where first = capid-guard = blocked. Prior wording ("blocked as it is forcapability_idand merely pending for the other") was unambiguous but paired a field against a class; naming the two classes outright - "blocked infpsm-capid-guardand merely pending infpsm-unbound-carrier-parity" - pairs class against class as ruled and removes the ordinal entirely.🔧 Fix: replace status ordinal with per-class attachment in pointer
3 infos still open:
AGENTS.md:20- The sentence lists twoadditionalPropertiesblind spots as if equivalent, but only one is real repo-wide. Forcapability_fields_match_capability_schema/manifest_fields_match_sidecar_schema,generated_schemas_match_checked_in_schemas(crates/contracts/src/lib.rs:609) already runsassert_object_parityoverCapabilityDescriptorvsCAPABILITY_SCHEMAandProviderManifestvsSIDECAR_SCHEMA(schema_cases, lib.rs:560-576), and that helper does compareadditionalProperties- so a generated side that drifts there fails the suite. By contrastresponse_variants_match_schema(crates/contracts/src/ipc.rs:326) is the only test that walks theBrokerResponseoneOf branches; the per-branchassert_same_shapetests only cover$defsentries, so a generated branch droppingdeny_unknown_fieldsreally does stay green everywhere. The clause "stays green at those sites" is literally true for both, but a reader closing gaps will write a redundant guard for capability/manifest and may not see that the response-variant hole is unbounded. Naming which of the two is covered elsewhere would fix it.docs/ARCHITECTURE.md:108-fpsm-capid-guardnamesChangeRequest::capability_idandProviderManifest::targets;fpsm-unbound-carrier-paritynames the lease floor,ChangeRequest::parameters, andDecisionBounds::min_samples. Sweeping the checked-in schemas for constraining keywords turns up further carriers neither entry mentions and no test binds:broker-request.schema.json.properties.owner(minLength),capability.schema.json.properties.id(pattern) and.properties.description(minLength),sidecar.schema.json.properties.id(pattern),metric-sample.schema.json.properties.errors(minimum),verdict.schema.json$defs.metric_summary.samplesand.total_errors(minimum),experiment.schema.jsonwarmup_samples/baseline_samples/candidate_samples(minimum) and$defs.decision_bounds.max_errors(minimum). The entries are class definitions and say "a further member", so nothing is false; but "Closing the class needs each such Rust type to carry its constraint" is not actionable without knowing the class is roughly a dozen fields rather than the two named. A member count, or a one-line note that the list is derivable by sweeping the schemas for constraining keywords, would make either entry closable.docs/ARCHITECTURE.md:106- Closingfpsm-lease-ceiling-paritymeans adding themaximumto$defs.ChangeRequest.lease_secondsinschemas/broker-request.schema.json. The moment that lands,lease_seconds_is_bounded_like_the_schema(crates/contracts/src/lib.rs:457) - which opens onlyEXPERIMENT_SCHEMA- stops binding every checked-in carrier of the ceiling, so the field that entry currently calls "compliant for its ceiling" (line 107) drops intofpsm-unbound-carrier-parityunless the test is extended in the same change. Neither entry states that coupling; the lease-ceiling entry says only "The deferred work is the binding test", which reads as one test rather than one test plus the schema edit that makes a second carrier exist. One clause noting that the ceiling test must gain the broker request schema alongside the schema edit would keep the two entries from trading a gap back and forth.🔧 Fix: document class derivability, pin what closes lease-ceiling parity
2 warnings still open:
docs/ARCHITECTURE.md:105- The new derivability line says class membership "is derivable by sweeping the checked-in schemas for constraining keywords". That sweep cannot reach thedeserialize_withhalf of the trigger, andfpsm-unbound-carrier-paritynames an instance the sweep misses.AGENTS.md:18states the trigger as two disjuncts - a checked-in constraint beyondtype, OR adeserialize_withvalidator, "which no checked-in schema can state". The validator lives on the Rust type (crates/contracts/src/lib.rs:90, the workspace's only one, onChangeRequest::parameters), and its checked-in counterpart is the bare declaredtype:parametersis{"type": "object"}inschemas/broker-request.schema.jsonandschemas/experiment.schema.json, carrying no constraining keyword. So a keyword sweep never surfaces it, yet line 108 namesChangeRequest::parametersas the class's second instance. A reader closing the class off this line closes it without the one member the entry works through by name. Remedy is one added clause covering the second disjunct's own sweep - the Rust types carrying adeserialize_withvalidator - which adds no count and no enumeration, so R7 is untouched, and keeps the checked-in-schema domain R30/R43 fixed for the general half.docs/ARCHITECTURE.md:107- R64 asked for the ambiguity to be resolved, and the applied wording resolves it into an exhaustive closure claim that no longer covers the entry's own second registered carrier. The entry registers two defects: the missingmaximuminschemas/broker-request.schema.json, and the gateway'stools/listinput schema "which no test compares against that constant in agreement". The clause now reads "Closing this is the schema edit together with the binding test, still not a reference from the gateway to the constant" - a closed list of two items, where the prior "The deferred work is the binding test, not a reference..." was the non-exhaustive contrast R64 describes. Neither item reaches the gateway:AGENTS.md:19defines binding as opening the checked-in file, and the gateway ceiling is inline Rust atapps/gateway/src/lib.rs:67with no reader ofMAX_LEASE_SECONDSanywhere inapps/gateway. The exclusion clause bars only the gateway reference remedy (R15's rejected fix), not a test comparing the copies, so the gateway leg is now closed by nothing named - and the entry's own stale-ceiling-with-green-suite scenario two sentences earlier survives both stated closure steps. Either the closure list needs the gateway comparison as a third item, or the clause needs to say explicitly which registered leg it is scoped to.🔧 Fix: extend class derivability sweep, add gateway closure item
2 warnings still open:
AGENTS.md:18- Thedeserialize_withdisjunct reads "for adeserialize_withvalidator, which no checked-in schema can state, that test binds each checked-in schema publishing it and asserts that field's declaredtypethere as the counterpart." The nearest antecedent for "it" is the validator, and the immediately preceding relative clause states that no checked-in schema can state it - so the set the rule quantifies over is empty and the disjunct requires nothing, which is the vacuity R24 exists to remove. The intended referent is the field (or its declaredtype): the pre-R56 wording was "that field's declaredtypein each checked-in schema publishing it", where "that field" preceded the pronoun; the R56 role fix moved the noun behind it and broke the chain. It also contradicts line 19 of the same file, wherechange_request_parameters_are_an_object_in_bothis said to bind "for fewer checked-in schemas than carry the one it asserts" - a claim that presupposes a non-empty publishing set - and contradicts the register, which namesChangeRequest::parametersas a class instance with two publishing schemas. Same byte-identical clause in all three copies:AGENTS.mdline 18, theassert_object_paritydoc comment, and theassert_same_shapedoc comment. Remedy is a referent, not a rule change: name the field in the slot the pronoun occupies. R24's substance and R43's checked-in-only domain both survive it; touching ruled text, so the captain should rule.docs/ARCHITECTURE.md:105- R65's second sweep landed, but the quantifier closing the same sentence undoes it: "membership is derivable by sweeping the checked-in schemas for constraining keywords and the Rust types for adeserialize_withvalidator ... then applying the entry's own criterion to each carrier either sweep turns up." This section fixes "carrier" to mean a checked-in schema that carries the constraint - "leaving the unbound carriers free to drift", "A lone carrier falls in the same way" introducing a constraint carried by one schema, and the class idfpsm-unbound-carrier-parityitself. The Rust-side sweep turns up Rust fields, which are not carriers in that sense, so under the section's own vocabulary the added sweep contributes nothing for the criterion to be applied to, andChangeRequest::parameters- the very member R65 was written to reach - is still underivable from this line. This is the shape R66a just resolved forbindby splitting verbs: one word carrying two referents inside one section with no rule reconciling them. Secondary defect in the same phrase: both entries are read per constraint, pinned by R33 forfpsm-unbound-carrier-parityand R44 forfpsm-capid-guard, while "each carrier" quantifies per schema, so the criterion is applied to the wrong kind of thing even for the first sweep. Remedy stays R7-clean - it changes what the criterion is applied to, adding no count and no enumeration - and is R30-clean, since the Rust-side domain is not widened. Touching text ruled at R63 and R65, so the captain should rule.🔧 Fix: name field referent, make constraint the sweep unit
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
cargo test --workspace(baseline and final, both green; run with CC/AR zig shims in target/ that this environment needs for libsqlite3-sys)cargo test -p fpsmaxxing-contractsx15 under a schema-mutation matrix (property rename, droppedrequiredentry,additionalPropertiesflip,typedrift on bound and unbound carriers, dropped bounds on bound and unbound carriers)Exhaustive constraint sweep: deleted each of 31 constraining keywords across all 7 files inschemas/one at a time, re-runningcargo test -p fpsmaxxing-contractsper deletion, to derive bound vs unbound carriersfpsm-lease-ceiling-paritytwo-step repro:MAX_LEASE_SECONDS300 -> 600 thenschemas/experiment.schema.jsonmaximum -> 600,cargo test --workspaceafter each, gateway inline ceiling read fromapps/gateway/src/lib.rs:67git check-ignoreover 7 real fixture files at real repo paths against HEAD's .gitignore and against the base 3f9a580 block in a throwaway repoRendered AGENTS.md throughnpx marked --gfmand inspected the Repository rules list markup to confirm the new three-sentence rule renders as one bulletgit status --porcelainafter every mutation run to confirm all schema and Rust mutations were reverted🔧 **Document** - 2 issues found → auto-fixed ✅
AGENTS.md:18- The operative rule reads "a dedicated test that binds each checked-in schema carrying that constraint" - the constraint rides on a participle attached to the object, so the predicate carries no "for" slot, which is the two-place form R57/R58 replaced one line later at :19 ("A test binds a schema for a constraint...") and at :20 ("bound for it by no test today"). R59a swept the register and line 19/20 and ruled the surviving participle forms "true-but-fragile ... Move both", but its domain never included line 18. The sentence is not false (its quantifier ranges over one triggering constraint, so participle and for-slot pick out the same thing), which is why this is left rather than fixed: it is ruled text, the remedy must move three copies together (AGENTS.md:18, crates/contracts/src/lib.rs assert_object_parity, crates/contracts/src/test_support.rs assert_same_shape) without disturbing R24's counterpart, R43's checked-in domain, R56's role split, or R67's "that field" referent, and whether R59a extends to the rule itself is the captain's call.crates/contracts/src/test_support.rs:4- The module doc says "Every schema-sync test module compares them the same way, so the comparison lives here once rather than being re-derived per module." Both clauses are contradicted by what this branch newly documents in AGENTS.md:20: crates/contracts/src/lib.rs imports properties/string_set/serialized_fields but defines its own assert_object_parity, a re-derivation of assert_same_shape, and capability_fields_match_capability_schema and manifest_fields_match_sidecar_schema compare a serialized sample's field set with no generated schema at all. Not fixed here because the only accurate rewording is a description of the helper duplication that R21 approved as-is, filed outside the repo, and ruled the branch silent about; deciding whether an honest softening of this sentence breaches that silence is a captain call.🔧 Fix: give bind predicate a for-slot, drop register overclaim
✅ Re-checked - no issues remain.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.