fix: suffixes_prefixes_titles always reflects current set state#166
Open
gaoflow wants to merge 1 commit into
Open
fix: suffixes_prefixes_titles always reflects current set state#166gaoflow wants to merge 1 commit into
gaoflow wants to merge 1 commit into
Conversation
The `suffixes_prefixes_titles` property on `Constants` cached its result in `_pst` after the first access. Any subsequent `add()` or `remove()` call on `titles`, `prefixes`, `suffix_acronyms`, or `suffix_not_acronyms` was silently ignored by the cache, so `is_rootname()` kept returning the stale result. Concretely, a word added to `C.titles` after the property was first accessed would still be treated as a rootname (first/middle/last) by `join_on_conjunctions`, even though `is_title()` correctly returned `True` for it. This contradicts the documented behaviour of per-instance config customisation described in AGENTS.md. Fix: drop the `_pst` cache entirely and compute the union fresh on every access. The four-set union is cheap and the simplest correct approach. Add five tests that assert the property and `is_rootname` stay consistent with the live sets after `add()`/`remove()` calls.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Constants.suffixes_prefixes_titlescached its result in_pstafter thefirst access and never invalidated that cache. Any subsequent
add()orremove()call ontitles,prefixes,suffix_acronyms, orsuffix_not_acronymswas silently ignored by the stale cache.This creates an observable inconsistency:
is_title()/is_prefix()/is_suffix()query the live sets and return the correct answer, butis_rootname()— which delegates tosuffixes_prefixes_titles— keepsreturning the stale cached answer, causing it to contradict the other helpers.
Minimal reproduction:
The same inconsistency affects titles and suffixes added or removed after the
property is first read.
join_on_conjunctionsrelies onis_rootnametocount
rootname_piecesand choose whether to join single-letter conjunctions,so a stale
_pstcan silently skew that decision.Fix
Drop
_pstentirely and compute the set union fresh on every access. Theunion of four
SetManagerinstances is a simpleO(n)operation with a smallconstant and is called only during name parsing, so there is no meaningful
performance cost.
Tests
Five new tests in
tests/test_constants.pyverify that:suffixes_prefixes_titlesreflects titles and prefixes added afterconstruction.
suffixes_prefixes_titlesno longer contains a title that was removed.is_rootnameandis_title/is_prefixremain consistent afteradd()/remove()calls.All 682 tests pass (672 pre-existing + 10 from the dual-run fixture applied
to the 5 new test methods).
This pull request was prepared with the assistance of AI, under my direction and review.