Repository navigation
FIX: make PUZZLED keyword selection independent of Unicode normalization form - #3524
Open
Nikhil Gupta (Denimworld12) wants to merge 1 commit into
Open
Nikhil Gupta (Denimworld12) wants to merge 1 commit into
Nikhil Gupta (Denimworld12) wants to merge 1 commit into
Conversation
…ion form _pos_lookup dropped every word carrying a combining mark, because token.is_alpha is str.isalpha and that is false once a mark is present. Pointed scripts were never tagged in any normalization form, and composed and decomposed spellings of the same word were given different parts of speech by the tagger, so the same prompt selected a different word to mask. Accept words with _MASKABLE_WORD_PATTERN, the test _iter_words already uses, and normalize the prompt before tagging. A 400-prompt randomized sweep with spaCy installed disagreed between NFC and NFD on 294 prompts; it now disagrees on none.
Richard Lundeen (richlundeen)
approved these changes
Oct 25, 2026
Richard Lundeen (richlundeen)
left a comment
Contributor
There was a problem hiding this comment.
Looks good. Normalizing the tagger input and reusing the existing word pattern fixes the inconsistent keyword selection without changing the original prompt.
Reviewed with GitHub Copilot.
This branch has not been deployed
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.
Description
PuzzledConverterpicks different words to mask, and prints different clues, depending onwhether the prompt arrives composed (NFC) or decomposed (NFD). The same sentence gives two
different puzzles.
This only shows up when spaCy is installed, which is why it has not surfaced: spaCy is an
optional extra, and every Unicode test in the suite is decorated
@_no_spacy.A randomized sweep of 400 prompts over an accented vocabulary, with spaCy live, disagrees
between NFC and NFD on 294 of 400. With this change, 0 of 400.
Follow-up to #3155.
Cause
Both causes live in
_pos_lookup, which is the one place in the module that had not beenmade normalization-safe.
1.
token.is_alphadiscards tags spaCy already produced. It istoken.text.isalpha(),which is
Falsefor any text carrying a combining mark. So the gate drops marked words eventhough the tagger labelled them:
is_alphapos_café(NFC)café(NFD)שָׁלוֹםعَرَبِيّहिन्दीPointed scripts are never tagged, in any normalization form — the same scripts #3155 made
maskable. They always fall back to the generic
"word"clue.2. The tagger itself is normalization-sensitive. NFC
cafétagsNOUN; NFDcafétagsADJ. Context propagates, so neighbouring ASCII words shift too (documentscame backVERBin one form andNOUNin the other). No gate change can fix this one — the text hasto be normalized before it reaches the pipeline.
Because the part of speech decides a tier in
_rank_candidates, a dropped or altered tagdoes not just change the clue text, it changes which word gets masked.
Changes
Two lines in
_pos_lookup:_MASKABLE_WORD_PATTERNis the module's own\p{L}[\p{L}\p{M}]*, already used by_iter_wordsto decide what a maskable word is. Using it here means the scanner and thetagger finally agree on that definition — which the module docstring already says they do
("Scanning and validation use the same Unicode properties").
Tagging is normalized to NFC only for the pipeline's benefit; the keys are still built with
_word_key, so lookups against prompt words in any form keep matching.Tests
Three tests in
tests/unit/converter/test_puzzled_keyword_masker.py. Onmainthe file is3 failed, 107 passed; with this change it is 110 passed, so the fix changes nothing
the existing tests assert:
test_pos_lookup_tags_words_with_combining_marks— a token whoseis_alphaisFalsestill contributes its tag.
test_pos_lookup_normalizes_before_tagging— the pipeline receives NFC text.test_mask_prompt_selection_is_stable_across_normalization_forms— NFC and NFD of oneprompt mask the same word with the same clue.
The existing
_FakeTokengained no new default: the gate test passesis_alpha=Falseexplicitly, and the recording fake derives it with
str.isalpha()the way spaCy does, sothe fakes now model the real behaviour instead of assuming it.
No new dependency: the tests use fakes, so they run whether or not spaCy is installed.