chore: add sentence case validation to verify-locales script #38221
+420
−106
Conversation
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
Contributor
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
metamaskbot
added
the
team-design-system
georgewrmarshall
changed the title
georgewrmarshall
added
the
no-changelog
Copilot finished reviewing on behalf of
georgewrmarshall
November 25, 2025 00:27
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enforces consistent sentence case capitalization across all English locale strings by adding automated validation to the verify-locales script. The validation detects title case violations (e.g., "Price Impact" → "Price impact") and can automatically fix them with the --fix flag. The PR includes a configuration file for special case exceptions (branded terms, product names, security terminology), preserves quoted text capitalization, and corrects 46 existing violations in the English locale files.
Key Changes
- Added sentence case validation logic with pattern detection and auto-fix support
- Created exceptions configuration for branded/technical terms that should maintain title case
- Fixed 46 capitalization violations across English locale files
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| development/verify-locale-strings.js | Adds 170+ lines implementing sentence case validation with detection, conversion, and auto-fix logic |
| app/_locales/sentence-case-exceptions.json | New configuration file defining 41 terms exempt from sentence case rules (brands, products, networks, acronyms) |
| app/_locales/en/messages.json | Corrects 46 title case violations to sentence case |
| app/_locales/en_GB/messages.json | Auto-synced with en locale corrections |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
development/verify-locale-strings.js
Outdated
Comment on lines
503
to
511
| if (sentenceCaseViolations.length > 0 && fix) { | ||
| const newLocale = { ...englishLocale }; | ||
| for (const violation of sentenceCaseViolations) { | ||
| if (newLocale[violation.key]) { | ||
| newLocale[violation.key].message = violation.suggested; | ||
| } | ||
| } | ||
| await writeLocale('en', newLocale); | ||
| } |
There was a problem hiding this comment.
When both unusedMessages and sentenceCaseViolations exist and --fix is used, the locale file is written twice (lines 495-500, then 503-511). The second write will overwrite the first, losing the unused message deletions. The fixes should be combined into a single write operation. Merge both changes into one newLocale object before calling writeLocale() once.
| "message": "Priority fee (aka “miner tip”) goes directly to miners and incentivizes them to prioritize your transaction." | ||
| }, | ||
| "airDropPatternDescription": { | ||
| "message": "The token's on-chain history reveals prior instances of suspicious airdrop activities." |
Contributor
Author
There was a problem hiding this comment.
Fixing all sentence case violations using updated script yarn verify-locales:fix
| }, | ||
| "notificationItemCheckBlockExplorer": { | ||
| "message": "Check on the Block Explorer" | ||
| "message": "Check on the block explorer" |
Contributor
Author
There was a problem hiding this comment.
Not sure about this one. Is "Block Explorer" considered a special term @coreyjanssen?
| }, | ||
| "rewardsOnboardingStep4LegalDisclaimer2": { | ||
| "message": "Supplemental Terms of Use and Privacy Notice" | ||
| "message": "Supplemental terms of use and privacy notice" |
Contributor
Author
There was a problem hiding this comment.
Should "Terms of Use" and "Privacy Notice" be special terms @coreyjanssen?
georgewrmarshall
force-pushed
the
fix-sentence-case
branch
from
4aef875 to
b1e70bc
Compare
| }, | ||
| "betaTerms": { | ||
| "message": "Beta Terms of use" | ||
| "message": "Beta terms of use" |
Contributor
Author
There was a problem hiding this comment.
Should "Terms of Use" be a special case @coreyjanssen?
| "message": "Priority fee (aka “miner tip”) goes directly to miners and incentivizes them to prioritize your transaction." | ||
| }, | ||
| "airDropPatternDescription": { | ||
| "message": "The token's on-chain history reveals prior instances of suspicious airdrop activities." |
Contributor
Author
There was a problem hiding this comment.
No need to check this file matches app/_locales/en/messages.json as the yarn verify-locales:fix automates copying the changes.
| @@ -0,0 +1,63 @@ | |||
| { | |||
Contributor
Author
There was a problem hiding this comment.
Special terms that are exempt and should remain Title Case or UPPERCASE
… from the words array before processing
- ~2,680 operations (one regex test per string) - ~50x faster performance improvement
… from the words array
Collaborator
Builds ready [b1e70bc]
UI Startup Metrics (1235 ± 96 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Implements 5 Copilot bot suggestions to improve code quality and fix bugs: 1. **Empty word guard**: Filter empty strings from word arrays to prevent false positives - Added `.filter(word => word.length > 0)` in hasTitleCaseViolation and convertToSentenceCase 2. **Performance optimization**: Pre-compiled regex for special case detection - Reduced complexity from O(n×m) to O(n) (~50x faster) - Built single regex at module load time instead of checking each exception per string 3. **Whitespace filtering**: Prevent edge cases with whitespace-only text - Filter empty strings after split to avoid `['']` arrays 4. **Overlapping terms fix** (CRITICAL BUG): - Sort special terms by position, then by length descending - Skip overlapping terms during processing - Prevents "MetaMask Portfolio" from being corrupted by overlapping "MetaMask" term All changes tested and validated with test scripts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Addresses Copilot suggestion about placeholder replacement safety.
**Issue**: Using the same placeholder `___QUOTED___` for all quoted strings
with `replace()` in a loop only replaces the first occurrence. While this
worked sequentially (each replace() removes one placeholder, making the next
one "first"), it was fragile and not immediately obvious.
**Fix**:
- Use unique placeholders: `___QUOTED_0___`, `___QUOTED_1___`, etc.
- Store placeholder-text pairs in objects: `{placeholder, text}`
- Check if word contains placeholder to handle punctuation edge cases
(e.g., `'Option A',` becomes `___QUOTED_0___,`)
**Testing**:
✅ All quoted strings correctly preserved
✅ Multiple quotes in sequence work correctly
✅ Mixed single and escaped double quotes handled
✅ Quoted text followed by punctuation preserved
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Collaborator
✨ Files requiring CODEOWNER review ✨👨🔧 @MetaMask/core-extension-ux (3 files, +4 -4)
🧪 @MetaMask/qa (2 files, +3 -3)
🔐 @MetaMask/web3auth (1 files, +2 -2)
|
Collaborator
Builds ready [056f18f]
UI Startup Metrics (1273 ± 102 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Fixed all linting issues: **require-unicode-regexp**: Added 'u' flag to all regex patterns - Line 50: escapeRegex function - Line 68: buildExceptionsRegex return - Lines 84-85: hasTitleCaseViolation quote removal - Line 89: hasTitleCaseViolation word split - Lines 97, 100: Title case pattern checks - Lines 190, 198: convertToSentenceCase quote replacement - Line 206: convertToSentenceCase word split **no-plusplus**: Replaced ++ with += 1 - Lines 193, 201: placeholderIndex increments **no-shadow**: Fixed variable shadowing - Line 227: Renamed destructured 'text' to 'quotedText' to avoid shadowing the outer 'text' parameter All changes verified - script still passes validation tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Comment on lines
+56
to
+57
| "Third Party Service", | ||
| "Third Party Services" |
Contributor
Author
There was a problem hiding this comment.
Should "Third Party Service/s" even be a special case? 🤔 cc @coreyjanssen
Copilot finished reviewing on behalf of
georgewrmarshall
November 25, 2025 02:25
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
46
to
262
| function buildExceptionsRegex(exceptions) { | ||
| const patterns = []; | ||
|
|
||
| // Escape special regex characters for exact matches | ||
| const escapeRegex = (str) => str.replace(/[.*+?^${}()|[\]\\]/gu, '\\$&'); | ||
|
|
||
| // Add exact matches (escaped to treat as literals) | ||
| exceptions.exactMatches.forEach((term) => { | ||
| patterns.push(escapeRegex(term)); | ||
| }); | ||
|
|
||
| // Add acronyms (escaped to treat as literals) | ||
| exceptions.acronyms.forEach((acronym) => { | ||
| patterns.push(escapeRegex(acronym)); | ||
| }); | ||
|
|
||
| // Add existing regex patterns (already in regex format) | ||
| Object.values(exceptions.patterns).forEach((pattern) => { | ||
| patterns.push(pattern); | ||
| }); | ||
|
|
||
| // Combine all patterns with OR operator | ||
| return new RegExp(patterns.join('|'), 'u'); | ||
| } | ||
|
|
||
| // Pre-compile the exceptions regex once at module load time | ||
| const specialCaseRegex = buildExceptionsRegex(sentenceCaseExceptions); | ||
|
|
||
| // Helper function to check if text contains special case terms | ||
| // Now uses pre-compiled regex for O(n) instead of O(n*m) performance | ||
| function containsSpecialCase(text) { | ||
| return specialCaseRegex.test(text); | ||
| } | ||
|
|
||
| // Helper function to detect title case violations | ||
| function hasTitleCaseViolation(text) { | ||
| // Remove quoted text (single quotes and escaped double quotes) before checking | ||
| // Quoted text refers to UI elements and should preserve capitalization | ||
| let textWithoutQuotes = text.replace(/'[^']*'/gu, ''); // Remove 'text' | ||
| textWithoutQuotes = textWithoutQuotes.replace(/\\"[^"]*\\"/gu, ''); // Remove \"text\" | ||
|
|
||
| // Ignore single words (filter out empty strings from whitespace) | ||
| const words = textWithoutQuotes | ||
| .split(/\s+/u) | ||
| .filter((word) => word.length > 0); | ||
| if (words.length < 2) { | ||
| return false; | ||
| } | ||
|
|
||
| // Check if multiple words start with capital letters (Title Case pattern) | ||
| // This pattern: "Word Word" or "Word Word Word" | ||
| const titleCasePattern = /^([A-Z][a-z]+\s+)+[A-Z][a-z]+/u; | ||
|
|
||
| // Also catch patterns like "In Progress", "Not Available" | ||
| const multipleCapsPattern = /\b[A-Z][a-z]+\s+[A-Z][a-z]+\b/u; | ||
|
|
||
| return ( | ||
| titleCasePattern.test(textWithoutQuotes) || | ||
| multipleCapsPattern.test(textWithoutQuotes) | ||
| ); | ||
| } | ||
|
|
||
| // Helper function to convert to sentence case while preserving special cases | ||
| function toSentenceCase(text) { | ||
| // If text contains special cases, we need to be careful | ||
| if (containsSpecialCase(text)) { | ||
| // Build a map of special terms and their positions | ||
| const specialTerms = []; | ||
|
|
||
| // Find all special terms from exact matches | ||
| for (const term of sentenceCaseExceptions.exactMatches) { | ||
| let index = text.indexOf(term); | ||
| while (index !== -1) { | ||
| specialTerms.push({ term, start: index, end: index + term.length }); | ||
| index = text.indexOf(term, index + 1); | ||
| } | ||
| } | ||
|
|
||
| // Find all acronyms | ||
| for (const acronym of sentenceCaseExceptions.acronyms) { | ||
| let index = text.indexOf(acronym); | ||
| while (index !== -1) { | ||
| specialTerms.push({ | ||
| term: acronym, | ||
| start: index, | ||
| end: index + acronym.length, | ||
| }); | ||
| index = text.indexOf(acronym, index + 1); | ||
| } | ||
| } | ||
|
|
||
| // Sort by position first, then by length descending (prefer longer matches) | ||
| // This ensures overlapping terms like "MetaMask Portfolio" are processed before "MetaMask" | ||
| specialTerms.sort((a, b) => { | ||
| if (a.start !== b.start) { | ||
| return a.start - b.start; | ||
| } | ||
| // If same start position, prefer longer match | ||
| return b.end - b.start - (a.end - a.start); | ||
| }); | ||
|
|
||
| // Build result preserving special terms | ||
| let result = ''; | ||
| let lastIndex = 0; | ||
|
|
||
| for (const special of specialTerms) { | ||
| // Skip overlapping terms (already covered by a previous term) | ||
| if (special.start < lastIndex) { | ||
| continue; | ||
| } | ||
|
|
||
| // Process text before this special term | ||
| const before = text.substring(lastIndex, special.start); | ||
| if (before) { | ||
| result += convertToSentenceCase(before); | ||
| } | ||
| // Add the special term as-is | ||
| result += special.term; | ||
| lastIndex = special.end; | ||
| } | ||
|
|
||
| // Process remaining text | ||
| if (lastIndex < text.length) { | ||
| result += convertToSentenceCase(text.substring(lastIndex)); | ||
| } | ||
|
|
||
| return result; | ||
| } | ||
|
|
||
| return convertToSentenceCase(text); | ||
| } | ||
|
|
||
| // Simple sentence case conversion | ||
| function convertToSentenceCase(text) { | ||
| if (!text) { | ||
| return text; | ||
| } | ||
|
|
||
| // Extract quoted text (single quotes and escaped double quotes) and preserve them | ||
| const quotedTexts = []; | ||
| let textToProcess = text; | ||
| let placeholderIndex = 0; | ||
|
|
||
| // Find all single-quoted text and replace with unique placeholders | ||
| textToProcess = textToProcess.replace(/'([^']*)'/gu, (match) => { | ||
| const uniquePlaceholder = `___QUOTED_${placeholderIndex}___`; | ||
| quotedTexts.push({ placeholder: uniquePlaceholder, text: match }); | ||
| placeholderIndex += 1; | ||
| return uniquePlaceholder; | ||
| }); | ||
|
|
||
| // Find all escaped double-quoted text and replace with unique placeholders | ||
| textToProcess = textToProcess.replace(/\\"([^"]*)\\"/gu, (match) => { | ||
| const uniquePlaceholder = `___QUOTED_${placeholderIndex}___`; | ||
| quotedTexts.push({ placeholder: uniquePlaceholder, text: match }); | ||
| placeholderIndex += 1; | ||
| return uniquePlaceholder; | ||
| }); | ||
|
|
||
| // Convert to sentence case | ||
| const words = textToProcess.split(/\s+/u).filter((word) => word.length > 0); | ||
| let converted = words | ||
| .map((word, index) => { | ||
| // Check if word contains a placeholder (exact match or with punctuation) | ||
| if ( | ||
| quotedTexts.some( | ||
| (q) => word === q.placeholder || word.includes(q.placeholder), | ||
| ) | ||
| ) { | ||
| return word; | ||
| } | ||
| if (index === 0) { | ||
| // First word: capitalize first letter | ||
| return word.charAt(0).toUpperCase() + word.slice(1).toLowerCase(); | ||
| } | ||
| // Other words: all lowercase | ||
| return word.toLowerCase(); | ||
| }) | ||
| .join(' '); | ||
|
|
||
| // Restore quoted text with unique placeholders | ||
| quotedTexts.forEach(({ placeholder, text: quotedText }) => { | ||
| converted = converted.replace(placeholder, quotedText); | ||
| }); | ||
|
|
||
| return converted; | ||
| } | ||
|
|
||
| // Validate sentence case compliance for a locale | ||
| function validateSentenceCaseCompliance(locale) { | ||
| const violations = []; | ||
|
|
||
| for (const [key, value] of Object.entries(locale)) { | ||
| if (!value || !value.message) { | ||
| continue; | ||
| } | ||
|
|
||
| const text = value.message; | ||
|
|
||
| // Skip if contains special cases | ||
| if (containsSpecialCase(text)) { | ||
| continue; | ||
| } | ||
|
|
||
| // Check for title case violations | ||
| if (hasTitleCaseViolation(text)) { | ||
| const suggested = toSentenceCase(text); | ||
| violations.push({ | ||
| key, | ||
| current: text, | ||
| suggested, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| return violations; | ||
| } |
There was a problem hiding this comment.
The new functions added for sentence case validation lack JSDoc documentation. Consider adding JSDoc comments to improve code maintainability:
/**
* Builds a compiled regex pattern from sentence case exceptions for efficient matching.
* @param {Object} exceptions - The exceptions object containing exactMatches, acronyms, and patterns
* @param {string[]} exceptions.exactMatches - Array of exact terms to match
* @param {string[]} exceptions.acronyms - Array of acronyms to match
* @param {Object} exceptions.patterns - Object containing regex patterns
* @returns {RegExp} Compiled regex pattern matching all exceptions
*/
function buildExceptionsRegex(exceptions) { ... }
/**
* Checks if text contains any special case terms that should be exempt from sentence case rules.
* @param {string} text - The text to check
* @returns {boolean} True if text contains special case terms
*/
function containsSpecialCase(text) { ... }
/**
* Detects if text violates sentence case by using title case capitalization.
* Ignores quoted text and single words.
* @param {string} text - The text to check
* @returns {boolean} True if text has title case violations
*/
function hasTitleCaseViolation(text) { ... }
/**
* Converts text to sentence case while preserving special case terms.
* @param {string} text - The text to convert
* @returns {string} Text converted to sentence case
*/
function toSentenceCase(text) { ... }
/**
* Simple sentence case conversion without special case preservation.
* @param {string} text - The text to convert
* @returns {string} Text converted to sentence case
*/
function convertToSentenceCase(text) { ... }
/**
* Validates that all locale messages follow sentence case conventions.
* @param {Object} locale - The locale object containing message keys and values
* @returns {Array<{key: string, current: string, suggested: string}>} Array of violations
*/
function validateSentenceCaseCompliance(locale) { ... }
Comment on lines
+189
to
+203
| // Find all single-quoted text and replace with unique placeholders | ||
| textToProcess = textToProcess.replace(/'([^']*)'/gu, (match) => { | ||
| const uniquePlaceholder = `___QUOTED_${placeholderIndex}___`; | ||
| quotedTexts.push({ placeholder: uniquePlaceholder, text: match }); | ||
| placeholderIndex += 1; | ||
| return uniquePlaceholder; | ||
| }); | ||
|
|
||
| // Find all escaped double-quoted text and replace with unique placeholders | ||
| textToProcess = textToProcess.replace(/\\"([^"]*)\\"/gu, (match) => { | ||
| const uniquePlaceholder = `___QUOTED_${placeholderIndex}___`; | ||
| quotedTexts.push({ placeholder: uniquePlaceholder, text: match }); | ||
| placeholderIndex += 1; | ||
| return uniquePlaceholder; | ||
| }); |
Contributor
Author
There was a problem hiding this comment.
We could simplify this logic by unifying the format in app/_locales/en/messages.json but not sure if there is a reason to have both?
Comment on lines
+524
to
+534
| if (sentenceCaseViolations.length) { | ||
| console.log( | ||
| `**en**: ${sentenceCaseViolations.length} sentence case violations`, | ||
| ); | ||
| log.info(`Messages not following sentence case:`); | ||
| sentenceCaseViolations.forEach(function (violation) { | ||
| log.info( | ||
| ` - [ ] ${violation.key}: "${violation.current}" → "${violation.suggested}"`, | ||
| ); | ||
| }); | ||
| } |
Contributor
Author
There was a problem hiding this comment.
We don't log any other violations so I'm not sure if we should log the sentence case ones but it might be worth it for the time being so it's clear to engineers that there has been an update made to the verify-locale script 🤔
Collaborator
Builds ready [4a35cc0]
UI Startup Metrics (1230 ± 100 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Collaborator
Builds ready [fe91217]
UI Startup Metrics (1214 ± 111 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
**Problem**: When both unusedMessages and sentenceCaseViolations exist and --fix is used, the locale file was written twice. The second write would overwrite the first, losing the unused message deletions. **Root cause**: - Line 544-549: First write deleted unused messages - Line 552-559: Second write applied sentence case fixes but used original englishLocale as base, not the already-fixed version - Result: Unused message deletions were lost **Solution**: Merged both fixes into a single operation: 1. Create one newLocale object from englishLocale 2. Apply unused message deletions to newLocale 3. Apply sentence case fixes to newLocale 4. Write locale file once with all changes applied This ensures both types of fixes are preserved in the final output. Addresses GitHub Copilot suggestion. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Collaborator
Builds ready [0a7579d]
UI Startup Metrics (1239 ± 104 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
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
This PR adds automated sentence case validation to the existing
verify-localesscript to enforce consistent capitalization across all English locale strings, as originally defined in PR #15285.What is the reason for the change?
Since PR #15285 (Aug 2022), we've had content guidelines requiring sentence case for all UI strings. However, over time, new features have introduced title case violations (e.g., "Price Impact", "Network Menu", etc). There was no automated enforcement, so violations accumulated.
What is the improvement/solution?
1. Automated CI Check
development/verify-locale-strings.jswith sentence case validation2. Special Cases Configuration
app/_locales/sentence-case-exceptions.jsonto define terms that should maintain title case:3. Quoted Text Preservation
'Get Signature'\"Switch to Smart Account\"4. Auto-Fix Support
yarn verify-locales --fixautomatically corrects all violationsen_GBauto-synced via existing script logicChangelog
CHANGELOG entry: null
Related issues
Part of: https://consensyssoftware.atlassian.net/browse/MDP-428
Manual testing steps
yarn verify-locales- should pass with no violationsapp/_locales/en/messages.json:yarn verify-locales- should fail with:yarn verify-locales --fix- should auto-correct to sentence caseScreenshots/Recordings
Before
After
--fixflag provides easy correctionafter720.mov
Pre-merge author checklist
Pre-merge reviewer checklist