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
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ The 2.0 rewrite landed as underscore-private modules beside the v1 code, which 2
- **Validation is eager and fail-loud**: every `raise` states the offending value, the expected form, and the fix. Exception taxonomy: wrong type — including wrong element type inside a collection, bare `str` where an iterable of strings is expected, or a `Mapping` where a plain iterable is expected — raises `TypeError`; well-typed but unacceptable values raise `ValueError`; failed enum lookups stay `ValueError` for any input (stdlib `EnumType` precedent). **When the message hands the reader code to paste, that code has to survive a type checker, and then RUN** — nameparser ships `py.typed`, and a placeholder can type-check and still raise: the 2.4 release sweep's first fix to the `capitalization_exceptions` hint offered `('key', 'Value')`, which passes mypy and fails #459's mask check, so `test_add_capitalization_exceptions_raises_pointing_at_replace` now executes the offered code. #337's segmenterless warning offered `Policy(segment_scripts=())`, an `arg-type` error, because these fields are annotated with what they STORE rather than everything the constructor accepts. Prefer the `frozenset()` / `()` spellings in messages and docstrings, and pin the offered spelling in a test — the warning tests matched on `ja_segmenter` and never checked the actionable half of the message. **A warning emitted in `Parser.__post_init__` needs `parser_for` to re-emit it from its own frame** (the `catch_warnings(record=True)` block at its return): `__post_init__`'s `stacklevel` is sized for direct `Parser(...)` construction, and through `parser_for`'s extra frame the default one-line rendering attributes the warning to the library's own `return Parser(...)` — the exact call the message tells the user to change becomes invisible. No single stacklevel serves both entry points; a new construction warning gets the re-emission for free, but a new CONSTRUCTION SITE for `Parser` inside this package needs its own re-emission or its callers get library-attributed warnings (#337 review).
- **Guard, hint, and emit for the WHOLE family, and parametrize the test over it**: a check added to one member of a set belongs on all of it, and the test must sweep the family, not one example. This session shipped `_reject_str_and_mapping` on `Policy` but not `PolicyPatch`, the bytes decode hint on three of five config entry points, and a regex-sync roster missing four of its copies — each a separate follow-up bug that a `{class} × {field} × {bad-value}` parametrization would have caught and a per-example test hid. When you find you're guarding member N, grep for the other members first.
- **Ambiguities are emitted at the DECISION site**: an `Ambiguity` records a fork the parse had to call, not a token that sits in an ambiguous vocabulary, so emit where the branch is taken, in every stage that takes one, and name the reading not taken. The reading TAKEN (`Token.reading`, 3.0) is the one read later, at assemble, where roles are final, from the role and the tags the deciding stages left, its suffix and name sources by the predicates that name the reading not taken. The contract is mechanisms.md#AMBIGUITY-AT-THE-DECISION-SITE; the working rules (branches that change nothing, the field a report names, the stages that decide `PARTICLE_OR_GIVEN`, `alternative`, the reading taken, and how to test an emitter) are in `nameparser/_pipeline/AGENTS.md`, which loads when a session reads a pipeline stage.
- **A kind is worth adding only if a reader would hesitate too**: the test is not "does the code take a branch" but whether a person reading that input would genuinely be unsure. "Smith, John V" reads as a middle initial to anyone -- the comma settles it -- so reporting it would be noise that teaches callers to ignore the field, which costs more than the missing report. Reachability of the second branch is necessary, not sufficient. Prefer leaving a fork silent and documenting the omission over emitting on input nobody finds ambiguous.
- **A kind is worth adding only if a reader would hesitate too**: the test is not "does the code take a branch" but whether a person reading that input would genuinely be unsure. "Smith, John V." reads as a middle initial to anyone -- the period settles it -- so reporting it would be noise that teaches callers to ignore the field, which costs more than the missing report. Reachability of the second branch is necessary, not sufficient. The example was "Smith, John V" until #877 measured FEC writing that bare letter as a middle initial about as often as letter frequency predicts, where the parser reads the generation: a fork that feels settled is a claim to measure against the data before it decides a report. Prefer leaving a fork silent and documenting the omission over emitting on input nobody finds ambiguous.
- **Parser owns config-dependent conveniences**: `Parser.matches`/`Parser.capitalized`/`Parser.revise` exist because the `ParsedName` equivalents fall back to DEFAULT config for str/omitted arguments (documented loudly in both docstrings). `revise` harvests tokens from a full sub-parse of each replacement value (tags kept minus `FOLDED_TAG`, roles forced, the R1 entry pass `suffix_entries` re-run over the forced state so a suffix value's entries follow its own commas, ambiguities discarded); the merge tail is shared with `replace()` via `ParsedName._with_field_tokens`. `Parser.capitalized` delegates through `name.capitalized(self.lexicon)` specifically so `_parser` never imports `_render` — keep it that way.
- **A reading is of one word, save a birth marker and a title addressing the given name** (the build's check 8, which `_lexicon._make_edit` runs on every `add` and RAISES on, since a reading is now a caller's statement the compile can refuse as the build does: 3.0's 4i-b). The two lists, `surnames` and `given_names`, and `capitalization_exceptions` keys still WARN on a multi-word entry (`_normset`/`_normpairs` via `_warn_at_caller`, UserWarning, never a raise — an inert entry produces nothing). The default vocabulary and every locale pack must stay warning-free (`test_default_lexicon_builds_warning_free`, `test_pack_vocabulary_entries_are_single_words`).
- **Invariants guard harm, not no-ops**: add a constructor check when violating it produces a *wrong parse*, not when it produces *nothing*. A false positive costs a working configuration; a true positive on an inert condition costs the user nothing, so that trade is never worth taking. Since 3.0's 4i-b a `Lexicon` compiles from readings, which hold the 2.x marker-over-base relations by construction; the checks a caller's edit reaches are the build's checks 3 (a kind and its properties, `may_be_initial` on a single cased letter alone), 4 (one suffix kind per word), 8 (a phrase), 9 (a bound given name that is a particle has a `given` reading) and 11's agnomen clause (an agnomen has a name reading), in `_lexicon._make_edit` and `_check_pool`, which `_lexicon._built` runs again under a choice of languages, refusing a caller's reading that clashes with one the choice turns on and naming the edit, each raising with the call that fixes it, because each is a wrong parse. A given-addressing title run the parse can never build is not checked, because an unreachable entry is simply never consulted (see Gotchas). Before adding one, construct the config it forbids and check what actually breaks. **Between raise and silence sits the construction-time `UserWarning`**, for a gap that is statically decidable, harmless to SOME deliberate caller, and indistinguishable-from-working for everyone else: the segmenterless activation (#337 — `parser_for(locales.JA)` without a segmenter behaved exactly like a working parser minus the feature) warns rather than raises because the inert JA registration is itself a pinned property, and a warning is filterable by the caller who wants exactly that. The message must carry every applicable remedy and no inapplicable one (the `ja_segmenter` hint fires only when a Japanese script is among the dead ones). Test fuzzers that legitimately construct such configs suppress the warning by MESSAGE, never by category — a blanket `UserWarning` ignore would mask the next construction diagnostic (`_quiet_parser` in `tests/v2/test_properties.py` is the pattern).
Expand Down
Loading
Loading