Skip to content

fix(i18n): emit settings expected-result codes from producers - #4551

Open
orangeCatDeveloper wants to merge 9 commits into
apache:mainfrom
orangeCatDeveloper:fix/locale-producer-codes
Open

fix(i18n): emit settings expected-result codes from producers#4551
orangeCatDeveloper wants to merge 9 commits into
apache:mainfrom
orangeCatDeveloper:fix/locale-producer-codes

Conversation

@orangeCatDeveloper

@orangeCatDeveloper orangeCatDeveloper commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Settings-area producers emitted zh-only result strings, so English users either saw Chinese or — where consumers guarded with /[㐀-鿿]/ content sniffing — silently lost the information (permission guidance was dropped entirely). Producers now emit stable machine codes (IPC payloads stay locale-free), and each settings catalog maps its union exhaustively per locale with an explicit unknown-code fallback. CJK sniffing was removed where this PR code-ified the producer. zh copy moved verbatim, en copy is new.

Covered producers: capability-snapshot reasons, memory ops, test-connection OAuth rate-limit, computer-use-host health reasons, the GitHub Copilot import path (github-copilot-local-credential + its IPC handler, re-based onto main's device-grant rewrite), plus the dev-singleton dialog, browser message-box close label, and artifact/config native dialog titles (wired through the main-process locale authority). subscriptionResultMessage now takes the result envelope and resolves code, then the typed reason (experimental_disabled), through the catalog before falling back to the keyword classifier; its trailing CJK passthrough is gone. It moved out of the useOAuthLoginFlow hook module into the catalog module beside the copy it renders, which also makes it testable from node.

The bot domain moved to its own PR so each PR is one reviewer context.

Memory messages now use complete per-locale templates instead of a shared sentence skeleton with translated fragments. The typed catalog owns counts, summaries, redaction notices, backup failures, and preview/action descriptions; existing wording and pluralization are preserved.

The Permission Center's guidance block is removed in this PR: no producer fills guidance after the code-ification, and #4526's layered rows carry the actionable lines (cuBackendStatus composition), so the dead column is dropped — its locale keys, the storybook fixture's zh guidance line, and the collapsed-story comment referencing the block are all gone.

Refs #2672

Verification

apps/desktop typecheck (4 tsconfigs):   0 errors
workspace typecheck:                    0 errors
desktop main tests (dist):              2209 pass / 0 fail
rendered-output tests (en+zh):          Copilot code, memory rejection codes, experimental_disabled reason, unknown code → fallback
packages/core tests:                    818 pass / 0 fail
CJK-sniff regexes in touched files:     0 (provider action fallback reworded to the locale fallback)
zh literals in copilot producers:       0 remaining (was 8)
renderer architecture check:            passed against origin/main
biome (changed files):                  clean
check:tui-copy:                         ok

Boundaries: one capability-reason-copy.ts catalog serves the Health center and the Permission Center; the three prose reason tokens are now platform_credentials_missing, macos_tcc_only and apple_events_tcc_status_unavailable. Bot capability reasons resolve through #4639's botStatusReasonCopy. The rate-limited connection test keeps its guidance keyed on statusCode === 429, the same stable signal the command palette already uses, rather than widening ConnectionTestErrorClass across the codec, CLI, and renderer. subscriptionResultMessage still matches three English Host messages (enrollment is disabled, already in progress, did not present OAuth) as a fallback for Hosts that predate the codes; an unclassified Host message renders the locale fallback rather than the raw text, per the locale policy in #2672. The storybook smoke keeps its full-sentence match on purpose: storybook-visual-smoke.test.mjs requires unrelated settings errors in the error story to stay fatal. Permission probe failures collapse to permission_probe_failed for the page, and the raw cause is kept observable via a console.warn in capability-snapshot.ts so the closed code does not swallow the diagnostic.

Follow-up verification: all 13 memory-copy and label tests pass, with exact formatted-output coverage across all three locales, draft states, singular/plural counts, archived entries, backup kinds, and preview messages. Desktop build and all four typechecks, repository format/lint, and the renderer architecture check pass.

AI use

Select exactly one:

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

Tool(s) and scope: Claude Code, Muse Spark, and OpenCode — analysis, implementation, review fixes, tests, and this description, under the contributor's direction; each affected commit carries the corresponding Generated-by trailer.

OpenCode also implemented the complete memory-message templates and their output tests, and updated this description.

Checklist

  • Tests cover the change and fail without it

@github-actions github-actions Bot added the effort/XL Under 2500 readable lines label Sep 2, 2026
@orangeCatDeveloper
orangeCatDeveloper force-pushed the fix/locale-producer-codes branch 7 times, most recently from ad2b11d to 89c58cd Compare September 3, 2026 08:49
@orangeCatDeveloper orangeCatDeveloper changed the title fix(i18n): emit expected-result codes from producers fix(i18n): emit settings expected-result codes from producers Sep 3, 2026
@orangeCatDeveloper
orangeCatDeveloper force-pushed the fix/locale-producer-codes branch 2 times, most recently from 4b6d344 to b526451 Compare September 3, 2026 10:12
@github-actions github-actions Bot added effort/L Under 1000 readable lines and removed effort/XL Under 2500 readable lines labels Sep 3, 2026
@orangeCatDeveloper
orangeCatDeveloper marked this pull request as ready for review September 4, 2026 06:50
@orangeCatDeveloper
orangeCatDeveloper force-pushed the fix/locale-producer-codes branch 9 times, most recently from 236c0da to daeb9ba Compare September 5, 2026 04:59
@orangeCatDeveloper
orangeCatDeveloper marked this pull request as draft September 5, 2026 05:01
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
@orangeCatDeveloper
orangeCatDeveloper marked this pull request as ready for review September 5, 2026 12:19
@github-actions github-actions Bot added effort/XL Under 2500 readable lines and removed effort/L Under 1000 readable lines labels Sep 5, 2026

@Astro-Han Astro-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 af07077 against 03b5a2cc5a, two independent passes. This is the CapabilityReasonCode follow-up #4524 asked for, and the producer side is right: CAPABILITY_REASON_CODES is one closed list in @maka/core, the memory and subscription codes are closed unions, the native dialog titles read the existing main-process locale authority rather than a new one, the interim CJK sniff in settings-health-copy.ts is gone, and zh-TW is real traditional copy.

One P1, three P2s, then smaller items.

P1: the 23 capability reason codes get two full three-locale tables, and they already say different things. settings-health-copy.ts:257-333 and permission-center-copy.ts:153-348 each map every code. For missing platform credentials the Health center renders 等待填写平台凭据。 and the Permission Center 未配置平台凭据; macOS TCC only is 仅 macOS 系统权限可探测。 in one and 仅 macOS TCC 权限适用 in the other. That is 138 strings maintained twice and two pages describing one state in two ways, which is the exact defect this family of PRs exists to remove. The body defers the reconciliation to #4524, but #4524 is merged and did not do it; it is this PR's job. One Record<CapabilityReasonCode, string> per locale in a shared module, both pages read it, and the Permission Center keeps only its cuBackendStatus composition. (Path ①: both pages show the same capability.)

P2: the Permission Center loses every bot capability's reason. capability-snapshot.ts:247-251 passes the bot's readinessReason into runtimeProbe.reason; on main localizedSnapshotText shows it when it is not CJK, so English users see "Telegram requires a Bot Token." The new resolver at permission-center-page.tsx:723-729 sends anything that is not a capability code to reasonFallback, and the comment says bot reasons are machine codes owned by the bot page. That is #4639's world, not main's, and even after #4639 the right answer is the bot copy, not a generic line. Land after #4639 and resolve bot:* reasons through its botStatusReasonMessage; until then keep the non-CJK pass-through.

P2: three of the new "codes" are English prose. capabilities.ts:115-117 lists 'missing platform credentials', 'macOS TCC only' and 'no Electron API for per-target Apple Events TCC status', and the producer now emits those where it used to emit Chinese. The body says the legacy tokens "keep their existing prose form"; they changed language. Any consumer that misses the map shows English to a Chinese user, which is the old failure with the locales swapped. Make them snake_case like the other twenty, or correct the body and add the missing-map test.

P2: subscriptionResultMessage drops an unclassified Host reason. The old subscription-result-message.ts:52 passed a non-CJK message through when nothing matched; the new function (settings-provider-copy.ts:681) returns the generic fallback when the classifier has no match, which can contradict what the Host said. The order (code → typed reason → three legacy matches → classifier) is right; keep the raw-text pass-through as the last step, and note at the three legacy matches when they can go (once every Host sends code).

Smaller:

  • test-connection.ts:508 deletes the 429 message rather than coding it, so a rate-limited OAuth account is now indistinguishable from provider_unavailable and the "retry later or switch model" guidance is gone; the body lists it under covered producers.
  • settings-error-copy.ts:33 replaces the zh-CN user-visible main-process message with unknownError plus console.error; and scripts/storybook-visual-smoke.mjs:188-193 now matches that console line by full text, so a story that changes the sentence fails the smoke. Match on the [settings] prefix.
  • settingsTestResultMessage's default branch loses the en pass-through (settings-test-result-copy.ts:163-166); this is the line #4639 also changes.
  • runtime-host-memory-ipc-main.ts:626-627 writes reason and code with the same value, the Copilot IPC writes message: code, and bridge-contract.d.ts:1511-1517 still carries message that nothing reads. One field.
  • localizedCapabilityGuidance maps capability.guidance, which no producer fills; health.ts:398-404 prefers runtimeProbe.reason, which Computer Use always has, so the Health cu_backend_status entry is unreachable; apps/desktop/src/main/permission-snapshot-e2e-fixture.ts:94 still emits Chinese prose and now renders reasonFallback.
  • "W2b" appears only in this body; if it is a roadmap item, link it.
  • Separable scope: the dev-singleton dialog, the message-box close label and the native dialog titles are a different producer family with no code union; fine to keep, but they are why the diff is 42 files.

Ordering: #4639 first, then this rebases and picks up its bot copy; with #4641 and #4526 the shared files differ only by key, any order.

Manual acceptance, three locales: Permission Center's Computer Use row against main word for word, a bot row with a failed test, the Health center row for the same capability (should now match the Permission Center), a memory save with counts and a backup, an OAuth subscription failure without a code.

Evidence boundary: static read against main; memory string fidelity was not re-checked line by line; no build, no suites, no Desktop.

AI-assisted review: drafted with Maka in two passes; I verified the two reason tables, the Permission Center resolver, the three prose codes and the sniff removal myself.

@orangeCatDeveloper
orangeCatDeveloper marked this pull request as draft September 5, 2026 22:40
@orangeCatDeveloper
orangeCatDeveloper force-pushed the fix/locale-producer-codes branch 4 times, most recently from e06b161 to ccae010 Compare September 6, 2026 02:29
@orangeCatDeveloper
orangeCatDeveloper marked this pull request as ready for review September 6, 2026 06:56
@orangeCatDeveloper

Copy link
Copy Markdown
Contributor Author

@Astro-Han All issues are addressed in the latest commit

@orangeCatDeveloper

Copy link
Copy Markdown
Contributor Author

@Astro-Han Rebased and CI is green again

Settings-area producers (memory, data, permission center, connection
test, computer-use health, dev dialogs, Copilot import) emitted zh prose
that reached en users verbatim, and presenters sniffed CJK to decide
whether to show it. Producers now return stable codes and redact probe
detail at the source; each settings catalog maps its codes per locale
with complete message templates and an explicit unknown fallback. One
capability reason catalog serves the Health center and the Permission
Center, bot reasons resolve through the bot catalog, and the memory and
Copilot results carry a single code field.

Generated-by: Claude Code
Generated-by: OpenCode
Remove the rebase-duplicated 429 branch that shadowed shared.rateLimit
behind shared.rateLimited and drop the now-dead rateLimited copy rows.
Finish the permission-center guidance-block removal: delete the unused
locale keys, empty the storybook fixture guidance, and fix the comment
that still described the block. Log capability probe failures before
collapsing them to permission_probe_failed. Drop redundant
cu_backend_status re-checks after the early return and shorthand the
locale pass-through in command actions.

Generated-by: Claude Code
…rthand

The `locale: locale` → `locale` shorthand in app-shell-command-actions.ts
drops two non-trivia tokens; regenerate the architecture ledger.

Generated-by: Claude Code
The storybook smoke failed on product-settings-pages--about-update-failed
across every CI run of this branch: the story pins
net::ERR_CONNECTION_RESET, classifyGeneralizedError did not recognize the
Chromium prefix (it only matches Node errno spellings), so the PR's new
[settings] operation failed console.error fired on a story that renders
normally, and the smoke treats console.error as fatal. Empty-commit
retriggers could not fix it.

Classify the net::ERR_ prefix as network_error so the update row renders
Network error / 网络错误 instead of the unknown-error fallback, which also
removes the diagnostic for this expected story path. Cover the prefix and
its per-locale rendering in the classifier tests.

Generated-by: Claude Code
The permission-center guidance list was removed in 02604e2 (no
producer fills guidance; apache#4526's layered rows carry the actionable
lines), but the snapshot-side plumbing survived: the
CapabilitySnapshot.guidance field, staticCapability's guidance input,
botCapability's empty array, and the fixtures' guidance: [] entries.
Every future producer had to keep feeding a write-only field.

Delete the field, the producer parameter, the botCapability entry, and
the fixture entries; fix the permission-center comment that still named
the removed guidance list.

Generated-by: Claude Code
…esult-copy

subscription-result-message.ts no longer exists — the module moved into
settings-provider-copy.ts beside the catalog it renders, so the old test
file name referenced nothing. Fold its two assertions (Traditional
Chinese rejects raw Simplified backend prose; the generalized classifier
still answers network failures) into oauth-result-copy.test.ts, which
already drives subscriptionResultMessage.

Also drop the unused lookupCopy import from settings-health-copy.ts.

Generated-by: Claude Code
The inline { type X } form counts as a value import in the renderer
architecture analysis, so the ledger recorded an @maka/core/ui-locale
dependency edge and CI's --base check failed against main, which uses
the type-only form. Restore import type and regenerate the ledger.

Generated-by: Claude Code

@Astro-Han Astro-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.

I reviewed this PR at head 8ccee58b (50 files, +1138/−443) against base bd3646b3, and re-verified every finding from the earlier review (which was bound to the older head af07077c) against the current code.

What the change does: settings-area producers stop emitting zh-only result strings and now emit stable machine codes; each settings catalog maps its union exhaustively per locale with an explicit unknown-code fallback. Covered producers: capability-snapshot reasons, memory operations, OAuth rate-limit test results, Computer Use host health, the GitHub Copilot import path, plus native dialog titles wired through the main-process locale authority.

Prior-review findings, re-verified at this head:

  • P1 (two parallel three-locale reason tables saying different things) — fixed. One shared capability-reason-copy.ts holds a single Record<CapabilityReasonCode, string> per locale; the Health center reads it via capabilityReasonMessage and the Permission Center via getCapabilityReasonCopy, and permission-center-copy.ts keeps only its cuBackendStatus composition. No duplicated 23×3 tables remain.
  • P2 (Permission Center losing bot capability reasons) — fixed. capabilityReasonText routes bot:* capabilities through botStatusReasonCopy (post-#4639) before falling back.
  • P2 (three prose reason tokens) — fixed. The tokens are now snake_case (platform_credentials_missing, macos_tcc_only, apple_events_tcc_status_unavailable), including in the e2e fixture.
  • P2 (subscriptionResultMessage dropping an unclassified Host reason) — deliberately diverged, with justification. The review asked to keep a raw-text pass-through as the last step; the author instead routes unclassified messages through the localized classifier and then the locale fallback, citing the #2672 locale policy (do not show untranslated raw text). The resolution order is preserved: code → typed reason → three legacy English Host matches → classifier → fallback (settings-provider-copy.ts:672-686). The loss is bounded to genuinely unclassifiable Host messages, which are rare; I record this as a P3 for human confirmation rather than a blocker.
  • Smaller items — all landed. The 429 rate-limit guidance is preserved renderer-side keyed on the stable statusCode === 429 signal (provider-panel-shared.ts:58); the memory IPC and Copilot IPC emit typed code instead of raw/Chinese message (dead field removed); the unused guidance plumbing is deleted outright (commit c04ded5d4); the e2e fixture emits the closed code; native dialog titles use a UiCatalog in the main process; the classifier gains net::err network-error classification.

New scope spot-check: the memory copy is a complete typed catalog (~150 keys × three locales plus results keyed by MemoryResultCode), covered by settings-memory-copy.test.ts; the OAuth/Copilot/dialog producers follow the same code-plus-catalog pattern. CI on this exact head is fully green (Build, Typecheck, affected workspace tests, Desktop e2e, Storybook smoke, Lint).

P3 (recorded): the unclassified-Host-message divergence above (English users may see a generic localized line instead of the Host's specific reason; bounded); and the Storybook smoke still matches the expected console line by full text (isExpectedConsoleError, with its own test) rather than the [settings] prefix — intentional so a changed error story fails the smoke, but it stays brittle to wording changes.

Not verified: I did not re-read every one of the ~150 memory template strings line by line for copy quality (structure is sound, CI green, tests cover exact output); I did not run the suites locally (CI on the exact head covers them). A manual pass over the three locales — Health vs Permission Center rendering the same capability identically, a failed bot row, and a memory save with counts — is worth doing before merge.

No P0–P2 findings. This is a behavior-changing fix; I am not approving — the merge decision belongs to humans.


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

@orangeCatDeveloper

Copy link
Copy Markdown
Contributor Author

Thanks — that matches what I intended, and I did the manual three-locale pass you asked for before merge. Both P3s are deliberate; details and evidence below.

P3-1 — unclassified Host message

Confirmed intentional. #2672 makes untranslated raw text a defect in its own right, so a Host sentence nobody can classify is exactly the case the policy is about: a zh-CN user seeing a raw English string is the failure this PR set exists to remove, and passing it through would reintroduce it at the last step. The resolution order still puts every path that can say something specific ahead of the fallback (settings-provider-copy.ts:672-686):

code → typed reason → three legacy English Host matches → classifier → locale fallback

The three legacy matches can go once every Host sends code; I'll drop them in that PR rather than leave the fallback carrying them.

P3-2 — Storybook smoke matches the full sentence

Keeping the full-sentence match. The suggested [settings] prefix would make the smoke miss the failure it exists to catch, because the real bridge-missing error shares that prefix:

[settings] operation failed: Runtime Host settings read failed in this story.   ← expected, must be tolerated
[settings] operation failed: Cannot read properties of undefined (reading 'getSnapshot')   ← must stay fatal

storybook-visual-smoke.test.mjs asserts the second one stays fatal inside the error story, along with ${expectedError} unexpected detail and an unrelated render failure. A prefix match makes all three pass, so the guard stops guarding. Brittleness to wording is the intended trade: if the story's sentence changes, the smoke should fail and be updated deliberately.

Manual three-locale pass

Head 8ccee58b, Electron fixture settings-permissions, platform=darwin, real window at 1440×960, one run per locale.

Structural result first: there is now exactly one reason table — apps/desktop/src/renderer/locales/capability-reason-copy.ts, 23 codes × 3 locales — and it has exactly two importers:

health-center-page.tsx:31      import { capabilityReasonMessage }   from '../locales/capability-reason-copy'
permission-center-page.tsx:59  import { getCapabilityReasonCopy }   from '../locales/capability-reason-copy'

Every reason string rendered on either page was matched byte-for-byte against its catalog entry in that locale:

locale    page          code
en        Permissions   apple_events_tcc_status_unavailable
en        Health        disabled, cu_executor_undistributable,
                        activity_recorder_probe_hint, memory_no_probe
zh-CN     (same set, all matched)
zh-TW     (same set, all matched)

zh-TW renders as real Traditional Chinese throughout, not a converted zh-CN string (外觀 / 記憶 / 遠端串接 / 匯入任務 / 執行態探測), and no page fell back to reasonFallback.

Permission Center, three locales Health center, three locales

Scope of this evidence. This fixture never puts the same code on both pages at once, so "the two pages describe one state the same way" rests on the single table plus the per-page byte matches above, not on a side-by-side row. Also not covered: a bot row with a failed test — the fixture's bots are paused, which renders disabled — and a memory save with counts and a backup; those stay on the typed-catalog tests.

One unrelated find

The zh-TW system-permissions help line reads 「對應分割槽」 (the disk-partition sense) where it means a Settings section. Pre-existing — it comes from b1369c808 (#3853), and this PR does not touch that line — so I'm leaving it out of this diff rather than widening the scope. Worth a separate one-line fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Under 2500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants