diff --git a/.claude/agents/design-docs-reviewer.md b/.claude/agents/design-docs-reviewer.md new file mode 100644 index 00000000..2f270f26 --- /dev/null +++ b/.claude/agents/design-docs-reviewer.md @@ -0,0 +1,68 @@ +--- +name: design-docs-reviewer +description: Reviews changes to docs/design/ (rules.md, decisions.md, mechanisms.md, and AGENTS.md itself, which carries the conventions the other three rest on) for the failure modes that this doc system's own tests cannot catch — unreproducible measurements, rules that contradict the decision entries they rest on, statements that contradict their own examples, and prose edits that change what the doc parsers see. Use before merging any PR that touches docs/design/, and after distilling a design session into those files. +tools: Bash, Read, Grep, Glob +--- + +You review changes to `docs/design/` for a Python name-parsing library +whose design docs are partly machine-checked. Your job is the part the +machines cannot do. + +**You report findings. You never edit files.** No fixes, no +suggestions phrased as edits — the calling session applies them. + +## The axes + +`docs/design/AGENTS.md` holds the review axes and the conventions they +rest on: what the suite already enforces — so a finding one of those +guards would catch is a false positive — the dated-count rule, and the +corpus-population caution. + +**Read that file and run every axis it lists.** Work from the file, +never from a summary, including one you have seen before: the list +grows, and individual axes get broadened in place. + +It is the only copy on purpose. This agent used to restate the axes +and drifted three ways against it — a missing axis, a narrowed one, +and a corpus count whose correction was recorded there and never +reached the copy here (#473, +decisions.md#review-agent-single-source). Do not reintroduce a +summary. + +## How to run + +1. `git diff master...HEAD -- docs/design/` for the scope. Also + `git diff HEAD -- docs/design/` for uncommitted work; review often + runs before the commit. +2. Read the changed rules and every decision entry they cite in full. + Not the diff hunks — the surrounding rule and entry, because the + rule-vs-examples, rule-vs-decision and precedence axes need + context the hunk does not carry. +3. Recompute. Use `uv run --frozen python - <<'EOF'` for measurement + scripts and `uv run --frozen pytest -q` for the suite. A bare + `python`/`pytest` picks up the wrong environment here. +4. State which tree you measured on — `git rev-parse --short HEAD` and + whether the tree is dirty. Stale measurements have produced rounds + of already-fixed findings in this repo. + +## Reporting + +Report each finding as: + + : — + Why it is wrong: + Concrete consequence: + Confidence: CONFIRMED (measured) | PLAUSIBLE (reasoned) + +Rank most severe first. Quote both sides of a contradiction verbatim — +a paraphrase of a doc claim is not evidence about the doc. + +If an axis found nothing, say so in one line. A silent axis is +indistinguishable from an unrun one, and this repo has a recurring +inert-measurement failure mode. + +**Do not report:** style, wording preferences, "consider adding", +missing content you cannot name a consequence for, or anything the +suite already enforces — AGENTS.md names those guards, and a finding +one of them would catch is a false positive. An empty report is a +fine outcome and is more useful than a padded one. diff --git a/.claude/commands/docs-review.md b/.claude/commands/docs-review.md new file mode 100644 index 00000000..772f81b9 --- /dev/null +++ b/.claude/commands/docs-review.md @@ -0,0 +1,33 @@ +--- +description: Review docs/design/ changes for the failure modes the doc tests cannot catch +argument-hint: "[base-ref, default master]" +allowed-tools: Bash, Read, Grep, Glob, Agent +--- + +Review this branch's changes to `docs/design/` using the +`design-docs-reviewer` agent. + +Base ref: `${1:-master}` + +1. Check there is something to review: + `git diff ${1:-master}...HEAD --stat -- docs/design/` plus + `git diff HEAD --stat -- docs/design/` for uncommitted work. If + both are empty, say so and stop — do not review an empty diff. +2. Confirm the suite is green first, so the agent can assume the + machine-checked layer holds: `uv run --frozen pytest -q`. If it + fails, report that and stop; a red suite falsifies the + machine-checked layer docs/design/AGENTS.md tells the agent to + assume, and it will waste effort rediscovering known breakage. +3. Dispatch the `design-docs-reviewer` agent with the base ref and + the changed-file list. Let it run its own axes — do not + pre-summarize the diff for it, and do not tell it what you think + the issues are. Its value is reading what is written rather than + what was meant, and a primed agent loses exactly that. +4. Relay its findings verbatim in your reply — the agent's report is + not shown to the user. Add your own assessment of each: agree, + disagree with reasoning, or need-more-information. You are not + obliged to accept a finding, but you are obliged to say why you + reject one. + +Do not apply fixes as part of this command. Report, then let the user +decide what lands. diff --git a/.gitignore b/.gitignore index 40f3ae89..dc4c8532 100644 --- a/.gitignore +++ b/.gitignore @@ -21,7 +21,9 @@ venv/ env/ # tools -.claude/ +.claude/* +!.claude/agents/ +!.claude/commands/ .codex/ .gemini/ .pytest_cache/ diff --git a/AGENTS.md b/AGENTS.md index d6db4b0e..b7f8feef 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -28,7 +28,7 @@ Three committed contributor docs carry the parser's normative rules and their re **Release-log claims.** Quantified or universal behavior claims in release bullets must come from the differential gate's classified summary, be verified against rules.md examples, or -- for a view the gate cannot see -- carry a recompute recipe stored with the design entry the bullet cites; never write one from memory. The gate compares the seven role fields and `_ambiguities`, so `capitalized()`, `initials()` and any future render view are invisible to it (decisions.md#R4, #R3) and the first two sources cannot reach a claim about one: a gate run is byte-identical across the change, and an example line witnesses an output without counting anything. A recipe names the corpus files, the policy sweep, and -- the part that is easy to omit and fatal -- THE COMPARATOR, which must be something the shipped tree is not: #408's first recipe said to compare `initials()` against a folded-first partition, which is what `initials()` now IS, so it reproduced 0 where the bullet claimed 660 and was the only stated provenance for the number. Run the recipe as written before shipping the bullet. Cross-version numbers (a released wheel, the pre-change tree) are dated snapshots under Counting claims, since nothing in the repository re-runs them. Per-rule ledger toml comments asserting PARSER behavior cite rule IDs under the excerpt discipline; free prose is for ledger mechanics only (owned by tools/differential/README.md). -**Working on docs/design/ has its own AGENTS.md.** `docs/design/AGENTS.md` carries the landing-a-design distillation checklist, the primary-source review rule, the dated-count convention, and the ten review axes. Claude Code loads it automatically when a session reads or edits anything under docs/design/; if your tool does not do nested discovery, read it yourself before touching those files or reviewing a change to them. +**Working on docs/design/ has its own AGENTS.md.** `docs/design/AGENTS.md` carries the landing-a-design distillation checklist, the primary-source review rule, the dated-count convention, and the review axes. Claude Code loads it automatically when a session reads or edits anything under docs/design/; if your tool does not do nested discovery, read it yourself before touching those files or reviewing a change to them. **Triaging a design backlog: shape before value.** When a pile of open questions has to be ordered, relatedness will not partition it — in a parser nearly everything touches particles or suffixes. The line that does is whether a decision changes WHAT THE MODEL CAN EXPRESS or fills in a value in a shape already fixed. Settle the shape questions first, as a batch: they are few, they are usually independent of each other (so their own ordering does not matter), and each one collapses or re-frames a run of the value questions below it. The rest are leaves needing one measurement and one answer apiece, in any order. Worked example, 2026-08-16 (#386): 30 open issues and ~13 open design questions reduced to three shape decisions, which decided or re-framed fourteen of them — and two of the three turned out smaller than their issues claimed, because the issues had gone stale. Check the age of an issue's premises before believing its cost estimate. diff --git a/docs/design/AGENTS.md b/docs/design/AGENTS.md index 3a599113..ff0d0641 100644 --- a/docs/design/AGENTS.md +++ b/docs/design/AGENTS.md @@ -14,7 +14,7 @@ rules.md states intended parsing behavior and is NORMATIVE; decisions.md is the Two shapes, needing different instruments. CROSS-SESSION distillation is what the rule above is written for — the #381 arc harvested nine named sessions, and for each the source transcript is an artifact independent of the reviewing session, so "review your own sections" is a two-artifact comparison. SAME-SESSION design work (#386) has no such artifact: spec and docs were written in one context, so the same instruction degenerates into re-reading the working memory that produced any error, and the rationale inverts — when the source is NOW, the source is what got it wrong. There, compare against artifacts instead, in this measured order of yield: re-derive every number with a freshly written script (2 wrong claims on #386); re-read the written spec against a checklist for FIDELITY, not just presence (1); interrogate NAMED contested rule pairs one at a time (2). Unaided prose reading found none — so a general "coherence read" by the author is the one form to distrust, because a green report from the weakest instrument manufactures confidence rather than supplying it. -**Reviewing a docs/design change: the ten axes.** Run these before merging anything that touches rules.md, decisions.md, mechanisms.md — or THIS FILE, which carries the conventions those three rest on and has the same failure modes. Axes 3, 5 and 6 are rules.md-specific (they need rule statements, examples and `interacts:`); the other seven apply to any of the four. Say which axes you ran and which you skipped as inapplicable — a checklist that asks for something meaningless teaches the reader to skim it. Each was earned by a defect that survived every other check on #386, where the review passes surfaced ten issues in total — nine of them by these axes, one by the spec self-review above — and unaided prose reading surfaced none. The parenthetical is the case that earned the axis. +**Reviewing a docs/design change: the eleven axes.** Run these before merging anything that touches rules.md, decisions.md, mechanisms.md — or THIS FILE, which carries the conventions those three rest on and has the same failure modes. Axes 3, 5 and 6 are rules.md-specific (they need rule statements, examples and `interacts:`); the other eight apply to any of the four — axis 11 included, since an invariant over two inputs is prompted by what a rule implies but is RUN without reading one, which is the whole of its value. Say which axes you ran and which you skipped as inapplicable — a checklist that asks for something meaningless teaches the reader to skim it. Axes 1–9 were each earned by a defect that survived every other check on #386, where the review passes surfaced ten issues in total — nine of them by these axes, one by the spec self-review above — and unaided prose reading surfaced none. Axes 10 and 11 were earned the same way on other arcs — #291's, which PREDATES this list, and #461/#466's, which follows it. The parenthetical is the case that earned the axis. Skip what the suite already enforces. `tests/v2/test_rules_doc.py` and `tests/v2/test_doc_citations.py` machine-check example lines and their `deviates:` today-values, boundaries, the one-of `implemented:`/`tracked:` rule, `implemented:` against the modules that actually cite the rule, `interacts:` and cross-doc anchor IDs, citation excerpts quoted verbatim, and legacy citation forms. Assume they pass; a finding one of them would catch is a false positive. Read those two files rather than trusting this sentence — it named five of the nine guards when written, and goes stale as guards are added. @@ -28,5 +28,6 @@ Skip what the suite already enforces. `tests/v2/test_rules_doc.py` and `tests/v2 8. **Guard docstring vs. what the guard enforces**, especially when the promise spans two test modules. 9. **Prose is input to the doc parsers.** A line starting with `"` inside a rule block is an example; a comment's quoted values join the citation block above them. 10. **Re-run every capability claim.** Axis 1 recomputes numbers; this one recomputes CAN and CANNOT. A statement that the parser never or cannot produce some reading is measured by a parse run written now, never derived from a mechanism — most of all when it justifies building something. (#291 derived "a multi-word credential is unparseable" from "vocabulary is matched one token at a time" — which `given_name_titles` had been an exception to since 2026-07-19, a month before that arc, and `maiden_markers` is a second since #434 — and specified a vocabulary set, a matching unit, a predicate and a `Lexicon` field on it; `parse("John Smith, MD PhD").suffix` had been `'MD PhD'` since 1.4.0.) +11. **State the invariant that relates two DIFFERENT inputs.** Axes 1–10 compare a document to another document, to its own examples, or to the implementation, so every one of them can only find code that DISAGREES with a rule — none of them can find a rule that is simply wrong. When a rule implies that two spellings of the same name must parse alike, write that as an invariant over both inputs and run it: it consults no rule statement, so it is not blind that way. Distinct from an invariant over two VIEWS of one parse, like `test_the_family_partitions_into_particles_and_base` in tests/v2/test_cases.py — that one holds however wrong the rule is. (Two review rounds a month apart found real defects and missed the premise under them: #461 was backed out after the rule turned out wrong, #466 closed unmerged for the same reason, and in both, every reviewer checked the code against the rule and THE RULE was at fault. The worked invariant: the family-comma writing under the default order and the comma-less writing under FAMILY_FIRST must parse alike. Measured, it holds for the great majority of the particle vocabulary and fails exactly on the words that are also suffix vocabulary — recovering P6's contested-membership set from behavior, having read no rule. Composition, measurement and recompute recipe: decisions.md#two-input-invariants.) -Also: before reporting "N names move", report the size of the population that COULD move — a small count over a corpus blind to the shape is evidence about the corpus. For rules.md#P6: of 782 corpus names, 245 carry a comma, two of those end in a particle, and one of the two clears the words-to-spare guard. Running axis 1 over this very list caught an earlier wording of that sentence conflating the population with the movers, so the count above is the corrected one. +Also: before reporting "N names move", report the size of the population that COULD move — a small count over a corpus blind to the shape is evidence about the corpus. For rules.md#P6 the population is the comma-bearing names that END in a particle — a small fraction of the comma-bearing names, which are themselves under a third of the corpus; decisions.md#P6 carries the dated measurement and the recompute. Do not restate a corpus count here. Axis 1 over this very sentence once caught it conflating the population with the movers, and the corrected figures that replaced it were superseded within the month as the corpus grew — a standing count in this file is wrong twice over, once for the class and once for the digits. diff --git a/docs/design/decisions.md b/docs/design/decisions.md index d63d6678..f16badd4 100644 --- a/docs/design/decisions.md +++ b/docs/design/decisions.md @@ -820,6 +820,24 @@ Every number below is a py3.11 measurement of 2026-08-31, recomputable with `uv - 2026-07-29 (script-ranges relocation) — REVERSES #271's documented choice ("tables deliberately live in _vocab"): the tables moved to _policy because packs cannot import the pipeline, evidenced by two hand copies with sync tests and a blocked 16–30× compiled-regex optimization (a module-level re.Pattern in a pack would flip its classification, hence the closure-held-pattern convention). Rider: U+3006 〆 joined the HAN span on a justification deliberately beyond UAX #24 — it is Script=Common but appears solely in Japanese surnames (〆木, 〆谷, 〆野). +### review-agent-single-source — the review axes live in one file + +- 2026-08-31 (#473) — `.claude/agents/design-docs-reviewer.md` restated AGENTS.md's review axes and, being untracked, drifted three ways against the file it implements: axis 10 never arrived, axis 4's outward half never arrived, and and the corpus-population caution kept a wording AGENTS.md itself records as corrected. That third one carried a second defect neither copy could show: BOTH quoted a corpus measurement that decisions.md#P6 had already superseded, so the endorsed wording was stale too. AGENTS.md's caution is rewritten in this change to carry the lesson without a standing count. The drift was one-directional — every correction landed in the tracked, reviewed, diffable file and none reached the copy. `.claude/agents/` and `.claude/commands/` are now tracked, and the agent points at AGENTS.md rather than restating it. The general form: when a duplicate carries no independent content, delete it; a duplicate is only worth keeping when it says something its source does not. + +Declined: a test pinning the agent's axis headings against AGENTS.md's list, which is what #473 proposed. Heading comparison would have caught the missing axis 10 and NEITHER of the other two drifts — both changed body prose and left headings intact — so the test detects a third of the defect while licensing the arrangement that produced all of it. Same shape as the `STABLE_TAGS` twin flagged in `nameparser/_types.py`, and the same remedy applies there if it is ever revisited. + +### two-input-invariants — checking a rule against something that is not the code + +- 2026-08-31 (#473; earned by #461 and #466) — AGENTS.md axis 11. Axes 1–10 compare a document to another document, to its own examples, or to the implementation, so all ten can only find code that disagrees with a rule; none can find a rule that is wrong. #461 was backed out and #466 closed unmerged for exactly that reason, a month apart, each after a review round that found real defects and missed the premise beneath them. An invariant relating two DIFFERENT inputs consults no rule statement and is not blind that way. + +- The worked invariant: for a name written `Family Given Particle`, the family-comma writing under the default order and the comma-less writing under FAMILY_FIRST must parse alike. Measured 2026-08-31 on 9f1d46e, comparing the non-empty role fields over `nameparser.config.particles.PARTICLES` × six families × six givens: it holds on all 36 pairs for 67 of the 70 particles, and fails on all 36 for `do`, `mc` and `vd` — precisely the words that are also suffix vocabulary, which rules.md#P6's Accepted consequence already names as the contested memberships. The invariant found that set from behavior alone, having read no rule. + +- Recompute: for each P in PARTICLES compare the non-empty role fields of `Parser().parse(f"{F}, {G} {P}")` against `Parser(policy=Policy(name_order=FAMILY_FIRST)).parse(f"{F} {G} {P}")` across any spread of families and givens that are not THEMSELVES particle vocabulary. That restriction is load-bearing, not hygiene: with F or G drawn from PARTICLES the correspondence fails for every particle, because the spread word joins the particle run and the invariant is then measuring a different shape (`Doe, Van de` gives given "Van" middle "de"; `Doe Van de` under FAMILY_FIRST gives given "Van de"). Measured, that is 0 of 70 rather than 67 of 70, so a reviewer who picks a particle word as a family — a natural choice when probing particle behavior — would report P6 broken. The argument survives the digits moving: what carries it is that the failures partition onto the particle-and-suffix words, not how many pairs there are. + +Declined: #473's "0 of 216 → 216 of 216" framing — but not re-argued here, because decisions.md#P6 already records that number's provenance (a script slicing `sorted(particles)[:14]`, which excluded exactly do, mc and vd), already names it axis 2 in its purest form, and already closes "Recompute over the WHOLE vocabulary, never a sample." Cite that entry for why the figure cannot be trusted; a second account of it in this file is the duplication decisions.md#review-agent-single-source was written to stop. #473's companion "72 of 216 — the ambiguous-particle half" measures a DIFFERENT partition of the vocabulary from the one above and must not be read as agreeing with it: the ambiguous particles are the 37 that are not never-given, and decisions.md#P6 records that #466 failed because a never-given test at the new site would have excluded all 37, `van` among them — not because of the three particle-and-suffix words. Two partitions, two different failures. + +Open: #469 — whether the invariant gets an executable form alongside the family-first input-shape documentation. + ### 3-0-reevaluations — decisions shaped by the v1 shim Promoted 2026-08-15 from session memory (Derek's 2026-07-30 ask; promotion approved 2026-08-15). Discipline when appending: mark each entry (A) "would decide differently without the shim" — real 3.0 work — or (B) "cited 1.4 parity but stands on its own", recorded so nobody re-litigates it. Append here whenever a design choice cites 1.4 parity or the shim as a load-bearing reason. Standing discipline for the removals themselves: every removal warns in a RELEASED version first — the rule established by the 1.3.0 eq/hash work (#223/#224), the reason the v1.4 milestone existed, and the reason FACADE-CONTRACT's "warning-free on 1.4" anchor works at all. 3.0's shim removals follow the same bridge. The CONVERSE rule, declined-with-reasoning twice in the migration design (2026-07-11): a bridge warning must WAIT when the replacement does not yet exist — 1.4 warned on neither shared-CONSTANTS mutation nor the subclass hooks, because users had no actionable response until 2.0, and #262's contract docs shipped in the same release (warn-while-documenting is a mixed message).