diff --git a/AGENTS.md b/AGENTS.md index 9bd8656..6f007b5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -65,9 +65,13 @@ uv run sphinx-build -b html docs dist/docs # And check for open Dependabot PRs on uv.lock (namedivider-python) and merge them # first — pyproject floats >=0.4 so fresh installs get the newest namedivider, but # CI's ja-extra job installs from uv.lock and only tests what the lock pins -# 1. Run the differential gate at BOTH baselines and confirm each exits 0: +# 1. Run the differential gate at EVERY baseline that has a ledger with rules, +# and confirm each exits 0. The dynamic check only runs for the ledger +# matching the baseline being run, so a ledger left out here gets no +# dynamic dormancy check at all -- see tools/differential/README.md: # uv run python tools/differential/compare.py --baseline 1.4.0 # the v1 compat contract # uv run python tools/differential/compare.py # the previous minor +# uv run python tools/differential/compare.py --baseline 2.0.0 # 2.0.0's ledger has rules too # Redirect to a file rather than piping — under zsh a pipe replaces the exit # code with the pipe's, so a failing run reads as a passing one. The # classified summary it prints is the source for the release notes' behavior @@ -128,6 +132,10 @@ uv run sphinx-build -b html docs dist/docs # added, or its name_regex/fields/examples edited, moves the record and # must be re-recorded deliberately, the same forcing function # _CORPUS_CLAIMS applies to rules. (A `why`-only edit does not move it.) +# A rule that explains nothing must carry `dormant = ""`: the +# CI guard checks regex reach, compare.py checks whether it explained a +# diff, and both fail undeclared. Removing the last name a rule reached is +# therefore a decision, not a silent no-op. # 9. Open the next cycle's VERSION: bump VERSION in nameparser/_version.py to # the minor now being worked, and set PRE_RELEASE = 'dev'. The tree then says # what it is building rather than what it last shipped -- docs/conf.py reads diff --git a/tests/v2/test_differential.py b/tests/v2/test_differential.py index ce45db3..0daeacb 100644 --- a/tests/v2/test_differential.py +++ b/tests/v2/test_differential.py @@ -279,6 +279,14 @@ def test_v2_fields_matches_the_Role_enum() -> None: ({"issue": "x", "fields": ["title", "given", "middle", "family", "suffix", "nickname", "maiden", "_ambiguities"]}, "all seven roles"), + ({"issue": "x", "fields": ["given"], "dormant": ""}, "not a non-empty"), + ({"issue": "x", "fields": ["given"], "dormant": True}, "not a non-empty"), + # widening _RULE_KEYS is exactly the edit that could let a near-miss + # through, and a silently-ignored `dormnat` would mean the rule is + # checked for dormancy while its author believes it is exempt + ({"issue": "x", "fields": ["given"], "dormnat": "typo"}, "unknown key"), + # a dormant declaration is not a pass for the rest of the checks + ({"issue": "x", "dormant": "reason"}, "neither 'name_regex' nor 'fields'"), ]) def test_validate_rules_rejects_a_rule_that_would_silently_widen( rule: dict, expect: str) -> None: @@ -354,7 +362,9 @@ def test_ambiguities_is_a_legal_field_name() -> None: def _run_main(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ledger_body: str, - baseline_facade: dict, baseline: str = "1.4.0", + baseline_facade: dict, + extra: list[tuple[str, dict]] | None = None, + baseline: str = "1.4.0", baseline_v2: dict | None = None, floor: int | None = 1) -> tuple[int, str]: """Drive main() end to end with a faked baseline worker. @@ -366,21 +376,31 @@ def _run_main(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ledger_body: str, `baseline` defaults to 1.4.0 (facade only). Pass 2.0.0 with `baseline_v2` to exercise the v2 surface, including the ambiguity-only diff that is the stated reason to compare it. + + `extra` appends more (name, baseline_facade) pairs to the corpus, + in order, alongside the fixture's own 'John Smith'. It exists so a + test can mix a diffing and a non-diffing name -- the single-name + corpus below is structurally incapable of that. """ + import json import sys corpus = tmp_path / "corpus_x.jsonl" - corpus.write_text('"John Smith"\n', encoding="utf-8") + names = ["John Smith"] + [n for n, _ in (extra or ())] + corpus.write_text( + "\n".join(json.dumps(n) for n in names) + "\n", encoding="utf-8") (tmp_path / f"expected_since_{baseline}.toml").write_text( ledger_body, encoding="utf-8") - row: dict = {"facade": baseline_facade} + rows: list[dict] = [{"facade": baseline_facade}] if baseline_v2 is not None: - row["v2"] = baseline_v2 + rows[0]["v2"] = baseline_v2 + for _, facade in (extra or ()): + rows.append({"facade": facade}) _WORKER_CALL.clear() def _fake(v: str, w: bool, n: list[str]) -> tuple[dict, list[dict]]: _WORKER_CALL.update(version=v, want_v2=w, names=list(n)) return ({"__version__": v, - "__file__": "/wheel/nameparser/__init__.py"}, [row]) + "__file__": "/wheel/nameparser/__init__.py"}, rows) # The fixture corpus needs a floor like any other. `floor=None` # leaves it unregistered, for the test that pins what happens when @@ -454,14 +474,90 @@ def test_main_sorts_a_name_regex_rule_ahead_of_a_fields_only_one( tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: """A broad fields-only rule written FIRST must not claim a diff the specific name_regex rule below it owns. Deleting main's - _sorted_rules call leaves _sorted_rules' own test passing.""" + _sorted_rules call leaves _sorted_rules' own test passing. + + 'broad' is declared dormant because in THIS fixture -- one corpus + name, always won by 'specific' -- it is permanently shadowed by + construction, which is exactly the case main()'s dormancy report + (#372) now calls out. Without the declaration this test would be + pinning main's sort order and main's dormancy report at once, and a + failure could not tell which one broke. + """ _, out = _run_main( tmp_path, monkeypatch, '[[change]]\nissue = "broad"\nfields = ["family"]\n' + 'dormant = "always shadowed by \'specific\' below, by construction ' + 'of this fixture"\n' '[[change]]\nissue = "specific"\nname_regex = "Smith"\n', _DIFFERS) assert "## specific (1)" in out and "broad" not in out +def test_main_exits_1_and_names_a_rule_that_explained_nothing( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """The #372 gate. `idle` matches no diffing name, so it explains + nothing and is not declared dormant -- the run must say so and fail, + even though every diff here IS explained. + + Nothing else pins this: dropping the dormancy terms from main's + return, or deleting the report loop, leaves every other test green. + """ + code, out = _run_main( + tmp_path, monkeypatch, + '[[change]]\nissue = "explains-it"\nfields = ["family"]\n' + '[[change]]\nissue = "idle"\nname_regex = "ZZNOSUCHNAME"\n' + 'fields = ["family"]\n', _DIFFERS) + assert code == 1 + assert "EXPLAINED NOTHING 'idle'" in out + assert "may have been reverted" in out + # the diff itself was explained; this failure is only about the rule + assert "unexplained: 0" in out + + +def test_main_only_feeds_diffing_names_to_the_dormancy_check( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """If a non-diffing name reached `diffing`, a rule matching only + that name would appear to match a name in the diff set -- and + since it is the only rule that matches it, dormant_rules would + classify it via that same rule and diagnose it as its own + shadower ("shadowed by 'idle'"), instead of the correct + 'reverted' diagnosis for a rule that matches no diffing name. + + 'Alice Jones' is added via `extra` with a baseline facade equal to + what the tree parses it as -- it does not diff -- alongside the + fixture's own diffing 'John Smith'. `idle`'s regex matches only + 'Alice Jones', so it must be reported reverted, never shadowed. + """ + code, out = _run_main( + tmp_path, monkeypatch, + '[[change]]\nissue = "explains-it"\nfields = ["family"]\n' + '[[change]]\nissue = "idle"\nname_regex = "Jones"\n' + 'fields = ["family"]\n', _DIFFERS, + extra=[("Alice Jones", + {"title": "", "first": "Alice", "middle": "", + "last": "Jones", "suffix": "", "nickname": "", + "maiden": ""})]) + assert code == 1 + assert "EXPLAINED NOTHING 'idle'" in out + assert "may have been reverted" in out + assert "shadowed by 'idle'" not in out + # the diffing name's diff was explained; this failure is only + # about the rule that matched no diffing name + assert "unexplained: 0" in out + + +def test_main_exits_1_when_a_declared_dormant_rule_wakes_up( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """The other direction. A `dormant` reason that stopped being true is + a false statement in the ledger, so it fails the run too.""" + code, out = _run_main( + tmp_path, monkeypatch, + '[[change]]\nissue = "awake"\nfields = ["family"]\n' + 'dormant = "claims to be idle, but explains the only diff here"\n', + _DIFFERS) + assert code == 1 + assert "NO LONGER DORMANT 'awake'" in out + + def test_check_tree_accepts_the_checkout_and_rejects_anything_else( tmp_path: Path) -> None: """The tree side is the half that had no proof at all: the baseline @@ -825,3 +921,138 @@ def test_an_excluded_shape_stays_classifiable_on_other_roles() -> None: # and a mixed diff is not a subset of the exclusion, so it survives assert compare.classify( name, {"nickname", "suffix"}, rules, never) == "catch-all" + + +def test_entry_matches_is_the_question_classify_asks() -> None: + """classify() and the dormancy check must agree on what a rule + would claim. They share this predicate so they cannot drift.""" + rule = {"issue": "x", "name_regex": "Smith", "fields": ["given"]} + assert compare._entry_matches(rule, "John Smith", {"given"}) + # regex misses + assert not compare._entry_matches(rule, "John Jones", {"given"}) + # fields is a SUBSET test, not an intersection + assert not compare._entry_matches(rule, "John Smith", {"given", "family"}) + # a rule with neither key admits everything validate_rules lets exist + assert compare._entry_matches({"issue": "x"}, "anyone", {"suffix"}) + # the ignore-don't-reject contract the docstring rests on: validate_* + # already rejected these at startup, so re-judging them here would put + # the two in a position to disagree + assert compare._entry_matches({"issue": "x", "name_regex": 5}, "anyone", {"suffix"}) + assert compare._entry_matches({"issue": "x", "fields": "given"}, "anyone", {"given"}) + # an exclusion entry narrows on the same two keys and carries no `issue` + assert compare._entry_matches( + {"why": "x", "name_regex": "Smith", "examples": ["John Smith"]}, + "John Smith", {"given"}) + + +def test_validate_rules_accepts_a_declared_dormant_rule() -> None: + """`dormant` is a legal key, so a rule that declares one is not + rejected as a misspelling.""" + compare.validate_rules( + [{"issue": "x", "fields": ["given"], "dormant": "no corpus name"}], + "expected_since_1.4.0.toml") + + +def test_dormant_rules_reports_a_rule_whose_behavior_vanished() -> None: + """The #372 case: a rule matching no diffing name at all. Its fix + was probably reverted, and today the run exits 0 regardless.""" + rules = [{"issue": "fix(a)", "name_regex": "Smith", "fields": ["given"]}, + {"issue": "fix(b)", "name_regex": "Jones", "fields": ["given"]}] + report = compare.dormant_rules( + rules, {"fix(a)"}, [("John Smith", {"given"})]) + assert report.awake == () + assert [i for i, _ in report.undeclared] == ["fix(b)"] + assert "reverted" in report.undeclared[0][1] + + +def test_dormant_rules_names_the_rule_that_shadows_one() -> None: + """A rule can explain nothing because a broader rule written ahead + of it in the same tier claimed every diff it would have claimed. + That is a different diagnosis with a different fix, so it gets + different words.""" + rules = [{"issue": "fix(broad)", "name_regex": "Smith", + "fields": ["given", "family"]}, + {"issue": "fix(narrow)", "name_regex": "John Smith", + "fields": ["given"]}] + report = compare.dormant_rules( + rules, {"fix(broad)"}, [("John Smith", {"given"})]) + assert [i for i, _ in report.undeclared] == ["fix(narrow)"] + assert "shadowed by 'fix(broad)'" in report.undeclared[0][1] + + +def test_dormant_rules_distinguishes_an_excluded_shape() -> None: + """Third diagnosis: the rule matches a diffing name, but a [[never]] + entry refuses it, so no rule claims it. Reporting that as `reverted` + would send someone hunting for a fix that was never undone.""" + rules = [{"issue": "fix(a)", "name_regex": "Smith", "fields": ["given"]}] + never = [{"why": "protected", "name_regex": "Smith"}] + report = compare.dormant_rules( + rules, set(), [("John Smith", {"given"})], never) + assert [i for i, _ in report.undeclared] == ["fix(a)"] + assert "[[never]]" in report.undeclared[0][1] + + +def test_dormant_rules_is_silent_about_a_declared_rule() -> None: + rules = [{"issue": "fix(a)", "name_regex": "Smith", "fields": ["given"], + "dormant": "no corpus name reaches it"}] + report = compare.dormant_rules(rules, set(), []) + assert report.undeclared == () and report.awake == () + + +def test_dormant_rules_reports_a_declared_rule_that_woke_up() -> None: + """The other direction. A `dormant` declaration that stopped being + true is a false statement in the ledger, and the roster pattern in + this tree checks both directions or it checks nothing.""" + rules = [{"issue": "fix(a)", "fields": ["given"], "dormant": "was idle"}] + report = compare.dormant_rules( + rules, {"fix(a)"}, [("John Smith", {"given"})]) + assert report.awake == ("fix(a)",) + assert report.undeclared == () + + +def test_dormant_rules_names_the_shadower_that_does_the_shadowing() -> None: + """Which rule to go and look at. Picking the alphabetically first + claimant would send someone to the rule that took one name while + another took the rest.""" + rules = [{"issue": "fix(a)", "name_regex": "Alpha", "fields": ["given"]}, + {"issue": "fix(z)", "name_regex": "Zeta", "fields": ["given"]}, + {"issue": "fix(idle)", "name_regex": "Alpha|Zeta", + "fields": ["given"]}] + report = compare.dormant_rules( + rules, {"fix(a)", "fix(z)"}, + [("Alpha One", {"given"}), ("Zeta One", {"given"}), + ("Zeta Two", {"given"}), ("Zeta Three", {"given"})]) + assert [i for i, _ in report.undeclared] == ["fix(idle)"] + assert "shadowed by 'fix(z)'" in report.undeclared[0][1] + + +def test_dormant_rules_sorts_before_diagnosing() -> None: + """classify() must be asked in the order main() asks it. These rules + are written broad-first, but _sorted_rules puts the name_regex rule + ahead of the fields-only one, so 'specific' is what actually claims + 'John Smith' -- which makes 'broad' the dormant one, shadowed by it. + + Without the internal sort this returns the diagnosis backwards, + naming 'specific' as dormant and shadowed by 'broad'. Nothing else + pins that line: main() always pre-sorts before calling this. + """ + rules = [{"issue": "broad", "fields": ["given"]}, + {"issue": "specific", "name_regex": "Smith", + "fields": ["given"]}] + report = compare.dormant_rules( + rules, {"specific"}, [("John Smith", {"given"})]) + assert [i for i, _ in report.undeclared] == ["broad"] + assert "shadowed by 'specific'" in report.undeclared[0][1] + + +def test_validate_rules_rejects_two_rules_sharing_an_issue() -> None: + """The dormancy check identifies a rule by its `issue`, so a + duplicate lets one rule hide behind the other -- it can explain + nothing and never be reported. Measured before this check existed: + dormant_rules returned undeclared=() awake=() for a rule that + genuinely explained nothing.""" + with pytest.raises(SystemExit, match="sharing the issue"): + compare.validate_rules( + [{"issue": "dup", "name_regex": "Smith", "fields": ["given"]}, + {"issue": "dup", "name_regex": "Jones", "fields": ["given"]}], + "expected_since_1.4.0.toml") diff --git a/tests/v2/test_ledger_guards.py b/tests/v2/test_ledger_guards.py index 9dc0152..5c68306 100644 --- a/tests/v2/test_ledger_guards.py +++ b/tests/v2/test_ledger_guards.py @@ -1318,3 +1318,41 @@ def test_a_fields_narrowing_actually_narrows_something() -> None: f"the whole entry.") assert checked, ( "no exclusion declares `fields`, so this pin is passing vacuously") + + +def test_a_rule_reaching_no_corpus_name_says_why_it_is_kept() -> None: + """The cheap half of #372, asked on every push. + + A rule whose regex reaches nothing explains nothing, and no other + guard here can tell that from a rule that is merely narrow: + _CORPUS_CLAIMS records the reach it HAS, whatever that is, so a + reach of zero is recorded as contentedly as any other number. + + This is deliberately about REACH, not about diffs. Asking whether a + rule explained a diff needs a baseline wheel and belongs in the + harness; asking whether it could reach any name at all needs only + the corpus, so it runs here, on every push, for free. + """ + silent = [] + checked = 0 + for ledger in _LEDGERS: + for rule in _rules(ledger): + if "dormant" in rule: + continue + # fields-only rules reach every name by construction, so only + # a name_regex can be statically silent -- see _claim(), which + # counts them as the whole corpus for the same reason + regex = rule.get("name_regex") + if not isinstance(regex, str): + continue + checked += 1 + if not _claimed(regex): + silent.append(f"{ledger.name}: {rule['issue']}") + assert not silent, ( + f"these rules reach no corpus name, so they explain nothing and " + f"nothing else would say so: {silent}. Either declare `dormant` " + f"with the reason the rule is worth keeping, or delete it.") + assert checked, ( + "no rule was examined, so this guard is passing vacuously -- " + "every rule either declares `dormant` or narrows by `fields` " + "alone") diff --git a/tools/differential/README.md b/tools/differential/README.md index e1c8371..b4108b5 100644 --- a/tools/differential/README.md +++ b/tools/differential/README.md @@ -270,12 +270,38 @@ written in settles every tie between them. Append a rule to the bottom of a file only after checking that nothing above it already claims the diff you meant it for. -Some entries in the seed list are for behavior families that a -particular corpus happens not to contain any example of (e.g. custom -suffix-delimiter rendering, which only fires under a non-default -`Policy` -- see the ceiling below). They're kept in the file anyway, -matching the family documented in `tests/v2/cases.py`, so the rule is -ready the moment a matching string is added to the corpus. +Some entries in `expected_since_1.4.0.toml` are for behavior families that +a corpus happens to contain no example of (e.g. custom suffix-delimiter +rendering, which only fires under a non-default `Policy` -- see the ceiling +below). They're kept in the file anyway, matching the family documented in +`tests/v2/cases.py`, so the rule is ready the moment a matching string is +added to the corpus. + +Such a rule must say so, with `dormant = ""`. Without it a rule +that explains nothing is indistinguishable from one that has stopped +explaining anything -- which is how a reverted fix leaves its rule inert +and the run still exits 0 (#372). Two tiers ask: + +- `tests/v2/test_ledger_guards.py` fails, in CI, when a rule's + `name_regex` reaches no corpus name and no `dormant` reason is given. It + needs no baseline wheel, so it is cheap and early. +- `compare.py` fails the run when a rule explained no diff, and equally + when a rule declaring `dormant` explained one -- a declaration that + stopped being true is a false statement in the ledger. It reports which + kind of nothing: **reverted** (matched no diffing name), **shadowed by + ``** (an earlier rule claimed every diff it would have), or + refused by a `[[never]]` exclusion. The three have three different fixes. + +One limit worth knowing. Only the ledger for the baseline being run gets the +dynamic check, so the release checklist runs `compare.py` at every baseline +that has a ledger with rules -- a ledger left out of the checklist gets no +dynamic dormancy check at all, and a `dormant` declaration in it goes +unaudited. Adding a new ledger means adding its baseline to the checklist. + +The static tier still exempts a rule on the mere PRESENCE of `dormant`; it +never asks whether the reason is still true. Only the dynamic check can +catch a `dormant` that quietly stops being true, which is why every ledger +with rules needs one. ### Shapes that must never be explained (`[[never]]`) diff --git a/tools/differential/compare.py b/tools/differential/compare.py index e1083ee..fe3c043 100644 --- a/tools/differential/compare.py +++ b/tools/differential/compare.py @@ -17,7 +17,9 @@ import subprocess import tempfile import tomllib +from collections import Counter from pathlib import Path +from typing import NamedTuple HERE = Path(__file__).resolve().parent FIELDS = ("title", "first", "middle", "last", "suffix", "nickname", @@ -331,7 +333,7 @@ def _is_latin_only(name: str) -> bool: #: ambiguity entry is legal and load-bearing -- a SEGMENTATION-only diff #: is facade-identical, so this is the one name that can classify it. _RULE_FIELDS = frozenset((*V2_FIELDS, "_ambiguities")) -_RULE_KEYS = frozenset(("issue", "name_regex", "fields")) +_RULE_KEYS = frozenset(("issue", "name_regex", "fields", "dormant")) #: Probe names for the over-match check, chosen to share no script, no #: vocabulary and no punctuation. A `name_regex` matching ALL of them is #: not targeting a behavior family, it is matching everything -- and @@ -387,7 +389,23 @@ def validate_rules(rules: list[dict[str, object]], ledger: str) -> None: `ledger` is named rather than hardcoded because there is one per baseline now: a message naming the wrong file sends the reader to edit a rule that is not the broken one. + + The `dormant` check is the one exception to that framing: it is not + about a rule's matching semantics drifting, but about an opt-out + carrying a justification someone can review. """ + seen: set[str] = set() + for rule in rules: + issue = rule.get("issue") + if not isinstance(issue, str): + continue # the per-rule loop below rejects it with a better message + if issue in seen: + raise SystemExit( + f"{ledger} has two rules sharing the issue {issue!r}. The " + f"dormancy check identifies a rule by its issue, so the " + f"second would hide behind the first: it could explain " + f"nothing and never be reported") + seen.add(issue) for i, rule in enumerate(rules): where = f"{ledger} rule #{i + 1}" issue = rule.get("issue") @@ -402,6 +420,15 @@ def validate_rules(rules: list[dict[str, object]], ledger: str) -> None: f"only {sorted(_RULE_KEYS)}. A misspelled key is not " f"ignored -- it drops that half of the rule's narrowing " f"and the rule matches on the other half alone") + if "dormant" in rule: + reason = rule["dormant"] + if not isinstance(reason, str) or not reason: + raise SystemExit( + f"{where} has a 'dormant' that is not a non-empty " + f"string ({reason!r}). 'dormant' declares that a rule " + f"is expected to explain nothing, and the reason is " + f"the whole safeguard -- an exemption nobody can " + f"justify means the rule should be deleted instead") has_regex, has_fields = "name_regex" in rule, "fields" in rule if not has_regex and not has_fields: raise SystemExit( @@ -555,6 +582,30 @@ def validate_exclusions(entries: list[dict[str, object]], f"'fields' to exclude any diff on a matching name") +def _entry_matches(rule: dict[str, object], name: str, + diff_fields: set[str]) -> bool: + """Does this entry's narrowing admit this diff? + + Called twice in classify() -- once for exclusions, once for rules -- + and again in dormant_rules(). All three narrow on the same two keys, + and the dormancy diagnosis is only meaningful if it asks the + question classify asks, so there is one predicate rather than three + copies of it. + + A non-str `name_regex` or non-list `fields` is IGNORED rather than + rejected here: validate_rules and validate_exclusions reject both at + startup, and duplicating that judgement in the hot path would put the + two in a position to disagree. + """ + name_regex = rule.get("name_regex") + if isinstance(name_regex, str) and not re.search(name_regex, name): + return False + fields = rule.get("fields") + if isinstance(fields, list) and not diff_fields <= set(fields): + return False + return True + + def classify(name: str, diff_fields: set[str], rules: list[dict[str, object]], exclusions: list[dict[str, object]] | None = None) -> str | None: @@ -580,22 +631,78 @@ def classify(name: str, diff_fields: set[str], about. """ for entry in exclusions or (): - pattern = entry.get("name_regex") - if isinstance(pattern, str) and not re.search(pattern, name): - continue - fields = entry.get("fields") - if isinstance(fields, list) and not diff_fields <= set(fields): - continue - return None + if _entry_matches(entry, name, diff_fields): + return None for rule in rules: - name_regex = rule.get("name_regex") - if isinstance(name_regex, str) and not re.search(name_regex, name): + if _entry_matches(rule, name, diff_fields): + return rule["issue"] # type: ignore[return-value] + return None + + +class _Dormancy(NamedTuple): + """What a run found out about rules that explained nothing.""" + #: (issue, diagnosis) for every rule that explained nothing and does + #: not declare `dormant` + undeclared: tuple[tuple[str, str], ...] + #: issues declaring `dormant` that explained at least one diff + awake: tuple[str, ...] + + +def dormant_rules(rules: list[dict[str, object]], explained: set[str], + diffing: list[tuple[str, set[str]]], + exclusions: list[dict[str, object]] | None = None, + ) -> _Dormancy: + """Which rules explained nothing, and which kind of nothing. + + A rule going inert is invisible to every other guard here. + _CORPUS_CLAIMS records what a rule's REGEX reaches, which is + parser-independent, so reverting the fix a rule describes leaves the + rule matching exactly as many names as before while it explains no + diff at all -- and the run exits 0 (#372). + + Pure on purpose: it needs only values main() already derives per + name, and no second baseline run. Wiring it in means threading the + diff-fields set through the existing loop, not adding new I/O. A + check reachable only through a full baseline run is a check nobody + mutates, and this tree has a long list of measurements that ran, + printed a plausible number, and measured nothing. + + Three diagnoses, because they have three different fixes: + reverted -- matches no diffing name; the behavior is likely gone + shadowed -- an earlier rule claimed every diff it would have + excluded -- a [[never]] entry refuses every name it matches + """ + # classify() must be asked in the order main() asked it, or the + # shadower named here is not the rule that actually won. Sorting + # internally makes that true whatever the caller passes; the sort is + # stable and idempotent, so doing it twice costs nothing. + ordered = _sorted_rules(rules) + undeclared: list[tuple[str, str]] = [] + awake: list[str] = [] + for rule in ordered: + issue = str(rule["issue"]) + declared = "dormant" in rule + if issue in explained: + if declared: + awake.append(issue) continue - fields = rule.get("fields") - if isinstance(fields, list) and not diff_fields <= set(fields): + if declared: continue - return rule["issue"] # type: ignore[return-value] - return None + matched = [(n, d) for n, d in diffing + if _entry_matches(rule, n, d)] + if not matched: + why = ("matched no diffing name -- the behavior it describes " + "may have been reverted") + else: + winners = Counter( + c for c in (classify(n, d, ordered, exclusions) + for n, d in matched) if c is not None) + why = (f"shadowed by {winners.most_common(1)[0][0]!r}" + if winners else + "every diffing name it matches is refused by a " + "[[never]] exclusion") + undeclared.append((issue, why)) + return _Dormancy(tuple(undeclared), tuple(awake)) def main() -> int: @@ -684,6 +791,9 @@ def main() -> int: # facade dicts would print such a name under UNEXPLAINED with no # field lines under it at all: a failure nobody can act on. unexplained: list[_Unexplained] = [] + # every name that diffed, with its diff, so dormant_rules can ask + # which rule WOULD have claimed one that no rule did + diffing: list[tuple[str, set[str]]] = [] for name, old in zip(corpus, old_rows): new = {k: v or "" for k, v in HumanName(name).as_dict().items() if k in FIELDS} @@ -702,6 +812,7 @@ def main() -> int: if old["v2"].get(f, "") != new_v2.get(f, "")} if not diff: continue + diffing.append((name, diff)) issue = classify(name, diff, rules, exclusions) if issue is None: unexplained.append( @@ -721,6 +832,15 @@ def main() -> int: for n in names[:10]: print(f" {n!r}") print() + dormancy = dormant_rules(rules, set(by_issue), diffing, exclusions) + for issue, why in dormancy.undeclared: + print(f"EXPLAINED NOTHING {issue!r}\n {why}") + for issue in dormancy.awake: + print(f"NO LONGER DORMANT {issue!r}\n it explained a diff in " + f"this run, so its `dormant` reason is now false -- remove " + f"the key") + if dormancy.undeclared or dormancy.awake: + print() if unexplained: print("Field names below are Role's, matching what a ledger " "`fields` rule must say.\n") @@ -745,7 +865,10 @@ def main() -> int: print(f" {_canonical_field(f)}: " f"{old_v2.get(f, '')!r} -> {new_v2.get(f, '')!r}" f" [v2 surface only]") - return 1 if unexplained else 0 + # A rule explaining nothing is as much a broken contract as an + # unexplained diff: both mean the ledger no longer describes what the + # code does. Same exit code, so neither can be the one nobody noticed. + return 1 if unexplained or dormancy.undeclared or dormancy.awake else 0 if __name__ == "__main__": diff --git a/tools/differential/expected_since_1.4.0.toml b/tools/differential/expected_since_1.4.0.toml index bfb3abb..899b06a 100644 --- a/tools/differential/expected_since_1.4.0.toml +++ b/tools/differential/expected_since_1.4.0.toml @@ -181,6 +181,7 @@ issue = "fix(suffix-delimiter-rendering) no-space delimiter core token kept whol # this rule is expected to match nothing here. Kept for documentation # parity with tests/v2/cases.py's 'suffix_delimiter_no_space_core' row # (anti-#100, migration plan deviation 5). +dormant = "the no-space delimiter rendering fires only under a non-default Policy, and the corpora run under the default one, so no corpus name can reach this rule. Kept because the behavior family is real and documented in tests/v2/cases.py: the rule is ready the moment a matching string is added" name_regex = "/" fields = ["suffix"] @@ -192,6 +193,7 @@ issue = "ambiguous-surname-acronym data change: parenthesized (MA)/(DO) now stay # unambiguous there) -- they now fall through to nickname parsing. # Not expected to fire against this corpus (no such strings survived # into the v1 banks); kept for documentation completeness. +dormant = "no corpus contains a parenthesized (MA) or (DO), so this data change has no diff to explain here. Kept so the rule is ready the moment such a name is harvested" name_regex = "(?i)[(\"'](m\\.?a\\.?|d\\.?o\\.?)[)\"']" fields = ["suffix", "nickname"]