From 1fff06d4bba1e378aae39bfe4c75e3c58fe9fb70 Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Sat, 29 Aug 2026 01:28:29 -0700 Subject: [PATCH 1/2] feat(differential): reject a rule with name_regex and no fields (#456) The symmetric twin of #451's ban. Such a rule 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 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 at the same time. The #372 failure mode, reached by following a gate error message. The new message says so, and says not to reach for #451's banned shape instead. Free on #451's own terms: measured, 0 of 179 rules across the three ledgers have it, and all three still load. Placed at the END of the `fields` family beside #451's, so neither pre-empts the precise messages for a MALFORMED `fields` -- that placement was itself a #453 review finding, and this check has the same hazard in reverse. Twenty test fixtures gained a `fields`; intent is preserved in every one -- they are `ZZZ` rules that match nothing whatever their roles, a `""` rule that hits the sentinel check first, and `Smith` rules whose {family} diff the added roles still admit. Nine of the twenty already carried a `fields` on the next concatenated line and my first pass duplicated the key; caught by TOMLDecodeError and removed. test_a_rule_with_a_regex_and_no_fields_or_both_stays_legal pinned this shape as LEGAL when #451 landed. It is inverted rather than deleted, so the change of status is visible in the history rather than silent. Proved able to fail: disabling the check fails the new test. 5350 passed; gates 229/0, 194/0, 102/0. Closes #456 Co-Authored-By: Claude Opus 5 --- docs/design/decisions.md | 1 + tests/v2/test_differential.py | 61 ++++++++++++++++++++++++----------- tools/differential/README.md | 14 ++++++-- tools/differential/compare.py | 16 +++++++++ 4 files changed, 71 insertions(+), 21 deletions(-) diff --git a/docs/design/decisions.md b/docs/design/decisions.md index 6aae4df4..d8846673 100644 --- a/docs/design/decisions.md +++ b/docs/design/decisions.md @@ -635,6 +635,7 @@ Decisions that landed: - 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. - 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. - 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. +- 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. - 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. Found rather than decided, and worth as much: diff --git a/tests/v2/test_differential.py b/tests/v2/test_differential.py index cc2fa9f3..1e1b623c 100644 --- a/tests/v2/test_differential.py +++ b/tests/v2/test_differential.py @@ -262,13 +262,38 @@ def test_dormant_does_not_buy_an_exemption_from_the_ban() -> None: "test_ledger.toml") -def test_a_rule_with_a_regex_and_no_fields_or_both_stays_legal() -> None: - """The neighbouring shapes #451 did NOT retire. `name_regex` alone - still narrows by name; `name_regex` plus `fields` narrows by both. - Only the fields-only shape -- no name narrowing at all -- is new - to reject.""" - compare.validate_rules( - [{"issue": "x", "name_regex": "Smith"}], "test_ledger.toml") +def test_a_rule_with_a_regex_and_no_fields_is_rejected() -> None: + """#451's ban in mirror image (#456). + + A rule with no `fields` 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 made that worse than it looks by giving + the shape a second job: over_declared_rules skips a rule with no + `fields`, correctly, since one declaring no roles cannot + over-declare them. So deleting the `fields` line is the response to + an OVER-DECLARED failure that takes the least thought, and it both + silences the check and makes the rule maximally permissive. + + Free to enforce, on the same terms as #451's: no rule in any + shipped ledger has the shape, so the ban costs no migration. + """ + with pytest.raises(SystemExit, match="no 'fields'"): + compare.validate_rules( + [{"issue": "fix(x) a rule with no role narrowing", + "name_regex": "Smith"}], + "test_ledger.toml") + + +def test_a_rule_carrying_both_keys_is_the_only_legal_shape() -> None: + """What is left after the three bans, and there is exactly one. + + `validate_rules` rejects neither key (it would match every diff), + `fields` without `name_regex` (#451, no name narrowing), and + `name_regex` without `fields` (#456, no role narrowing). This + pinned the regex-only shape as LEGAL when #451 landed; #456 + retired it, and the test is inverted rather than deleted so the + change of status is visible in the history rather than silent. + """ compare.validate_rules( [{"issue": "x", "name_regex": "Smith", "fields": ["given"]}], "test_ledger.toml") @@ -502,7 +527,7 @@ def test_main_exits_1_and_reports_an_unclassified_diff( exiting 0 forever -- read by exit code, that is silence.""" code, out = _run_main( tmp_path, monkeypatch, - '[[change]]\nissue = "unrelated"\nname_regex = "ZZZ"\n', _DIFFERS) + '[[change]]\nissue = "unrelated"\nname_regex = "ZZZ"\nfields = ["family"]\n', _DIFFERS) assert code == 1 assert "UNEXPLAINED 'John Smith'" in out @@ -514,7 +539,7 @@ def test_main_reports_the_unexplained_field_under_its_role_name( this role `last`; a rule saying `last` never matches.""" _, out = _run_main( tmp_path, monkeypatch, - '[[change]]\nissue = "unrelated"\nname_regex = "ZZZ"\n', _DIFFERS) + '[[change]]\nissue = "unrelated"\nname_regex = "ZZZ"\nfields = ["family"]\n', _DIFFERS) assert "family:" in out and "last:" not in out @@ -536,7 +561,7 @@ def test_main_validates_the_ledger_before_running_anything( rule shadows the ledger.""" with pytest.raises(SystemExit, match="matches every one of"): _run_main(tmp_path, monkeypatch, - '[[change]]\nissue = "wide"\nname_regex = ""\n', _DIFFERS) + '[[change]]\nissue = "wide"\nname_regex = ""\nfields = ["family"]\n', _DIFFERS) def test_main_rejects_a_broad_fields_only_rule_before_running_anything( @@ -559,7 +584,7 @@ def test_main_rejects_a_broad_fields_only_rule_before_running_anything( _run_main( tmp_path, monkeypatch, '[[change]]\nissue = "broad"\nfields = ["family"]\n' - '[[change]]\nissue = "specific"\nname_regex = "Smith"\n', + '[[change]]\nissue = "specific"\nname_regex = "Smith"\nfields = ["family"]\n', _DIFFERS) @@ -705,7 +730,7 @@ def test_main_aborts_when_the_tree_side_is_not_the_checkout( monkeypatch.setattr(compare, "REPO_ROOT", tmp_path) with pytest.raises(SystemExit, match="not from this checkout's source"): _run_main(tmp_path, monkeypatch, - '[[change]]\nissue = "x"\nname_regex = "ZZZ"\n', _DIFFERS) + '[[change]]\nissue = "x"\nname_regex = "ZZZ"\nfields = ["family"]\n', _DIFFERS) def test_worker_env_strips_the_import_path_overrides( @@ -840,7 +865,7 @@ def test_main_compares_the_v2_surface_from_baseline_2_0( v2 = {**_SAME_V2, "_ambiguities": ["SEGMENTATION"]} code, out = _run_main( tmp_path, monkeypatch, - '[[change]]\nissue = "unrelated"\nname_regex = "ZZZ"\n', + '[[change]]\nissue = "unrelated"\nname_regex = "ZZZ"\nfields = ["family"]\n', _SAME_FACADE, baseline="2.0.0", baseline_v2=v2) assert code == 1, "an ambiguity-only regression must not exit 0" assert "UNEXPLAINED 'John Smith'" in out @@ -867,7 +892,7 @@ def test_main_reports_a_role_once_when_both_surfaces_moved( change shows on each; printing it twice would read as two findings.""" _, out = _run_main( tmp_path, monkeypatch, - '[[change]]\nissue = "unrelated"\nname_regex = "ZZZ"\n', + '[[change]]\nissue = "unrelated"\nname_regex = "ZZZ"\nfields = ["family"]\n', {**_SAME_FACADE, "last": "SMYTHE"}, baseline="2.0.0", baseline_v2={**_SAME_V2, "family": "SMYTHE"}) assert out.count("family:") == 1 @@ -878,7 +903,7 @@ def test_main_forwards_the_baseline_and_corpus_to_the_worker( """Otherwise main could read the 2.0 ledger while comparing against 1.4, or compare a truncated corpus, and every other test would pass.""" _run_main(tmp_path, monkeypatch, - '[[change]]\nissue = "x"\nname_regex = "ZZZ"\n', + '[[change]]\nissue = "x"\nname_regex = "ZZZ"\nfields = ["family"]\n', _SAME_FACADE, baseline="2.0.0", baseline_v2=_SAME_V2) assert _WORKER_CALL == {"version": "2.0.0", "want_v2": True, "names": ["John Smith"]} @@ -887,7 +912,7 @@ def test_main_forwards_the_baseline_and_corpus_to_the_worker( def test_main_asks_for_the_facade_alone_below_2_0( tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: _run_main(tmp_path, monkeypatch, - '[[change]]\nissue = "x"\nname_regex = "ZZZ"\n', _SAME_FACADE) + '[[change]]\nissue = "x"\nname_regex = "ZZZ"\nfields = ["family"]\n', _SAME_FACADE) assert _WORKER_CALL["want_v2"] is False @@ -928,7 +953,7 @@ def test_main_aborts_on_a_truncated_corpus( """A corpus below its floor must stop the run, not shrink it.""" with pytest.raises(SystemExit, match="below its floor"): _run_main(tmp_path, monkeypatch, - '[[change]]\nissue = "x"\nname_regex = "ZZZ"\n', + '[[change]]\nissue = "x"\nname_regex = "ZZZ"\nfields = ["family"]\n', _SAME_FACADE, floor=50) @@ -939,7 +964,7 @@ def test_main_aborts_on_a_corpus_with_no_floor( tables use.""" with pytest.raises(SystemExit, match="no entry in _CORPUS_FLOORS"): _run_main(tmp_path, monkeypatch, - '[[change]]\nissue = "x"\nname_regex = "ZZZ"\n', + '[[change]]\nissue = "x"\nname_regex = "ZZZ"\nfields = ["family"]\n', _SAME_FACADE, floor=None) diff --git a/tools/differential/README.md b/tools/differential/README.md index 2e084c91..039d68ce 100644 --- a/tools/differential/README.md +++ b/tools/differential/README.md @@ -304,9 +304,17 @@ string). It may narrow further with `fields` (the diffing rule matches only if the observed diff fields are a subset of this list), which since #452 must also name EXACTLY the roles that rule's own diffs move -- see below. Keep both as tight as the actual diff allows -- a loose -rule can mask a real regression. `name_regex` is REQUIRED since #451: -`validate_rules` rejects a rule carrying `fields` and no `name_regex`, -as it already rejected one carrying neither. +rule can mask a real regression. **Both keys are REQUIRED**, and each +ban has its own issue: `validate_rules` rejects a rule carrying +neither (it would match every diff), one carrying `fields` and no +`name_regex` (#451 -- no name narrowing, so it claims every name whose +diff fits its roles), and one carrying `name_regex` and no `fields` +(#456 -- no role narrowing, so on any name its regex reaches it claims +every diff shape there is, measured 127 of 127). The three are one +rule with one reason: a rule narrows by name AND by role, or it is not +a rule. Note the two bans are each other's obvious wrong answer -- +deleting `fields` to silence an over-declaration failure lands on +#456's, and adding `fields` while dropping the regex lands on #451's. **That closes the SHAPE, not the property.** A required `name_regex` is not a bound on how much a rule reaches: the only width check is the diff --git a/tools/differential/compare.py b/tools/differential/compare.py index 15a9d8fc..33344695 100644 --- a/tools/differential/compare.py +++ b/tools/differential/compare.py @@ -536,6 +536,22 @@ def validate_rules(rules: list[dict[str, object]], ledger: str) -> None: f"unrelated behavior families, with every guard green " f"the whole time. Narrow by name instead, or split this " f"into the rules the diffs actually need") + if has_regex and not has_fields: + raise SystemExit( + f"{where} has 'name_regex' but no 'fields' (#456). It " + f"narrows by name and by nothing else, so on any name " + f"its regex reaches it claims EVERY diff shape there is " + f"-- 127 of them. #452 makes that worse than it looks: " + f"over_declared_rules skips a rule with no 'fields', " + f"correctly, since one declaring no roles cannot " + f"over-declare them -- so deleting the line is the " + f"cheapest way to silence an OVER-DECLARED failure and " + f"the most permissive thing you can do to the rule at " + f"the same time. Name the roles the diffs actually " + f"move. Do NOT reach for the other shape instead: a " + f"'fields' with no 'name_regex' is banned by #451 for " + f"the mirror-image reason, and the two bans are each " + f"other's obvious wrong answer") def validate_exclusions(entries: list[dict[str, object]], From c856c33ba619f353cf1cd228f5889fdf910c4e98 Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Sat, 29 Aug 2026 01:41:00 -0700 Subject: [PATCH 2/2] 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 --- docs/design/decisions.md | 2 +- tests/v2/test_differential.py | 34 ++++++++++++++++++++++++++++------ tools/differential/README.md | 9 ++++----- tools/differential/compare.py | 27 ++++++++++++++++++--------- 4 files changed, 51 insertions(+), 21 deletions(-) diff --git a/docs/design/decisions.md b/docs/design/decisions.md index d8846673..c5440b58 100644 --- a/docs/design/decisions.md +++ b/docs/design/decisions.md @@ -635,8 +635,8 @@ Decisions that landed: - 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. - 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. - 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. -- 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. - 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. +- 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. Found rather than decided, and worth as much: diff --git a/tests/v2/test_differential.py b/tests/v2/test_differential.py index 1e1b623c..c8583b54 100644 --- a/tests/v2/test_differential.py +++ b/tests/v2/test_differential.py @@ -267,7 +267,8 @@ def test_a_rule_with_a_regex_and_no_fields_is_rejected() -> None: A rule with no `fields` 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 made that worse than it looks by giving + measured, every one of the 255 shapes + eight roles allow at a 2.x baseline. #452 made that worse than it looks by giving the shape a second job: over_declared_rules skips a rule with no `fields`, correctly, since one declaring no roles cannot over-declare them. So deleting the `fields` line is the response to @@ -276,12 +277,29 @@ def test_a_rule_with_a_regex_and_no_fields_is_rejected() -> None: Free to enforce, on the same terms as #451's: no rule in any shipped ledger has the shape, so the ban costs no migration. + + This assertion is an INVERSION. Its other half pinned the + regex-only shape as LEGAL when #451 landed -- "the neighbouring + shapes #451 did NOT retire" -- and #456 retired it. Inverted rather + than deleted, so the change of status is visible to a `git log -L` + on the assertion rather than vanishing with the test. """ with pytest.raises(SystemExit, match="no 'fields'"): compare.validate_rules( [{"issue": "fix(x) a rule with no role narrowing", "name_regex": "Smith"}], "test_ledger.toml") + # `dormant` buys no exemption here either, for the reason it buys + # none from #451's ban: it is a claim about today's corpus, not a + # bound on reach, and a regex-only rule sits at every diff shape + # the moment one matching name arrives -- at which point its + # dormancy claim is false too. Pinned because disabling the check + # otherwise fails exactly one test (#457 review). + with pytest.raises(SystemExit, match="no 'fields'"): + compare.validate_rules( + [{"issue": "fix(x) idle and unbounded", + "name_regex": "Smith", "dormant": "a reason nobody faults"}], + "test_ledger.toml") def test_a_rule_carrying_both_keys_is_the_only_legal_shape() -> None: @@ -289,10 +307,8 @@ def test_a_rule_carrying_both_keys_is_the_only_legal_shape() -> None: `validate_rules` rejects neither key (it would match every diff), `fields` without `name_regex` (#451, no name narrowing), and - `name_regex` without `fields` (#456, no role narrowing). This - pinned the regex-only shape as LEGAL when #451 landed; #456 - retired it, and the test is inverted rather than deleted so the - change of status is visible in the history rather than silent. + `name_regex` without `fields` (#456, no role narrowing). One + shape survives all three, and this is it. """ compare.validate_rules( [{"issue": "x", "name_regex": "Smith", "fields": ["given"]}], @@ -865,7 +881,13 @@ def test_main_compares_the_v2_surface_from_baseline_2_0( v2 = {**_SAME_V2, "_ambiguities": ["SEGMENTATION"]} code, out = _run_main( tmp_path, monkeypatch, - '[[change]]\nissue = "unrelated"\nname_regex = "ZZZ"\nfields = ["family"]\n', + # `_ambiguities`, not `family`: this test's diff IS the + # ambiguity-only one, and a `family` declaration refuses that + # shape -- which left the rule inert even with name narrowing + # disabled, costing this test the mutation it was written to + # catch (#457 review). Declare the role the diff moves. + '[[change]]\nissue = "unrelated"\nname_regex = "ZZZ"\n' + 'fields = ["_ambiguities"]\n', _SAME_FACADE, baseline="2.0.0", baseline_v2=v2) assert code == 1, "an ambiguity-only regression must not exit 0" assert "UNEXPLAINED 'John Smith'" in out diff --git a/tools/differential/README.md b/tools/differential/README.md index 039d68ce..2d859e72 100644 --- a/tools/differential/README.md +++ b/tools/differential/README.md @@ -300,17 +300,16 @@ history rather than being edited into the next one's. Each `[[change]]` entry needs `issue` (a short label, ideally an issue number or `fix()` matching a `tests/v2/cases.py` classification) and `name_regex` (searched against the raw input -string). It may narrow further with `fields` (the diffing rule matches -only if the observed diff fields are a subset of this list), which -since #452 must also name EXACTLY the roles that rule's own diffs move --- see below. Keep both as tight as the actual diff allows -- a loose +string) AND `fields` (the diffing rule matches only if the observed +diff fields are a subset of this list), which since #452 must also +name EXACTLY the roles that rule's own diffs move -- see below. Keep both as tight as the actual diff allows -- a loose rule can mask a real regression. **Both keys are REQUIRED**, and each ban has its own issue: `validate_rules` rejects a rule carrying neither (it would match every diff), one carrying `fields` and no `name_regex` (#451 -- no name narrowing, so it claims every name whose diff fits its roles), and one carrying `name_regex` and no `fields` (#456 -- no role narrowing, so on any name its regex reaches it claims -every diff shape there is, measured 127 of 127). The three are one +every diff shape there is, measured, 255 of them from baseline 2.0 on and 127 below it). The three are one rule with one reason: a rule narrows by name AND by role, or it is not a rule. Note the two bans are each other's obvious wrong answer -- deleting `fields` to silence an over-declaration failure lands on diff --git a/tools/differential/compare.py b/tools/differential/compare.py index 33344695..1955f156 100644 --- a/tools/differential/compare.py +++ b/tools/differential/compare.py @@ -390,9 +390,13 @@ def validate_rules(rules: list[dict[str, object]], ledger: str) -> None: buy a precise message rather than safety; they are here because the family is easier to reason about whole than split by direction. - The `fields`-without-`name_regex` check belongs to the dangerous - direction, and it is the family's sharpest example: no other - malformed shape can widen invisibly. #451 is the rule that lived + Two checks belong to the dangerous direction and are the family's + sharpest examples, one per missing key -- #451 for a `fields` with + no `name_regex`, #456 for a `name_regex` with no `fields`. An + earlier version of this paragraph said the first was the only shape + that could widen invisibly; #456 falsified that, and worse, since + over_declared_rules skips a fieldless rule so nothing narrows it + either. #451 is the rule that lived it -- no name narrowing, so it claimed every name whose diff fit its `fields`, and _CORPUS_CLAIMS (the guard tracking each rule's 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: raise SystemExit( f"{where} has 'name_regex' but no 'fields' (#456). It " f"narrows by name and by nothing else, so on any name " - f"its regex reaches it claims EVERY diff shape there is " - f"-- 127 of them. #452 makes that worse than it looks: " + f"its regex reaches it claims EVERY diff shape there is -- " + f"255 of them from baseline 2.0 on, where `_ambiguities` " + f"joins the seven roles, and 127 below it. #452 makes " + f"that worse than it looks: " f"over_declared_rules skips a rule with no 'fields', " f"correctly, since one declaring no roles cannot " f"over-declare them -- so deleting the line is the " @@ -691,8 +697,9 @@ def classify(name: str, diff_fields: set[str], rules -- so a new entry's blast radius is exactly the set of names it captures, independent of rule order. - An exclusion narrows by `name_regex` and optionally by `fields`, - exactly as a rule does. Without `fields` it refuses any diff on a + An exclusion narrows by `name_regex` and optionally by `fields`. + The optionality is the exclusion's alone since #456: a RULE must + now carry both. Without `fields` it refuses any diff on a matching name; with them it refuses only the reading it names, so a name whose parens mark a nickname to one rule and a suffix to another stays classifiable on the reading the exclusion is not @@ -840,8 +847,10 @@ def over_declared_rules( can have one, and the test that pins this skip uses exactly that input, because the empty-union input cannot discriminate.) A rule with no `fields` declares no roles and so has nothing to - over-declare; one with `fields` and no `name_regex` cannot exist - since #451. And a rule that explained nothing is dormant_rules' + over-declare -- and cannot exist since #456, which banned that + shape precisely because this skip made deleting `fields` the + cheapest way to silence the check. One with `fields` and no + `name_regex` cannot exist since #451. And a rule that explained nothing is dormant_rules' too, which is the third `continue` below. What this does NOT bound is a diff shape no single name produced.