Repository navigation
Pin the canonicalization depth silence, correct OQ-007's account of it - #124
Merged
Merged
Conversation
OQ-007 already named excess nesting as one of two remaining silent throws and said the two "suppress the document-id signal identically". That understates this one badly. assertBoundedDepth runs at the top of resolveContentReferences, and document-verdict.ts wraps that call, resolveAssetReferences and resolveSemanticReferences in one try whose catch swallows CanonicalizationError. Two further passes are gated on contentIds, which the throw leaves null, and the id recompute trips its own copy of the bound. Seventeen codes go silent, measured one document per code, headed by CDX-E-ID-COLLISION -- an Error in every state -- and CDX-E-DOCUMENT-ID-MISMATCH, so a sufficiently deep document with a fabricated manifest.id reports nothing at all. That makes it an integrity suppression rather than a reporting gap. The negative control places the boundary: the pre-canonicalization scans and the raw-byte hash still fire at any depth, so the silence is exactly the canon-gated passes and "a deep document loads clean" holds only of one carrying nothing those scans catch. Three test-document-verdict cases pin it -- the hashed anchor position, the out-of-hash annotation position, and the id collision, that last because documentVerdict deliberately orders the reference pass before the id recompute so a duplicate id surfaces as its own finding, and past the bound that ordering buys nothing. Each searches for the boundary rather than hard-coding it, so the pin stays tied to the constant; each carries a floor, the defect being reported below the bound before its absence above means anything; and each carries a live witness -- a non-NFC string scanned by an iterative walk with no depth guard -- that still fires above the bound. Without the witness, "reports nothing" is equally satisfied by a reader that never read the part, and the assertion cannot tell a silence from a failure to load. An unknown block type does not serve: classifyContent carries its own bound over block levels and goes silent around 256 blockquotes. One arm lands a document at exactly the bound. A blockquote contributes two containers, so the chain steps in twos and straddles the bound without landing on it, leaving the guard's comparison untested on the one value separating > from >=; tightening it by one was measured to pass every gate in the repository. An empty marks array shifts the parity by one container and closes it. Mutation-verified: disabling the depth bound fails all three, silencing the witness fails all three, and tightening the guard by one fails the arm written for it. No suite artifact changes. errors.json untouched at v0.14/86, spec/ untouched, suiteVersion held at 0.2.3 -- the suite a claimant executes is unchanged at 209 vectors + 157 fixtures, and every prior bump was driven by an added vector, fixture or code. Two of #114's four items are withdrawn and a third reclassified. Item 1 stays withdrawn to #121. Item 4 goes to #122: three drafts were each measurably wrong, and the schema's silence is not the specification's permission. Item 3 was drafted as an open question about whether §4.3.2's number and text arms should share a scope; it is a defect and goes to #123 -- §4.3.2 scopes the number rule to hashed content twice and the published summary reads "A hashed JSON number" with an empty note, yet firstNonRepresentableNumber walks raw and false-REJECTs a conformant crdt payload. Refs #114, #121, #122, #123
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
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.
Closes the reportable half of #114. Two of its four items are withdrawn and a third is reclassified — the detail is below, because what didn't land is most of the value here.
What lands
OQ-007 corrected. It already named excess nesting as one of two remaining silent throws, and said the two "suppress the document-id signal identically". That understates this one.
assertBoundedDepthruns at the top ofresolveContentReferences, anddocument-verdict.tswraps that call,resolveAssetReferencesandresolveSemanticReferencesin onetrywhosecatchswallowsCanonicalizationError. Two further passes are gated oncontentIds, which the throw leaves null, and the id recompute trips its own copy of the bound. Seventeen codes go silent, measured one document per code, by four distinct mechanisms — headed byCDX-E-ID-COLLISION, an Error in every state, andCDX-E-DOCUMENT-ID-MISMATCH. A sufficiently deep document with a fabricatedmanifest.idreports nothing at all, which makes this an integrity suppression rather than a reporting gap.The negative control places the boundary: the pre-canonicalization scans and the raw-byte hash still fire at any depth. So the silence is exactly the canon-gated passes, and "a deep document loads clean" is true only of one carrying nothing those scans catch. The over-broad version of that sentence was in an earlier draft and is the reason the control exists.
Three
test-document-verdictcases pin it — the hashed anchor position, the out-of-hash annotation position, and the id collision. The last earns its own case becausedocumentVerdictdeliberately orders the reference pass before the id recompute so a duplicate id surfaces as its own finding; past the bound that ordering buys nothing, and a REJECT reads as clean.Each test carries three properties:
classifyContentcarries its own bound over block levels and goes silent around 256 blockquotes, so a test resting on it would pin a claim false one bound further out.One arm lands a document at exactly the bound. A blockquote contributes two containers, so the chain steps in twos and straddles the bound without landing on it — leaving the guard's comparison untested on the one value separating
>from>=. Tightening it by one was measured to pass every gate in the repository. An emptymarksarray shifts the parity by a single container and closes it.No suite artifact changes
conformance/errors.jsonuntouched at v0.14 / 86 codes,spec/untouched,suiteVersionheld at 0.2.3. The suite a claimant executes is unchanged at 209 vectors + 157 fixtures = 366 checks, and every prior bump was driven by an added vector, fixture or defect code — 0.2.2→0.2.3 is the direct precedent, having shipped four harness cases plus a new gate and an oracle selftest while attributing the bump to one fixture.What was withdrawn, and why
Item 1 → #121. The concatenation scope fix, unchanged: it scoped four walks where six model the merge, so canon REJECTS a duplicate id the document verdict reports as IGNORE, and the Python oracle shares the wrong model so no gate sees it.
Item 4 → #122. Stating that a text leaf node is admissible as a root-level block. Three drafts, each measurably wrong and each differently: "permitted in any array that holds block content" contradicts §7.2 rules 3, 4, 5, 7, 8 and 9; the schema-derived enumeration contradicts §7.2 rules 6 and 11 ("MUST be block-level content") and §4.12.1, which makes text-in-
tableCellan OPTIONAL shorthand that SHOULD NOT be used; and that enumeration is under-inclusive against fourteen extension-schema arrays resolving to the same unconstrained$defs/block— one of which is the array #121 turns on, so the closure pre-judges #121 by omission. The through-line: the schema's silence is not the specification's permission. Every draft derived a normative claim from whatcontent.schema.jsonfails to constrain, and each time §7.2 or a section-level rule constrained it anyway.Item 3 → #123, reclassified. Drafted as an open question about whether §4.3.2's number and text arms should share a scope. It is a defect. §4.3.2 item 1 scopes the number rule to "hashed content" twice, and
CDX-E-PART-NUMBER-NON-REPRESENTABLEwas published reading "A hashed JSON number" with an empty note — yetfirstNonRepresentableNumberwalks raw and false-REJECTs a conformant document whosecrdtpayload holds a large integer. Narrowing it would align the implementation with what was published, not flip an expectation, so the stated reason for recording rather than fixing did not survive its own citation. The draft entry also inverted §4.3.2's own words, claiming NFC is "a property of rendered content" where the clause says "NFC is a property of the stored bytes".Review
Two
/review-fixrounds, nine agents in isolated worktrees. Both rounds found real defects, and every one was in work written earlier in the same session.Round 1 (six lenses) is why this PR is three files instead of seven: it found the §3.1 contradiction, the 17-vs-5 miscount, and the over-broad "reported clean outright" claim. Round 2's verifiers were invalidated — I edited the tree while they were running, which is my error and breaks the freeze the loop depends on. A third round was refused by the review-cycle guard as a fix-my-fix loop, which is a fair reading: each round found a genuine defect in the previous round's fix, so each looked justified on its own.
What that leaves: every claim in this PR was measured directly, with a negative control, and the tests are mutation-verified against the specific mutants named above — including the off-by-one that previously survived every gate. Nothing outstanding is known.
Gates
All 29 green, plus
npm ci,npm audit --audit-level=high(0 vulnerabilities),npx tsc --noEmit,npm test, andgenerate:templateinto a fresh directory. 366/366 (209 vectors + 157 fixtures).test:document-verdictat 79 passed.