Skip to content

feat(i18n): make web and CLI language-aware#1748

Merged
magyargergo merged 10 commits into
abhigyanpatwari:mainfrom
ChamHerry:codex/i18n-20260521
May 23, 2026
Merged

feat(i18n): make web and CLI language-aware#1748
magyargergo merged 10 commits into
abhigyanpatwari:mainfrom
ChamHerry:codex/i18n-20260521

Conversation

@ChamHerry

@ChamHerry ChamHerry commented May 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds web i18n with English and Simplified Chinese resources, browser-language detection, persisted language preference, and a language switcher.
  • Replaces web UI literals across the main panels/components with translated resources.
  • Adds CLI i18n resources and localized command/message rendering for serve/status/list/remove/tool/doctor/clean paths.
  • Keeps embedding auto-start opt-in by localStorage to avoid noisy background model startup during normal web UI loading.

Why

GitNexus currently exposes both a web UI and CLI. Internationalization needs to cover both surfaces so users do not get mixed-language behavior after switching locale.

Stack / review note

This PR is submitted directly to upstream main per request, so it currently includes the prerequisite LadybugDB sidecar recovery commit from #1747:

If #1747 lands first, this branch should be rebased onto upstream main so this PR becomes a pure i18n diff.

Validation

  • cd gitnexus && npx vitest run test/unit/cli-i18n.test.ts test/unit/cli-message.test.ts && npx tsc --noEmit
  • cd gitnexus-web && npm test && npx tsc -b --noEmit
  • Pre-commit hook: eslint/prettier + gitnexus-web and gitnexus typechecks

Not tested

  • Full browser e2e screenshot pass for every translated panel.

Follow-up / out of scope

These items are intentionally left out of this PR to keep the current i18n diff focused and reviewable:

  • Backend API error localization contract: add stable machine-readable API error codes while preserving the existing error: string response for compatibility, then update the web BackendError / assertOk / formatBackendError path to translate by code instead of matching English messages. This should be a separate PR because the backend client error path has broad impact.
  • Remaining CLI runtime strings: migrate setup, group, and wiki command runtime output to the existing CLI t() resource system in staged follow-up PRs (setup -> group -> wiki). Keep machine-readable --json output, paths, env vars, model names, URLs, and raw external/tool/LLM messages unchanged.
  • errors.json parity regression coverage: add a small test proving the en and zh-CN error resources load the expected languages. No file swap is needed; the current files are not reversed.

@vercel

vercel Bot commented May 21, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the NexusCore Team on Vercel.

A member of the Team first needs to authorize it.

ChamHerry pushed a commit to ChamHerry/GitNexus that referenced this pull request May 21, 2026
Add shared CLI translation resources and web i18next/react-i18next wiring so GitNexus can present core command and UI text in English or Chinese while preserving existing defaults.

Constraint: PR abhigyanpatwari#1748 was previously stacked on obsolete abhigyanpatwari#1747 work, so this branch is rebuilt from upstream/main and contains only i18n changes.\nRejected: Keeping the old stacked branch | it carried obsolete lbug sidecar commits and stayed conflicting after abhigyanpatwari#1747 merged.\nConfidence: medium\nScope-risk: broad\nDirective: Keep future i18n follow-ups separated from native-worker and lbug recovery fixes; update both locale trees together when adding translatable UI text.\nTested: GitNexus impact analysis for App, Header, useAppState, cleanCommand, doctorCommand; GitNexus detect_changes on staged linked worktree; CLI i18n vitest targets; web i18n vitest targets; gitnexus npx tsc --noEmit; gitnexus-web npx tsc -b --noEmit; gitnexus-web npm test; gitnexus-web npm run build; gitnexus npm run build.\nNot-tested: Browser visual/manual language switching; production Vercel deployment; full gitnexus npm test ended with Vitest unhandled simulated worker-pool startup crash after all 355 files and 9180 assertions passed.
@ChamHerry ChamHerry force-pushed the codex/i18n-20260521 branch from ece02ae to 2f0eebc Compare May 21, 2026 12:13
ChamHerry pushed a commit to ChamHerry/GitNexus that referenced this pull request May 21, 2026
Add shared CLI translation resources and web i18next/react-i18next wiring so GitNexus can present core command and UI text in English or Chinese while preserving existing defaults. Keep the heartbeat reconnect banner on a dedicated translation key so disconnect recovery remains distinguishable from initial server connection failures.

Constraint: PR abhigyanpatwari#1748 was previously stacked on obsolete abhigyanpatwari#1747 work, so this branch is rebuilt from upstream/main and contains only i18n changes.
Rejected: Keeping the old stacked branch | it carried obsolete lbug sidecar commits and stayed conflicting after abhigyanpatwari#1747 merged.
Confidence: medium
Scope-risk: broad
Directive: Keep future i18n follow-ups separated from native-worker and lbug recovery fixes; update both locale trees together when adding translatable UI text.
Tested: GitNexus impact analysis for App, Header, useAppState, cleanCommand, doctorCommand, and AppContent; GitNexus detect_changes on the linked worktree; CLI i18n vitest targets; web i18n vitest targets; gitnexus npx tsc --noEmit; gitnexus-web npx tsc -b --noEmit; gitnexus-web npm test; gitnexus-web npm run build; gitnexus npm run build; local Playwright heartbeat-reconnect e2e spec.
Not-tested: Full browser visual/manual language switching; production Vercel deployment; full gitnexus npm test ended with Vitest unhandled simulated worker-pool startup crash after all 355 files and 9180 assertions passed.
@ChamHerry ChamHerry force-pushed the codex/i18n-20260521 branch from 2f0eebc to ea5a012 Compare May 21, 2026 12:23
@github-actions

github-actions Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

CI Report

All checks passed

Pipeline Status

Stage Status Details
✅ Typecheck success tsc --noEmit
✅ Tests success unit tests, 3 platforms
✅ E2E success gitnexus-web changes only

Test Results

Tests Passed Failed Skipped Duration
9584 9583 0 1 551s

✅ All 9583 tests passed

1 test(s) skipped — expand for details
  • buildTypeEnv > known limitations (documented skip tests) > Ruby block parameter: users.each { |user| } — closure param inference, different feature

Code Coverage

Tests

Metric Coverage Covered Base Delta Status
Statements 77.69% 31564/40626 N/A% 🟢 ███████████████░░░░░
Branches 66.41% 20163/30360 N/A% 🟢 █████████████░░░░░░░
Functions 81.25% 3238/3985 N/A% 🟢 ████████████████░░░░
Lines 81.12% 28465/35087 N/A% 🟢 ████████████████░░░░

📋 View full run · Generated by CI

ChamHerry pushed a commit to ChamHerry/GitNexus that referenced this pull request May 21, 2026
Add shared CLI translation resources and web i18next/react-i18next wiring so GitNexus can present core command and UI text in English or Chinese while preserving existing defaults. Keep the heartbeat reconnect banner on a dedicated translation key so disconnect recovery remains distinguishable from initial server connection failures.

Constraint: PR abhigyanpatwari#1748 was previously stacked on obsolete abhigyanpatwari#1747 work, so this branch is rebuilt from upstream/main and contains only i18n changes.
Rejected: Keeping the old stacked branch | it carried obsolete lbug sidecar commits and stayed conflicting after abhigyanpatwari#1747 merged.
Confidence: medium
Scope-risk: broad
Directive: Keep future i18n follow-ups separated from native-worker and lbug recovery fixes; update both locale trees together when adding translatable UI text.
Tested: GitNexus impact analysis for App, Header, useAppState, cleanCommand, doctorCommand, and AppContent; GitNexus detect_changes on the linked worktree; CLI i18n vitest targets; web i18n vitest targets; gitnexus npx tsc --noEmit; gitnexus-web npx tsc -b --noEmit; gitnexus-web npm test; gitnexus-web npm run build; gitnexus npm run build; local Playwright heartbeat-reconnect e2e spec.
Not-tested: Full browser visual/manual language switching; production Vercel deployment; full gitnexus npm test ended with Vitest unhandled simulated worker-pool startup crash after all 355 files and 9180 assertions passed.
@ChamHerry ChamHerry force-pushed the codex/i18n-20260521 branch from ea5a012 to 044a48f Compare May 21, 2026 15:47
Add shared CLI translation resources and web i18next/react-i18next wiring so GitNexus can present core command and UI text in English or Chinese while preserving existing defaults. Keep the heartbeat reconnect banner on a dedicated translation key so disconnect recovery remains distinguishable from initial server connection failures.

Constraint: PR abhigyanpatwari#1748 was previously stacked on obsolete abhigyanpatwari#1747 work, so this branch is rebuilt from upstream/main and contains only i18n changes.
Rejected: Keeping the old stacked branch | it carried obsolete lbug sidecar commits and stayed conflicting after abhigyanpatwari#1747 merged.
Confidence: medium
Scope-risk: broad
Directive: Keep future i18n follow-ups separated from native-worker and lbug recovery fixes; update both locale trees together when adding translatable UI text.
Tested: GitNexus impact analysis for App, Header, useAppState, cleanCommand, doctorCommand, and AppContent; GitNexus detect_changes on the linked worktree; CLI i18n vitest targets; web i18n vitest targets; gitnexus npx tsc --noEmit; gitnexus-web npx tsc -b --noEmit; gitnexus-web npm test; gitnexus-web npm run build; gitnexus npm run build; local Playwright heartbeat-reconnect e2e spec.
Not-tested: Full browser visual/manual language switching; production Vercel deployment; full gitnexus npm test ended with Vitest unhandled simulated worker-pool startup crash after all 355 files and 9180 assertions passed.
@ChamHerry ChamHerry force-pushed the codex/i18n-20260521 branch from 044a48f to eea4b2a Compare May 21, 2026 15:48
@github-actions

github-actions Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 7m 54s —— View job


PR #1748 Production-Readiness Review

Tasks completed:

  • Post initial task list
  • Branch hygiene check
  • Read repo docs (DoD.md, AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md, TESTING.md, ARCHITECTURE.md)
  • Verify dependency additions (package.json, lockfile)
  • Inspect web i18n runtime (index.ts, resources.ts, languages.ts, main.tsx)
  • Inspect locale resource completeness & key parity (en vs zh-CN)
  • Inspect CLI i18n wiring (i18n/index.ts, env detection, command safety)
  • Inspect web components for hard-coded strings and state-preservation risks
  • Inspect error/progress message handling
  • Inspect tests (cli-i18n.test.ts, i18n.test.tsx, embedding-auto-start.test.ts)
  • Hidden/bidi Unicode scan
  • Check CI status
  • Write final review

Review bar for this PR

Applied: DoD.md (v2.0.0), TESTING.md, CONTRIBUTING.md, ARCHITECTURE.md, AGENTS.md, GUARDRAILS.md. All present and read. Key DoD items for this PR: §2.1 (real runtime path), §2.4 (user-visible behavior docs), §2.7 (tests that break on wiring failure), §4.3 (web E2E when browser flows change), §2.8 (actionable errors), §2.3 (smallest correct solution).


Problem being solved

GitNexus serves both a Vite/React web UI and a Node CLI. This PR adds i18n to both surfaces: English (default) and Simplified Chinese, with browser-language detection, localStorage persistence, a LanguageSwitcher component, and env-var-driven CLI locale detection (GITNEXUS_LANGLC_ALLLC_MESSAGESLANG).


Current PR state

  • One commit: eea4b2a — a focused feat(i18n) commit only. Verified via git show --name-status eea4b2a.
  • 69 files changed, all i18n or test/setup files. No lbug/sidecar contamination.
  • CI: 5/6 workflows completed (all green: Dependency Review ✅, CodeQL ✅, Docker Build ✅, Gitleaks ✅, PR Autofix ✅). Main CI workflow is still in_progress at review time.
  • Vercel deployment pending team authorization (unrelated to code correctness).

Merge status and mergeability

Not mergeable yet. Main CI workflow (in_progress) has not posted a final result. Branch protection requires the full CI pass before merge is allowed.


Repository history considered

  • PR fix(lbug): keep serve stable when sidecars are missing #1747 (fix(lbug): keep serve stable when sidecars are missing) merged May 21, 2026. The lbug/sidecar commit (a9fef2c) appears in git log below eea4b2a, confirming it is an ancestor already on main — it is not re-introduced by this PR.
  • PR body claims the branch was rebuilt from upstream/main as a pure i18n diff. Verified directly: git show --name-status eea4b2a shows only i18n-related files with no lbug/server-recovery paths.

Branch hygiene assessment

Clean. Single i18n-scoped commit, no unrelated churn, no merge-from-main commits, no workflow/Docker changes, no shared-contract drift (gitnexus-shared/ untouched). No hidden Unicode found (scanned via Grep pattern for U+202A–202E, U+2066–2069, U+200B–200D, U+FEFF).


Understanding of the change

The PR implements a two-layer i18n system:

Web (i18next + react-i18next): Eager-loaded JSON locale modules via import.meta.glob in resources.ts. Initialization is synchronous enough (all modules bundled) with useSuspense: false so no key flash risk. A LanguageSwitcher <select> appears in the Header. Language changes update document.lang/document.dir via i18n.on('languageChanged', ...). Detection order: ?lng= querystring → localStorage(gitnexus.lng) → navigator → htmlTag.

CLI (custom t()): A minimal template engine ({{var}} substitution) backed by static cliResources dictionaries. Language detected at call-time from GITNEXUS_LANG > LC_ALL > LC_MESSAGES > LANG, defaulting to 'en'. Machine-facing output (JSON on stdout via writeSync(1,...)) is untouched. Human-facing messages (errors, banners) go to stderr via cliErrorKey/cliInfoKey — correct channel separation verified in cli-message.ts.


Findings


Finding 1 — gitnexus doctor output misaligned for zh-CN (cosmetic but confirmed)

  • Risk: The doctor.ts command uses .padEnd(N) to align labels into columns. .padEnd() counts Unicode code points, not terminal display columns. CJK characters are double-width in most terminals. A 3-character label like '系统:' occupies 6 display columns, but .padEnd(10) only adds 7 spaces (totalling 10 code points), producing 13 display columns instead of 10. All zh-CN doctor rows will be misaligned.
  • Evidence found: doctor.ts lines 13–38 — e.g. ` ${t('doctor.labels.os').padEnd(10)}${fingerprint.platform}/${fingerprint.arch}`. zh-CN: '系统:'.padEnd(10)'系统: ' (3 chars + 7 spaces = 10 code points; terminal displays as 6+7 = 13 columns vs intended 10).
  • Recommended fix: Replace .padEnd(N) with a display-width-aware helper (e.g. wcwidth or a local CJK-width function), or use a two-column separator (tab or fixed spacer) independent of label width. Fix this →
  • Blocks merge: maybe (cosmetic regression for all zh-CN users of gitnexus doctor)

Finding 2 — t in auto-connect useEffect dependency array (low risk, but unnecessary)

  • Risk: App.tsx line 187 includes t in the useEffect dependency array for the auto-connect effect. When the user switches language, t is a new reference, which re-schedules the effect. The autoConnectRan.current ref guards actual reconnection (returns early). However, t is only used inside the async callback body for progress messages, not in the effect's conditional logic — it should not be a dependency.
  • Evidence found: App.tsx:187: }, [handleServerConnect, setProgress, setViewMode, setServerBaseUrl, setAvailableRepos, t]);. autoConnectRan.current = true is set before any async call (line 117), so switching language causes a no-op re-fire but no reconnect loop.
  • Recommended fix: Wrap t in useRef that stays current, or move translation calls to render time. At minimum, verify eslint-plugin-react-hooks is not silenced for this dep. Fix this →
  • Blocks merge: no (ref guard prevents actual harm)

Finding 3 — Unsupported ?lng= value pollutes localStorage

  • Risk: i18n/index.ts has caches: ['localStorage']. If a user visits with ?lng=fr, i18next stores fr in gitnexus.lng. On the next visit (no querystring), i18next reads fr, finds no resources, and falls back to English. The invalid key persists indefinitely, making the language switcher show English while localStorage claims French.
  • Evidence found: i18n/index.ts:32-36detection.caches: ['localStorage'], supportedLngs: [...SUPPORTED_LANGUAGE_CODES, 'zh-cn', 'zh'], nonExplicitSupportedLngs: true. The nonExplicitSupportedLngs option allows fr-FR to resolve as fr, which also has no resources.
  • Recommended fix: Add a saveMissing-safe guard or validate ?lng= against SUPPORTED_LANGUAGE_CODES before caching, or set caches: [] for the querystring order entry so querystring lookups do not write to localStorage.
  • Blocks merge: no (edge case, English fallback works)

Finding 4 — fallbackNS spans all namespaces (fragile, not a current bug)

  • Risk: i18n/index.ts:8 sets fallbackNS to all namespaces except common. If a zh-CN key is missing, i18next cascades through every other namespace before giving up. A key with the same path in graph.json could silently "satisfy" a lookup for a missing settings.json key, showing wrong content rather than an English fallback.
  • Evidence found: const FALLBACK_NAMESPACES = namespaceList.filter((namespace) => namespace !== DEFAULT_NAMESPACE) — currently 7 namespaces in fallback list. The key parity test in i18n.test.tsx lines 22–33 verifies all key paths match between en and zh-CN today, which mitigates this for the current state. Future contributors may not know this guard exists.
  • Recommended fix: Set fallbackNS: false (or fallbackNS: 'common' only) and rely on fallbackLng: 'en' for missing-key recovery. The parity test already enforces structural completeness. Document the rationale either way.
  • Blocks merge: no (parity test guards current state)

Finding 5 — zh_TW/zh_HK environment mapped to Simplified Chinese without documentation

  • Risk: detectCliLanguage maps any env value starting with zh to zh-CN. Users with LANG=zh_TW.UTF-8 or LANG=zh_HK.UTF-8 (Traditional Chinese) will receive Simplified Chinese output, which is not the same script.
  • Evidence found: gitnexus/src/cli/i18n/index.ts:12return raw.toLowerCase().startsWith('zh') ? 'zh-CN' : 'en';. The cli-i18n.test.ts does not test this case.
  • Recommended fix: Document the limitation explicitly in a code comment (no Traditional Chinese locale is planned). Consider mapping zh_TW/zh_HK back to en until a Traditional Chinese locale is available. Add a test case. Fix this →
  • Blocks merge: no (no Traditional Chinese users are targeted today, but creates a correctness gap)

Finding 6 — CLI plural keys defined but ignored by the t() resolver

  • Risk: en.ts and zh-CN.ts define doctor.nodes_one, doctor.nodes_other, doctor.chunks_one, doctor.chunks_other, but the CLI t() function has no plural logic — callers use the base key ('doctor.nodes'). For count=1 in English, t('doctor.nodes', { count: 1 }) returns '1 nodes' (grammatically incorrect). The _one/_other keys are unreachable dead code.
  • Evidence found: doctor.ts:35t('doctor.nodes', { count: embeddingConfig.batchSize }). i18n/index.ts:23-30 — no plural resolver in the custom t(). en.ts:74-78 — both base and plural variants defined. Batch sizes are typically ≥ 16 in practice, so the "1 nodes" case is unlikely but possible for custom configs.
  • Recommended fix: Either implement basic plural logic (key + _one suffix when count === 1), or remove the _one/_other keys and use a context-free form in English (e.g. '{{count}} node(s)'), or always use _other for CLI simplicity.
  • Blocks merge: no (cosmetic English grammar issue at count=1)

Finding 7 — No browser E2E coverage for language switching (DoD §4.3 gap)

  • Risk: The LanguageSwitcher is a new user-facing browser component in the Header. DoD.md §4.3 requires npm run test:e2e when browser flows or user-facing UI behavior changes. The PR explicitly states "Not tested: Full browser visual/manual language switching." The i18n.test.tsx tests i18next wiring (unit level) but does not exercise the Header render, the switcher within the actual app layout, language changes during onboarding/analyze flows, or the reconnecting banner text in zh-CN.
  • Evidence found: gitnexus-web/test/unit/i18n.test.tsx — renders LanguageSwitcher in isolation (line 55). No E2E test file for language switching found. PR body explicitly names this as not tested.
  • Recommended fix: Add a targeted Playwright spec for: (a) switcher is present in the Header, (b) selecting zh-CN updates visible UI text and document.lang, (c) refresh preserves language. The existing startup/onboarding e2e spec is a natural host. If the team accepts the gap for this PR, it should be tracked in an issue.
  • Blocks merge: maybe — the DoD requires E2E for user-facing browser flow changes. The language switcher is a new user-facing interaction in the header of every view.

Finding 8 — Main CI not yet concluded

  • Risk: The main CI workflow (id: 26236888336) was in_progress at review time. It cannot be confirmed green.
  • Evidence found: mcp__github_ci__get_ci_status returned status: "in_progress", conclusion: null for the main CI run.
  • Blocks merge: yes (per DoD and branch-protection policy — do not merge while checks are pending)

PR-specific assessment sections

A. Web i18n initialization and language persistence

  • import './i18n' side-effect in main.tsx — synchronous module evaluation; eager import.meta.glob bundles all JSON. With useSuspense: false, components render immediately with the pre-loaded resources. No first-render key flash expected.
  • i18nReady is a Promise but nothing awaits it before ReactDOM.createRoot(...).render(...). For the eager-resource pattern this is safe — i18n.init() completes synchronously enough before first render. The test (i18n.test.tsx:36-42) awaits i18nReady explicitly.
  • localStorage in private/restricted mode: setup.ts adds a working in-memory shim for test environments. Browser private mode still has localStorage (per-session only) — the language preference just won't persist across sessions, which is acceptable.

B. Locale resource completeness and key parity

  • i18n.test.tsx lines 22–33 run a recursive flattenKeys() comparison for all namespaces. This is the correct structural guard.
  • Spot-checked: en/common.jsonzh-CN/common.json — full key parity confirmed. en/errors.jsonzh-CN/errors.json — full key parity confirmed. Interpolation variables match (e.g. {{seconds}}, {{message}}).
  • One intentional mixed-language choice: list.unknown = 'unknown' in zh-CN (zh-CN.ts:21). Acceptable for a commit-hash fallback technical term, but should be a conscious decision.

C. Web UI conversion correctness

  • All changed components use useTranslation consistently. Hard-coded English accepted: product names ("GitNexus", "Nexus AI"), GitHub URLs, API parameter names, tool identifiers ("cypher", "impact"), code tokens. No unexpected raw English in UI flows spotted in the reviewed components.

D. Error/progress behavior

  • formatBackendError (error-messages.ts) preserves original error.message as defaultValue fallback for all switch branches. If a translation key is missing, the raw server error message is shown. Diagnostic detail is preserved. ✅
  • translateAnalyzePhase and translateProgressMessage use defaultValue: '' with fallback to message || phase — unknown phases show the server-provided text. ✅
  • errors:backend.rateLimited interpolates {{seconds}} — confirmed matching variable in both locales. ✅

E. Browser flow and state preservation

  • Language switching updates document.lang/dir only; does not touch graph state, repo, panels, or server connection. Confirmed: i18n.on('languageChanged', syncDocumentLanguage) only touches document.documentElement. ✅
  • t in auto-connect useEffect dep (Finding 2): autoConnectRan.current ref guards reconnection. No loop risk confirmed.

F. Embedding auto-start

  • shouldAutoStartEmbeddings() reads localStorage.getItem('gitnexus.autoStartEmbeddings') === 'true' — opt-in only, defaults to false. Unchanged by i18n changes. Test (embedding-auto-start.test.ts) covers default-off, opt-in, and non-true value cases. ✅

G. CLI localization and automation safety

  • Confirmed safe: JSON tool output (query, context, impact, cypher, detect-changes) goes to stdout via writeSync(1, ...) — untouched by localization. ✅
  • Confirmed safe: MCP protocol files — grep found no imports from cli/i18n in any MCP file. ✅
  • Confirmed safe: All localized output goes to stderr via cliInfo/cliError/cliWarn or console.log (the list/status/doctor commands write tabular output to stdout). The tabular stdout output of list/status/doctor is human-formatted (indented labels, not JSON), so localization is appropriate.
  • Flag: formatDetectChangesResult in tool.ts lines 169–204 contains hardcoded English ("No changes detected.", "Changes:", "Affected processes:", "Risk level:", "Changed symbols:", "Affected execution flows:"). This was pre-existing and not changed in this PR, but it creates an inconsistency: running gitnexus detect-changes in zh-CN locale gives English output while other commands are in Chinese.

H. Dependency and security posture

  • Added: i18next@^26.2.0, i18next-browser-languagedetector@^8.2.1, react-i18next@^17.0.8. These are the standard, well-maintained i18next ecosystem packages (millions of weekly downloads, CNCF-backed). Dependency Review CI check passed. ✅
  • No new XSS surface: interpolation: { escapeValue: false } is correct for React (React escapes JSX by default). Translated strings are plain text values rendered as React children — not dangerouslySetInnerHTML. ✅
  • lockfile includes updated hashes for the three new deps. Spot-checked package names and maintainer consistent with official packages.

I. Branch hygiene and stacked PR contamination

  • Clean. Single commit eea4b2a. No lbug/sidecar files. No unrelated churn. Verified via git show --name-status eea4b2a.

Back-and-forth avoided by verifying

  • No reconnect loop: autoConnectRan.current = true is set before tryConnect(), so language-change-triggered effect re-fires are immediately no-ops.
  • Parity tests exist: Both i18n.test.tsx and cli-i18n.test.ts run key-parity checks, so missing translations fail loudly in CI.
  • MCP is clean: No CLI i18n imported in any MCP server file.
  • README examples are command-only: gitnexus list, gitnexus status appear as invocation examples only — no expected output labels that would break.
  • lang attribute correctness: getLanguageMetadata case-insensitively maps zh-cn → canonical 'zh-CN', then document.documentElement.lang is set to the canonical code. Correct.

Open questions

  1. Is the Traditional Chinese locale (zh_TW/zh_HK) mapping to Simplified Chinese (Finding 5) a known, accepted limitation, or does it need a note in README/docs?
  2. Will the team accept the E2E gap for language switching (Finding 7) with a tracked follow-up issue, or does it block merge per DoD §4.3?
  3. Is the formatDetectChangesResult English hardcoding in tool.ts intentional (machine-facing enough to leave in English), or should it be added to the CLI locale resources?

Final verdict

not production-ready

CI's main workflow is still in_progress — merge is blocked by live checks alone. Beyond the pending CI, there are two actionable issues that need resolution before the code quality bar is met: (1) gitnexus doctor output is visually broken for all zh-CN users due to .padEnd() not accounting for CJK double-width characters (Finding 1); (2) the language switcher is a new user-facing browser interaction but has no browser E2E coverage, which the repo's own DoD §4.3 requires for UI flow changes (Finding 7). The remaining findings (Findings 2–6) are minor and do not individually block merge, but the combination of pending CI, the cosmetic regression, and the E2E gap means this PR does not yet clear the production-readiness bar. Once CI is green, the doctor alignment is fixed, and the E2E gap is either covered or tracked as an accepted follow-up issue with maintainer sign-off, the PR would be in production-ready with minor follow-ups territory.
· Branch: codex/i18n-20260521

@magyargergo

Copy link
Copy Markdown
Collaborator

@ChamHerry Can you please look into these findings? Also could you please answer the open questions?

magyargergo and others added 4 commits May 21, 2026 18:26
Merge the latest upstream main into the i18n branch while resolving the web package lockfile against the merged dependency set. Preserve the existing merge-based PR history instead of rewriting the branch.

Constraint: PR branch already contained merge-from-main commits, so merge avoids force-push/rebase risk.

Rejected: Rebasing the PR branch | would rewrite published branch history and complicate review continuity.

Confidence: medium

Scope-risk: broad

Directive: Re-run full CI after pushing because upstream main brought ingestion/lbug/analyze changes beyond the lockfile conflict resolution.

Tested: gitnexus-web npm install --package-lock-only --ignore-scripts; git diff --check; gitnexus-web npm test -- test/unit/i18n.test.tsx; gitnexus-web npx tsc -b --noEmit; gitnexus npx vitest run test/unit/cli-i18n.test.ts test/unit/cli-message.test.ts; gitnexus npx tsc --noEmit; GitNexus detect_changes on staged merge.

Not-tested: full npm test suites; Playwright e2e; post-push GitHub CI.
Keep the fix scoped to review findings: terminal-width-safe doctor output, canonical language normalization, CLI plural selection, stable auto-connect translation usage, and browser coverage for header language switching.

Constraint: PR abhigyanpatwari#1748 review requested i18n correctness fixes without broad architecture changes.
Rejected: Adding a width/persistence dependency | Local helpers and existing i18next detector hooks cover the reviewed cases with less dependency churn.
Confidence: high
Scope-risk: moderate
Directive: Keep Traditional Chinese unsupported until a zh-TW resource is added; do not map zh-Hant locales to zh-CN.
Tested: cd gitnexus && npx vitest run test/unit/cli-i18n.test.ts test/unit/cli-message.test.ts test/unit/doctor-format.test.ts
Tested: cd gitnexus-web && npm test -- i18n.test.tsx
Tested: cd gitnexus && npx tsc --noEmit
Tested: cd gitnexus-web && npx tsc -b --noEmit
Tested: npx eslint <changed files>
Tested: git diff --check
Tested: cd gitnexus-web && npm run test:e2e -- language-switching.spec.ts
Tested: cd gitnexus-web && npm test
Tested: mcp__gitnexus__.detect_changes(scope="staged", repo="GitNexus")
Not-tested: cd gitnexus && npm test currently fails 5 pre-existing/local-env owner-guard cases in test/unit/hooks.test.ts unrelated to the i18n changes.
@ChamHerry

Copy link
Copy Markdown
Contributor Author

@magyargergo I pushed commit 3cc30ee7 with the review fixes and answers to the open questions.

Addressed:

  • Fixed gitnexus doctor zh-CN/CJK alignment with display-width-aware padding and focused regression coverage.
  • Added Header language-switching Playwright coverage for zh-CN selection, document.lang, localStorage persistence, reload persistence, and switching back to English.
  • Prevented unsupported ?lng= values from being cached as invalid gitnexus.lng values; browser-detected aliases now normalize only to canonical supported languages.
  • Removed cross-namespace fallback (fallbackNS) so missing keys do not silently resolve from unrelated namespaces.
  • Updated CLI locale detection so Traditional Chinese locales (zh_TW, zh_HK, zh-Hant) fall back to English until a Traditional Chinese resource exists.
  • Implemented minimal CLI plural suffix resolution for existing _one / _other keys.
  • Removed t from the auto-connect effect dependency path by using a current translator ref.

Open questions:

  1. Traditional Chinese is not supported in this PR; those locales now fall back to English rather than Simplified Chinese.
  2. The language switcher E2E gap is covered by gitnexus-web/e2e/language-switching.spec.ts.
  3. formatDetectChangesResult hardcoded English remains out of scope/pre-existing; I recommend tracking it separately unless you want that included in this PR.

Local validation:

  • cd gitnexus && npx vitest run test/unit/cli-i18n.test.ts test/unit/cli-message.test.ts test/unit/doctor-format.test.ts
  • cd gitnexus-web && npm test -- i18n.test.tsx
  • cd gitnexus && npx tsc --noEmit
  • cd gitnexus-web && npx tsc -b --noEmit
  • npx eslint <changed files>
  • git diff --check
  • cd gitnexus-web && npm run test:e2e -- language-switching.spec.ts
  • cd gitnexus-web && npm test
  • mcp__gitnexus__.detect_changes(scope="staged", repo="GitNexus")

Note: cd gitnexus && npm test still fails 5 existing/local-env owner-guard cases in test/unit/hooks.test.ts; the targeted CLI tests for these i18n changes pass. The remaining PR blocker currently visible is Vercel team authorization, not a code/test failure.

Route Commander help headings, root command descriptions, subcommand descriptions, option descriptions, defaults labels, and analyze custom help text through the existing CLI i18n resources so `GITNEXUS_LANG=zh-CN gitnexus -h` no longer shows English-only copy.

Constraint: PR abhigyanpatwari#1748 review fixes must keep machine-facing tool output stable; this only changes human-facing Commander help text.
Rejected: Rewriting every command registration inline | A post-registration help localization pass keeps lazy command setup stable and avoids broad command action churn.
Confidence: high
Scope-risk: narrow
Directive: Keep CLI syntax tokens and env var names untranslated; localize explanatory prose only.
Tested: cd gitnexus && npx vitest run test/unit/cli-index-help.test.ts test/unit/cli-i18n.test.ts test/unit/cli-message.test.ts test/unit/doctor-format.test.ts
Tested: cd gitnexus && npx tsc --noEmit
Tested: cd gitnexus && npx eslint src/cli/index.ts src/cli/help-i18n.ts src/cli/i18n/en.ts src/cli/i18n/zh-CN.ts test/unit/cli-index-help.test.ts
Tested: cd gitnexus && git diff --check
Tested: GITNEXUS_LANG=zh-CN node --import tsx src/cli/index.ts --help
Tested: GITNEXUS_LANG=zh-CN node --import tsx src/cli/index.ts query --help
Tested: GITNEXUS_LANG=zh-CN node --import tsx src/cli/index.ts analyze --help
Tested: mcp__gitnexus__.detect_changes(scope="staged", repo="GitNexus")
Not-tested: cd gitnexus && npm test not rerun; earlier full suite still had 5 pre-existing/local-env owner-guard failures in test/unit/hooks.test.ts unrelated to CLI i18n help.
@ChamHerry

Copy link
Copy Markdown
Contributor Author

@magyargergo Follow-up pushed in b4d521d0 for the additional CLI help i18n gap.

What changed:

  • Localized Commander help headings (Usage, Options, Commands, etc.).
  • Localized root gitnexus -h command descriptions, version/help option text, and implicit help [command] text.
  • Localized command-specific option descriptions, including query --help and group subcommand help.
  • Localized the custom analyze --help environment-variable help block.
  • Kept command syntax tokens, flags, env var names, URLs, and machine-facing tool output unchanged.

Validation:

  • cd gitnexus && npx vitest run test/unit/cli-index-help.test.ts test/unit/cli-i18n.test.ts test/unit/cli-message.test.ts test/unit/doctor-format.test.ts
  • cd gitnexus && npx tsc --noEmit
  • cd gitnexus && npx eslint src/cli/index.ts src/cli/help-i18n.ts src/cli/i18n/en.ts src/cli/i18n/zh-CN.ts test/unit/cli-index-help.test.ts
  • cd gitnexus && git diff --check
  • GITNEXUS_LANG=zh-CN node --import tsx src/cli/index.ts --help
  • GITNEXUS_LANG=zh-CN node --import tsx src/cli/index.ts query --help
  • GITNEXUS_LANG=zh-CN node --import tsx src/cli/index.ts analyze --help
  • mcp__gitnexus__.detect_changes(scope="staged", repo="GitNexus") reported LOW risk / 0 affected processes.

The PR head is now b4d521d069d9b3b03673dd54ad1736f5304fce8c. CI restarted and is queued; Vercel still shows the same team authorization failure.

@github-actions

github-actions Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 12m 1s —— View job


PR #1748 Production-Readiness Review — Second Pass

Tasks completed

  • Post initial task list
  • Read repo docs (DoD.md, AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md, TESTING.md, ARCHITECTURE.md)
  • Check git log / branch hygiene — compare final diff vs origin/main
  • Check CI status for latest head SHA (b4d521d)
  • Inspect web i18n runtime (index.ts, resources.ts, languages.ts, main.tsx)
  • Inspect locale resource completeness & key parity (en vs zh-CN, all namespaces)
  • Inspect CLI i18n wiring (i18n/index.ts, locale detection, plural/interpolation)
  • Inspect doctor.ts CJK display-width fix (verify vs prior blocker)
  • Inspect App.tsx auto-connect effect (tRef fix verification)
  • Inspect LanguageSwitcher accessibility & Header wiring
  • Inspect E2E spec (language-switching.spec.ts) quality and CI wiring
  • Inspect machine-readable output boundaries (tool.ts, MCP paths, writeSync)
  • Hidden Unicode scan (git diff --check clean)
  • Inspect dependency additions (package.json, lockfile)
  • Write final review

Review bar for this PR

Applied: DoD.md v2.0.0, TESTING.md, CONTRIBUTING.md, ARCHITECTURE.md, AGENTS.md, GUARDRAILS.md. All present and read. Key DoD items: §2.1 (real runtime path), §2.7 (tests that break on wiring failure), §4.3 (web E2E when browser flows change), §4.2 (CLI typecheck + npm test), §2.4 (contracts and user-visible docs updated). This is a second-pass review; each prior finding is verified directly in the file at current HEAD, not trusted from the author's comment.


Problem being solved

GitNexus exposes both a Vite/React web UI and a Node CLI. This PR makes both surfaces language-aware: English (default) and Simplified Chinese, with browser-language detection, localStorage persistence, a LanguageSwitcher component, and env-var-driven CLI locale detection. The prior Claude review identified seven blockers; the author claimed all were addressed in 3cc30ee and b4d521d. This review verifies every claim.


Current PR state

  • Head: b4d521dfix(i18n): localize CLI help output (pushed 2026-05-22T16:43)
  • Commits: eea4b2a (initial i18n) → 27ffb80 / 57178f0 (merge-from-main) → f19adf6 (chore merge) → 3cc30ee (review fixes) → b4d521d (CLI help i18n)
  • CI: Gitleaks ✅, Dependency Review ✅, PR Autofix ✅, CodeQL ✅ — Main CI queued, Docker Build in_progress for b4d521d. Not yet confirmed green.
  • Vercel: team authorization blocked (preview only, not a merge blocker).
  • cd gitnexus && npm test has 5 pre-existing owner-guard failures in hooks.test.ts, unrelated to i18n. Author acknowledged this; CI is expected to isolate them.

Merge status and mergeability

Not mergeable yet. The main CI workflow is queued (not started) for head b4d521d. All required checks must be green before merge. No conflicts with main detected (merge commits resolved them). Beyond CI, no code-level blockers remain per this review.


Repository history considered

  • PR fix(lbug): keep serve stable when sidecars are missing #1747 (fix(lbug): keep serve stable when sidecars are missing) merged. The f19adf6 chore-merge brought in several main commits (87b91c8 lbug WAL, 954b184 --no-stats, be3833d security bump, etc.). These are all already on main, so the GitHub three-dot PR diff does not include them. Confirmed: the 73-file changed-files list in the PR context is i18n-scoped only.
  • No open i18n/language/localization issues found in the referenced context that apply to this PR.
  • Prior Claude review comment verified in detail: all seven prior blockers are now addressed in the actual files.

Branch hygiene assessment

Merge-from-main commits present; final diff is i18n-scoped.

The commit history contains 27ffb80, 57178f0, f19adf6 (three merge-from-main commits) which bring Kotlin, C++, lbug/WAL, deferred-profile, and other unrelated changes into the branch's reachable history. However, git diff <merge-base> HEAD (GitHub's PR view) is clean: 73 files, all in gitnexus-web/src/, gitnexus-web/e2e/, gitnexus-web/test/, gitnexus/src/cli/, gitnexus/test/unit/, and package*.json. No lbug, sidecar, native-worker, ingestion, Docker, workflow, or schema files appear in the PR diff. git diff --check HEAD~4..HEAD returned no whitespace errors. No hidden Unicode detected.

Classification: merge-from-main history present but PR diff is clean. Rebase not required.


Understanding of the change

Web i18next/react-i18next layer: import.meta.glob eagerly bundles all locale JSON. i18nReady is a Promise that resolves after i18n.init() (synchronous enough for eager resources). Detection order: querystringlocalStoragenavigatorhtmlTag. caches: [] disables auto-write to localStorage; persistSupportedLanguage writes only validated canonical codes. fallbackNS: false prevents cross-namespace key resolution. convertDetectedLanguage normalizes any detected code before i18next uses it. LanguageSwitcher is a controlled <select> with accessible aria-label in the header namespace.

CLI i18n layer: Custom t() engine backed by static cliResources dictionaries (TypeScript satisfies EnglishMessages enforces key parity at type level). Detection: GITNEXUS_LANGLC_ALLLC_MESSAGESLANG. normalizeCliLanguage applies the same trim().split('.')[0].replace(/_/g, '-').toLowerCase() pipeline as the web layer. _one/_other plural suffix lookup implemented. Traditional Chinese (zh_TW, zh_HK, zh-Hant) falls back to English. localizeCliHelp(program) applies i18n post-registration via Commander's configureHelp({ styleTitle, optionDescription }) callbacks and direct option.description mutation; called after registerGroupCommands(program) and before program.parse().

Automation safety: tool.ts output() writes JSON to writeSync(1, ...) — untouched. No import of cli/i18n in any MCP file confirmed. Human-facing messages go to stderr via cliError/cliInfo/cliWarn. formatDetectChangesResult remains hardcoded English (author-accepted out-of-scope).


Findings


Finding 1 — Prior blocker: CJK doctor alignment — CONFIRMED FIXED

  • gitnexus/src/cli/doctor.ts now exports displayWidth(value: string): number and padDisplayEnd(value: string, columns: number): string. isWideCodePoint covers the full Unicode wide-character range (CJK Unified, Hangul, fullwidth forms, emoji). label(key, width) calls padDisplayEnd(t(key), width).
  • gitnexus/test/unit/doctor-format.test.ts tests ASCII equivalence, CJK double-width padding ('系统:'displayWidth = 6, padDisplayEnd('系统:', 10) = '系统: ' (4 spaces, total display width 10)), and over-wide labels (no truncation).
  • Status: FIXED. No remaining alignment regression for zh-CN users of gitnexus doctor.

Finding 2 — Prior blocker: t in auto-connect useEffect dependency array — CONFIRMED FIXED

  • gitnexus-web/src/App.tsx:109-112:
    const tRef = useRef(t);
    useEffect(() => {
      tRef.current = t;
    }, [t]);
  • The auto-connect useEffect at line 191 dependency array: [handleServerConnect, setProgress, setViewMode, setServerBaseUrl, setAvailableRepos]t is absent. tRef.current is used inside the effect callback.
  • Language changes no longer re-schedule the auto-connect effect.
  • Status: FIXED.

Finding 3 — Prior blocker: Unsupported ?lng= values polluting localStorage — CONFIRMED FIXED

  • gitnexus-web/src/i18n/index.ts:54: caches: [] — i18next's language detector NEVER auto-writes to localStorage.
  • persistSupportedLanguage at lines 26–34 calls normalizeSupportedLanguage(language) and only writes if non-null. ?lng=frconvertDetectedLanguage('fr') returns 'en'persistSupportedLanguage('en') writes 'en', not 'fr'.
  • i18n.test.tsx:75–84: explicit test for changeLanguage('fr')document.lang = 'en', localStorage not set to 'fr'.
  • Status: FIXED.

Finding 4 — Prior blocker: fallbackNS spanning all namespaces — CONFIRMED FIXED

  • gitnexus-web/src/i18n/index.ts:46: fallbackNS: false. Cross-namespace key leakage impossible.
  • i18n.test.tsx:57–64: explicit test probes settings:crossNamespaceProbe from zh-CN — expects English value (from settings namespace), not the zh-CN graph namespace value.
  • Status: FIXED.

Finding 5 — Prior blocker: zh_TW/zh_HK/zh-Hant mapping to Simplified Chinese — CONFIRMED FIXED

CLI: gitnexus/src/cli/i18n/index.ts:10–28normalizeCliLanguage only maps zh, zh-cn, zh-cn-*, zh-hans, zh-hans-* to zh-CN. A comment explicitly documents the Traditional Chinese exclusion. cli-i18n.test.ts:14–18: tests that zh_TW.UTF-8, zh_HK.UTF-8, zh-Hant.UTF-8'en'.

Web: gitnexus-web/src/i18n/languages.ts:25–33 — same logic; zh-TWnullDEFAULT_LANGUAGE. i18n.test.tsx:66–73: normalizeSupportedLanguage('zh-TW')null confirmed.

  • Status: FIXED.

Finding 6 — Prior blocker: CLI plural suffix handling — CONFIRMED FIXED

  • gitnexus/src/cli/i18n/index.ts:43–57: t() computes pluralKey = key + (count === 1 ? '_one' : '_other'). Falls back to base key if plural suffix not present. Falls back to English if locale missing.
  • cli-i18n.test.ts:26–36: tests doctor.nodes with count=1'1 node' (en), count=2'2 nodes' (en), zh-CN both forms → 'N 个节点'.
  • zh-CN.ts has doctor.nodes_one and doctor.nodes_other both equal (grammatically correct for Chinese). Type-enforced by satisfies EnglishMessages.
  • Status: FIXED.

Finding 7 — Prior blocker: No browser E2E coverage for language switching — CONFIRMED FIXED

  • gitnexus-web/e2e/language-switching.spec.ts tests: zh-CN selection → document.lang = 'zh-CN' → visible Chinese text ('觉得不错就点星') → localStorage['gitnexus.lng'] = 'zh-CN' → reload persists → switcher shows zh-CN → switch back to English → document.lang = 'en'.
  • Backend is fully mocked via page.route() intercepting /api/repos, /api/repo, /api/graph, /api/heartbeat.
  • After reload with ?server=http://localhost:4747&project=mock-repo in URL (set by App.tsx handleServerConnect), auto-connect fires and reaches exploring mode within 20s timeout.
  • E2E workflow .github/workflows/ci-e2e.yml runs npx playwright test (all specs) when gitnexus-web/** changes — language-switching.spec.ts is auto-included. ✓
  • Status: FIXED.

Finding 8 — NEW CONFIRMED: CI not yet concluded for head b4d521d

  • Risk: The main CI workflow (id: 26300320688) is queued and Docker Build is in_progress. Per DoD §4 and branch protection, merge requires all required checks to be green.
  • Evidence: mcp__github_ci__get_ci_status at review time: main CI status: queued, Docker Build status: in_progress. Commits b4d521d was pushed just before this review (2026-05-22T16:43 UTC).
  • Recommended fix: Wait for CI to complete. No code change needed.
  • Blocks merge: yes — hard policy gate.

Finding 9 — NEW CONFIRMED (non-blocking): help-i18n.ts Commander metadata labels use hardcoded locale check

  • Risk: localizeOptionDescription in gitnexus/src/cli/help-i18n.ts:148–175 uses getCliLanguage() === 'zh-CN' ? '可选值' : 'choices' (and similar for default, preset, env) rather than routing through the t() resource system. Adding a third locale would require editing this function rather than adding resource keys.
  • Evidence: help-i18n.ts:148-184. These labels are for Commander's inline option metadata (choices list, default value, env var) appended in parentheses. They are correct for the current 2-locale setup.
  • Recommended fix: Accept as-is; document at locale addition time. Alternatively, add the 4 keys to the resource files. Neither is urgent.
  • Blocks merge: no.

Finding 10 — NEW CONFIRMED (non-blocking, pre-existing): formatDetectChangesResult English hardcoding

  • Risk: gitnexus/src/cli/tool.ts:169-205detect-changes tabular output ('No changes detected.', 'Changes: ...', 'Affected processes: ...', etc.) is hardcoded English. Running GITNEXUS_LANG=zh-CN gitnexus detect-changes produces mixed-language output (zh-CN error messages, English result table). The output goes to stdout via writeSync(1, ...), which is also used for machine-readable JSON output from other commands — detect-changes specifically outputs human-readable formatted text, not JSON.
  • Evidence: tool.ts:169-205, tool.ts:218 (output(formatDetectChangesResult(result))). Author explicitly accepted this as out-of-scope in the PR comment.
  • Recommended fix: Track as follow-up issue. The mixed output is an accepted limitation for this PR.
  • Blocks merge: no (pre-existing, author-accepted out-of-scope).

Finding 11 — NEW CONFIRMED (non-blocking): setup.ts beforeEach does not reset gitnexus.lng

  • Risk: gitnexus-web/test/setup.ts:34-37 only removes gitnexus-llm-settings from localStorage between tests. If a test outside i18n.test.tsx writes gitnexus.lng without cleanup, subsequent tests could detect the wrong language. Current test suite has no such leaking tests — i18n.test.tsx afterEach calls window.localStorage.clear().
  • Evidence: setup.ts:35-37, i18n.test.tsx:17-25.
  • Recommended fix: Add localStorage.removeItem('gitnexus.lng') to setup.ts's beforeEach, or localStorage.clear() after each test. Low urgency.
  • Blocks merge: no.

PR-specific assessment sections

A. React/i18next runtime

  • i18n.init() with eager-loaded resources is synchronous enough; useSuspense: false prevents render-time Promise suspension. No first-render key flash risk confirmed.
  • tRef pattern correctly isolates the auto-connect effect from language changes. ✓
  • LanguageSwitcher has aria-label={t('selectLanguage')} on the <select> and <span className="sr-only">{t('language')}</span> for screen readers. Accessible. ✓
  • syncDocumentLanguage sets both document.documentElement.lang and document.documentElement.dir (always ltr for en/zh-CN). Called on init and on every languageChanged event. No listener leak — i18n.on() is called at module scope once. ✓
  • i18n.on('languageChanged', ...) is a module-scope side effect in i18n/index.ts. Not cleaned up across hot reload. Acceptable — the module is a singleton. ✓
  • Language change does not touch any AppState, graph, repo, or connection state. Verified by reading App.tsx, Header.tsx, useAppState.tsx: i18n.changeLanguage() only updates translation context and triggers syncDocumentLanguage/persistSupportedLanguage. ✓

B. Locale resources and fallback semantics

  • i18n.test.tsx:27-37: flattenKeys() recursively compares en ↔ zh-CN for all namespaces. Structural parity enforced at test level. ✓
  • CLI: zh-CN.ts satisfies EnglishMessages — TypeScript type-system enforces exact key parity. ✓
  • Interpolation variables spot-checked: {{seconds}} in errors:backend.rateLimited, {{message}} in error paths, {{count}} in plural keys — all match between en and zh-CN in both CLI and web resources. ✓
  • fallbackNS: false confirmed. fallbackLng: 'en' handles missing zh-CN translations at namespace level. ✓
  • zh-hans and zh-hans-* correctly map to zh-CN. zh-TW/zh-HK/zh-Hant fall back to English. ✓

C. Web UI component replacement

  • All 26 modified components use useTranslation consistently. Reviewed Header.tsx, App.tsx, LanguageSwitcher.tsx in detail; spot-checked others.
  • Hard-coded acceptable English: product name "GitNexus", GitHub URLs, API parameter names, Commander tokens, emoji characters (, ). None are localized — correct distinction.
  • errors:backend.reconnecting key used for the reconnecting banner in App.tsx:309. ✓
  • header:starIfCool'觉得不错就点星' is the zh-CN translation visible in the E2E test. ✓

D. CLI i18n

  • Detection precedence: GITNEXUS_LANGLC_ALLLC_MESSAGESLANG'' → default 'en'. Confirmed in i18n/index.ts:31. ✓
  • localizeCliHelp(program) called after registerGroupCommands(program) (line 253) and before program.parse() (line 256). All commands including group subcommands are covered recursively. ✓
  • addHelpText('after', () => t('help.analyze.environment')) uses a callback for lazy evaluation at help-print time. analyze env var block localizes correctly. ✓
  • Machine-readable stdout paths: query/context/impact/cypher/detect-changes all use output()writeSync(1, ...). No t() call touches these. ✓
  • No cli/i18n imports in any MCP file. ✓

E. Embedding auto-start and localStorage

  • gitnexus.autoStartEmbeddings key is separate from gitnexus.lng. No conflict. Embedding opt-in defaults to false unchanged. Language switching does not call startEmbeddingsWithFallback. ✓
  • embedding-auto-start.test.ts covers default-off, opt-in, and non-true values. ✓

F. Dependency and security review

  • New: i18next@^26.2.0, i18next-browser-languagedetector@^8.2.1, react-i18next@^17.0.8. All are the official i18next ecosystem packages (well-maintained, widely deployed). Dependency Review CI passed. ✓
  • interpolation: { escapeValue: false } is correct for React — JSX escapes values by default. No dangerouslySetInnerHTML usage found in changed components. ✓
  • Locale resources loaded via import.meta.glob (Vite bundled at build time), not fetched from user-controlled URLs. ✓

G. Test adequacy and CI wiring

  • language-switching.spec.ts is picked up by ci-e2e.yml which runs npx playwright test (all specs) when gitnexus-web/** changes. ✓
  • The test would fail if LanguageSwitcher were removed from Header: page.getByTestId('language-switcher') would time out. ✓
  • i18n.test.tsx key-parity test would fail if zh-CN added or removed keys without updating en. ✓
  • cli-i18n.test.ts parity test (Object.keys(messages).sort() equality) would fail on key count mismatch. TypeScript satisfies provides compile-time enforcement too. ✓
  • cli-index-help.test.ts uses spawnSync to run the actual CLI binary — tests the real help output path including Commander rendering. Tests would fail if localizeCliHelp were removed or if zh-CN resource keys were missing. ✓
  • CI report for prior head (SHA 3cc30ee at the time of the github-actions CI report comment): 9565 tests passed, 1 skipped. The current head adds only cli-index-help.test.ts additions and no new test failures expected. However, the CI report is for a PRIOR head SHA (3cc30ee), not the current b4d521d. The current head's CI is queued.

H. Branch hygiene and stacked-history audit

  • Final PR diff (73 files, from the PR context changed_files list) contains only i18n, test, locale JSON, and package files. Confirmed: no lbug, sidecar, ingestion, Docker, workflow, or schema files.
  • README.md changes visible in branch history are from merge-from-main commits (WAL checkpoint docs, deferred-profile env vars). Already in main. Not in the PR diff.
  • gitnexus-web/vercel.json npm ci change also from merge-from-main. Not in the PR diff.

Back-and-forth avoided by verifying

  • E2E reload works: After clicking a repo card, App.tsx handleServerConnect sets ?project=<name> and the DropZone.onServerConnect callback adds ?server=http://localhost:4747. After page.reload(), the URL retains both params; auto-connect fires with serverUrlParam = 'http://localhost:4747'; mocked routes answer; exploring mode loads with Header+LanguageSwitcher. Timeout of 20s is adequate. ✓
  • Plural fallback chain: t('doctor.nodes', {count: 1}) → tries doctor.nodes_one → found in zh-CN → '1 个节点'. For a non-plural key with count, the base key fallback handles it cleanly. ✓
  • No reconnect loop: autoConnectRan.current = true is set before tryConnect() at line 121, blocking any re-fire even if tRef sync effect reschedules. ✓
  • Type-enforced CLI key parity: satisfies EnglishMessages in zh-CN.ts means a missing or extra key is a TypeScript compile error, caught by tsc --noEmit. ✓
  • caches: [] vs persistSupportedLanguage: These are complementary guards. caches: [] prevents i18next's LanguageDetector from writing anything. persistSupportedLanguage is the only write path, and it validates before writing. No way for an unsupported locale to enter localStorage. ✓

Open questions that remain

None that block the code review itself. The one pre-existing open question about formatDetectChangesResult was answered explicitly by the author: out-of-scope, accept English for now. Suggest tracking as a follow-up issue once this PR merges.


Final verdict

not production-ready

The code quality has cleared the bar. All seven prior blockers are confirmed fixed by direct file inspection: CJK terminal-width-aware padding, stable tRef translator reference, guarded localStorage persistence, fallbackNS: false, Traditional Chinese correct fallback to English, CLI plural suffix resolution, and Playwright E2E coverage for language switching. The remaining findings (CLI hardcoded metadata labels in help-i18n.ts, formatDetectChangesResult English, minor test-setup gap) are all non-blocking and either pre-existing or author-accepted out-of-scope.

The sole blocker is operational: the main CI workflow for head b4d521d is queued at review time — the pipeline has not yet started, let alone concluded. Merge is blocked by pending required checks per DoD §4 and branch protection. Once CI completes green, the PR is production-ready with minor follow-ups (formatDetectChangesResult localization, Commander metadata labels in resource files) that can be tracked as separate issues. No code changes are required before merge beyond CI completing.
· Branch: codex/i18n-20260521

Localize Commander metadata and detect-changes result labels while sharing the formatter across CLI and eval-server paths. Reset the persisted web language key in Vitest setup so language state cannot leak across tests.

Constraint: PR abhigyanpatwari#1748 second-pass review identified three non-blocking i18n gaps to close without changing machine-readable MCP/tool JSON contracts.
Rejected: Leaving eval-server with a duplicated detect-changes formatter | would preserve future English-only drift.
Confidence: high
Scope-risk: narrow
Directive: Keep protocol/JSON paths separate from human-readable CLI localization changes.
Tested: cd gitnexus && npx vitest run test/unit/cli-index-help.test.ts test/unit/cli-i18n.test.ts test/unit/tool-direct-cli.test.ts test/unit/eval-formatters.test.ts test/unit/cli-message.test.ts test/unit/doctor-format.test.ts; cd gitnexus && npx tsc --noEmit; cd gitnexus-web && npx vitest run test/unit/i18n.test.tsx test/unit/embedding-auto-start.test.ts test/unit/settings-service.test.ts test/unit/test-setup-storage.test.ts; cd gitnexus-web && npx tsc -b --noEmit; npm test in gitnexus-web; gitnexus detect_changes --scope staged.
Not-tested: cd gitnexus && npm test full combined run remained flaky in hooks.test.ts; reran test/unit/hooks.test.ts alone and it passed.
@ChamHerry

Copy link
Copy Markdown
Contributor Author

@magyargergo Fixed the three second-pass follow-ups and pushed commit b7a444dc.

What changed:

  • Commander help metadata labels (choices/default/preset/env) now use CLI i18n resources instead of hardcoded locale branches.
  • detect-changes human-readable formatter is shared by CLI and eval-server, with en/zh-CN labels covered by tests.
  • Web Vitest setup now clears gitnexus.lng at setup time and before each test, with a dedicated isolation test.

Call-chain / implementation plan artifact:

  • docs/todo/2026-05-23-pr1748-second-pass-followups-callchain-plan.md

Validation:

  • cd gitnexus && npx vitest run test/unit/cli-index-help.test.ts test/unit/cli-i18n.test.ts test/unit/tool-direct-cli.test.ts test/unit/eval-formatters.test.ts test/unit/cli-message.test.ts test/unit/doctor-format.test.ts
  • cd gitnexus && npx tsc --noEmit
  • targeted CLI eslint ✅ (existing any warnings remain in pre-existing files only)
  • cd gitnexus-web && npx vitest run test/unit/i18n.test.tsx test/unit/embedding-auto-start.test.ts test/unit/settings-service.test.ts test/unit/test-setup-storage.test.ts
  • cd gitnexus-web && npx tsc -b --noEmit
  • cd gitnexus-web && npm test ✅ (245 passed)
  • GitNexus MCP detect_changes(scope=staged): low risk, 0 affected execution flows ✅

Note: a full combined cd gitnexus && npm test run hit 6 hook-output parsing failures in test/unit/hooks.test.ts; rerunning test/unit/hooks.test.ts alone passed (131/131), so I treated it as the existing combined-run flake and did not change hook code.

@@ -0,0 +1,204 @@
# PR #1748 第二轮审查 Follow-up 修复方案与调用链分析

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can you please remove this doc file?

问题描述(What): 将 docs/ 文件夹从 git 版本控制中移除并加入 .gitignore
根本原因(Why): docs/ 为本地文档目录,不应纳入 PR diff 和版本控制
修复内容(How): git rm --cached -r docs/ + 更新 .gitignore 排除整个 docs/ 目录
技术细节(Details): 13 个文件从跟踪中移除,磁盘文件保留不变
影响范围(Impact): PR abhigyanpatwari#1748 diff 将不再包含 docs/ 文件
@ChamHerry

Copy link
Copy Markdown
Contributor Author

@magyargergo Done! Removed the entire docs/ folder from git tracking in commit 5606a870 and added docs/ to .gitignore. The files remain on disk locally but are no longer tracked or included in the PR diff.

@ChamHerry ChamHerry requested a review from magyargergo May 22, 2026 17:40
@github-actions

github-actions Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 7m 23s —— View job


PR #1748 Production-Readiness Review — Last Round (Translation Coverage Focus)

Tasks completed:

  • Post initial task list
  • Read repo docs (DoD.md, AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md, TESTING.md, ARCHITECTURE.md)
  • Verify branch hygiene (git log, diff, merge commits)
  • Check CI status for latest head
  • Inspect web i18n runtime (index.ts, resources.ts, languages.ts, main.tsx)
  • Run hardcoded-string searches across all web components
  • User-facing page translation sweep (all lanes)
  • Inspect locale resource integrity (key parity, interpolation, zh-CN prose)
  • Inspect CLI i18n (channel separation, plurals, CJK width, unsupported locale)
  • Verify prior review blocker closure (all 7 findings)
  • Inspect tests and CI wiring
  • Hidden Unicode / hygiene checks
  • Write final review

Review bar for this PR

Applied: DoD.md v2.0.0, TESTING.md, CONTRIBUTING.md, ARCHITECTURE.md, AGENTS.md, GUARDRAILS.md. Key items for this pass: §2.1 (end-to-end real runtime path), §2.4 (user-visible behavior), §4.3 (web E2E for browser-flow changes), §2.7 (tests that break on wiring failure).


Problem being solved

GitNexus web UI and CLI are English-only. This PR adds web i18n (English + Simplified Chinese, browser-language detection, persisted preference, header language switcher) and CLI i18n (env-var-driven locale, localized human-facing output for serve/status/list/remove/tool/doctor/clean). The stated goal is to make all GitNexus-owned user-facing text language-aware across both surfaces.


Current PR state

  • Head: 5606a87 (chore: remove docs/ from git tracking)
  • Commit chain: eea4b2a (initial i18n) → merge commits → 3cc30ee (review blockers) → b4d521d (CLI help i18n) → b7a444d (second-pass follow-ups) → 5606a87 (docs/ removal)
  • CI: Gitleaks ✅, Dependency Review ✅, CodeQL ✅, PR Autofix ✅, Docker Build ✅ — Main CI in_progress (started 2026-05-22T17:33). Not concluded at review time.
  • Vercel: team authorization blocked (preview only; not a required status check).

Merge status and mergeability

Not mergeable. The main CI workflow is in_progress. Required checks are not fully green. No code conflicts detected.


Repository history considered


Branch hygiene assessment

Clean PR diff, merge-from-main history present but harmless. All five i18n commits are scoped to i18n/test/locale files only. git show --name-status on each commit confirms no lbug/sidecar/ingestion/native-worker/Docker/CI/release churn in the PR diff. git diff --check HEAD returned no whitespace errors. No hidden/bidi Unicode control characters found in source files.


Understanding of the change

Web layer: i18next with eager import.meta.glob bundles. caches: [] prevents language-detector auto-write; persistSupportedLanguage validates before writing to localStorage. fallbackNS: false prevents cross-namespace key leakage. convertDetectedLanguage normalizes zh-TW/zh-HK to null → falls back to English. tRef pattern keeps translator stable in auto-connect effect. LanguageSwitcher in header with aria-label and sr-only label. Language change touches only document.lang/dir and localStorage — no graph, repo, or connection state mutation.

CLI layer: Custom t() with {{var}} interpolation, _one/_other plural suffix resolution, Traditional Chinese correctly falls back to English, detectCliLanguage reads GITNEXUS_LANG > LC_ALL > LC_MESSAGES > LANG. localizeCliHelp(program) post-registers Commander help text. Machine-readable stdout (writeSync(1, ...)) is untouched. formatDetectChangesResult is now in detect-changes-format.ts and routed through t().


User-facing page translation sweep

Landing / repo selection / upload / drop

DropZone.tsx, RepoLanding.tsx, OnboardingGuide.tsxPASS. All user-visible text uses t(). Phase cards (SuccessCard, LoadingCard), step rows, terminal window labels, polling bar, copy button aria-labels, repo cards, time-relative labels — all translated. Path example placeholders (/home/you/project, C:\Users\you\project) in RepoAnalyzer.tsx:439 are OS-specific technical examples, not prose — acceptable.

Analyze / onboarding / progress

AnalyzeOnboarding.tsx, AnalyzeProgress.tsx, LoadingOverlay.tsx, RepoAnalyzer.tsxPASS. Progress messages route through translateProgressMessage. Analyze phases and progress keys have full en/zh-CN parity.

Header / nav / language switcher

Header.tsx, LanguageSwitcher.tsxPASS. Search placeholder, aria-label, repo dropdown labels, reanalyze/delete tooltips, counts, AI button, GitHub star button text — all via t(). Language switcher has proper aria-label and sr-only span.

Graph exploration

GraphCanvas.tsxPASS. AI highlights toggle title uses t('canvas.turnOnHighlights')/t('canvas.turnOffHighlights'). Zoom/fit/layout buttons reviewed in locale files — all present.

File tree / code references / processes / right panel

FileTreePanel.tsx, CodeReferencesPanel.tsx, ProcessesPanel.tsx, RightPanel.tsxPASS for all visible labels, empty states, filter placeholder, process highlight/view button titles, tabs, and chat send/stop/clear buttons.

Query / chat / FAB / tool calls

QueryFAB.tsx, RightPanel.tsx, ToolCallCard.tsxPARTIALLY PASSING. The Cypher query textarea placeholder in QueryFAB.tsx:240 is a hardcoded Cypher query example (MATCH (n:Function) RETURN n.name...) — this is a code token / technical example, acceptable as non-translatable. However, MarkdownRenderer.tsx:130 has confirmed hardcoded English tooltips (see Finding 1 below).

Settings / provider config

SettingsPanel.tsx, ProviderConfigCard.tsxPARTIALLY PASSING. Most provider config is translated. Provider titles ("OpenAI", "Google Gemini", "Anthropic", "OpenRouter", "MiniMax") are product names — acceptable. Model name placeholders (e.g., gpt-4o, gpt-4-turbo) are API identifiers — acceptable. However, two hardcoded English prose strings remain (see Findings 2 and 3 below).

Help panel

HelpPanel.tsxPASS. Uses useTranslation. Help content is large but routed through i18n resources.

Status / loading / error / reconnect / embedding / WebGPU / modals

StatusBar.tsx, LoadingOverlay.tsx, EmbeddingStatus.tsx, WebGPUFallbackDialog.tsx, ProcessFlowModal.tsxPASS. All verified to use t(). Error messages route through formatBackendError with defaultValue fallback. Reconnect banner at App.tsx:309 uses t('errors:backend.reconnecting'). WebGPU fallback dialog has full zh-CN coverage including CPU mode options.


CLI translation sweep

All human-facing CLI output confirmed localized: serve.ts, status.ts, list.ts, remove.ts, clean.ts, doctor.ts, tool.ts. Machine-readable stdout (writeSync(1, ...) for query/context/impact/cypher/detect-changes JSON output) confirmed untouched. formatDetectChangesResult now routes through detect-changes-format.ts and t(). Commander help text localized via localizeCliHelp post-registration. Structured error fields in tool.ts:142-146 are JSON protocol fields — acceptable as English.


Findings


Finding 1 — CONFIRMED MERGE-BLOCKING: MarkdownRenderer.tsx:130 hardcoded English tooltips in chat grounding links

  • Risk: When a zh-CN user is in the AI chat panel and the AI response contains grounding references like [[Function:validateUser]] or [[src/auth.ts:42]], hovering over those reference chips shows the English tooltips View Function:validateUser in Code panel or Open in Code panel • src/auth.ts:42. These are title attributes visible on hover to sighted users and read by screen readers. The component imports useTranslation and uses t() for the copy button, but these two tooltip strings are completely absent from locale resources.
  • Evidence found: gitnexus-web/src/components/MarkdownRenderer.tsx:130:
    ? `View ${inner} in Code panel` : `Open in Code panel • ${inner}`}
    Confirmed by grep -rn "codeRef\|nodeRef\|viewNode\|openInCode\|code.panel\|codePanelView" gitnexus-web/src/locales/ — no matching locale key exists.
  • Recommended fix: Add locale keys (e.g., common:chat.viewNodeInCodePanel / common:chat.openInCodePanel) and pass inner as an interpolation variable. Since markdownComponents is memoized with [handleLinkClick] deps, the simplest fix is to call t() outside the memo and capture the strings, or add t to the useMemo deps (via a stable useTranslation ref). Fix this →
  • Blocks merge: YES — reachable in a core app flow (AI chat); GitNexus-owned text; completely absent from i18n resources.

Finding 2 — CONFIRMED MERGE-BLOCKING: SettingsPanel.tsx:494-497 hardcoded English help text for OpenAI custom base URL

  • Risk: When a zh-CN user opens Settings and selects the OpenAI provider, they see the following hardcoded English paragraph below the base URL input:
    Leave empty to use the default OpenAI API. Set a custom URL for proxies or compatible APIs.
    
    This is a full prose sentence that GitNexus owns, not a product name or API identifier. It provides instructional guidance to the user about the base URL field.
  • Evidence found: gitnexus-web/src/components/SettingsPanel.tsx:494-497. Confirmed by grep -rn "Leave empty\|default OpenAI\|proxies or" gitnexus-web/src/locales/ — no matching locale key exists.
  • Recommended fix: Add a locale key (e.g., settings:providers.openai.baseUrlHint) to both en/settings.json and zh-CN/settings.json, and use {t('settings:providers.openai.baseUrlHint')} in the component. Fix this →
  • Blocks merge: YES — reachable user-facing settings flow; GitNexus-owned English prose; completely absent from i18n resources.

Finding 3 — CONFIRMED MERGE-BLOCKING: SettingsPanel.tsx:702 hardcoded , then run: in Ollama quickstart block

  • Risk: The Ollama provider quickstart block partially uses t() for the intro sentence but then appends hardcoded English , then run:. In zh-CN mode the sentence renders as: 从这里安装 Ollama:ollama.ai, then run: followed by the ollama serve code block. This creates a mid-sentence English fragment in an otherwise Chinese UI.
  • Evidence found: gitnexus-web/src/components/SettingsPanel.tsx:700-706:
    {t('settings:providers.ollama.installFrom')}{' '}
    <a href="https://ollama.ai" ...>ollama.ai</a>
    , then run:
    settings:providers.ollama.installFrom is translated, but , then run: is hardcoded. Confirmed by grep — no matching locale key exists.
  • Recommended fix: Add settings:providers.ollama.thenRun key to both locale files (e.g. ", then run:" in en, ",然后运行:" in zh-CN) and replace the hardcoded fragment. Or restructure the JSX to render the full sentence via t() with a component interpolation. Fix this →
  • Blocks merge: YES — creates a visibly broken mixed-language sentence in the Ollama settings panel for zh-CN users.

Finding 4 — CONFIRMED (operational): Main CI workflow in_progress

  • Risk: The main CI workflow (id: 26302626059) is still in_progress at review time. Per DoD §4 and branch protection, merge requires all required checks to be green.
  • Evidence: mcp__github_ci__get_ci_status at review time: status: in_progress, conclusion: null.
  • Blocks merge: YES — hard operational gate. No code change needed; wait for completion.

PR-specific assessment sections

A. Web page translation sweep
Three hardcoded strings confirmed as merge-blocking (Findings 1–3). All other lanes checked and passing: landing/onboarding, graph canvas, header, file tree, code references, processes panel, right panel/chat, help, status bar, loading overlay, embedding status, WebGPU fallback dialog, reconnect banner, settings (except OpenAI base URL hint and Ollama "then run:").

B. i18n runtime behavior
caches: [] confirmed — language detector never auto-writes localStorage. persistSupportedLanguage validates before writing. convertDetectedLanguage normalizes on detection. fallbackNS: false confirmed. tRef pattern isolates auto-connect from language changes. Switching language touches only document.lang/dir and localStorage — confirmed no graph/repo/connection state mutation. LanguageSwitcher aria-label and sr-only span present and translated.

C. Locale resource integrity
Key parity: i18n.test.tsx flattenKeys() recursive comparison covers all 8 namespaces. TypeScript satisfies EnglishMessages in zh-CN.ts enforces CLI key parity at compile time. Interpolation variables: spot-checked {{count}}, {{seconds}}, {{message}}, {{mb}}, {{percent}} — all match between en and zh-CN. zh-CN prose quality: all values reviewed in common.json, settings.json, graph.json — translations are correct; technical tokens left in English (product names, API names, CLI commands) are appropriate. list.unknown: 'unknown' is an intentional accepted technical term.

D. CLI localization and automation safety
Machine-readable JSON stdout via writeSync(1, ...) confirmed untouched. detect-changes-format.ts now routes all human-readable detect-changes output through t(). Commander help text localized post-registration. Plural _one/_other suffix resolution confirmed working. CJK displayWidth/padDisplayEnd fix in doctor.ts confirmed correct with doctor-format.test.ts coverage. zh_TW/zh_HK/zh-Hant → English confirmed in cli-i18n.test.ts.

E. Prior review blocker closure
All 7 prior blockers confirmed fixed by direct file inspection at current HEAD:

  • CJK doctor alignment: ✅ displayWidth/padDisplayEnd in doctor.ts, covered by doctor-format.test.ts
  • t/auto-connect dep: ✅ tRef = useRef(t) at App.tsx:109-112; dep array at line 191 does not include t
  • Invalid ?lng= cache: ✅ caches: [] + persistSupportedLanguage validation
  • fallbackNS: ✅ fallbackNS: false at i18n/index.ts:46
  • Traditional Chinese: ✅ normalizeSupportedLanguage returns null for zh-TW/zh-HK/zh-Hant
  • CLI plural resolver: ✅ _one/_other suffix lookup in i18n/index.ts:43-57
  • Browser E2E: ✅ language-switching.spec.ts tests switcher, document.lang, localStorage, reload persistence, switch-back

F. Tests and CI
Key parity test (i18n.test.tsx:27-37) is meaningful — it would fail if any zh-CN namespace added or removed keys without updating en. CLI TypeScript satisfies EnglishMessages catches key drift at compile time. language-switching.spec.ts verifies the real app header switcher, document.lang, localStorage, and reload persistence. The E2E backend is fully mocked via page.route(). However, none of the tests would catch the three hardcoded strings in Findings 1–3 because those strings are not in locale files — they are in JSX props. A future regression prevention test would need to render the specific components and check that tooltip/help text values change on language switch.

G. Security and dependency posture
i18next@^26.2.0, i18next-browser-languagedetector@^8.2.1, react-i18next@^17.0.8 — well-maintained official packages, Dependency Review CI passed. interpolation: { escapeValue: false } is correct for React — all translated strings are rendered as React children, not dangerouslySetInnerHTML. No translator-controlled code/protocol mutation found.

H. Branch hygiene and history
Five i18n-scoped commits, three merge-from-main commits. PR diff contains only i18n, test, locale JSON, package files — confirmed by git show --name-status on each commit. docs/ removal was appropriate (maintainer-requested, removes local artifact files from PR diff). git diff --check HEAD returned no whitespace errors.


Back-and-forth avoided by verifying

  • markdownComponents memoization: React.useMemo at line 108 depends on [handleLinkClick]. If t were referenced inside but not in deps, it would silently use a stale translator after language switch. Confirming this is a separate concern from the missing locale keys — even if keys were added, the useMemo would need t in its deps array or use a stable ref.
  • Settings panel t() usage is extensive: SettingsPanel.tsx has 81 t(/useTranslation occurrences. The three hardcoded strings are genuinely missed cases in otherwise well-translated code, not a systematic failure.
  • formatDetectChangesResult is now localized: Confirmed in detect-changes-format.ts:33-86 — all output goes through t(). The previous review's acceptance of this as "out-of-scope" has been superseded by the fix in b7a444d.

Final verdict

not production-ready

Three hardcoded English strings remain in reachable user-facing components that the PR itself claims to have translated: tooltip titles on AI chat grounding links (MarkdownRenderer.tsx:130), an instructional help paragraph in the OpenAI settings panel (SettingsPanel.tsx:494-497), and a sentence fragment in the Ollama provider quickstart (SettingsPanel.tsx:702). All three are reachable in normal zh-CN user flows, contain GitNexus-owned English prose (not product names or code tokens), and are entirely absent from locale resources — the key parity test does not catch them because they are hardcoded in JSX rather than keyed. Additionally, the main CI workflow is still in_progress at review time. Once CI completes and the three missing locale strings are added (small, targeted fixes), the PR clears the production-readiness bar. All seven prior review blockers are confirmed fixed, the i18n architecture is sound, CLI automation contracts are intact, and branch hygiene is clean.


· Branch: codex/i18n-20260521

@magyargergo

Copy link
Copy Markdown
Collaborator

@ChamHerry Pleas also run your sweep for all of the user-facing pages and make sure they are all translated 🙏

Localize the remaining PR abhigyanpatwari#1748 blocker strings and follow-up sweep findings without changing UI state or backend contracts. Keep technical examples and command tokens as-is while moving GitNexus-owned prose into locale resources.

Constraint: Latest maintainer/Claude review requested all user-facing pages be swept for translation coverage.

Rejected: Translating code/query examples and product names | they are technical identifiers and would reduce clarity.

Confidence: high

Scope-risk: moderate

Directive: Add both en and zh-CN keys for any future GitNexus-owned visible prose, including title/placeholder/helper text.

Tested: GitNexus impact analysis for MarkdownRenderer, SettingsPanel, RepoAnalyzer, FileTreePanel, CodeReferencesPanel, useAppState, and translateProgressMessage.

Tested: custom TypeScript/JSX hardcoded user-facing text scan; blocker grep for prior review strings.

Tested: cd gitnexus-web && npx prettier --check <changed files>.

Tested: cd gitnexus-web && npx eslint <changed TS/TSX files> (warnings only, pre-existing style warnings).

Tested: cd gitnexus-web && npx tsc -b --noEmit.

Tested: cd gitnexus-web && npm test -- i18n.test.tsx; cd gitnexus-web && npm test.

Tested: cd gitnexus-web && npm run test:e2e -- language-switching.spec.ts after starting Vite on localhost:5173.

Tested: git diff --check; GitNexus detect_changes(scope=staged, repo=GitNexus).

Not-tested: Full manual visual sweep in a browser for every panel; full backend/CLI test suites unchanged.
@ChamHerry

Copy link
Copy Markdown
Contributor Author

@magyargergo pushed 0a14c734 with the translation coverage fixes and a full user-facing text sweep.

Fixed:

  • Localized MarkdownRenderer grounding link tooltips (View/Open in Code panel) and updated memo deps so titles follow language changes.
  • Localized the OpenAI base URL placeholder/help text and Ollama then run fragment from the latest Claude blockers.
  • Localized additional sweep findings: GitLab repo analyzer labels/help text, file-tree hop labels, collapsed code-panel Selected label, provider model placeholders, and app-state progress/chat messages.

Sweep results:

  • Ran a TypeScript/JSX scan across gitnexus-web/src for hardcoded user-facing title/placeholder/JSX text/label-like props.
  • Re-ran blocker grep for the review strings; no remaining component hits for View ... in Code panel, Open in Code panel, Leave empty to use, then run:, GitLab Repository URL, Supports GitLab.com, raw 1 hop/2 hops, or SELECTED.
  • Remaining scan hits are technical identifiers/examples only (AI, line marker L, keyboard shortcuts, Cypher query example, ollama serve, ollama pull ..., and LLM tool schema descriptions).

Validation:

  • cd gitnexus-web && npx prettier --check <changed files>
  • cd gitnexus-web && npx eslint <changed TS/TSX files> (warnings only; no errors)
  • cd gitnexus-web && npx tsc -b --noEmit
  • cd gitnexus-web && npm test -- i18n.test.tsx
  • cd gitnexus-web && npm test (17 files / 245 tests passed)
  • cd gitnexus-web && npm run test:e2e -- language-switching.spec.ts after starting Vite on localhost:5173 (1 passed)
  • git diff --check
  • GitNexus impact analysis + detect_changes(scope=staged)

CI is running on the new head now; Vercel still appears to require team authorization as before.

@magyargergo

Copy link
Copy Markdown
Collaborator

Can you please check all the cli options ad well? 🙏

Add a regression sweep that enumerates every registered CLI help page and verifies zh-CN help no longer contains the original English command or option descriptions from the Commander registration files.

Constraint: PR abhigyanpatwari#1748 follow-up asked to check all CLI options, not just sampled commands.

Rejected: Manual-only CLI option review | it would not prevent future untranslated options from slipping in.

Confidence: high

Scope-risk: narrow

Directive: Keep new CLI commands/options in the Commander registration files covered by help localization resources.

Tested: GITNEXUS_LANG=zh-CN node --import tsx src/cli/index.ts <all commands> --help dynamic sweep.

Tested: cd gitnexus && npx vitest run test/unit/cli-index-help.test.ts test/unit/cli-i18n.test.ts test/unit/cli-message.test.ts.

Tested: cd gitnexus && npx tsc --noEmit.

Tested: cd gitnexus && npx eslint test/unit/cli-index-help.test.ts.

Tested: git diff --check; GitNexus detect_changes(scope=staged, repo=GitNexus).

Not-tested: Full gitnexus npm test suite; no production code changed.
@ChamHerry

Copy link
Copy Markdown
Contributor Author

Follow-up for the CLI options request: pushed 094d4953 with a full CLI help/options coverage test.

What I checked:

  • Enumerated every CLI help surface in src/cli/index.ts and src/cli/group.ts:
    • root, setup, analyze, index, serve, mcp, list, status, doctor, clean, remove, wiki, augment, publish
    • query, context, impact, cypher, detect-changes/detect_changes, eval-server
    • group, group create/add/remove/list/status/sync/impact/query/contracts
  • Ran zh-CN help for all of them.
  • Extracted all registered Commander command descriptions and option descriptions from the source files and verified none of the original English descriptions remain verbatim in zh-CN help output.

Result:

  • No CLI option/help translation gaps found.
  • Remaining English in help is limited to intentional technical syntax/identifiers: flags, placeholders like <repo>, env vars, provider/product names, URLs, and command names.

Added regression coverage:

  • test/unit/cli-index-help.test.ts now has a full sweep test so future CLI commands/options must be covered by help i18n.

Validation:

  • GITNEXUS_LANG=zh-CN node --import tsx src/cli/index.ts <all commands> --help sweep
  • cd gitnexus && npx vitest run test/unit/cli-index-help.test.ts test/unit/cli-i18n.test.ts test/unit/cli-message.test.ts (25 tests passed)
  • cd gitnexus && npx tsc --noEmit
  • cd gitnexus && npx eslint test/unit/cli-index-help.test.ts
  • Pre-commit hook also ran formatting + gitnexus typecheck and passed

@magyargergo magyargergo merged commit fc6007e into abhigyanpatwari:main May 23, 2026
38 of 39 checks passed
@ChamHerry ChamHerry deleted the codex/i18n-20260521 branch May 28, 2026 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants