Skip to content

fix(dialect): whole-word entity match + wire lang_regex into extraction (#50, #53) - #2099

Open
KeilerHirsch wants to merge 3 commits into
MemPalace:developfrom
KeilerHirsch:fix/dialect-extraction-accuracy
Open

fix(dialect): whole-word entity match + wire lang_regex into extraction (#50, #53)#2099
KeilerHirsch wants to merge 3 commits into
MemPalace:developfrom
KeilerHirsch:fix/dialect-extraction-accuracy

Conversation

@KeilerHirsch

Copy link
Copy Markdown
Contributor

What

Two independent extraction-accuracy fixes in dialect.py (audit Group 8), both directly relevant to non-English (German) content.

#53encode_entity substring collision

The fallback loop matched a registered name as a bare substring of the query, so a short registered name wrongly claimed a longer, unrelated name's code:

# "Ann" registered  →  encode_entity("Annabelle") returned Ann's code
if key.lower() in name.lower():   # "ann" in "annabelle" == True

Fixed to a whole-word match (re.search(r"\b{escape(key)}\b", ...)): "Alice" still matches "Alice Cooper", but "Ann" no longer matches "Annabelle". The same substring bug existed in the key-sentence decision-word scoring ("key" matched inside "monkey") and is fixed the same way.

#50lang_regex loaded but never used

Dialect.__init__ loaded self.lang_regex = get_regex() but _extract_topics / _detect_entities_in_text / _extract_key_sentence never consulted it, so non-English content ran through ASCII-only English patterns:

  • The tokenizer [a-zA-Z][a-zA-Z_-]{2,} split German words at umlauts (GrößeGr/e).
  • The English-only decision-word list scored German decision sentences at ~0.

Now __init__ derives _stop_words, _topic_pattern, and _decision_words from self.lang_regex, falling back to the English module defaults (_STOP_WORDS, a new _DECISION_WORDS, and a unicode-aware topic pattern) when a locale omits a key. Added regex.decision_words to en.json (mirrors the English default) and de.json (German). The other 13 locales fall back to the English defaults — no guessed translations.

Regression fix (caught mid-work by the existing suite)

My first en.json topic_pattern ([A-Z][a-z]{2,}|…) was more restrictive than the original hardcode and split GraphQLGraph, breaking test_boosts_technical_terms. Replaced with a unicode-aware [^\W\d_][\w-]{2,} that keeps umlaut words whole and preserves CamelCase/snake_case/kebab-case.

Honest scope notes

  • English behavior: no change on the existing test suite or normal English prose. The substring→whole-word swap is a deliberate, correctness-improving behavior change that only fires on edge inputs ("key" in "monkey"); it is not a bit-for-bit no-op, so this is worded as "no observed change on English text," not "identical."
  • Intended side effect: the unicode-aware topic_pattern now treats getUserById / kebab-case-word as single compound tokens (previously split at each internal capital/hyphen). This is the same "don't split at internal boundaries" property that fixes the GraphQL bug.
  • Out of scope (follow-ups, not this PR): (a) encode_entity with an empty-string registered key matches any query via a zero-width \b\b — pre-existing (the old substring code had the identical bug), worth a guard later. (b) Dialect.__init__ calls the module-level i18n.load_lang(), mutating process-global locale state; harmless for single-threaded CLI/test use but a latent race if Dialect is ever constructed concurrently with different lang= (e.g. an MCP server serving multiple locales). Neither is introduced by this PR.

Tests

tests/test_dialect_extraction_accuracy.py — 4 tests (3 demonstrate the regressions RED-before / GREEN-after, 1 positive whole-word check), plus an autouse fixture that restores i18n's process-global current language after each test so Dialect(lang="de") does not leak German into later tests. Verified: 3 bug-tests RED before the fix, all GREEN after; broad suite (-k "dialect or compress or i18n or extract or miner or convo") 519 passed / 0 failed; ruff check + ruff format --check clean.

Audit Group 8. MemPalace#53: encode_entity's fallback matched a registered name as a bare substring ('Ann' claimed 'Annabelle'); now requires a whole-word match (\b...\b), preserving legitimate multi-word matches ('Alice Cooper'->Alice). MemPalace#50: _extract_topics/_detect_entities_in_text/_extract_key_sentence now consult self.lang_regex (topic_pattern, stop_words, decision_words) with English module defaults as fallback, so German content is no longer tokenized by an ASCII-only pattern (umlaut words were split) or scored against an English-only decision-word list. Added regex.decision_words to en.json (mirrors default) and de.json (German).
…cision_words

Regression fix: the first en.json topic_pattern ([A-Z][a-z]{2,}|...) was more restrictive than the original hardcode and split 'GraphQL'->'Graph' (broke test_boosts_technical_terms). Replaced with unicode-aware [^\W\d_][\w-]{2,} in en.json/de.json and the code fallback: preserves CamelCase/snake_case/kebab-case AND keeps umlaut words whole. Test assert on decision sentence now checks the sentence START (survives the ~55-char truncation) instead of a trailing word.
Rewrote the en.json/de.json changes as minimal in-place edits (2 keys each: topic_pattern + new decision_words) instead of a json.dump re-serialization that reflowed the whole file into 600+ lines of formatting noise. Added an autouse fixture restoring i18n's process-global _current_lang after each test, so Dialect(lang='de') in the new tests no longer leaks German into a later Dialect() with no lang (was failing test_extract_key_sentence only in combined test order).

@fatkobra fatkobra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: the same substring collision still exists in the user-facing
plain-text compression path.

encode_entity() now uses a whole-word match, but compress() does not call
that method. It calls _detect_entities_in_text(), which still does:

if not name.islower() and name.lower() in text.lower():

Therefore, with entities={"Ann": "X99"}, compressing text that contains
Annabelle can still record X99 even though Ann was never mentioned.

The new regression tests call encode_entity() directly, so they do not
exercise the normal compress() path.

Please use one shared entity-boundary helper in both encode_entity() and
_detect_entities_in_text(), and add a regression test through compress()
or _detect_entities_in_text() proving that:

  • Annabelle does not match the registered entity Ann;
  • Alice Cooper still matches the registered entity Alice.

It would also be useful for the shared helper to handle registered entities
that begin or end with punctuation, for example .NET, C++, C#, or
@alice; explicit (?<!\w) / (?!\w) boundaries are safer for those than
\b...\b.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants