Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/design/decisions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
83 changes: 65 additions & 18 deletions tests/v2/test_differential.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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

Expand All @@ -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


Expand All @@ -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(
Expand All @@ -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)


Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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"]}
Expand All @@ -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


Expand Down Expand Up @@ -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)


Expand All @@ -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)


Expand Down
21 changes: 14 additions & 7 deletions tools/differential/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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(<slug>)` 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
Expand Down
39 changes: 32 additions & 7 deletions tools/differential/compare.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]],
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down