Skip to content

PRO-824: Use Unicode-aware regex in ambiguous text check - #1730

Open
SteveJonesDev wants to merge 1 commit into
developfrom
steve/pro-824-localization-and-phrase-filters
Open

PRO-824: Use Unicode-aware regex in ambiguous text check#1730
SteveJonesDev wants to merge 1 commit into
developfrom
steve/pro-824-localization-and-phrase-filters

Conversation

@SteveJonesDev

@SteveJonesDev SteveJonesDev commented May 29, 2026

Copy link
Copy Markdown
Member

Summary

  • Replaces the ASCII-only /[^a-z]+/g regex with the Unicode-aware /[^\p{L}]+/gu variant so that non-Latin characters (e.g. æ, ø, å) are preserved during normalization rather than stripped
  • Normalizes the translated phrases array with the same regex so both sides of the comparison are treated consistently — prevents translator punctuation from causing false negatives
  • Fixes detection of ambiguous link text in non-English sites (e.g. Danish "Læs mere", "Klik her")

Closes #84

Test plan

  • Verify English ambiguous phrases still trigger (e.g. "Read more", "Click here", "Learn more")
  • Verify a non-Latin ambiguous phrase triggers when the matching translation exists in the bundle (e.g. Danish "Læs mere")
  • Verify phrases with punctuation variants (e.g. "more...") still match correctly
  • Verify aria-label and aria-labelledby paths work the same way

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced ambiguous text detection to provide better accuracy with international text and non-ASCII character handling.

Review Change Stack

…languages

Fixes detection of translated ambiguous phrases (e.g. Danish "Læs mere") by
replacing the ASCII-only /[^a-z]+/g regex with the Unicode-aware /[^\p{L}]+/gu
variant. Also normalizes the phrases array on both sides of the comparison so
translator punctuation variations don't cause missed matches.

Closes #84

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 067ed752-72a4-4d26-8deb-7297edef82cb

📥 Commits

Reviewing files that changed from the base of the PR and between 6986dfa and 42abe66.

📒 Files selected for processing (1)
  • src/pageScanner/checks/has-ambiguous-text.js

📝 Walkthrough

Walkthrough

The ambiguous text detection in has-ambiguous-text.js shifts from ASCII-only character filtering to Unicode letter-based normalization. The change precomputes normalized phrase variants by lowercasing and filtering to Unicode letters, then applies matching normalization to the input text before comparison. This improves consistency and performance by normalizing phrases once during initialization rather than repeatedly during checks.

Changes

Ambiguous Text Detection Normalization

Layer / File(s) Summary
Unicode-based phrase normalization in ambiguous text detection
src/pageScanner/checks/has-ambiguous-text.js
Precomputed normalizedPhrases now normalizes each translated phrase to lowercase with Unicode letter characters only. checkAmbiguousPhrase applies matching normalization to input text and compares against the precomputed set, replacing the previous ASCII-only filtering approach.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

codex

Suggested reviewers

  • pattonwebz

Poem

In phrases lost to tongues diverse,
Unicode letters now converse—
No ASCII bounds restrict the way,
Ambiguity caught throughout the day! 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: switching from ASCII-only regex to Unicode-aware regex for ambiguous text detection.
Linked Issues check ✅ Passed The PR successfully addresses the linked issue #84 by adding Unicode-aware normalization to support non-Latin languages in ambiguous phrase detection.
Out of Scope Changes check ✅ Passed All changes are focused on the ambiguous text check normalization logic and directly support the Unicode language support objective from issue #84.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch steve/pro-824-localization-and-phrase-filters

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request improves the ambiguous text check by pre-normalizing phrases and utilizing a Unicode-aware regular expression to strip non-letter characters. The reviewer noted that using only \p{L} will strip combining diacritical marks (\p{M}) used in many languages, potentially breaking words and causing matching to fail. They recommended including \p{M} in the regex and applying .normalize() to both the predefined phrases and the input text to ensure consistent and robust matching.

Comment on lines +26 to +28
const normalizedPhrases = ambiguousPhrases.map(
( p ) => p.toLowerCase().replace( /[^\p{L}]+/gu, ' ' ).trim()
);

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.

high

Using only \p{L} (Letters) in the Unicode regex will strip combining diacritical marks (\p{M}), which are used in many languages (e.g., Hindi, Arabic, Thai, or even Latin languages when strings are in NFD/decomposed form, such as text copied from macOS). Stripping these marks will break words into fragments and cause matching to fail.

To prevent this, we should:

  1. Include \p{M} in the character class: /[^\p{L}\p{M}]+/gu.
  2. Use .normalize() to ensure consistent Unicode normalization (NFC) before processing.
Suggested change
const normalizedPhrases = ambiguousPhrases.map(
( p ) => p.toLowerCase().replace( /[^\p{L}]+/gu, ' ' ).trim()
);
const normalizedPhrases = ambiguousPhrases.map(
( p ) => p.normalize().toLowerCase().replace( /[^\p{L}\p{M}]+/gu, ' ' ).trim()
);

Comment on lines +34 to +35
text = text.toLowerCase().replace( /[^\p{L}]+/gu, ' ' ).trim();
return normalizedPhrases.includes( text );

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.

high

Apply the same Unicode-aware normalization (including \p{M} and .normalize()) to the input text to ensure consistent matching with the normalized phrases.

Suggested change
text = text.toLowerCase().replace( /[^\p{L}]+/gu, ' ' ).trim();
return normalizedPhrases.includes( text );
text = text.normalize().toLowerCase().replace( /[^\p{L}\p{M}]+/gu, ' ' ).trim();
return normalizedPhrases.includes( text );

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 42abe667f7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

}
text = text.toLowerCase().replace( /[^a-z]+/g, ' ' ).trim();
return ambiguousPhrases.includes( text );
text = text.toLowerCase().replace( /[^\p{L}]+/gu, ' ' ).trim();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve combining marks when normalizing Unicode text

For locales with accented translations, this drops combining marks instead of treating canonically equivalent text the same. For example, the Vietnamese translation for “continue” is tiếp tục; if the DOM contains the same visible text in decomposed form from copy/paste, textContent normalizes to something like tie p tu c while the precomposed translated phrase normalizes to tiếp tục, so the ambiguous link is missed. Include marks in the character class or normalize both strings to the same Unicode form before stripping punctuation.

Useful? React with 👍 / 👎.

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.

Localization and Phrase filters

1 participant