Skip to content

Commit 6787631

Browse files
derek73claude
andcommitted
test(rules-doc): close both /code-review findings
The exactly-one-pointer guard added two commits ago claimed more than it enforced. Its docstring said a shipped rule "cannot keep a stale tracking pointer once its issues close", but the test reads the doc only and cannot see code. Meanwhile test_implemented_matches_citing_modules skipped any rule without implemented:, so a rule that gained code and a citation while keeping tracked: passed both. That is exactly the state the docstring called unrepresentable. The citations test now handles the tracked: branch, and the docstring states its own scope instead of the pair's: neither test alone makes a stale pointer unrepresentable, the pair does. Mutation-tested on the finding's own scenario -- adding a P6 citation to _post_rules.py while leaving tracked: in place now fails with the swap instruction, and the restore was verified by diff. The DEVIATION note on P1 wrote its example values in double quotes, which put four extra quoted spans inside the citation block that test_citations_are_verbatim_excerpts parses -- it passed only because the check takes the FIRST span. Moving the note above the citation line, a natural edit, would have made the excerpt resolve to "de Mesnil Juan" and failed against prose that is entirely correct. Values are unquoted now, and the note says why so the next editor does not helpfully re-add them. Considered and rejected: asserting exactly one quoted span per citation block, which would be the unrepresentable-states fix. Measured first -- 38 blocks carry one, but _group.py's M1 block legitimately carries six, so the invariant does not hold today and imposing it would mean rewriting an unrelated comment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 1f5e0b8 commit 6787631

3 files changed

Lines changed: 28 additions & 7 deletions

File tree

nameparser/_pipeline/_post_rules.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,14 @@ def post_rules(state: ParseState) -> ParseState:
9999
# attaches to are the family, and any name words beyond that
100100
# read by position." (v1 handle_non_first_name_prefix; history:
101101
# decisions.md#P1)
102-
# DEVIATION #364: the fold below still takes every remaining
103-
# name word, not just the particle's own; "de Mesnil Juan" gives
104-
# family="de Mesnil Juan" where the rule says family="de Mesnil"
105-
# plus given="Juan". Pinned by the deviates: markers on P1.
102+
# DEVIATION #364: the fold below still takes every remaining name
103+
# word, not just the particle run's own -- de Mesnil Juan gives
104+
# family=de Mesnil Juan where the rule says family=de Mesnil plus
105+
# given=Juan. Pinned by the deviates: markers on P1.
106+
# Values written unquoted deliberately: this note sits INSIDE the
107+
# citation block above (# decisions.md#P1) does not close it --
108+
# _CITE_RE wants a colon after the ID), and the excerpt check
109+
# takes the first quoted span in the block.
106110
# Code-local: a lone PIECE is the test at both sites, so a
107111
# particle group already chained forward is not a lone particle,
108112
# and rule H1 above cannot be what produces the fold's family

tests/v2/test_doc_citations.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,13 +102,23 @@ def test_implemented_matches_citing_modules() -> None:
102102
citing.setdefault(cid, set()).add(str(path.relative_to(REPO)))
103103
problems = []
104104
for rule in parse_rules_doc(RULES_DOC.read_text(encoding="utf-8")):
105+
actual = citing.get(rule.rule_id, set())
105106
if rule.implemented:
106-
actual = citing.get(rule.rule_id, set())
107107
declared = set(rule.implemented)
108108
if actual != declared:
109109
problems.append(
110110
f"{rule.rule_id}: implemented: says {sorted(declared)} "
111111
f"but citations found in {sorted(actual)}")
112+
elif rule.tracked and actual:
113+
# The other half of test_rules_doc.py's exactly-one-pointer
114+
# rule: that test cannot see code, so a rule that SHIPPED
115+
# while keeping tracked: would pass it. Without this branch
116+
# the stale pointer is invisible -- the loop above skips
117+
# any rule with no implemented: at all.
118+
problems.append(
119+
f"{rule.rule_id}: declares tracked: {sorted(rule.tracked)} "
120+
f"but code cites it in {sorted(actual)}; swap tracked: for "
121+
f"implemented: now that something implements it")
112122
assert not problems, "\n".join(problems)
113123

114124

tests/v2/test_rules_doc.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,15 @@ def test_every_rule_has_examples_and_boundary(rule: Rule) -> None:
3737
def test_every_rule_points_at_code_or_at_the_work(rule: Rule) -> None:
3838
"""A normative rule names the code honoring it, or the issues that
3939
would ship it -- never neither, so an unimplemented rule cannot sit
40-
in the doc untracked, and never both, so a shipped rule cannot keep
41-
a stale tracking pointer once its issues close."""
40+
in the doc untracked, and never both.
41+
42+
Scope, precisely: this test reads the DOC only, so it cannot tell
43+
that a tracked: rule has since been implemented -- adding the code
44+
and its citation while leaving tracked: in place passes here. That
45+
half is test_doc_citations.py::test_implemented_matches_citing_
46+
modules, which sees the citing modules. Neither test alone makes a
47+
stale pointer unrepresentable; the pair does.
48+
"""
4249
assert rule.implemented or rule.tracked, (
4350
f"{rule.rule_id}: add 'implemented: <path>' or, if nothing "
4451
f"implements it yet, 'tracked: #N' naming the issues that would")

0 commit comments

Comments
 (0)