Skip to content

Commit c856c33

Browse files
derek73claude
andcommitted
fix(differential,docs): the review findings on the fields ban
One real coverage loss and two falsified sentences, all from the focused review of #457. test_main_compares_the_v2_surface_from_baseline_2_0 stopped catching its own mutation. Its diff IS the ambiguity-only one, and the blanket `fields = ["family"]` my fixture sweep gave it refuses that shape, so the ZZZ rule stayed inert even with name narrowing disabled: measured, that mutant failed 11 tests on master and 10 here. The fixture now declares `_ambiguities`, the role its diff actually moves, and the mutant is back to 11. This is exactly the weakening the sweep's own commit message claimed not to have caused, in the one fixture whose diff is not {family}. "127 of 127" is wrong at both 2.x baselines, including the default. _RULE_FIELDS has eight members, and from 2.0 on main unions `_ambiguities` into the same diff set, so the banned shape claims 255 there and 127 below. A live count with no date, quoted in an error message a contributor reads while a 2.1.0 gate is failing. Both figures now, in the message, the README and decisions.md. validate_rules' docstring said the fields-without-name_regex check was "the family's sharpest example: no other malformed shape can widen invisibly". #456 is the counterexample, and worse -- over_declared_rules skips a fieldless rule, so nothing narrows it either. The paragraph names both now. Also: `dormant` gains a pin, since disabling this check otherwise failed exactly one test where #451's fails three; the README's "It may narrow further with `fields`" is folded into the required list rather than left contradicting the sentence eight lines below it; over_declared_rules' and classify's docstrings stop presenting the banned shape as legal; the decisions.md entry moves after the #452 entry it cites, restoring date order; and the inversion note moves to the assertion that was actually inverted, where a `git log -L` finds it. 5350 passed; gates 229/0, 194/0, 102/0. Refs #456 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 1fff06d commit c856c33

4 files changed

Lines changed: 51 additions & 21 deletions

File tree

docs/design/decisions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -635,8 +635,8 @@ Decisions that landed:
635635
- 2026-08-27 #451 — past-tense counts stay in a ledger comment; live ones do not. "It claimed 25 until #372" is a fixed fact. "Four corpus names, and the prose fits all four" was written when the rule explained four and still read that way at fourteen, because nothing recomputes it. Where a count is owned by another comment, point at the owner rather than restating it — two copies means one gets updated and the other does not.
636636
- 2026-08-28 #452 — a rule's declared `fields` must EQUAL the union of the diffs it explains, checked by `compare.py` at the end of every run and failing it like an unexplained diff. The statement is exact rather than heuristic, which is what makes it cheap: `classify()` already requires `declared >= union` for the rule to match the names it matches, so the only possible error is the other direction, and the union is simultaneously the check and the repair. Narrowing to it cannot orphan a name, since every name a rule explains contributed to it. Measured before landing: 3 of 67 explaining rules over-declared at 1.4.0, 5 of 58 at 2.0.0, 6 of 51 at 2.1.0 — all fourteen narrowed first, so the check was silent the day it arrived.
637637
- 2026-08-28 #452 — over-declaration is BASELINE-RELATIVE, and each ledger is measured on its own run rather than copied. `fix(#296) a lone post-comma credential is a suffix` declared `{family, given, suffix, title}` in all three ledgers: exactly exercised at 1.4.0, where v1 reads the pre-comma word as `first` and all four roles move, and over-declared at both 2.x baselines, where the same behaviour moves only `{suffix, title}` — which is all it declares in those two files now. A reader comparing the three sees one rule with TWO different field lists and should read that as correct rather than as drift. Two and not three, measured: the 2.x pair narrowed to the same set, so the split is 1.4.0 against both 2.x ledgers, not one list per file.
638-
- 2026-08-29 #456 — a rule carrying `name_regex` and no `fields` is REJECTED, the symmetric twin of #451's ban. It narrows by name and by nothing else, so on any name its regex reaches it claims every diff shape there is — measured, 127 of 127. #452 is what made it urgent rather than merely untidy: `over_declared_rules` skips a rule with no `fields`, correctly, since one declaring no roles cannot over-declare them — so deleting the line is the cheapest way to silence an OVER-DECLARED failure AND the most permissive thing that can be done to the rule, which is the #372 failure mode reached by following a gate error message. Free to enforce on #451's own terms: measured, 0 of 179 rules across the three ledgers had the shape. After it, every rule carries both keys, and the three rejections in `validate_rules` read as one rule — narrow by name and by role, or it is not a rule. NO escape hatch for a "genuinely unbounded" rule, declined until one appears, same call and same reason as #452's.
639638
- 2026-08-28 #452 — NO escape hatch, decided rather than deferred. `dormant` already covers the explains-nothing case in both directions, a rule with no `fields` has nothing to over-declare, and the ledger's own doctrine — "a rule that pre-claims shapes it has never seen is the #372 failure mode" — makes strictness the existing principle. Accepted cost, stated so it is not rediscovered as a surprise: the first rule that genuinely needs a wider declaration has to argue for a key the way `dormant` was argued for in #373, rather than reaching for one that already exists.
639+
- 2026-08-29 #456 — a rule carrying `name_regex` and no `fields` is REJECTED, the symmetric twin of #451's ban. It narrows by name and by nothing else, so on any name its regex reaches it claims every diff shape there is — measured, 255 shapes from baseline 2.0 on and 127 below it. #452 is what made it urgent rather than merely untidy: `over_declared_rules` skips a rule with no `fields`, correctly, since one declaring no roles cannot over-declare them — so deleting the line is the cheapest way to silence an OVER-DECLARED failure AND the most permissive thing that can be done to the rule, which is the #372 failure mode reached by following a gate error message. Free to enforce on #451's own terms: measured, 0 of 179 rules across the three ledgers had the shape. After it, every rule carries both keys, and the three rejections in `validate_rules` read as one rule — narrow by name and by role, or it is not a rule. NO escape hatch for a "genuinely unbounded" rule, declined until one appears, same call and same reason as #452's.
640640

641641
Found rather than decided, and worth as much:
642642

tests/v2/test_differential.py

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,8 @@ def test_a_rule_with_a_regex_and_no_fields_is_rejected() -> None:
267267
268268
A rule with no `fields` narrows by name and by nothing else, so on
269269
any name its regex reaches it claims every diff shape there is --
270-
measured, 127 of 127. #452 made that worse than it looks by giving
270+
measured, every one of the 255 shapes
271+
eight roles allow at a 2.x baseline. #452 made that worse than it looks by giving
271272
the shape a second job: over_declared_rules skips a rule with no
272273
`fields`, correctly, since one declaring no roles cannot
273274
over-declare them. So deleting the `fields` line is the response to
@@ -276,23 +277,38 @@ def test_a_rule_with_a_regex_and_no_fields_is_rejected() -> None:
276277
277278
Free to enforce, on the same terms as #451's: no rule in any
278279
shipped ledger has the shape, so the ban costs no migration.
280+
281+
This assertion is an INVERSION. Its other half pinned the
282+
regex-only shape as LEGAL when #451 landed -- "the neighbouring
283+
shapes #451 did NOT retire" -- and #456 retired it. Inverted rather
284+
than deleted, so the change of status is visible to a `git log -L`
285+
on the assertion rather than vanishing with the test.
279286
"""
280287
with pytest.raises(SystemExit, match="no 'fields'"):
281288
compare.validate_rules(
282289
[{"issue": "fix(x) a rule with no role narrowing",
283290
"name_regex": "Smith"}],
284291
"test_ledger.toml")
292+
# `dormant` buys no exemption here either, for the reason it buys
293+
# none from #451's ban: it is a claim about today's corpus, not a
294+
# bound on reach, and a regex-only rule sits at every diff shape
295+
# the moment one matching name arrives -- at which point its
296+
# dormancy claim is false too. Pinned because disabling the check
297+
# otherwise fails exactly one test (#457 review).
298+
with pytest.raises(SystemExit, match="no 'fields'"):
299+
compare.validate_rules(
300+
[{"issue": "fix(x) idle and unbounded",
301+
"name_regex": "Smith", "dormant": "a reason nobody faults"}],
302+
"test_ledger.toml")
285303

286304

287305
def test_a_rule_carrying_both_keys_is_the_only_legal_shape() -> None:
288306
"""What is left after the three bans, and there is exactly one.
289307
290308
`validate_rules` rejects neither key (it would match every diff),
291309
`fields` without `name_regex` (#451, no name narrowing), and
292-
`name_regex` without `fields` (#456, no role narrowing). This
293-
pinned the regex-only shape as LEGAL when #451 landed; #456
294-
retired it, and the test is inverted rather than deleted so the
295-
change of status is visible in the history rather than silent.
310+
`name_regex` without `fields` (#456, no role narrowing). One
311+
shape survives all three, and this is it.
296312
"""
297313
compare.validate_rules(
298314
[{"issue": "x", "name_regex": "Smith", "fields": ["given"]}],
@@ -865,7 +881,13 @@ def test_main_compares_the_v2_surface_from_baseline_2_0(
865881
v2 = {**_SAME_V2, "_ambiguities": ["SEGMENTATION"]}
866882
code, out = _run_main(
867883
tmp_path, monkeypatch,
868-
'[[change]]\nissue = "unrelated"\nname_regex = "ZZZ"\nfields = ["family"]\n',
884+
# `_ambiguities`, not `family`: this test's diff IS the
885+
# ambiguity-only one, and a `family` declaration refuses that
886+
# shape -- which left the rule inert even with name narrowing
887+
# disabled, costing this test the mutation it was written to
888+
# catch (#457 review). Declare the role the diff moves.
889+
'[[change]]\nissue = "unrelated"\nname_regex = "ZZZ"\n'
890+
'fields = ["_ambiguities"]\n',
869891
_SAME_FACADE, baseline="2.0.0", baseline_v2=v2)
870892
assert code == 1, "an ambiguity-only regression must not exit 0"
871893
assert "UNEXPLAINED 'John Smith'" in out

tools/differential/README.md

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -300,17 +300,16 @@ history rather than being edited into the next one's.
300300
Each `[[change]]` entry needs `issue` (a short label, ideally an
301301
issue number or `fix(<slug>)` matching a `tests/v2/cases.py`
302302
classification) and `name_regex` (searched against the raw input
303-
string). It may narrow further with `fields` (the diffing rule matches
304-
only if the observed diff fields are a subset of this list), which
305-
since #452 must also name EXACTLY the roles that rule's own diffs move
306-
-- see below. Keep both as tight as the actual diff allows -- a loose
303+
string) AND `fields` (the diffing rule matches only if the observed
304+
diff fields are a subset of this list), which since #452 must also
305+
name EXACTLY the roles that rule's own diffs move -- see below. Keep both as tight as the actual diff allows -- a loose
307306
rule can mask a real regression. **Both keys are REQUIRED**, and each
308307
ban has its own issue: `validate_rules` rejects a rule carrying
309308
neither (it would match every diff), one carrying `fields` and no
310309
`name_regex` (#451 -- no name narrowing, so it claims every name whose
311310
diff fits its roles), and one carrying `name_regex` and no `fields`
312311
(#456 -- no role narrowing, so on any name its regex reaches it claims
313-
every diff shape there is, measured 127 of 127). The three are one
312+
every diff shape there is, measured, 255 of them from baseline 2.0 on and 127 below it). The three are one
314313
rule with one reason: a rule narrows by name AND by role, or it is not
315314
a rule. Note the two bans are each other's obvious wrong answer --
316315
deleting `fields` to silence an over-declaration failure lands on

tools/differential/compare.py

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -390,9 +390,13 @@ def validate_rules(rules: list[dict[str, object]], ledger: str) -> None:
390390
buy a precise message rather than safety; they are here because the
391391
family is easier to reason about whole than split by direction.
392392
393-
The `fields`-without-`name_regex` check belongs to the dangerous
394-
direction, and it is the family's sharpest example: no other
395-
malformed shape can widen invisibly. #451 is the rule that lived
393+
Two checks belong to the dangerous direction and are the family's
394+
sharpest examples, one per missing key -- #451 for a `fields` with
395+
no `name_regex`, #456 for a `name_regex` with no `fields`. An
396+
earlier version of this paragraph said the first was the only shape
397+
that could widen invisibly; #456 falsified that, and worse, since
398+
over_declared_rules skips a fieldless rule so nothing narrows it
399+
either. #451 is the rule that lived
396400
it -- no name narrowing, so it claimed every name whose diff fit
397401
its `fields`, and _CORPUS_CLAIMS (the guard tracking each rule's
398402
reach) recorded that reach as the whole corpus from the start, so
@@ -540,8 +544,10 @@ def validate_rules(rules: list[dict[str, object]], ledger: str) -> None:
540544
raise SystemExit(
541545
f"{where} has 'name_regex' but no 'fields' (#456). It "
542546
f"narrows by name and by nothing else, so on any name "
543-
f"its regex reaches it claims EVERY diff shape there is "
544-
f"-- 127 of them. #452 makes that worse than it looks: "
547+
f"its regex reaches it claims EVERY diff shape there is -- "
548+
f"255 of them from baseline 2.0 on, where `_ambiguities` "
549+
f"joins the seven roles, and 127 below it. #452 makes "
550+
f"that worse than it looks: "
545551
f"over_declared_rules skips a rule with no 'fields', "
546552
f"correctly, since one declaring no roles cannot "
547553
f"over-declare them -- so deleting the line is the "
@@ -691,8 +697,9 @@ def classify(name: str, diff_fields: set[str],
691697
rules -- so a new entry's blast radius is exactly the set of names
692698
it captures, independent of rule order.
693699
694-
An exclusion narrows by `name_regex` and optionally by `fields`,
695-
exactly as a rule does. Without `fields` it refuses any diff on a
700+
An exclusion narrows by `name_regex` and optionally by `fields`.
701+
The optionality is the exclusion's alone since #456: a RULE must
702+
now carry both. Without `fields` it refuses any diff on a
696703
matching name; with them it refuses only the reading it names, so a
697704
name whose parens mark a nickname to one rule and a suffix to
698705
another stays classifiable on the reading the exclusion is not
@@ -840,8 +847,10 @@ def over_declared_rules(
840847
can have one, and the test that pins this skip uses exactly that
841848
input, because the empty-union input cannot discriminate.) A rule
842849
with no `fields` declares no roles and so has nothing to
843-
over-declare; one with `fields` and no `name_regex` cannot exist
844-
since #451. And a rule that explained nothing is dormant_rules'
850+
over-declare -- and cannot exist since #456, which banned that
851+
shape precisely because this skip made deleting `fields` the
852+
cheapest way to silence the check. One with `fields` and no
853+
`name_regex` cannot exist since #451. And a rule that explained nothing is dormant_rules'
845854
too, which is the third `continue` below.
846855
847856
What this does NOT bound is a diff shape no single name produced.

0 commit comments

Comments
 (0)