refactor(text): one C0/DEL predicate (#1253); the dangling-citation detector can now fail (#1235) - #390
Merged
Merged
Conversation
… #1253) `ord(ch) < 0x20 or ord(ch) == 0x7F` was written out seven times across six files. Every copy agreed, so nothing was mis-screened; the cost was future-tense and is the one #1239 named -- a later hardening applied to one copy silently does not apply to the rest, and nothing reports the omission. THIS SHARES THE PREDICATE, NOT THE ACTION, and that is the design rather than an implementation detail. #1239 explicitly ruled out "collapsing the call sites into one helper with a flag", because the differing wrappers are appropriate: a raise suits a path context, a bool suits a filter, and the exception differs by layer (WiringError in config, a PHI-safe NegativeAckError in FHIR). So every call site keeps its own refusal and its own message; only the TEST moved. A flag parameter would have re-created the coupling this item exists to remove, one indirection further away. TWO ACTIONS ARE PRESERVED, and one must never be "simplified" into the other. Six sites REJECT. transports/rest.py STRIPS, on a message-derived header VALUE, and that is defensible rather than a second instance of the mutation pattern the owner ruled against in #1238: that ruling turns on basename() converting a path into a valid-but-DIFFERENT target, handing an attacker a real file. A header value has no such property -- removing CR/LF cannot redirect a request anywhere -- and rest.py already REJECTS a header NAME failing its RFC 7230 token check. Name-rejected, value-stripped. parsing/sniff.py is deliberately NOT folded in: it tests the same code points but is byte-wise rather than character-wise and subtracts an allowlist, because a text sniffer must tolerate tab, CR and LF. Folding it in would change its behaviour. TESTS. #1239 asked for proof that "the two predicates agree across a shared character corpus". With one predicate that obligation becomes a CHARACTERISATION test: the caught set is pinned over U+0000-U+02FF against an independently-written definition, the boundaries are pinned at each edge (0x1F in, 0x20 out, 0x7E out, 0x7F in, 0x80 out), and C1 plus the Unicode separators are pinned as deliberately NOT caught -- widening this is now a behaviour change at seven sites at once, which is the leverage and also the risk. 47 new tests; 256 existing tests across the six touched modules pass unchanged. BASE VERIFIED BEFORE BUILDING, because #1242's limb 4 was refused on exactly this ground: all six files are byte-identical between this branch and origin/main, and the three commits this branch is behind touch BACKLOG.md, scripts/asvs/apply.py and .claude/settings.json only. That is the discriminator -- apply.py differs here, these do not.
…ite runs it (BACKLOG #1235) #1235 was correctly ruled PARTIAL after PR #385: the detector shipped, but it was wired into nothing and could not fail. Two of that ruling's three measurements are addressed here. The third -- the written RULE -- is a convention change and is not mine to make. IT COULD NOT FAIL, AND THAT IS THE DEFECT THIS TOOL EXISTS TO CATCH ELSEWHERE. `--fail` was opt-in and nothing passed it, so a planted dangling citation was reported correctly AND the process exited 0. A checker that cannot fail is not a check. The default is now fail-closed with `--advisory` as the escape; flipping it cost nothing, because repo-wide the script was referenced by exactly two lines, both inside its own unit test. THE EXIT CODE KEYS ON THE LIVE SHAPE, NOT THE HIT COUNT. A number at or below the allocator's floor can never be issued, and a PR/issue/foreign-repo reference is not a backlog citation at all. Both are still REPORTED for a human to read, and neither reds the tree: failing on them would red it today for hits that are correct, and a gate that cries wolf gets switched off. THE SUITE IS NOW THE CALLER. A test walks the real docs/ tree and asserts no citation names a still-issuable number. Measured on origin/main before writing it: 312 markdown files, 6 above-floor hits, ALL foreign-repo shaped, ZERO live-shape -- so the gate passes today on merit rather than by being lenient. PROVED IT CAN FAIL, which is the whole point of the item. A live-shape citation was planted in docs/, the test ran, and it failed naming the file, line and number; the plant was removed in a finally block and the run without it passes. A gate whose failing arm has never been observed is the state this item is about. The population walk is pinned too (>200 files asserted, not merely printed): a walk that collapses to nothing would otherwise report clean forever.
wshallwshall
enabled auto-merge (squash)
August 14, 2026 14:34
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.
Two commits.
#1253 is behaviour-preserving BY CONSTRUCTION, not by suite result
The author proved it structurally and asked to be checked rather than believed. Re-verified by the Lander:
No changed condition, no changed message, no changed exception type, no reordering.
The two actions stay two, which is the part a "simplification" would have destroyed:
rest.pystill strips a header value; the other six still reject. Collapsing those into one behaviour would have looked like tidying and changed what the engine does.That is checkable in one command rather than resting on 12,398 passing tests -- which is why it is stated this way. A refactor collapsing seven copies into one is exactly where a behaviour change hides in a diff that reads as cleanup.
#1235 closes the limb the PARTIAL ruling named
The detector previously exited 0 even when it fired, because
--failwas opt-in and passed by nothing, and it was wired into nothing -- two repo-wide references, both in its own test. It now fails, and the suite runs it.With #389's rule (CLAUDE.md section 5 + LEDGER-GATE.md) both of #1235's limbs discharge -- the detector that can fail, and the rule it enforces.
Verification
PYTEST_EXITcaptured deliberately -- the wrapper's exit code is not pytest's, and exit codes have misled three distinct ways today: atailprocess's code, a killed run's 127, and a margin gate under a check named for tests.+2 over the previous green, exactly the two tests #1235 added -- the delta accounts for itself rather than merely having moved.
Scope