Repository navigation
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds mobile i18n v2 runtime support, locale catalog content, and localization coverage updates. It wires i18n startup into the app layout, adds UI language persistence and a language settings screen, expands the shared locale files with mobile strings, and migrates mobile screens and components to translated text. It also extracts several inline styles into separate stylesheet modules and updates the coverage audit to scan mobile sources. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
mobile/app/troubleshoot.tsx (1)
34-38: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDon't persist translated diagnostics in state.
checksnow stores already-translatedlabel/detailstrings. After a language switch, this screen can show mixed locales until the user reruns diagnostics because only the livet(...)calls re-render.Minimal fix
const [checks, setChecks] = useState<CheckResult[]>([]) + + useEffect(() => { + setChecks([]) + setDiagnosticStatus('idle') + }, [t])Also applies to: 79-208
mobile/app/h/[hostId]/session/[worktreeId].tsx (1)
2185-2204: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
tmissing fromuseCallbackdependencies.
addDiffCommentForFilenow usest(Lines 2188, 2196) but its dependency array (Line 2204) omits it;deleteDiffCommentForFilehas the same gap (usestat Line 2228, deps at Line 2235).useTranslate()returns a freshteach render, so the memoized callbacks capture a stale translator (wrong language after a switch), and this is inconsistent with the other callbacks here that already listt(e.g. Line 2251).react-hooks/exhaustive-depswill also flag these.🔧 Add
tto both dependency arrays- [diffCommentBusy, persistDiffComments, showToast, worktreeId] + [diffCommentBusy, persistDiffComments, showToast, t, worktreeId]- [diffCommentBusy, persistDiffComments, showToast] + [diffCommentBusy, persistDiffComments, showToast, t]mobile/src/components/NewWorktreeModal.tsx (1)
786-801: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLocalize the workspace-name placeholder too.
This input still falls back to
MobileWorkspaceNameInput’s hardcoded"Workspace name"placeholder because noplaceholderprop is passed here. The rest of the modal is translated, so this field will stay English in localized builds.Suggested fix
<MobileWorkspaceNameInput style={styles.input} value={name} + placeholder={t( + 'mobile.newWorktree.workspaceNamePlaceholder', + 'Workspace name' + )} onChangeText={(value) => { setName(value) setError('')config/scripts/audit-localization-coverage.mjs (1)
492-516: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake explicit
--source-rootvalues replace the defaults.Line 492 seeds the built-in roots, and Lines 510-516 only append. That means
--source-root mobile/srccan never scope the audit to mobile only, and passing a default root again makesmain()scan the same files twice, inflating candidate counts and allowlist misses.Proposed fix
function parseArgs(argv) { const options = { allowlistPath: path.join('config', 'localization-coverage-allowlist.json'), check: false, format: 'summary', outputPath: null, sourceRoots: [path.join('src', 'renderer', 'src'), path.join('mobile', 'src')] } + let sawExplicitSourceRoot = false for (let index = 0; index < argv.length; index += 1) { const arg = argv[index] @@ } else if (arg === '--source-root') { // Why: support repeated --source-root flags so the mobile and // renderer surfaces share the same coverage gate. const next = argv[index + 1] if (next) { + if (!sawExplicitSourceRoot) { + options.sourceRoots = [] + sawExplicitSourceRoot = true + } options.sourceRoots.push(next) index += 1 } } }
🟡 Minor comments (22)
docs/superpowers/specs/2026-06-29-mobile-i18n-v2-design.md-4-4 (1)
4-4: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate document status before merge.
Line 4 lists
**Status**: Draft — awaiting user review. Since this PR is being reviewed for merge, the status should be updated to reflect the current state (e.g., "Approved" or "Ready for implementation").docs/superpowers/specs/2026-06-29-mobile-i18n-v2-design.md-60-62 (1)
60-62: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDesign claims v1 files reused without changes, but actual implementation has important differences.
The spec states v1's
init.ts/I18nProvider.tsx/loadUiLanguage/saveUiLanguageare reused "without changes" (line 61). However, the actual implementation shows meaningful improvements:
translate.tsadds try/catch fallback protection (seemobile/src/i18n/translate.ts)language-settings.tsxaddspendingstate anddisabledprop to prevent race conditionsThe spec should acknowledge these deliberate improvements over v1 rather than claiming identical reuse.
docs/superpowers/plans/2026-06-29-mobile-i18n-v2-pr1.md-417-434 (1)
417-434: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPR1 plan's translate.ts lacks try/catch that actual implementation includes.
The plan shows bare
return getI18n().t(...)without error handling. The actual implementation (and the design spec's relevant code snippet) wraps this in try/catch to return the fallback on failure. Update the plan to match the implementation that was actually shipped.export function translate( key: string, fallback: string, options?: TOptions ): string { - return getI18n().t(key, { defaultValue: fallback, ...options }) + try { + return getI18n().t(key, { defaultValue: fallback, ...options }) + } catch { + return fallback + } }docs/superpowers/plans/2026-06-29-mobile-i18n-v2-pr1.md-217-222 (1)
217-222: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSingleton test expectation is misleading—
initI18nignores language changes on re-init.The test expects
a === b(same instance) after callinginitI18n('en')theninitI18n('zh'). While the instance is indeed the same, the language remains'en'—the second call's'zh'parameter is silently ignored. This is correct for the singleton pattern but the test doesn't verify the language stays'en'or document that re-init with different language is a no-op. Add an assertion to clarify intent:it('returns the same instance on repeated calls', async () => { const a = await initI18n('en') const b = await initI18n('zh') expect(a).toBe(b) + // Singleton: language does NOT change on re-init; use changeLanguage() instead + expect(a.language).toBe('en') })Or, if language switching on re-init is desired,
initI18nneeds to callchangeLanguage()when an instance exists with a different resolved language.docs/superpowers/plans/2026-06-29-mobile-i18n-v2-pr1.md-999-1001 (1)
999-1001: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winVerification script uses hardcoded absolute path.
Line 1000 uses
cd /home/ppw/orca/workspaces/orca/汉化支持which is a machine-specific absolute path. This won't work on other developers' machines or CI. Use a relative path or environment variable.- cd /home/ppw/orca/workspaces/orca/汉化支持 + cd "$(git rev-parse --show-toplevel)"docs/superpowers/plans/2026-06-29-mobile-i18n-v2-pr2.md-317-317 (1)
317-317: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExpo-router routing misconception:
.gitignoredoes not control routing.The plan suggests "Or update the
.gitignoreof expo-router config" as an alternative to renaming files. Expo Router's file-based routing is determined by the file system, not.gitignore. Files inapp/are routes regardless of.gitignorestatus. Remove this incorrect option.- - Or update the `.gitignore` of expo-router config + - Or move them to `app/_styles/` (outside `app/` routing scope)docs/superpowers/specs/2026-06-29-mobile-i18n-v2-design.md-120-133 (1)
120-133: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winSpec's translate.ts snippet missing try/catch fallback that exists in implementation.
The code snippet shows bare
return getI18n().t(...)without error handling. The actualmobile/src/i18n/translate.tsimplementation wraps this in try/catch to return the fallback on any i18n failure. Update the spec to match the actual implementation.export function translate( key: string, fallback: string, options?: TOptions ): string { + try { return getI18n().t(key, { defaultValue: fallback, ...options }) + } catch { + return fallback + } }mobile/app/terminal-settings.tsx-74-80 (1)
74-80: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign the unknown-scale fallback with the picker selection.
Line 75 falls back to
'default', but Line 79 falls back tooptions[0]('smallest'). If a stale or unsupported saved scale is loaded, the row says “Smallest” while the picker opens on “Default”.Suggested fix
function textSizeSummary(options: TextSizeOption[], scale: number): string { - return (options.find((o) => o.scale === scale) ?? options[0]!).label + return ( + options.find((o) => o.scale === scale) ?? + options.find((o) => o.value === 'default') ?? + options[0]! + ).label }mobile/app/index.tsx-689-693 (1)
689-693: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the real Tasks fallback here.
Both
mobile.tasks.titlecalls fall back to"Title", so a missing key renders the wrong label in the home screen. Use"Tasks"in both places.Suggested fix
- <Text style={styles.taskHomeTitle}>{t('mobile.tasks.title', 'Title')}</Text> + <Text style={styles.taskHomeTitle}>{t('mobile.tasks.title', 'Tasks')}</Text> ... - {t('mobile.tasks.title', 'Title')} + {t('mobile.tasks.title', 'Tasks')} ... - {t('mobile.tasks.title', 'Title')} + {t('mobile.tasks.title', 'Tasks')}Also applies to: 962-974
mobile/app/index.tsx-907-914 (1)
907-914: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRestore plural-aware worktree copy.
This fallback hard-codes singular
"worktree", so hosts with 2+ worktrees render incorrect English. Use a count-neutral fallback or wire this key through pluralization.mobile/app/h/[hostId]/index.tsx-952-954 (1)
952-954: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse i18next interpolation syntax in these fallbacks.
useTranslate()forwards fallbacks to i18next, so{label},{name}, and{branch}are treated as literal text. These strings need{{...}}placeholders or they'll render with braces when a key is missing.Suggested fix
- accessibilityLabel={t('mobile.workspaces.sortA11y', 'Sort by {label}', { + accessibilityLabel={t('mobile.workspaces.sortA11y', 'Sort by {{label}}', { label: selectedSortLabel })} ... - {t('mobile.workspaces.deleteConfirm', 'Delete "{name}" ({branch})?', { + {t('mobile.workspaces.deleteConfirm', 'Delete "{{name}}" ({{branch}})?', { name: confirmDelete.displayName || confirmDelete.repo, branch: confirmDelete.branch })} ... - 'Remove "{name}"? You can re-pair later.', + 'Remove "{{name}}"? You can re-pair later.',Also applies to: 1346-1349, 1444-1450
mobile/app/h/[hostId]/history/[worktreeId].tsx-55-59 (1)
55-59: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the error path localized.
The common
Errorbranch still renderserr.message, so this screen shows untranslated transport/backend text on most failures. Use the translated generic message here and log raw details separately if needed.mobile/app/h/[hostId]/accounts.tsx-296-300 (1)
296-300: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix fallback interpolation for the host name.
Connecting to {name}…won't interpolate through i18next. Use{{name}}or the fallback will render literally when that key is missing.Suggested fix
- {t('mobile.accounts.connectingToHost', 'Connecting to {name}…', { + {t('mobile.accounts.connectingToHost', 'Connecting to {{name}}…', { name: hostName || 'host' })}mobile/src/components/MobileDiffReviewFileSummary.tsx-84-87 (1)
84-87: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the note count label too.
This still hardcodes
"note"/"notes", so localized summaries will show an English count even when the surrounding pills are translated.Proposed fix
{item.noteCount > 0 ? ( <Text style={styles.fileMeta}> - {mobileReviewCountLabel(item.noteCount, 'note', 'notes')} + {mobileReviewCountLabel( + item.noteCount, + t('mobile.diffReview.noteSingular', 'note'), + t('mobile.diffReview.notePlural', 'notes') + )} </Text> ) : null}mobile/src/components/MobileDiffReviewDrawers.tsx-49-61 (1)
49-61: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPass a translated
cancelLabelhere too.
ConfirmModalstill defaults its cancel button to English"Cancel", so this dialog remains mixed-language even after localizing the title and confirm action. The default comes frommobile/src/components/ConfirmModal.tsx:16-57.Proposed fix
<ConfirmModal visible={controller.discardTarget !== null} title={t('mobile.diffReview.discardFile', 'Discard File')} message={ controller.discardTarget ? t( 'mobile.diffReview.discardFileConfirm', 'Discard changes to "{file}"? This cannot be undone.', { file: controller.discardTarget.filePath } ) : undefined } confirmLabel={t('mobile.diffReview.discard', 'Discard')} + cancelLabel={t('mobile.diffReview.cancel', 'Cancel')} destructivemobile/src/components/pr-sidebar/ReviewerPickerDrawer.tsx-127-127 (1)
127-127: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLocalize the full accessibility phrase, not just the verb.
Concatenating
Request/Removewith the login locks the screen-reader phrase to English word order. Use one translation key with{login}interpolation instead.Proposed fix
- accessibilityLabel={`${requested ? t('mobile.reviewerPicker.remove', 'Remove') : t('mobile.reviewerPicker.request', 'Request')} ${item.login}`} + accessibilityLabel={ + requested + ? t('mobile.reviewerPicker.removeReviewer', 'Remove {{login}}', { + login: item.login + }) + : t('mobile.reviewerPicker.requestReviewer', 'Request {{login}}', { + login: item.login + }) + }mobile/src/components/MobileDiffReviewHeader.tsx-116-122 (1)
116-122: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the rendered chip item in the accessibility label.
Line 120 interpolates
filter, which is the currently selected filter prop, not the chip being rendered. That makes every chip announce the same label after selection, so screen-reader users cannot distinguish the actions.Suggested fix
accessibilityLabel={t( 'mobile.diffHeader.showFilterA11y', 'Show {filter} review files', { - filter + filter: item } )}mobile/src/components/MobileMarkdown.tsx-168-178 (1)
168-178: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle the singular hidden-row/column case.
When exactly one row or column is hidden, this renders
"1 more rows"/"1 more columns". Please split singular vs plural here so the English fallback stays correct.Suggested fix
{hiddenRows > 0 || hiddenColumns > 0 ? ( <Text style={styles.tableTruncated}> {hiddenRows > 0 - ? t('mobile.markdown.moreRows', '{n} more rows', { n: hiddenRows }) + ? hiddenRows === 1 + ? t('mobile.markdown.moreRow', '{n} more row', { n: hiddenRows }) + : t('mobile.markdown.moreRows', '{n} more rows', { n: hiddenRows }) : ''} {hiddenRows > 0 && hiddenColumns > 0 ? ' · ' : ''} {hiddenColumns > 0 - ? t('mobile.markdown.moreColumns', '{n} more columns', { - n: hiddenColumns - }) + ? hiddenColumns === 1 + ? t('mobile.markdown.moreColumn', '{n} more column', { + n: hiddenColumns + }) + : t('mobile.markdown.moreColumns', '{n} more columns', { + n: hiddenColumns + }) : ''} </Text> ) : null}mobile/src/components/MobileDiffReviewHeader.tsx-92-97 (1)
92-97: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake the unsent-count string fully translatable.
Passing translated nouns into
mobileReviewCountLabel()still forces the output shape to"{count} {label}", so translators cannot change word order or plural rules here. This will read incorrectly in some locales even though the individual words are translated.Suggested fix
- <Text style={styles.progressText}> - {mobileReviewCountLabel( - unsentCount, - t('mobile.diffHeader.unsentNote', 'unsent note'), - t('mobile.diffHeader.unsentNotes', 'unsent notes') - )} - </Text> + <Text style={styles.progressText}> + {unsentCount === 1 + ? t('mobile.diffHeader.unsentCountOne', '{{count}} unsent note', { + count: unsentCount + }) + : t('mobile.diffHeader.unsentCountOther', '{{count}} unsent notes', { + count: unsentCount + })} + </Text>mobile/src/components/TextInputModal.tsx-42-44 (1)
42-44: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCustom submit labels still bypass i18n.
This only localizes the default
"Save"path. A supplied caller inmobile/app/h/[hostId]/session/[worktreeId].tsxstill passessubmitLabel="Open", so that modal stays English in non-English UI. Either require callers to pass translated labels or extend this API so custom actions can be localized too.mobile/src/components/pr-sidebar/PRCommentsSection.tsx-246-252 (1)
246-252: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMove the count suffix into the translation.
(${count})is still composed outside the localized string, so locales that need different word order or punctuation cannot translate this header correctly. Put the count inside the translation instead of appending it afterward.mobile/src/i18n/README.md-28-28 (1)
28-28: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRefresh the rollout details in this README.
Line 28 still documents only
{en,zh}.jsonand~470PR2 keys, but the PR scope described here is broader than that. Leaving the old locale/count details in the local i18n README will send the next migration to the wrong files.
🧹 Nitpick comments (3)
docs/superpowers/specs/2026-06-29-mobile-i18n-v2-design.md (1)
295-299: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMetro config advice contradicts PR objectives and may cause build issues.
The spec says "v2 PR1 must retain v1's full metro.config.js changes" including
nodeModulesPathsanddisableHierarchicalLookup. However, the PR objectives state "tightening Metro watch folders" and the actualmetro.config.jschanges in the PR stack only add workspace root watching for locale JSON access. The spec's advice to keep all v1 changes is overly conservative—JSON files don't require node module resolution from the workspace root. Align the spec with the actual minimal Metro changes.mobile/src/i18n/__tests__/useTranslate.test.ts (1)
27-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThis never exercises
useTranslate()itself.The test only reimplements
getI18n().t(...), so hook-specific regressions inuseTranslate()would still pass. If the mobile test stack already supports it, a tiny component smoke test would give you coverage for the actual returned contract without depending on a hook-only harness.mobile/.oxlintrc.json (1)
19-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftKeep shrinking these screens instead of raising the ceiling again.
Bumping
app/h/*/tasks.tsxandapp/h/*/session/*.tsxto 15,500 and 5,500 lines turns the current escape hatch into the new baseline. Please keep extracting focused modules/styles/helpers rather than ratcheting the limit upward. As per coding guidelines, "Never add amax-linesdisable comment (eslint-disable max-lines,oxlint-disable max-lines, or line-specific variants). Split the file, extract focused modules, move fixtures/builders into named files, or otherwise reduce the counted lines instead."Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 889accb1-2014-4c3c-86a8-05a5964e928c
📥 Commits
Reviewing files that changed from the base of the PR and between d877d94 and c8a20890c9749ba813d2e50a788b115d2516801d.
⛔ Files ignored due to path filters (1)
mobile/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (74)
config/localization-coverage-allowlist.jsonconfig/scripts/audit-localization-coverage.mjsdocs/superpowers/plans/2026-06-29-mobile-i18n-v2-pr1.mddocs/superpowers/plans/2026-06-29-mobile-i18n-v2-pr2.mddocs/superpowers/specs/2026-06-29-mobile-i18n-v2-design.mdmobile/.oxlintrc.jsonmobile/app/_layout.tsxmobile/app/about.tsxmobile/app/browser-settings.tsxmobile/app/h/[hostId]/accounts.tsxmobile/app/h/[hostId]/history/[worktreeId].tsxmobile/app/h/[hostId]/index-styles.tsmobile/app/h/[hostId]/index.tsxmobile/app/h/[hostId]/session/[worktreeId].tsxmobile/app/h/[hostId]/tasks.tsxmobile/app/index-styles.tsmobile/app/index.tsxmobile/app/language-settings.tsxmobile/app/notifications.tsxmobile/app/pair-confirm.tsxmobile/app/pair-scan-styles.tsmobile/app/pair-scan.tsxmobile/app/pair.tsxmobile/app/settings.tsxmobile/app/terminal-settings.tsxmobile/app/troubleshoot-sections.tsxmobile/app/troubleshoot.tsxmobile/app/voice-settings-styles.tsmobile/app/voice-settings.tsxmobile/metro.config.jsmobile/package.jsonmobile/src/components/ConnectionLog.tsxmobile/src/components/CustomKeyModal.tsxmobile/src/components/MobileDictationSetupSheet.tsxmobile/src/components/MobileDiffReviewDrawers.tsxmobile/src/components/MobileDiffReviewFileSummary.tsxmobile/src/components/MobileDiffReviewFooter.tsxmobile/src/components/MobileDiffReviewHeader.tsxmobile/src/components/MobileMarkdown.tsxmobile/src/components/NewWorktreeModal.tsxmobile/src/components/ProtocolBlockScreen.tsxmobile/src/components/TerminalShortcutSettings.tsxmobile/src/components/TextInputModal.tsxmobile/src/components/VoiceModelList.tsxmobile/src/components/WorktreeListRow.tsxmobile/src/components/custom-key-modal-styles.tsmobile/src/components/new-worktree-modal-styles.tsmobile/src/components/pr-sidebar/MobileLinkPrForm.tsxmobile/src/components/pr-sidebar/MobilePrComposeForm.tsxmobile/src/components/pr-sidebar/PRActionsSection.tsxmobile/src/components/pr-sidebar/PRCheckDetail.tsxmobile/src/components/pr-sidebar/PRChecksSection.tsxmobile/src/components/pr-sidebar/PRCommentCard.tsxmobile/src/components/pr-sidebar/PRCommentsSection.tsxmobile/src/components/pr-sidebar/PRConflictingFilesSection.tsxmobile/src/components/pr-sidebar/PRSidebarHeader.tsxmobile/src/components/pr-sidebar/PrSidebarCreateEmptyState.tsxmobile/src/components/pr-sidebar/ReviewerPickerDrawer.tsxmobile/src/components/terminal-shortcut-settings-styles.tsmobile/src/i18n/I18nProvider.tsxmobile/src/i18n/README.mdmobile/src/i18n/__tests__/init.test.tsmobile/src/i18n/__tests__/translate.test.tsmobile/src/i18n/__tests__/useTranslate.test.tsmobile/src/i18n/init.tsmobile/src/i18n/translate.tsmobile/src/i18n/useTranslate.tsmobile/src/storage/preferences.test.tsmobile/src/storage/preferences.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.json
| ```tsx | ||
| import { useCallback, useEffect, useState } from 'react' | ||
| import { View, Text, StyleSheet, Pressable } from 'react-native' | ||
| import { useSafeAreaInsets } from 'react-native-safe-area-context' | ||
| import { useRouter } from 'expo-router' | ||
| import { Check, ChevronLeft, Globe } from 'lucide-react-native' | ||
|
|
||
| import { useTranslate } from '../src/i18n/useTranslate' | ||
| import { getI18n, resolveLanguage, type MobileUiLanguage } from '../src/i18n/init' | ||
| import { loadUiLanguage, saveUiLanguage } from '../src/storage/preferences' | ||
| import { colors, spacing, typography } from '../src/theme/mobile-theme' | ||
|
|
||
| // Why: language pickers must NOT translate their own labels — the user | ||
| // needs to see the language name in its native script to recognize it. | ||
| const LANGUAGE_CHOICES: ReadonlyArray<{ value: MobileUiLanguage; label: string }> = [ | ||
| { value: 'system', label: 'Follow System' }, | ||
| { value: 'en', label: 'English' }, | ||
| { value: 'zh', label: '中文（简体）' } | ||
| ] | ||
|
|
||
| export default function LanguageSettingsScreen() { | ||
| const router = useRouter() | ||
| const insets = useSafeAreaInsets() | ||
| const { t } = useTranslate() | ||
| const [lang, setLang] = useState<MobileUiLanguage>('system') | ||
|
|
||
| useEffect(() => { | ||
| void loadUiLanguage().then(setLang) | ||
| }, []) | ||
|
|
||
| const onPick = useCallback(async (next: MobileUiLanguage) => { | ||
| setLang(next) // 立即更新 UI | ||
| // Why: saveUiLanguage is best-effort persistence. Blocking the UI | ||
| // switch on a failing write makes the user think the tap did nothing. | ||
| void saveUiLanguage(next) | ||
| await getI18n().changeLanguage(resolveLanguage(next)) | ||
| }, []) | ||
|
|
||
| return ( | ||
| <View style={[styles.container, { paddingTop: insets.top + spacing.sm }]}> | ||
| <View style={styles.topRow}> | ||
| <Pressable style={styles.backButton} onPress={() => router.back()}> | ||
| <ChevronLeft size={22} color={colors.textSecondary} /> | ||
| </Pressable> | ||
| <Text style={styles.heading}> | ||
| {t('mobile.language.title', 'Language')} | ||
| </Text> | ||
| </View> | ||
|
|
||
| <View style={styles.section}> | ||
| {LANGUAGE_CHOICES.map((choice, index) => ( | ||
| <View key={choice.value}> | ||
| <Pressable | ||
| style={({ pressed }) => [styles.row, pressed && styles.rowPressed]} | ||
| onPress={() => void onPick(choice.value)} | ||
| > | ||
| <Globe size={16} color={colors.textSecondary} /> | ||
| <Text style={styles.rowLabel}>{choice.label}</Text> | ||
| {lang === choice.value && <Check size={16} color={colors.textPrimary} />} | ||
| </Pressable> | ||
| {index < LANGUAGE_CHOICES.length - 1 && <View style={styles.separator} />} | ||
| </View> | ||
| ))} | ||
| </View> | ||
| </View> | ||
| ) | ||
| } | ||
|
|
||
| const styles = StyleSheet.create({ | ||
| container: { flex: 1, backgroundColor: colors.background }, | ||
| topRow: { | ||
| flexDirection: 'row', | ||
| alignItems: 'center', | ||
| paddingHorizontal: spacing.md, | ||
| paddingBottom: spacing.md | ||
| }, | ||
| backButton: { padding: spacing.xs, marginRight: spacing.sm }, | ||
| heading: { ...typography.heading, color: colors.textPrimary }, | ||
| section: { | ||
| marginHorizontal: spacing.md, | ||
| backgroundColor: colors.surface, | ||
| borderRadius: 12, | ||
| overflow: 'hidden' | ||
| }, | ||
| row: { | ||
| flexDirection: 'row', | ||
| alignItems: 'center', | ||
| paddingVertical: spacing.md, | ||
| paddingHorizontal: spacing.md, | ||
| gap: spacing.sm | ||
| }, | ||
| rowPressed: { backgroundColor: colors.surfacePressed }, | ||
| rowLabel: { ...typography.body, color: colors.textPrimary, flex: 1 }, | ||
| separator: { height: StyleSheet.hairlineWidth, backgroundColor: colors.separator, marginLeft: spacing.md } | ||
| }) | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
PR1 plan's language-settings.tsx lacks race-condition guard from actual implementation.
The plan shows a simplified onPick without pending state or disabled prop. The actual implementation adds these to prevent sequential tap races. Update the plan to match the actual code for accuracy.
export default function LanguageSettingsScreen() {
...
const [lang, setLang] = useState<MobileUiLanguage>('system')
+ const [pending, setPending] = useState<MobileUiLanguage | null>(null)
useEffect(() => {
void loadUiLanguage().then(setLang)
}, [])
- const onPick = useCallback(async (next: MobileUiLanguage) => {
+ const onPick = useCallback(async (next: MobileUiLanguage) => {
+ if (pending) return
+ setPending(next)
setLang(next)
void saveUiLanguage(next)
- await getI18n().changeLanguage(resolveLanguage(next))
- }, [])
+ try {
+ await getI18n().changeLanguage(resolveLanguage(next))
+ } finally {
+ setPending(null)
+ }
+ }, [pending])And add disabled={pending !== null} to the Pressable.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ```tsx | |
| import { useCallback, useEffect, useState } from 'react' | |
| import { View, Text, StyleSheet, Pressable } from 'react-native' | |
| import { useSafeAreaInsets } from 'react-native-safe-area-context' | |
| import { useRouter } from 'expo-router' | |
| import { Check, ChevronLeft, Globe } from 'lucide-react-native' | |
| import { useTranslate } from '../src/i18n/useTranslate' | |
| import { getI18n, resolveLanguage, type MobileUiLanguage } from '../src/i18n/init' | |
| import { loadUiLanguage, saveUiLanguage } from '../src/storage/preferences' | |
| import { colors, spacing, typography } from '../src/theme/mobile-theme' | |
| // Why: language pickers must NOT translate their own labels — the user | |
| // needs to see the language name in its native script to recognize it. | |
| const LANGUAGE_CHOICES: ReadonlyArray<{ value: MobileUiLanguage; label: string }> = [ | |
| { value: 'system', label: 'Follow System' }, | |
| { value: 'en', label: 'English' }, | |
| { value: 'zh', label: '中文（简体）' } | |
| ] | |
| export default function LanguageSettingsScreen() { | |
| const router = useRouter() | |
| const insets = useSafeAreaInsets() | |
| const { t } = useTranslate() | |
| const [lang, setLang] = useState<MobileUiLanguage>('system') | |
| useEffect(() => { | |
| void loadUiLanguage().then(setLang) | |
| }, []) | |
| const onPick = useCallback(async (next: MobileUiLanguage) => { | |
| setLang(next) // 立即更新 UI | |
| // Why: saveUiLanguage is best-effort persistence. Blocking the UI | |
| // switch on a failing write makes the user think the tap did nothing. | |
| void saveUiLanguage(next) | |
| await getI18n().changeLanguage(resolveLanguage(next)) | |
| }, []) | |
| return ( | |
| <View style={[styles.container, { paddingTop: insets.top + spacing.sm }]}> | |
| <View style={styles.topRow}> | |
| <Pressable style={styles.backButton} onPress={() => router.back()}> | |
| <ChevronLeft size={22} color={colors.textSecondary} /> | |
| </Pressable> | |
| <Text style={styles.heading}> | |
| {t('mobile.language.title', 'Language')} | |
| </Text> | |
| </View> | |
| <View style={styles.section}> | |
| {LANGUAGE_CHOICES.map((choice, index) => ( | |
| <View key={choice.value}> | |
| <Pressable | |
| style={({ pressed }) => [styles.row, pressed && styles.rowPressed]} | |
| onPress={() => void onPick(choice.value)} | |
| > | |
| <Globe size={16} color={colors.textSecondary} /> | |
| <Text style={styles.rowLabel}>{choice.label}</Text> | |
| {lang === choice.value && <Check size={16} color={colors.textPrimary} />} | |
| </Pressable> | |
| {index < LANGUAGE_CHOICES.length - 1 && <View style={styles.separator} />} | |
| </View> | |
| ))} | |
| </View> | |
| </View> | |
| ) | |
| } | |
| const styles = StyleSheet.create({ | |
| container: { flex: 1, backgroundColor: colors.background }, | |
| topRow: { | |
| flexDirection: 'row', | |
| alignItems: 'center', | |
| paddingHorizontal: spacing.md, | |
| paddingBottom: spacing.md | |
| }, | |
| backButton: { padding: spacing.xs, marginRight: spacing.sm }, | |
| heading: { ...typography.heading, color: colors.textPrimary }, | |
| section: { | |
| marginHorizontal: spacing.md, | |
| backgroundColor: colors.surface, | |
| borderRadius: 12, | |
| overflow: 'hidden' | |
| }, | |
| row: { | |
| flexDirection: 'row', | |
| alignItems: 'center', | |
| paddingVertical: spacing.md, | |
| paddingHorizontal: spacing.md, | |
| gap: spacing.sm | |
| }, | |
| rowPressed: { backgroundColor: colors.surfacePressed }, | |
| rowLabel: { ...typography.body, color: colors.textPrimary, flex: 1 }, | |
| separator: { height: StyleSheet.hairlineWidth, backgroundColor: colors.separator, marginLeft: spacing.md } | |
| }) | |
| ``` |
| - 869 untranslated `<Text>` tags across 26 files | ||
| - 5 locale files (`en.json` / `zh.json` / `es.json` / `ja.json` / `ko.json`) need new keys | ||
| - Extend desktop's `verify:localization-coverage` to also scan `mobile/` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
PR2 adds es/ja/ko locales, contradicting spec scope of system/en/zh only.
The PR2 plan adds es.json, ja.json, and ko.json with English placeholders, but the design spec explicitly decided "mobile 仅 system/en/zh" (line 381) and marked Korean/Japanese/Spanish as "v3" candidates. Either the spec needs updating to reflect this expanded scope, or PR2 should limit to en/zh as originally designed.
| } catch (error) { | ||
| // Why: if resource loading fails (corrupt zh.json, missing module), | ||
| // fall back to English so the app still boots. | ||
| console.warn('[i18n] init failed; defaulting to en', error) | ||
| try { | ||
| await initI18n('en') | ||
| } catch (fallbackError) { | ||
| console.error('[i18n] fallback init also failed', fallbackError) | ||
| } | ||
| } | ||
| setReady(true) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Don't flip ready after both init attempts fail.
Line 60 still runs when initI18n(lang) and the English fallback both reject. The next render then calls getI18n() on Line 176, which throws by contract, so the app still hard-crashes on the exact startup path this catch block is trying to save.
Proposed fix
useEffect(() => {
void (async () => {
+ let initialized = false
try {
const lang = await loadUiLanguage()
await initI18n(lang)
+ initialized = true
} catch (error) {
// Why: if resource loading fails (corrupt zh.json, missing module),
// fall back to English so the app still boots.
console.warn('[i18n] init failed; defaulting to en', error)
try {
await initI18n('en')
+ initialized = true
} catch (fallbackError) {
console.error('[i18n] fallback init also failed', fallbackError)
}
}
- setReady(true)
+ if (initialized) {
+ setReady(true)
+ }
})()
}, [])Also applies to: 176-176
| useEffect(() => { | ||
| void loadUiLanguage().then(setLang) | ||
| }, []) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Prevent the initial preference load from overwriting a fresh tap.
Line 32 can race with onPick(): if the user taps before loadUiLanguage() resolves, the stale persisted value can still win the final setLang(...), so the checkmark ends up on the wrong option even though changeLanguage(...) already switched the app.
Suggested fix
-import { useCallback, useEffect, useState } from 'react'
+import { useCallback, useEffect, useRef, useState } from 'react'
@@
export default function LanguageSettingsScreen() {
@@
const { t } = useTranslate()
const [lang, setLang] = useState<MobileUiLanguage>('system')
+ const hasUserPickedRef = useRef(false)
@@
useEffect(() => {
- void loadUiLanguage().then(setLang)
+ void loadUiLanguage().then((storedLang) => {
+ if (!hasUserPickedRef.current) {
+ setLang(storedLang)
+ }
+ })
}, [])
@@
async (next: MobileUiLanguage) => {
if (pending) {
return
}
+ hasUserPickedRef.current = true
setPending(next)
setLang(next) // 立即更新 UIAlso applies to: 35-52
| export async function initI18n(language: MobileUiLanguage): Promise<I18nInstance> { | ||
| if (instance) { | ||
| return instance | ||
| } | ||
| const resolved = resolveLanguage(language) | ||
| // Why: store the candidate in a local first, only assign to the module-level | ||
| // singleton after init() resolves. If init() rejects, instance stays null and | ||
| // getI18n() will throw a clear error rather than returning a broken object. | ||
| const candidate = i18next.createInstance() | ||
| await candidate.use(initReactI18next).init({ | ||
| lng: resolved, | ||
| fallbackLng: FALLBACK_LANGUAGE, | ||
| defaultNS: 'translation', | ||
| interpolation: { escapeValue: false }, | ||
| returnEmptyString: false, | ||
| resources: { | ||
| zh: { translation: zh }, | ||
| en: { translation: en } | ||
| } | ||
| }) | ||
| instance = candidate | ||
| return instance |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make initI18n() share one in-flight initialization.
instance is only set after await candidate.init(...), so two overlapping callers can both create and initialize separate instances. That breaks the “same instance” contract and makes startup order-dependent. Cache the pending initialization promise so every caller awaits the same work.
Suggested fix
let instance: I18nInstance | null = null
+let initPromise: Promise<I18nInstance> | null = null
export async function initI18n(language: MobileUiLanguage): Promise<I18nInstance> {
if (instance) {
return instance
}
+ if (initPromise) {
+ return initPromise
+ }
const resolved = resolveLanguage(language)
- // Why: store the candidate in a local first, only assign to the module-level
- // singleton after init() resolves. If init() rejects, instance stays null and
- // getI18n() will throw a clear error rather than returning a broken object.
- const candidate = i18next.createInstance()
- await candidate.use(initReactI18next).init({
- lng: resolved,
- fallbackLng: FALLBACK_LANGUAGE,
- defaultNS: 'translation',
- interpolation: { escapeValue: false },
- returnEmptyString: false,
- resources: {
- zh: { translation: zh },
- en: { translation: en }
- }
- })
- instance = candidate
- return instance
+ initPromise = (async () => {
+ const candidate = i18next.createInstance()
+ await candidate.use(initReactI18next).init({
+ lng: resolved,
+ fallbackLng: FALLBACK_LANGUAGE,
+ defaultNS: 'translation',
+ interpolation: { escapeValue: false },
+ returnEmptyString: false,
+ resources: {
+ zh: { translation: zh },
+ en: { translation: en }
+ }
+ })
+ instance = candidate
+ return candidate
+ })()
+ try {
+ return await initPromise
+ } finally {
+ initPromise = null
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function initI18n(language: MobileUiLanguage): Promise<I18nInstance> { | |
| if (instance) { | |
| return instance | |
| } | |
| const resolved = resolveLanguage(language) | |
| // Why: store the candidate in a local first, only assign to the module-level | |
| // singleton after init() resolves. If init() rejects, instance stays null and | |
| // getI18n() will throw a clear error rather than returning a broken object. | |
| const candidate = i18next.createInstance() | |
| await candidate.use(initReactI18next).init({ | |
| lng: resolved, | |
| fallbackLng: FALLBACK_LANGUAGE, | |
| defaultNS: 'translation', | |
| interpolation: { escapeValue: false }, | |
| returnEmptyString: false, | |
| resources: { | |
| zh: { translation: zh }, | |
| en: { translation: en } | |
| } | |
| }) | |
| instance = candidate | |
| return instance | |
| let initPromise: Promise<I18nInstance> | null = null | |
| export async function initI18n(language: MobileUiLanguage): Promise<I18nInstance> { | |
| if (instance) { | |
| return instance | |
| } | |
| if (initPromise) { | |
| return initPromise | |
| } | |
| const resolved = resolveLanguage(language) | |
| initPromise = (async () => { | |
| const candidate = i18next.createInstance() | |
| await candidate.use(initReactI18next).init({ | |
| lng: resolved, | |
| fallbackLng: FALLBACK_LANGUAGE, | |
| defaultNS: 'translation', | |
| interpolation: { escapeValue: false }, | |
| returnEmptyString: false, | |
| resources: { | |
| zh: { translation: zh }, | |
| en: { translation: en } | |
| } | |
| }) | |
| instance = candidate | |
| return candidate | |
| })() | |
| try { | |
| return await initPromise | |
| } finally { | |
| initPromise = null | |
| } | |
| } |
Replaces the v1 no-op <T> wrapper (closed at PR stablyai#6627) with a translate() function pattern mirroring the desktop i18n flow (src/renderer/src/i18n/i18n.ts:43). Every mobile screen, component, and modal now renders through the same useTranslate() hook used by the desktop app. What ships in this PR: - mobile/src/i18n/{init,translate,useTranslate,I18nProvider}.{ts,tsx} + init test (F11 — translate() returns fallback when uninit) - 1004 mobile.* keys added to all 5 locales (en, zh, es, ja, ko) with full parity. Single-brace {{var}} → {var} interpolation fixed in 3 pair error keys (F1). - All mobile screens migrated (app/about.tsx, browser-settings.tsx, notifications.tsx, troubleshoot.tsx, voice-settings.tsx, terminal-settings.tsx, pair-confirm.tsx, index.tsx, h/[hostId]/{index,accounts,history/[worktreeId],tasks,session/[worktreeId]}.tsx). - All mobile components migrated (NewWorktreeModal, CustomKeyModal, MobileMarkdown, TerminalShortcutSettings, MobileDictationSetupSheet, WorktreeListRow, MobileDiffReview*, VoiceModelList, ProtocolBlockScreen, ConnectionLog, TextInputModal + 8 pr-sidebar files). - Language picker at app/language-settings.tsx with race-safe async changeLanguage (F3). - storage/preferences.ts: loadUiLanguage uses normalizeUiLanguage to coerce legacy or corrupt values to 'system' (F12). - metro.config.js: narrowed watchFolders to src/shared + i18n locales to avoid hot-reload cache thrash (F6). - config/scripts/audit-localization-coverage.mjs: extended to scan mobile/ with allowlist for prop-driven strings (F4, P20). - Desktop zh.json: full Chinese translation of 236 keys (F5). Japanese and Korean translations across all mobile.* blocks. Mobile: 1004/1004 zh keys (60 byte-identical to en — all brand names). Verified on Android emulator (Pixel, Android 14, x86_64): - language switch via UI Settings → language-settings → 中/英 - persistence across app cold restart (AsyncStorage round-trip) - all migrated screens render Chinese - coverage gate passes (0 candidates, 198 allowlist entries with per- sub-block reasons) Refs: supersedes stablyai#6627 (closed: <T> no-op bug, desktop had zero mobile.* keys). PR2 plan: docs/superpowers/plans/2026-06-29-mobile-i18n-v2-pr2.md Design spec: docs/superpowers/specs/2026-06-29-mobile-i18n-v2-design.md
c8a2089 to
f6c8cb6
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (3)
docs/superpowers/plans/2026-06-29-mobile-i18n-v2-pr1.md (1)
1178-1184: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRace condition in
language-settings.tsxonPickpersists from prior review.The
onPickcallback still lacks thependingstate guard and thePressableis missingdisabled={pending !== null}. Two rapid taps interleavechangeLanguagecalls and can leave the UI showing language A while i18n is set to language B. This was flagged in the previous review and remains unfixed in the plan document.const onPick = useCallback(async (next: MobileUiLanguage) => { + if (pending) return + setPending(next) setLang(next) void saveUiLanguage(next) - await getI18n().changeLanguage(resolveLanguage(next)) + try { + await getI18n().changeLanguage(resolveLanguage(next)) + } finally { + setPending(null) + } - }, []) + }, [pending])docs/superpowers/specs/2026-06-29-mobile-i18n-v2-design.md (2)
209-244: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winLanguage-settings snippet still omits race-condition guard from prior review.
The spec's
language-settings.tsxexcerpt is missing thependingstate, thedisabled={pending !== null}prop onPressable, and the guard logic inonPick. As noted in the previous review, this makes the spec misleading for implementers. Update the snippet to match the actual implementation.+ // Why: sequential taps would race — disable the row while a pick is in flight. + const [pending, setPending] = useState<MobileUiLanguage | null>(null) + const onPick = useCallback( async (next: MobileUiLanguage) => { + if (pending) return + setPending(next) setLang(next) try { void saveUiLanguage(next) await getI18n().changeLanguage(resolveLanguage(next)) } finally { + setPending(null) } }, - [] + [pending] )And add
disabled={pending !== null}to thePressable.
381-381: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSpec locale scope contradicts actual PR implementation.
Line 381 states v2 only supports
system/en/zh, yet the PR objectives explicitly deliver parity acrossen,zh,es,ja, andko. Resolve the contradiction by updating the spec to acknowledge the expanded locale scope, or adjust the PR to match the spec's v2 boundary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6279bec2-50b2-469e-bff2-6054f5dfe059
📥 Commits
Reviewing files that changed from the base of the PR and between c8a20890c9749ba813d2e50a788b115d2516801d and f6c8cb6.
⛔ Files ignored due to path filters (1)
mobile/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (69)
config/localization-coverage-allowlist.jsonconfig/scripts/audit-localization-coverage.mjsdocs/superpowers/plans/2026-06-29-mobile-i18n-v2-pr1.mddocs/superpowers/plans/2026-06-29-mobile-i18n-v2-pr2.mddocs/superpowers/specs/2026-06-29-mobile-i18n-v2-design.mdmobile/.oxlintrc.jsonmobile/app/_layout.tsxmobile/app/about.tsxmobile/app/browser-settings.tsxmobile/app/h/[hostId]/accounts.tsxmobile/app/h/[hostId]/history/[worktreeId].tsxmobile/app/h/[hostId]/index-styles.tsmobile/app/h/[hostId]/index.tsxmobile/app/h/[hostId]/session/[worktreeId].tsxmobile/app/h/[hostId]/tasks.tsxmobile/app/index-styles.tsmobile/app/index.tsxmobile/app/language-settings.tsxmobile/app/notifications.tsxmobile/app/pair-confirm.tsxmobile/app/pair-scan-styles.tsmobile/app/pair-scan.tsxmobile/app/pair.tsxmobile/app/settings.tsxmobile/app/terminal-settings.tsxmobile/app/troubleshoot-sections.tsxmobile/app/troubleshoot.tsxmobile/app/voice-settings-styles.tsmobile/app/voice-settings.tsxmobile/metro.config.jsmobile/package.jsonmobile/src/components/ConnectionLog.tsxmobile/src/components/CustomKeyModal.tsxmobile/src/components/MobileDictationSetupSheet.tsxmobile/src/components/MobileDiffReviewDrawers.tsxmobile/src/components/MobileDiffReviewFileSummary.tsxmobile/src/components/MobileDiffReviewFooter.tsxmobile/src/components/MobileDiffReviewHeader.tsxmobile/src/components/MobileMarkdown.tsxmobile/src/components/NewWorktreeModal.tsxmobile/src/components/ProtocolBlockScreen.tsxmobile/src/components/TerminalShortcutSettings.tsxmobile/src/components/TextInputModal.tsxmobile/src/components/VoiceModelList.tsxmobile/src/components/WorktreeListRow.tsxmobile/src/components/custom-key-modal-styles.tsmobile/src/components/new-worktree-modal-styles.tsmobile/src/components/pr-sidebar/MobileLinkPrForm.tsxmobile/src/components/pr-sidebar/MobilePrComposeForm.tsxmobile/src/components/pr-sidebar/PRActionsSection.tsxmobile/src/components/pr-sidebar/PRCheckDetail.tsxmobile/src/components/pr-sidebar/PRChecksSection.tsxmobile/src/components/pr-sidebar/PRCommentCard.tsxmobile/src/components/pr-sidebar/PRCommentsSection.tsxmobile/src/components/pr-sidebar/PRConflictingFilesSection.tsxmobile/src/components/pr-sidebar/PRSidebarHeader.tsxmobile/src/components/pr-sidebar/PrSidebarCreateEmptyState.tsxmobile/src/components/pr-sidebar/ReviewerPickerDrawer.tsxmobile/src/components/terminal-shortcut-settings-styles.tsmobile/src/i18n/I18nProvider.tsxmobile/src/i18n/README.mdmobile/src/i18n/__tests__/init.test.tsmobile/src/i18n/__tests__/translate.test.tsmobile/src/i18n/__tests__/useTranslate.test.tsmobile/src/i18n/init.tsmobile/src/i18n/translate.tsmobile/src/i18n/useTranslate.tsmobile/src/storage/preferences.test.tsmobile/src/storage/preferences.ts
💤 Files with no reviewable changes (21)
- mobile/src/i18n/I18nProvider.tsx
- mobile/src/components/terminal-shortcut-settings-styles.ts
- mobile/src/i18n/README.md
- mobile/src/i18n/useTranslate.ts
- mobile/src/storage/preferences.test.ts
- mobile/src/i18n/tests/useTranslate.test.ts
- mobile/src/i18n/tests/init.test.ts
- mobile/src/components/pr-sidebar/ReviewerPickerDrawer.tsx
- mobile/src/i18n/translate.ts
- mobile/src/components/pr-sidebar/PRCheckDetail.tsx
- mobile/src/i18n/tests/translate.test.ts
- mobile/src/storage/preferences.ts
- mobile/src/components/pr-sidebar/PRCommentsSection.tsx
- mobile/src/i18n/init.ts
- mobile/src/components/pr-sidebar/PRCommentCard.tsx
- mobile/src/components/pr-sidebar/MobilePrComposeForm.tsx
- mobile/src/components/pr-sidebar/PRSidebarHeader.tsx
- mobile/src/components/pr-sidebar/PRActionsSection.tsx
- mobile/src/components/pr-sidebar/PRChecksSection.tsx
- mobile/src/components/pr-sidebar/PrSidebarCreateEmptyState.tsx
- mobile/src/components/pr-sidebar/PRConflictingFilesSection.tsx
✅ Files skipped from review due to trivial changes (4)
- mobile/app/pair-scan-styles.ts
- config/localization-coverage-allowlist.json
- mobile/app/h/[hostId]/accounts.tsx
- docs/superpowers/plans/2026-06-29-mobile-i18n-v2-pr2.md
🚧 Files skipped from review as they are similar to previous changes (42)
- mobile/app/troubleshoot-sections.tsx
- mobile/app/index-styles.ts
- mobile/app/h/[hostId]/index-styles.ts
- mobile/app/about.tsx
- mobile/app/pair.tsx
- mobile/app/browser-settings.tsx
- mobile/app/voice-settings-styles.ts
- mobile/src/components/pr-sidebar/MobileLinkPrForm.tsx
- mobile/src/components/ConnectionLog.tsx
- mobile/.oxlintrc.json
- mobile/app/notifications.tsx
- mobile/app/h/[hostId]/history/[worktreeId].tsx
- mobile/package.json
- mobile/src/components/TextInputModal.tsx
- mobile/src/components/MobileDiffReviewFileSummary.tsx
- config/scripts/audit-localization-coverage.mjs
- mobile/src/components/MobileDiffReviewHeader.tsx
- mobile/src/components/MobileDiffReviewFooter.tsx
- mobile/app/settings.tsx
- mobile/src/components/ProtocolBlockScreen.tsx
- mobile/metro.config.js
- mobile/app/language-settings.tsx
- mobile/src/components/new-worktree-modal-styles.ts
- mobile/app/_layout.tsx
- mobile/app/pair-confirm.tsx
- mobile/src/components/custom-key-modal-styles.ts
- mobile/app/pair-scan.tsx
- mobile/app/troubleshoot.tsx
- mobile/src/components/MobileDictationSetupSheet.tsx
- mobile/src/components/WorktreeListRow.tsx
- mobile/app/terminal-settings.tsx
- mobile/src/components/CustomKeyModal.tsx
- mobile/src/components/MobileDiffReviewDrawers.tsx
- mobile/src/components/MobileMarkdown.tsx
- mobile/app/h/[hostId]/index.tsx
- mobile/src/components/VoiceModelList.tsx
- mobile/app/h/[hostId]/tasks.tsx
- mobile/app/voice-settings.tsx
- mobile/src/components/NewWorktreeModal.tsx
- mobile/src/components/TerminalShortcutSettings.tsx
- mobile/app/index.tsx
- mobile/app/h/[hostId]/session/[worktreeId].tsx
…label Address two findings from /review on PR stablyai#6752: 1. tasks.tsx styles extraction (stablyai#9) CodeRabbit: "Bumping tasks.tsx to 15,500 lines turns the current escape hatch into the new baseline. Please keep extracting focused modules/styles/helpers rather than ratcheting the limit upward." The 1392-line StyleSheet.create block moved into a sibling `tasks-styles.ts` (1400 lines) — same pattern we already used for index.tsx → index-styles.ts. tasks.tsx drops from 15,518 → 14,126 lines, fitting under the original 14,682 oxlint limit so the bumped entry is no longer needed. mobile/.oxlintrc.json reverts the tasks.tsx bump (14682 stays) and adds a `tasks-styles.ts` entry at 1500. 2. Filter chip accessibility label (#5) mobile/src/components/MobileDiffReviewHeader.tsx line 120 was interpolating `filter` (the currently-selected filter prop) instead of `item` (the chip being rendered). Every chip in the FlatList announced the same label after a selection, so screen-reader users couldn't distinguish actions. Changed to `{ filter: item }` matching the suggested fix. Out of scope: session/[worktreeId].tsx still sits at 5666 lines (the 5500 limit is now 678 lines above the previous 4988). The keyboard listener / tab action sheet logic that main added in commits 93867ba and 31d35b1 isn't a clean candidate for a 1-commit extraction — extracting it would require passing several session-scoped refs and setters across the boundary. That refactor is tracked as follow-up work and will land separately before the limit can be reverted. Verification: - pnpm test: 147/147 files, 979 tests pass - pnpm exec tsc --noEmit: No errors - pnpm exec oxlint: clean - pnpm run verify:localization-catalog: 5 locales × 10898 keys - pnpm run verify:localization-coverage: 208 allowlisted
Address three i18n correctness findings from /review on PR stablyai#6752. All three translate() call sites now let translators change word order and plural rules; previously the surrounding JS composed the count outside the translated string, forcing English grammar even on localized UIs. 1. MobileMarkdown.tsx:168-178 — singular vs plural rows/columns Before: always rendered "1 more rows" / "1 more columns" because we used the plural key for both. After: split into mobile.markdown .moreRow + moreRows and .moreColumn + moreColumns, picked by `hiddenRows === 1` / `hiddenColumns === 1`. 2. MobileDiffReviewHeader.tsx:92-97 — unsent note count Before: passed two pre-translated nouns into mobileReviewCountLabel() which forced the output shape to "{count} {label}", so translators could not change word order. After: dropped the helper, render unsentCountOne / unsentCountOther keys directly with {{count}} in the string. Removed now-unused mobileReviewCountLabel import. 3. PRCommentsSection.tsx:253 — "(count)" suffix Before: `count > 1 ? \` (${count})\` : ''` composed the count outside the translation, so non-English locales couldn't restyle it (e.g. full-width parens, "共 N 条"). After: route the suffix through t() as mobile.prComments.replyCountSuffix with locale-specific formatting ('({{count}})' for en/es/ko, '（{{count}}）' for zh/ja, '（共 {{count}} 条）' for zh). 5 new keys × 5 locales = 25 new translation strings; catalog parity verified at 10903 keys per locale. Verification: - pnpm test: 147/147 files, 979 tests pass - pnpm exec tsc --noEmit: No errors - pnpm exec oxlint: clean - pnpm run verify:localization-catalog: 5 locales × 10903 keys - pnpm run verify:localization-coverage: 208 allowlisted
Address three documentation and test-coverage findings from /review on PR stablyai#6752. 1. mobile/src/i18n/README.md (#6) The "Translation keys" section still described a PR1/PR2 split with ~30 + ~470 keys across only {en,zh}.json. v2 ships 1004 keys across all 5 locales with strict parity. Refreshed to reflect the actual rollout and add a sentence on interpolation syntax + pluralization (the recent #1 fix made this distinction load-bearing). Also refreshed "Testing" to note the hook now has a real rendered test instead of the original "no RN test infrastructure" framing. 2. docs/superpowers/specs/2026-06-29-mobile-i18n-v2-design.md (#7) The "metro.config.js 兼容性" section said "v2 PR1 must retain v1's full metro.config.js changes (watchFolders + nodeModulesPaths + disableHierarchicalLookup)". Actual implementation: - only added watchFolders = [sharedRoot, i18nLocalesRoot] - did NOT add nodeModulesPaths - did NOT add disableHierarchicalLookup (which broke pnpm's @expo/metro-runtime resolution during local validation) Updated the section to describe the actual minimal change with a "Spec 修订说明" callout explaining why the original draft was wrong and how the v2 implementation ended up aligned with upstream defaults. 3. mobile/src/i18n/__tests__/useTranslate.test.ts (stablyai#8) The previous test reimplemented the hook's t() closure inline instead of calling the real hook — so any regression in useTranslate itself would still pass. Replaced with a real hook test: - Renders a tiny HookHost component via react-dom's createRoot - Captures the hook's return value through a useEffect callback - Asserts resolvedLanguage === 'zh' (the active language) - Asserts fallback path (missing key → defaultValue) - Asserts hit path (existing zh key → '设置' not 'Settings') Uses `@vitest-environment jsdom` per-file directive to opt in to DOM access for `document.createElement('div')` while keeping the rest of the mobile test suite on the faster 'node' environment. jsdom is already in mobile's transitive deps. Verification: - pnpm test: 147/147 files, 979 tests pass - pnpm exec tsc --noEmit: No errors - pnpm exec oxlint: clean - pnpm run verify:localization-catalog: 5 locales × 10903 keys - pnpm run verify:localization-coverage: 208 allowlisted
Address review finding #3 from /review on PR stablyai#6752: > "Custom submit labels still bypass i18n. This only localizes the > default 'Save' path. A supplied caller in > mobile/app/h/[hostId]/session/[worktreeId].tsx still passes > submitLabel='Open', so that modal stays English in non-English UI. > Either require callers to pass translated labels or extend this API > so custom actions can be localized too." Two-part fix: 1. mobile/src/components/TextInputModal.tsx — drop the submitLabel === 'Save' special case. The previous logic silently translated the magic string 'Save' but left every other literal English label untranslated. That distinction was a foot-gun: callers passing 'Save' got translation, callers passing 'Open' (or anything else) did not — there's no way to know which is which from the call site. Replaced with a clear contract: "submitLabel: caller passes a final string. Pass t('mobile.*', 'English') if you want it translated, or a plain English string if the call site owns the layout." Added a comment at the prop default explaining the rule so future callers don't trip on it. 2. mobile/app/h/[hostId]/session/[worktreeId].tsx:5615 — the 'New Browser' modal had three untranslated strings (title, message, submitLabel). Wired them through t() with three new keys: - mobile.session.newBrowserTitle - mobile.session.newBrowserMessage - mobile.session.open Added to all 5 locales; 打开 (zh) / Abrir (es) / 開く (ja) / 열기 (ko). The other TextInputModal call sites (pr-sidebar) already passed t('mobile.prComment.*', ...) so they continue to work unchanged. Verification: - pnpm test: 147/147 files, 979 tests pass - pnpm exec tsc --noEmit: No errors - pnpm exec oxlint: clean - pnpm run verify:localization-catalog: 5 locales × 10904 keys - pnpm run verify:localization-coverage: 208 allowlisted
|
will get this in soon |
Replaces v1 PR #6627 (closed by CodeRabbit for a no-op
<T>wrapper and missingmobile.*keys). v2 is a completerewrite that fixes both root causes and ships a full mobile Chinese translation layer.
Summary
translate(key, fallback, options?)function that mirrors the desktop'ssrc/renderer/src/i18n/i18n.ts:43shape — both
keyandfallbackare required positional args, so missing translations fall back to the English literal(no blank key paths, no silently-empty render).
zh.jsonanden.jsondirectly via MetrowatchFolders— single source of truth, zeroduplication.
useTranslate()/translate().orca:uiLanguage; reads on boot, applies before firstt()call.mobile.*keys translated to Chinese; full parity acrossen/zh/es/ja/ko(10712 keys × 5locales).
verify:localization-coverageto scanmobile/, blocking any future untranslated string at CI.Spec / Plan
docs/superpowers/specs/2026-06-29-mobile-i18n-v2-design.mddocs/superpowers/plans/2026-06-29-mobile-i18n-v2-pr1.mddocs/superpowers/plans/2026-06-29-mobile-i18n-v2-pr2.mdWhat changed
New code (in
mobile/src/i18n/)init.tsinit()resolves)translate.tstranslate(key, fallback, options?)— synchronous, returns fallback on uninituseTranslate.tsuseTranslate()hook for components; re-renders onlanguageChangedI18nProvider.tsxreact-i18next's<I18nextProvider>README.md__tests__/init.test.ts__tests__/translate.test.tstranslate()behavior tests__tests__/useTranslate.test.tsNew screen / migration
mobile/app/language-settings.tsx(new) — 3-option picker (Follow System / English / 中文（简体）), persisted vialoadUiLanguage/saveUiLanguageindex,about,troubleshoot,voice-settings,terminal-settings,browser-settings,notifications,pair,pair-scan,pair-confirm,settings, plus all 12 routes underapp/h/[hostId]/(workspaces,accounts, history, tasks, session, source-control, files, review, pr)
NewWorktreeModal,CustomKeyModal,MobileMarkdown,pr-sidebar/*(12 files),TerminalShortcutSettings,MobileDiffReview*,MobileDictationSetupSheet,WorktreeListRow,VoiceModelList,ProtocolBlockScreen,ConnectionLog,TextInputModal,ReviewerPickerDrawermax-linesrule (troubleshoot-sections.tsx,voice-settings-styles.ts,index-styles.ts,pair-scan-styles.ts)Translations
mobile.*keys added to all 5 locales (en.json/zh.json/es.json/ja.json/ko.json)Configuration
mobile/metro.config.js—watchFoldersnarrowed to[sharedRoot, i18nLocalesRoot](was the entire workspace root).Hot-reload latency dropped; Metro cache no longer invalidated by desktop-only edits.
mobile/.oxlintrc.json— bumpedmax-linesoverrides fortasks.tsxandsession/[worktreeId].tsx(now exceed priorlimits due to ~153 added
t()calls)mobile/src/storage/preferences.ts— addedloadUiLanguage/saveUiLanguage(4 tests)mobile/app/_layout.tsx—useEffectto init i18n on mount, gate render behindreadyflag, try/catch fallback chainto
enCoverage / catalog tooling
config/scripts/audit-localization-coverage.mjs—parseArgsaccepts repeated--source-root; defaultsourceRootsnow includes both
src/renderer/srcandmobile/src.main()iterates over all source roots.config/localization-coverage-allowlist.json— 198 entries, each with a per-sub-block reason explaining the specificdeferral pattern (prop attribute / object property / jsx-expression). 37 unique reason categories.
v1 → v2 root cause fixes
v1 PR #6627 was closed by CodeRabbit for two issues. Both are fixed:
<T>was a no-op wheni18nKeywas missing (612/944 migrations broken)translate(key, fallback)makes both argszh.jsonhad zeromobile.*keysmobile.*block in all 5 locales, hand-translatedVerification
pnpm test(mobile/)pnpm run verify:localization-catalogpnpm run verify:localization-coveragepnpm exec oxlint(mobile/)useGitStatusPolling.tswarning)pnpm exec expo export --platform androidgradle assembleDebug/language-settings→ pick 中文 → UI immediately switches),Manual smoke test steps for the reviewer
pnpm startinmobile/Chinese
loadUiLanguage()Test plan
pnpm test— confirms 979 vitest tests passpnpm run verify:localization-catalog— confirms 5 locales × 10712 keys paritypnpm run verify:localization-coverage— confirms no new unlocalized stringspnpm exec oxlintinmobile/— confirms no new lint errorspnpm exec expo export --platform android— confirms bundle buildsOut of scope (PR3 follow-up)
without per-site refactoring
LABEL_OPTIONSarrays)+{n}/-{n}), URLs, brand namesEach entry in the allowlist has a per-sub-block reason explaining the specific pattern. PR3 will sweep through these by
replacing
LABEL_OPTIONSarrays withtranslate()calls and refactoring PickerModal / TextInput to acceptt()results.
PR3 scope: also covers the remaining
mobile.*keys with English placeholders ines.json(full es translation), andthe
mobile.*keys still in English ines.json(mostly legacy brand-name strings).Breaking changes
None. This PR only adds new files and modifies files that have not been released yet (the
mobile/app has not beenpublished to a stable release).
Risk assessment
zh.json+en.json(~1.2 MB).Hermes-precompile + Metro tree-shaking bring the runtime bundle to 11 MB. Acceptable for a 50 MB app budget.
[sharedRoot, i18nLocalesRoot]. Edits outside these directories no longertrigger Metro hot-reload. This is a strict improvement.
orca:uiLanguageis a fresh key. No migration needed.CodeRabbit v1 feedback addressed
All 16 actionable comments from v1 PR #6627 were resolved:
<T>no-op wheni18nKeymissing (11 occurrences)translate(key, fallback)makes both requiredi18next::pluralResolvermissing in tests (1)i18next.init()with zh resourcesloadUiLanguagesave failure swallowed (1)console.warn+ best-effort write preserved; UI never blocksonPickrace condition (1)pendingstate disables all rows mid-switchinit()resolvesThe 9 additional findings from this PR's own /review pass are documented in the commit log.
Open follow-ups (out of scope for this PR)
mobile.*blockpr-sidebar/*and the diff review components were notmigrated; these are in the allowlist
verify:localization-coverageto verify interpolation placeholder formatELI5
Orca Mobile gains Chinese translations through a proper translate helper with English fallbacks. Missing keys no longer blank out; users see Chinese when available and English otherwise.