refactor(cli): centralize localized TUI copy - #3990
Conversation
72523cf to
c3f716c
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
The English fix is real and worth landing — pi-tui-pickers.ts:725 on main renders 搜索模型 / 服务商 / 连接… under locale: 'en' — and collapsing the render-site literals removes genuine duplication ('Select Model' in both pi-tui-runner.ts:2562 and pi-tui-pickers.ts:724, '搜索' in three places). Packaging is sound end to end: tsc emits the JSON into dist/ (the new test reads dist/locales/ and CI is green), and smoke-release-cli-package.mjs:477 waits on 'Set Up Provider', which is locales/en/pickers.json:21.
My concern is direction, not correctness.
The interface worth designing is the message contract, not the file format. Storage (TS/JSON/PO) is one mechanical conversion away; the message format and key contract are what's expensive to change. This PR changes the cheap thing and pins the expensive one to {name} substitution (tui-copy-catalog.ts:89) — which doesn't survive a third locale. The repo already shows why: externalLinks: (count) => `… ${count} ${count === 1 ? 'link' : 'links'}` is a hand-written plural rule. It works only because Chinese has no plural; Russian has three forms and Arabic six, and that pattern is spread across 34 UiCatalog modules. ICU ({count, plural, one {# link} other {# links}}) puts plural rules in CLDR data instead of in whoever writes the string.
ExactCopyShape blocks the incremental translation it's meant to enable. Requiring every locale to carry every key is a net win at two engineer-written locales. Adding ja means translating all 34 modules before anything compiles — and incremental is the only way an outside translator works. Keep the compile-time requirement for en (it's the fallback, it can't have holes); make the rest a CI coverage check.
The one durable piece is in a package nobody can import. formatTuiCopy / defineTuiCopyCatalog are the only genuinely new shared capability, and packages/cli is private: true with exports: {} — no workspace depends on maka-agent (I checked every package.json). The body promises follow-ups migrating packages/ui and Desktop; they can't reach this. Home is packages/core/src/ui-locale.ts, where UiCatalog lives and relative-time.ts already uses Intl.
On the JSON migration itself. Of +806: 290 JSON copy (relocated), 193 tests, 94 the new module, 217 product TS against 250 deleted. Product code is flat; 287 lines (35%) is machinery supporting "copy lives in JSON." It currently buys none of the three things JSON is chosen for — no translation workflow, static imports so every locale compiles into dist anyway, and a translator still reads a TS2345 to learn they missed a key. Suggest deferring it until there are outside translators to serve; TS literals give compile-time checking, comments and an ASF header for free, and exporting JSON later is mechanical once the contract is stable.
So: keep the English fix and the formatter in this PR, move the formatter to @maka/core and make it an ICU subset with en fallback rather than {name} substitution, and split the TS→JSON move out.
Side note on the body's opening line — adding a locale still requires editing three places, since getTuiCopyCatalog returns a hardcoded { zh, en } (:79-82). TuiDomainCatalog is already a [Locale in UiLocale] mapped type; transposing TUI_COPY_RESOURCES removes the third.
P3. formatTuiCopy reads values[name] on a plain object, so {constructor} resolves through the prototype chain instead of hitting the undefined guard — and when the guard does fire it throws a bare Error from inside render(), tearing down the TUI. release-cli-package.mjs:60 calls walkFiles at module scope with no existsSync guard, above argument validation; an empty directory silently reduces the required-file list to []. validatePackedFiles matches with endsWith (:788) where the locale entries are exact paths.
Undeclared changes. verifyFailed moved the detail to a trailing {detail} (empty detail leaves a trailing space in en); current/default were English for all locales on main and now render 当前 / 默认连接 in zh; pi-tui-mcp-status.ts:182 replaced if (locale === 'en') return state — total over any string — with a table lookup returning undefined off-union; release-cli-file-policy.mjs:254-256 relaxes the dist-JSON filter for every package, not just the CLI. Each defensible, none mentioned. Relatedly the English fix is narrower than the body implies: pi-tui-runner.ts still hardcodes Chinese for rewind (:650-654), skills (:2321), and transcript notices.
AI-assisted review: three Claude Code subagents ran architecture, correctness and packaging passes. I verified independently: en/zh key and placeholder sets are identical across all four domains (scripted), packages/cli is private with empty exports and no reverse dependency, the smoke marker is pickers.json:21, the test reads from dist/ so CI green proves tsc emits the JSON, and the line breakdown is from git diff --numstat. No tests, typecheck or release build run. AI review is not independent human review.
简体中文
英文修复是真实价值,打包链路端到端核过是安全的。问题在方向:值得设计的接口是消息契约,不是文件格式。存储格式换起来便宜,消息格式和 key 契约换起来贵——这个 PR 换了便宜的,却把贵的钉死在 {name} 平替上。仓库里已有手写复数(count === 1 ? 'link' : 'links'),它在第三种语言上必然崩。
ExactCopyShape 要求全量 key,加 ja 就得先翻完 34 个模块才能编译,而增量是外部翻译者唯一的工作方式;应保留 en 的编译期强制,其余改为 CI 覆盖率检查。
formatTuiCopy 放在 private + exports: {} 的包里,packages/ui 和 Desktop 取不到,而它们正是 body 承诺的迁移目标;应放进 packages/core/src/ui-locale.ts。
+806 里 287 行(35%)是支撑"文案在 JSON 里"的机制自重,产品代码平进平出,而 JSON 的三样好处一样没拿到。建议:英文修复和格式化留在本 PR,格式化搬进 core 并做成带 fallback 的 ICU 子集,JSON 迁移单独拆出、等真有外部翻译者时再做。
7f12a3c to
7e38423
Compare
303ad50 to
013c835
Compare
Replacing an accidentally empty approval with the already confirmed formal review body.
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for reworking this around the message contract rather than the storage format. The original problem is real: English TUI paths were rendering Chinese search controls, and the related copy was duplicated across presentation sites.
The current solution is much cleaner. The formatter and fallback contract now live in @maka/core, English remains complete, partial translations fall back recursively, ICU/CLDR owns plural behavior, and the earlier JSON packaging machinery is gone. Machine-readable output and protocol state remain unchanged.
I found no P0 or P1 issues, and the reviewed before/after presentation looks good. Approved.
I found two non-blocking P2 consistency gaps:
- The generic
PickerOverlaystill defaults to the hard-coded English hintenter select / esc close. Chinese/modelfallback and/thinkingpaths do not override it, so otherwise localized pickers can still display mixed-language controls. - Several onboarding rejection paths pass Host-provided English text directly into the Chinese TUI, including stale verification and save rejection. Stable reason codes should cross the boundary and be localized by the TUI presentation owner rather than parsing or displaying backend strings.
The branch also needs to be rebased before merge. A mechanical rebase and fixes limited to these two P2s do not require another full architectural review.
Review analysis was assisted by Codex and independent @reviewer agents. Astro-Han reviewed the visual evidence and verified the exact head, production locale flow, package boundary, severity and reachability, and owns this review.
中文对照
谢谢你把方案从“存储格式迁移”重新收敛到真正重要的消息契约。原问题是真实的:英文 TUI 路径会显示中文搜索控件,相关文案也散落在多个 presentation site。
当前方案干净很多。formatter 和 fallback contract 已归属 @maka/core;英文保持完整,部分翻译可以递归回退;复数交给 ICU/CLDR;之前没有实际收益的 JSON 打包机制也已经删除。机器可读输出和协议状态没有变化。
没有发现 P0 或 P1;人工检查的前后视觉也没有问题,Approve。
仍有两个不阻塞的 P2 一致性问题:
- 通用
PickerOverlay的默认提示仍硬编码为英文enter select / esc close。中文/modelfallback 和/thinking没有覆盖它,因此本地化 picker 仍可能混入英文控件提示。 - 一些 onboarding 拒绝路径把 Host 返回的英文文本直接展示在中文 TUI,包括 stale verification 和 save rejection。这里应跨边界传递稳定 reason code,再由 TUI presentation owner 本地化,而不是解析或直接展示后端字符串。
分支合并前还需要 rebase。仅包含机械 rebase 和这两个 P2 修复的更新,不需要重新做一次完整架构审查。
本次审查分析由 Codex 和独立的 @reviewer 子代理协助;Astro-Han 人工检查了视觉证据,并核验了精确 head、生产 locale 流程、package boundary、问题分级与可达性,对本次 Review 负责。
English mode rendered Chinese picker controls because locale selection was mixed into presentation code with no shared message contract. Generated-by: OpenAI Codex Generated-by: Claude Code
013c835 to
48067a0
Compare
|
@Astro-Han Rebased and fixed all P2 issues. CI passed |
Part of #2672
Locale-specific TUI screens can mix Chinese and English copy: the English model picker used Chinese controls, Chinese generic pickers used an English fallback hint, and Chinese onboarding displayed Host-provided English rejection text.
The affected copy lived in presentation branches without a shared message contract.
PickerOverlayalso owned an English default, while the onboarding adapter converted stable Host outcomes into display strings before the TUI could localize them.This change centralizes the four migrated domains as typed TypeScript message resources.
@maka/core/ui-localeprovides recursive English fallback and ICU formatting through the new runtime dependencyintl-messageformat; catalog tests require complete Chinese coverage, matching variables, and successful formatting. Picker callers now supply a localized hint, including the/modelfallback and/thinkingpaths.Onboarding now carries stable rejection reasons and failure classes across the Runtime Host boundary. The TUI owns their English and Chinese presentation, including stale verification, save rejection, transport failure, provider-list failure, and post-save refresh failure; backend exception text is not rendered.
This covers only the migrated domains. Connection identity, rewind, skill, and transcript notices in
pi-tui-runner.ts, plus MCP management editor copy inpi-tui-mcp-status.ts, still contain pre-existing hardcoded Chinese and remain follow-up work. No locale, machine-readable output, or Runtime Host protocol behavior changes.Evidence
Both screenshots render the real
ModelSearchOverlaywith the same fixture and 100-column viewport.Before:
After:
Verification
AI use
Select exactly one:
Tool(s) and scope: OpenAI Codex implemented and tested the refactor and follow-up fixes, reviewed the final diff, rebased the branch, and prepared this description. Claude Code reviewed the earlier diff and added locale-guard coverage.
Checklist
Does this PR entail a change in behavior?