fix(i18n): emit settings expected-result codes from producers - #4551
Conversation
ad2b11d to
89c58cd
Compare
89c58cd to
3ffb1a2
Compare
4b6d344 to
b526451
Compare
b526451 to
c9390a5
Compare
236c0da to
daeb9ba
Compare
2a2c114 to
bc9228e
Compare
e06b161 to
ccae010
Compare
ccae010 to
f932a35
Compare
|
@Astro-Han All issues are addressed in the latest commit |
|
@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
…R_CONNECTION_RESET)
…ION_RESET, 3rd occurrence)
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
a6f9379 to
066e288
Compare
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
left a comment
There was a problem hiding this comment.
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.tsholds a singleRecord<CapabilityReasonCode, string>per locale; the Health center reads it viacapabilityReasonMessageand the Permission Center viagetCapabilityReasonCopy, andpermission-center-copy.tskeeps only itscuBackendStatuscomposition. No duplicated 23×3 tables remain. - P2 (Permission Center losing bot capability reasons) — fixed.
capabilityReasonTextroutesbot:*capabilities throughbotStatusReasonCopy(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→ typedreason→ 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 === 429signal (provider-panel-shared.ts:58); the memory IPC and Copilot IPC emit typedcodeinstead of raw/Chinesemessage(dead field removed); the unusedguidanceplumbing is deleted outright (commitc04ded5d4); the e2e fixture emits the closed code; native dialog titles use aUiCatalogin the main process; the classifier gainsnet::errnetwork-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.
|
Thanks — that matches what I intended, and I did the manual three-locale pass you asked for before merge. Both P3s are deliberate. |
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for confirming the three-locale manual pass and the intentional P3 tradeoffs. The head remains 8ccee58b6, which was already reviewed without P0–P2 findings. Current CI is green, there are no unresolved threads, and the maintainer has confirmed proceeding with the merge.
AI-assisted source/integration review; the manual locale check was reported by the contributor, not rerun by this agent.
中文
感谢确认已完成三语言人工验收及两项 P3 的取舍。head 仍是此前无 P0–P2 的 8ccee58,当前 CI 绿色、无未解决讨论,维护者已确认合并。本轮为 AI 辅助源码/整合评审;三语言人工检查由贡献者报告,并非本代理重新执行。
Upstream 5a1514c..93a8dd7. Backend, preload and shared take upstream verbatim. Renderer adaptations: - apache#5001 transcript navigation: `lib/ported/desktop-transcript-range-store.ts` and `lib/ported/transcript-reading-position.ts` re-ported from the new upstream versions (navigationVersion admission, retired generations, the restore lifecycle, prepareTranscriptForSend). `store/active-session-store.ts` owns what the old React controller did: one restore lifecycle per selection, `prepareSend` (called from the send path, never awaited), `setReadingAnchor` → controller `setReadingAnchor`/`loadLatest`, gate replacement on `latest`, and one `createTranscriptViewportNavigation()` passed to `useChatScroll`. - apache#4551 coded settings results: memory, config-import, permission and health surfaces resolve Host codes through the upstream catalogs (`capability-reason-copy.ts`, `settings-memory-copy.ts` `results`, `importFailures`, `reasonFallback`); Copilot `SubscriptionActionCode`s added to `settings-models-copy.ts` and read first by `oauthFailureMessage`. Enterprise-only memory copy keys re-applied over the upstream file. - Old-renderer / Astryx / stories / WorkHub files re-added by rename detection deleted again; upstream main tests retargeted to `lib/ported/` where they only need the ported modules, deleted where they drive the removed React controller (recorded in the release checklist). - e2e fixture now collapses CSS transitions (`[data-maka-e2e-fixture]`), which the scroll-cost spec assumed but nothing implemented; the message action bar's hover fade made that spec flaky. Gates: build, typecheck, lint, format, asf, locale hygiene, renderer architecture, knip (per workspace), renderer-state 162, desktop dist 1470, ui dist 208, renderer smoke 43 checks, e2e 28/28, real-window smoke, root npm test all workspaces. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…#4551) Replace settings-area producer prose with stable expected-result codes so users receive localized output instead of raw Chinese messages or silently hidden guidance. Domain catalogs provide complete templates and explicit fallbacks, with one capability-reason catalog shared by Health and Permission Center. Preserve typed memory and Copilot outcomes, bot reason routing, rate-limit guidance, and redacted unexpected-error diagnostics. Remove unused guidance plumbing and duplicated presentation paths. Native dialog labels use the existing main-process locale authority. Validation: reviewed without P0–P2 findings, current-head CI passed, and the contributor reported completing the three-locale manual pass. Maintainer confirmed merging. Refs apache#2672 Generated-by: Claude Code Generated-by: OpenCode Generated-by: Muse Spark
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-snapshotreasons, memory ops,test-connectionOAuth rate-limit,computer-use-hosthealth 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).subscriptionResultMessagenow takes the result envelope and resolvescode, then the typedreason(experimental_disabled), through the catalog before falling back to the keyword classifier; its trailing CJK passthrough is gone. It moved out of theuseOAuthLoginFlowhook 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
guidanceafter the code-ification, and #4526's layered rows carry the actionable lines (cuBackendStatuscomposition), 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
Boundaries: one
capability-reason-copy.tscatalog serves the Health center and the Permission Center; the three prose reason tokens are nowplatform_credentials_missing,macos_tcc_onlyandapple_events_tcc_status_unavailable. Bot capability reasons resolve through #4639'sbotStatusReasonCopy. The rate-limited connection test keeps its guidance keyed onstatusCode === 429, the same stable signal the command palette already uses, rather than wideningConnectionTestErrorClassacross the codec, CLI, and renderer.subscriptionResultMessagestill 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.mjsrequires unrelated settings errors in the error story to stay fatal. Permission probe failures collapse topermission_probe_failedfor the page, and the raw cause is kept observable via aconsole.warnincapability-snapshot.tsso 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:
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-bytrailer.OpenCode also implemented the complete memory-message templates and their output tests, and updated this description.
Checklist