Skip to content

fix(i18n): code-ize health signals and drop silent zh locale defaults - #4524

Merged
Astro-Han merged 1 commit into
apache:mainfrom
orangeCatDeveloper:fix/locale-gap-wave1
Sep 5, 2026
Merged

fix(i18n): code-ize health signals and drop silent zh locale defaults#4524
Astro-Han merged 1 commit into
apache:mainfrom
orangeCatDeveloper:fix/locale-gap-wave1

Conversation

@orangeCatDeveloper

@orangeCatDeveloperorangeCatDeveloper commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Health signals emitted zh product copy from packages/core/src/health.ts, and the English presenter reverse-mapped it by exact string match; any wording edit silently broke English, and the runtime-probe detail was regex-parsed back out of a formatted zh string.

Signals now carry a closed HealthSignalMessageCode plus structured HealthSignalDetail; each locale maps codes in its catalog and the producer is locale-free. Connection-test failure classes are a closed detail mapped on both the Health and connection-detail surfaces. Runtime-probe error classes use the turn's failure vocabulary (rate_limit, …), so unmapped classes stay visible as their token instead of collapsing to "unknown error".

Interim: capability reasons are still zh prose from capability-snapshot.ts, so the zh-CN presenter shows them as main does while en / zh-TW show a fixed settings-page pointer. Coding them as a CapabilityReasonCode is the follow-up that removes this sniff and the Permission Center's.

The ui stream/projection seams also lose their silent 'zh' locale defaults; every production caller already passes the real locale, so tests pin zh via a shared wrapper.

Sequencing: follows #4823 and #4640 (merged, in this base). #4824 and #4551 rebase onto this; #4551 touches the same health.ts and provider-panel-shared.ts.

Refs #2672

Verification

packages/ui suite: 362 pass / 0 fail
core health tests: 9 pass
desktop health-center-copy tests: 8 pass
desktop typecheck: 4 tsconfigs pass
renderer architecture check: pass
format:check / knip (desktop): clean

The app was not launched for manual acceptance.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code — analysis, implementation, tests. OpenCode — review follow-up and tests. Each commit carries its Generated-by trailer.

Checklist

  • Tests cover the change and fail without it

@github-actionsgithub-actionsBot added the effort/L Under 1000 readable lines label Sep 2, 2026
@orangeCatDeveloper
orangeCatDeveloper marked this pull request as draft September 2, 2026 05:07
@orangeCatDeveloper
orangeCatDeveloper marked this pull request as ready for review September 2, 2026 09:24
@orangeCatDeveloper
orangeCatDeveloperforce-pushed the fix/locale-gap-wave1 branch 3 times, most recently from 646c302 to 6ddc0acCompareSeptember 3, 2026 07:29
@orangeCatDeveloper
orangeCatDeveloperforce-pushed the fix/locale-gap-wave1 branch 2 times, most recently from 22879d0 to cfe5ba2CompareSeptember 3, 2026 09:00

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The shape is right. The producer emits codes, the existing UiCatalog maps them, Record<HealthSignalMessageCode, string> makes a missing entry a compile error, and the never checks pin detail exhaustiveness. The reverse-mapping it deletes was genuinely fragile: matching signal.message === '不是工作区的默认模型来源。' meant any wording edit in health.ts silently broke English. And I checked the "code without copy leaves a blank" worry, it is not reachable, both maps are total by type and nothing persists a HealthSignal.

[P2] The zh health center now prints raw English bot diagnostics

Two lines apart in the same switch:

// signalDetailZhreturncapabilityReasonsZh[detail.reason]??detail.reason;// signalDetailEnreturncapabilityReasonsEn[detail.reason]??'See the corresponding settings page for details.';

The zh side passes the unknown reason through verbatim. Bot reasons are English: settings-ipc-helpers.ts:235 produces ${label} requires a Bot Token. and ${label} rejected the Bot Token., which reach capabilityDetail unchanged through readinessReason and runtimeProbeFromBotReadiness. So configure a bot with a bad token, test it, open Settings and Health in Chinese, and an English sentence is what you get.

On main the zh side had a fallback for exactly this: /[㐀-鿿]/.test(raw) ? raw : '状态详情请见对应设置页。'. So a PR whose purpose is removing locale leakage introduces the mirror leak. The comment above that line says unknown reasons "are today's zh snapshot diagnostics", and the new test only pins the zh-text case, which is why it went unnoticed.

Fix is to mirror the en branch, plus a test with an English unknown reason.

[P2] last_test_message puts a machine token on the user-visible surface

runtime-host-connections-ipc-main.ts:429 sends lastTestMessage: connection.lastTest.errorClass, and errorClass is the closed six-value union in connection-effects.ts:69. So the string the user sees is literally auth. The type comments call it raw producer text rendered verbatim by design, but it is an enum, which is the thing this PR code-izes everywhere else. zh is unchanged from main; en regresses from the fallback sentence to the bare token. There is already a seam for this exact field, connectionLastTestMessageDisplay(message, locale) in provider-panel-shared.ts:94.

[P3] Three smaller ones

capabilityReasonsZh and capabilityReasonsEn carry four keys each (disabled, missing platform credentials, macOS TCC only, and the Apple Events one) that capability-snapshot.ts never emits. Main had four dead mappings in the producer; this moves them faithfully and doubles them.

permission-center-page.tsx:705 still uses the /[㐀-鿿]/u CJK sniff on the same capability.*.reason field this PR just stopped sniffing. Not a regression, both predate this, but it means an English user sees the bot's English message in the Permission Center and the fallback sentence in Health, for one field. Worth naming as a known remainder given the PR's stated obligation.

settings-pages.stories.tsx:600 maps a storage-scope signal to capability_ok, so the story renders capability copy for a SQLite row. Root cause is that HealthSignalMessageCode has no code for the app and storage scopes those fixtures use, and no producer emits them.

Next step

Both P2s are one catalog entry each. P3-1 is a free deletion in the same file.

This is a copy change with no screenshots, so it needs manual acceptance in Settings and Health, both locales: a connection in each state, the runtime row with a successful and a failed send, the runtime label suffix that moved from producer to presenter, and a bot capability after a failed credential test, which is the P2 reproduction.

Evidence boundary: read at cfe5ba2 against base b1ec289, no tests, typecheck or build, and the app was not launched. Both P2s are traced through source but not observed at runtime. The claim that no production caller relied on the removed zh defaults rests on a repo-wide grep plus the compiler, not a typecheck run.

AI-assisted review: drafted with Maka.

@orangeCatDeveloper
orangeCatDeveloperforce-pushed the fix/locale-gap-wave1 branch 2 times, most recently from b40cbd4 to 297488fCompareSeptember 3, 2026 10:08
@orangeCatDeveloper

Copy link
Copy Markdown
ContributorAuthor

@Astro-Han Comments are addressed.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The P2 is closed, and closed harder than I asked for. Neither presenter passes raw producer text through any more: capability_reason and last_test_message both return a fixed, localized sentence, so the whole class of "English bot diagnostic surfaces in the Chinese Health center" is gone rather than patched at the one call site I found. The exhaustive switch with no default is what makes that hold — a new detail kind is a compile error in both locales.

CI is unrelated: the failure is host-kernel.test.js:613 (elects one owner ... releases ownership after true-idle shutdown, got unavailable where it wanted connected), which no i18n change can reach. I've rerun it.

P2 — two detail fields are carried but never read, and the comment promises otherwise.

// Raw machine token from the capability snapshot; the presenter maps the// known vocabulary per locale and falls back to a generic hint.|{ kind: 'capability_reason'; reason: string}// Legacy connection-test diagnostic; the presenter uses locale-specific fallback copy.|{ kind: 'last_test_message'; text: string}

Neither reason nor text has a reader. signalDetailZh and signalDetailEn both return the generic sentence unconditionally for those two kinds — the per-locale vocabulary map the first comment describes doesn't exist any more. So the producer computes a value, it crosses the IPC boundary, and nothing consumes it.

Two coherent ways out, and I don't mind which:

  • Implement what the comment says for capability_reason. reason is a machine token, not free text, which is exactly the shape a per-locale map wants — and it recovers the specific wording that the generic hint currently costs.
  • Or drop both fields. If the decision is that a generic pointer to the settings page is the right detail, then the payload should say so. text in particular is free-form English from the producer; keeping it around is an invitation to render it later and reintroduce the leak this PR just removed.

Either way the comments need to match. Right now the type documents behaviour the presenters don't have, which is the same failure mode as the string matching you deleted — a promise held in prose instead of in types.

Everything else stands: Record<HealthSignalMessageCode, string> making a missing entry a compile error is the right shape, and the reverse-mapping it replaces was genuinely fragile.

@orangeCatDeveloper
orangeCatDeveloperforce-pushed the fix/locale-gap-wave1 branch 2 times, most recently from 17b73e9 to 06b1f63CompareSeptember 4, 2026 08:38
@orangeCatDeveloper

Copy link
Copy Markdown
ContributorAuthor

Addressed by dropping the unused reason and text payloads. The producer now carries only a typed detail discriminator, and both presenters render fixed localized fallback copy. I also removed the stale comments and added bilingual coverage for both fallback cases.

@orangeCatDeveloper
orangeCatDeveloperforce-pushed the fix/locale-gap-wave1 branch 5 times, most recently from 8883ac8 to 1acd169CompareSeptember 5, 2026 05:23

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed at 1acd169. Last round's P2 is closed as asked and the regressions from round one stay closed: no presenter passes producer text through, the auth token is mapped on both surfaces, the four dead capability mappings went with userVisibleCapabilityReason, and the app / storage fixtures were removed because nothing produces them (I checked; health.ts only emits llm_connection and capability). Three things this round, and the first one is partly on me.

P2: Chinese users lose the capability diagnostics they see today. I offered "drop both fields" as one of two outs last round; for last_test_message that was right, for capability_reason it was not. The reasons in capability-snapshot.ts:83, 93, 103, 111, 246, 349 are live Chinese product copy (未配置平台凭据, 本地 MEMORY.md 已可见;自动抽取/写入仍需用户确认, …), and main shows them verbatim in the zh Health center. After this PR every one becomes 状态详情请见对应设置页。, while the Permission Center (permission-center-page.tsx:707) keeps showing the specific sentence for the same fact. Either code these reasons too (CapabilityReasonCode, same medicine as the rest of the PR, producer side in capability-snapshot.ts), or keep the reason payload and let the zh presenter pass CJK through as main does, labeled as the interim state. Whichever you pick, the body needs to name this as a user-visible change.

P2: the never checks are gone. The previous head had exhaustiveness guards on the three signalDetail* switches; this one has none, the switches have no default, the return type is string | undefined, and nothing enables noImplicitReturns. A new HealthSignalDetail kind now compiles and silently drops the detail in all three locales, which is the failure mode the Record<HealthSignalMessageCode, string> side was built to prevent. Three default: { const _: never = detail; return undefined; }.

P2: runtime probe error classes are looked up in the wrong vocabulary.localizedRuntimeErrorClass indexes connectionTestErrorMessages (auth | timeout | provider_unavailable | network | unknown), but the probe's errorClass is the turn's failureClass, i.e. ModelFailureKind (model-protocol.ts:331: rate_limit, context_overflow, provider_capacity, provider_billing, abort, …). Everything outside the five overlapping names falls to ?? unknownRuntimeErrorClass, so a rate-limited send that main reports as 错误类型=rate_limit now reads 错误类型=未知错误, and the new future_runtime_error → 未知错误 test pins the wrong side. Fall back to the raw token on a miss, or key a runtime table on ModelFailureKind.

Smaller:

  • HealthConnectionTestErrorClass duplicates ConnectionTestErrorClass (llm-connections.ts:520) and adds invalid_response, which the wire codec rejects (connection-catalog-codec.ts:707-719), so its seven copy entries are unreachable and a new class in core will not be a compile error here. Alias the core type.
  • Six of the eight HealthSignalDetail variants carry no data and four of them map 1:1 to a message code; the presenter can derive those from the code, leaving runtime_probe_result and last_test_error_class.
  • The 15-line applyLiveTurnEvent wrapper is pasted into three test files, plus three stream wrappers, all to avoid writing 'zh-CN' at the call site; the diff also leaves runs of 3–4 blank lines in five test files.
  • With app / storage fixtures gone, HealthSignalScope's 'app' | 'storage' and the three-locale scopes.app / scopes.storage / sources.storage entries are dead too.

Ordering: provider-panel-shared.ts is touched here in the body right under the signature line #4640 changes, so one of the two rebases; #4551 edits the same file. materialize.test.ts and live-turn-projection.test.ts merge mechanically with #4640 but the second one to land should re-run packages/ui. Against #4817 this PR only lowers counts.

Manual acceptance still owed (body says the app was not launched), three locales: Health center rows for configuration / validation / runtime_probe, a failed send's runtime detail with the error-class segment, a capability signal with a reason (the first P2), the connection detail lastTest row, and the runtime label suffix that moved to the presenter.

Evidence boundary: static read at 1acd169 against cbeb1a93c7; vocabularies traced through source, not observed at runtime; no tests, no typecheck.

AI-assisted review: drafted with Maka; I verified the capability reasons on main, the two error-class unions, and the missing never guards myself.

@orangeCatDeveloper
orangeCatDeveloperforce-pushed the fix/locale-gap-wave1 branch 2 times, most recently from 94ddb8f to f5c725fCompareSeptember 5, 2026 07:47
Health signals carried zh product copy from the producer; the en
presenter reverse-mapped it by exact string match, so any wording edit
silently broke English rendering. Signals now carry stable message
codes and structured details, both locales map codes in the presenter
catalog, and raw capability or connection diagnostics no longer leak
machine or wrong-locale text into Settings surfaces. Capability reasons
are still zh prose from the snapshot, so the zh presenter keeps showing
them until they are coded. The ui stream/projection seams lose their
silent 'zh' locale defaults so callers must thread the real locale.
Generated-by: Claude Code
Generated-by: OpenCode
Claude-Session: https://claude.ai/code/session_01AqdSkg56F2x55wEGRWvzcB
@orangeCatDeveloper

Copy link
Copy Markdown
ContributorAuthor

@Astro-Han RFR, after this get merged, I will rebase and reopen draft PR #4824#4641 and #4551

@hqhq1025hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed exact head 2f75546d5e8b4a95f8a8b9f541901fd6e6932b81 against base 1e543a7385614adc671623efe2586cf5317582d4.

No P0-P3 findings.

The change now has one typed health-copy boundary: HealthSignalMessageCode and HealthSignalDetail replace producer-owned prose (packages/core/src/health.ts:52-92), while the three locale catalogs own the rendered messages and structured details (apps/desktop/src/renderer/locales/settings-health-copy.ts:172-355). I also traced the live producers and callers: Runtime Host connection tests project the closed error-class vocabulary at apps/desktop/src/main/runtime-host-connections-ipc-main.ts:422-430, health aggregation consumes those projections at apps/desktop/src/main/runtime-host-permissions-ipc-main.ts:101-121, and the removed stream/projection locale defaults have explicit production callers, including apps/desktop/src/renderer/app-shell-session-events.ts:146-152 and packages/ui/src/chat-view.tsx:767-772.

The prior review items are closed on this head:

  • zh-CN keeps current CJK capability detail while English and Traditional Chinese use fixed locale copy (settings-health-copy.ts:256-338; covered at health-center-copy.test.ts:99-116).
  • all three detail presenters restore the never exhaustiveness guard (settings-health-copy.ts:281-282,309-310,337-338).
  • runtime probe classes outside the connection-test vocabulary stay visible as their raw diagnostic token instead of becoming “unknown” (settings-health-copy.ts:342-355; covered at health-center-copy.test.ts:77-97).
  • connection-test classes alias the core union and are localized on both the Health and connection-detail surfaces (health.ts:71-81,407-418; provider-panel-shared.ts:85-93).

Verification on this exact head:

  • Hosted test: success, run 33954160323.
  • build:test, full workspace typecheck, renderer production build, and Storybook build: passed.
  • @maka/ui tests: 373 passed; @maka/core tests: 825 passed; focused changed-path tests: 39 passed.
  • format, ASF headers, AppShell hook scope, Desktop knip, renderer architecture, and git diff --check: passed.
  • Fresh origin/main (3fdaddb4b9a8383a0612c55bf28f7a0e5c624ea0) merges cleanly; the 16 post-base main files do not overlap this PR's 25 changed files.

Evidence boundary: I did not launch the interactive Desktop app, so the three-locale visual/manual acceptance described in the PR remains unobserved. The local full Desktop suite reports 2,161 passed, 0 failed, and 8 cancelled in the unchanged mcp-oauth-controller.test deadline group; I reproduced that with Node 22.22.1 and 22.22.2. The exact-head hosted check is green, and the changed-path focused suite has no cancellations.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-reviewed at 2f75546. All three P2s from last round are closed, and the smaller items with them:

  • Capability reasons: reason is back on the detail, the zh-CN presenter passes CJK prose through as main does, en and zh-TW keep the fixed pointer (which is what main's zh-TW presenter already did at settings-health-copy.ts:140), the sniff is labeled interim with the CapabilityReasonCode follow-up named, and the body says so. Covered both ways in health-center-copy.test.ts.
  • never guards: all three switches end in default: return unhandledDetail(detail) with _detail: never.
  • Runtime error classes: unmapped failureClass values fall back to the raw token; the rate_limit fixture now pins the right side.
  • HealthConnectionTestErrorClass aliases ConnectionTestErrorClass, the unreachable invalid_response entries are gone, the dead app / storage scope and source entries are gone, and the three pasted test wrappers collapsed into one shared live-turn-zh.ts.

Checked against #4817's ratchet from its current head: this branch passes, and it retires the two packages/uisilent-locale-default hits it says it retires. Merges clean against main and against #4526 at b01cb0c (the two shared files, settings-provider-copy.ts and the zh-TW peer-mesh test, do not collide).

One body nit, not blocking: the checklist lost its second line and the "change in behavior" question; the answer is yes, for the en Health center's capability rows, which the Interim paragraph already describes.

Manual acceptance is still owed by whoever lands it (body says the app was not launched): Health center rows in the three locales, a failed send's runtime detail with an unmapped class, and a capability signal with a reason in zh-CN vs en.

Evidence boundary: static read of the delta since 1acd169 plus one run of the #4817 script on this head; no build, no suites, no Desktop.

AI-assisted review: drafted with Maka; I verified the three presenters, the error-class fallback and the ratchet run myself.

Astro-Han pushed a commit that referenced this pull request Sep 5, 2026
Adding a UI locale compiled cleanly and still rendered the wrong language wherever code compared `locale` to a literal, defaulted a `locale: UiLocale` parameter, sniffed CJK in a payload, or translated by looking up one locale's string. The type system cannot see any of the four, and Biome cannot express "no growth against base" for the two unformatted trees.
`scripts/check-locale-hygiene.mjs` counts those four patterns per file in `apps/desktop/src`, `packages/core/src` and `packages/ui/src`, both quote styles, per match, and fails CI when any (file, rule) count grows against the merge base or `BASE_SHA`. There is no ledger to maintain; only files in the diff are scanned. With no resolvable base the step fails instead of skipping. The step runs with the install-free gates before `setup-node`. `locale-literal-compare` is deliberately narrower than `check-tui-copy`'s AST rule so it stays dependency-free.
No runtime or migration impact. Baseline on this commit: 67 / 4 / 12 / 32; the four `silent-locale-default` hits retire with #4524 and #4551, after which that rule can become a hard zero.
Refs #2672
Generated-by: Claude Code
@Astro-Han
Astro-Han merged commit 6803967 into apache:mainSep 5, 2026
1 check passed
@orangeCatDeveloper
orangeCatDeveloper deleted the fix/locale-gap-wave1 branch September 5, 2026 11:12
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/LUnder 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@orangeCatDeveloper@hqhq1025@Astro-Han