diff --git a/docs/design/decisions.md b/docs/design/decisions.md index 6aae4df4..c5440b58 100644 --- a/docs/design/decisions.md +++ b/docs/design/decisions.md @@ -636,6 +636,7 @@ Decisions that landed: - 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-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 cc2fa9f3..c8583b54 100644 --- a/tests/v2/test_differential.py +++ b/tests/v2/test_differential.py @@ -262,13 +262,54 @@ 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, 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 + 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. + + 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: + """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). One + shape survives all three, and this is it. + """ compare.validate_rules( [{"issue": "x", "name_regex": "Smith", "fields": ["given"]}], "test_ledger.toml") @@ -502,7 +543,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 +555,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 +577,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 +600,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 +746,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 +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"\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 @@ -867,7 +914,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 +925,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 +934,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 +975,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 +986,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..2d859e72 100644 --- a/tools/differential/README.md +++ b/tools/differential/README.md @@ -300,13 +300,20 @@ 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 -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. +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, 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 +#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..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 @@ -536,6 +540,24 @@ 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"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 " + 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]], @@ -675,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 @@ -824,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.