fix: vocabulary replacement fails for English words adjacent to CJK characters#93
fix: vocabulary replacement fails for English words adjacent to CJK characters#93sysalpha01 wants to merge 1 commit into
Conversation
…haracters
The word boundary regex used \p{L} (all Unicode letters) which includes
CJK characters, preventing replacement of English words when they appear
next to Japanese/Chinese/Korean text (e.g., "Xavixの設定" would not
replace "Xavix" because "の" matched \p{L} in the lookahead).
Changed \p{L} to \p{Script=Latin} so the word boundary check only
considers Latin script characters. This allows vocabulary replacements
to work correctly in CJK contexts while still preventing partial matches
within Latin words (e.g., "apple" in "pineapple" is still protected).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe change modifies the word-boundary regex in the non-CJK branch of Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
JiwaniZakir
left a comment
JiwaniZakir
left a comment
There was a problem hiding this comment.
The fix in text-replacement.ts correctly unblocks the CJK adjacency issue, but the change from \p{L} to \p{Script=Latin} introduces a new edge case worth considering: non-Latin, non-CJK scripts such as Cyrillic, Arabic, Hebrew, or Devanagari are no longer treated as word-boundary blockers. For example, if the vocabulary entry is "API" and the input text contains something like "APIв" (a Latin word directly concatenated with a Cyrillic character), the new regex would match and replace "API" there, whereas the original \p{L} lookahead would have correctly blocked it. The comment says "Only check Latin script boundaries" but doesn't acknowledge this trade-off — if this behavior is intentional (i.e., the assumption is that vocabulary terms are always Latin and adjacency to any non-Latin script is fine), that rationale should be documented explicitly. A more surgical fix might instead use a negative lookahead/lookbehind that excludes only ideographic/CJK blocks (\p{Script=Han}, \p{Script=Hiragana}, \p{Script=Katakana}, etc.) while keeping the broader \p{L} guard for other alphabetic scripts. It would also be worth adding test cases covering the Cyrillic/Arabic adjacency scenario to codify the intended behavior.
2 participants
Summary
\p{L}to\p{Script=Latin}in word boundary lookahead/lookbehindProblem
The word boundary regex in
applyTextReplacements()uses\p{L}(all Unicode letters) in negative lookahead/lookbehind. Since\p{L}matches CJK characters, English words adjacent to Japanese text (e.g.,XavixinXavixの設定) are never replaced because the CJK character triggers the boundary check.Example
With vocabulary entry
Xavix → ZABBIX:Xavixの設定Xavixの設定(no replacement -のmatches\p{L})ZABBIXの設定(correctly replaced -のdoesn't match\p{Script=Latin})Changes
apps/desktop/src/utils/text-replacement.ts:\p{L}with\p{Script=Latin}in the word boundary regexTest Plan
Xavixの設定→ZABBIXの設定works correctlypineappledoes not haveapplereplaced (Latin boundary still works)ザビックス→ZABBIX) still workSummary by CodeRabbit