Skip to content

fix(i18n): emit bot expected-result codes from producers - #4639

Merged
Astro-Han merged 1 commit into
apache:mainfrom
orangeCatDeveloper:fix/locale-bot-codes
Sep 6, 2026
Merged

fix(i18n): emit bot expected-result codes from producers#4639
Astro-Han merged 1 commit into
apache:mainfrom
orangeCatDeveloper:fix/locale-bot-codes

Conversation

@orangeCatDeveloper

@orangeCatDeveloper orangeCatDeveloper commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Bot producers (bot-test error/hint pairs, bot-events replies and status reasons, wechat-bridge setup hints, bot onboarding) emitted zh-only strings. The bot settings pages did not sniff with /[㐀-鿿]/ regexes on main — they branch on locale === 'zh-CN' (bot-chat-detail.tsx, bot-chat-overview.tsx, bot-onboarding-modal.tsx) — so English users saw a generic fallback while zh users saw the producer's zh sentence. Producers now emit stable machine codes; settings-bot-copy maps each union exhaustively per locale with an explicit unknown-code fallback, and the locale gates in the bot settings pages are gone. zh copy mostly moved verbatim (send-failed and get-me-failed are rewrites), en copy is new. Bot platform display names and zh command keyword sets are protocol/matching data and are untouched; bot-channel notices follow the bot audience language and are annotated rather than re-plumbed.

packages/core/redaction.ts is restructured on the way: the keyword classifier becomes classifyGeneralizedError() → GeneralizedErrorClass (bot onboarding emits the class as its code), and the two hand-mirrored en/zh message maps collapse into one UiCatalog<Record<GeneralizedErrorClass, string>> behind generalizedErrorMessageForLocale(…, locale). generalizedErrorMessage stays as the en wrapper for its callers; the zh-CN and zh-TW wrappers are removed and their last caller goes through the locale form. Adding a locale now means adding one catalog block, not a third classifier.

Split out of #4551 so each PR is one reviewer context. main now ships generalizedErrorMessageForLocale; this PR keeps its signature and adds the exported classifier and code union.

Allowlist warnings and help text are complete per-locale formatters rather than UI-assembled fragments. Each locale owns the three-ID preview and count wording: Chinese retains its total-count phrasing, while English now reports only the omitted IDs as remaining. The adjacent limit-reached help follows the same complete-message rule.

Review follow-ups (squashed into the single commit):

  • Permission Center and Health center resolve bot capability reasons through the bot copy table instead of showing raw codes (gateway-closed-4004, stream-failed) in every locale. They call botStatusReasonCopy, which returns copy only for codes the catalog knows; anything else falls through to the pre-existing prose path, so no surface sniffs the text and the locale-hygiene ratchet stays flat. fix(i18n): emit settings expected-result codes from producers #4551 codes the remaining configuration prose as CapabilityReasonCode. Copy catalogs may not runtime-import each other (renderer architecture check), so the resolution lives at the page layer.
  • onboarding.errors and testHints are closed with satisfies over their producer unions, so a new code fails the typecheck instead of silently rendering as failed at runtime; the modal resolves codes through the catalog's botOnboardingErrorMessage, keeping the version-skew fallback the rendering tests pin.
  • The unread hint plumbing is gone: BotTestHintCode, the eight per-locale hint tables no surface reads, and the SettingsTestResult.details.hintCode passthrough (these hints were already dead on main — zh prose went into details.hint with no reader). QR-dialog hints keep their WechatBridgeQrHintCode table.
  • The WeChat QR failure dialog renders localized hint-code titles; the raw English diagnostic (error.error) moves to console.warn.
  • botTestFailure collapses to code resolution; the SettingsTestResult.message field keeps a stable English diagnostic for support dumps and is never rendered.
  • Dead ?? fallback expressions at the five botStatusReasonMessage call sites are removed; the shared default gains a definite-string overload so only the undefined-reason path falls back.
  • The five GeneralizedErrorClass sentences live once in the bot catalog (BOT_TRANSPORT_ERRORS, spread into both statusReasons.codes and onboarding.errors) with platform wording, not the shared GENERALIZED_ERROR_COPY, whose sentences name the model service. The eight remaining BotTestErrorCode sentences still exist in both settings-bot-copy.ts and settings-test-result-copy.ts because catalogs may not import each other, and the pinning test keeps them equal.
  • A provider start() failure during onboarding now returns the error snapshot with its errorCode instead of throwing, so the modal renders botOnboardingErrorMessage for it the same way it does for a failed poll; a test covers the producer-to-presenter path.

User-visible behavior notes from review:

  • isWechatIlinkChannel no longer requires a token, so an iLink URL with an empty token now reports wechat_ilink_credentials_incomplete instead of taking the bridge path; unrelated to codes but user-visible.
  • zh-CN users no longer see the platform's own text (Telegram me.description, WebSocket close reason) in "last failure"; producers log it via console.warn and the presenter shows localized copy or detailsInLogs.
  • A persisted pre-PR lastError/readinessReason (prose) degrades to the generic line until the next connection test.

Refs #2672

Verification

workspace typecheck:                    0 errors (all four desktop tsconfigs)
desktop main tests (dist):              2214 pass / 0 fail
packages/core tests:                    826 pass / 0 fail (incl. en+zh render via generalizedErrorMessageForLocale)
packages/runtime tests:                 3208 pass (5 sandbox-only filesystem-worker failures, unrelated)
desktop build + renderer-architecture:  pass
rendered-output tests (en+zh):          bot connection-test codes — pass
biome (changed files):                  clean

The review follow-ups re-verified all of the above after the Permission Center / Health Center bot-reason resolution, the satisfies catalog closures, and the hint-plumbing removal. The new permission-center-bot-reason.test.ts (4 tests) pins that a raw code like gateway-closed-4004 never reaches the page in any locale. The earlier follow-up verification still holds: 26 bot-detail rendering and settings-copy tests pass, including all three locales, empty lists, overflow counts, and the allowlist limit.

Four invalid IDs, with the first three shown:

Before:
These entries are not numeric IDs and may be usernames, so they will not match anyone: @alice, @bob, @carol and 4 more

After:
These entries are not numeric IDs and may be usernames, so they will not match anyone: @alice, @bob, @carol and 1 more

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, and this description, under the contributor's direction; the commit carries a Generated-by: Claude Code trailer.

Claude Code implemented the complete allowlist messages and count fix, added rendering tests, updated this description, and landed the review follow-ups (Permission Center / Health Center bot-reason resolution, satisfies catalog closures, hint-plumbing removal, WeChat QR localized titles); commits carry a Generated-by: Claude Code trailer.

Checklist

  • Tests cover the change and fail without it

@github-actions github-actions Bot added the effort/L Under 1000 readable lines label Sep 3, 2026
@orangeCatDeveloper
orangeCatDeveloper force-pushed the fix/locale-bot-codes branch 12 times, most recently from 1518efa to 560790e Compare September 5, 2026 04:58
@orangeCatDeveloper
orangeCatDeveloper force-pushed the fix/locale-bot-codes branch 3 times, most recently from ae17f95 to b87baf2 Compare September 5, 2026 11:27
@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 3e60c8c against 03b5a2cc5a, two independent passes. The direction is right and the anchor is a good one: BotStatusReason is a closed type on base-adapter.ts, so the compiler, not a test, is what stops a bridge from writing prose again; the eight bridges and bot-test.ts all emit codes; the redaction.ts split is needed because onboarding now emits the class as its code, the classifier's behavior is unchanged, and the two zh wrappers had exactly one non-test caller on main, which is updated. zh-TW is real traditional copy and the en count fix ("and 1 more" for four invalid IDs with three shown) is correct.

Two P2s, then smaller items and body corrections.

P2: the Permission Center now shows the raw code. capability-snapshot.ts:247-251 feeds status.reason ?? channel.readinessReason into the bot capability's runtimeProbe.reason, and permission-center-page.tsx:707-709 (localizedSnapshotText) only filters CJK, so ASCII passes straight to the page. On main that string is an English sentence; after this PR it is connection_failed, gateway-closed-4004, stream-failed, in all three locales. Path ①: any bot whose test fails, then open Permission Center. Smallest fix: resolve bot:* capability reasons through botStatusReasonMessage at that call site (#4551 touches the same function and currently replaces it with a generic fallback; whichever lands second should end up with the bot copy, not the fallback).

P2: onboarding.errors is not exhaustive over BotOnboardingErrorCode. It is an inferred object and bot-onboarding-modal.tsx:273 falls back at runtime with Object.hasOwn, so a new code silently renders as failed. satisfies Record<BotOnboardingErrorCode, string> closes it and the hasOwn guard goes; testHints wants the same satisfies.

Smaller:

  • The ten BotTestErrorCode sentences exist twice, in settings-bot-copy.ts (testErrors) and settings-test-result-copy.ts (bot.errors); a test pins them equal per locale, which is why this is not a P2, but one table would need no test. rate_limited has three different sentences inside this PR (statusReasons.codes, onboarding.errors, and the five GeneralizedErrorClass sentences redaction.ts already owns and exposes through generalizedErrorMessageForLocale); read those five from redaction.ts.
  • details.hintCode has no renderer reader (the WeChat QR dialog reads a different hintCode on WechatBridgeQrCodeResult). main's details.hint had no reader either, so this is inherited, but the PR adds eight hint codes × 3 locales to a field nothing shows. Either wire it or drop the eight.
  • WeChat QR failure: the hint is coded, the title is not; wechat-bridge.ts:333 and :349 still return English sentences that bot-wechat-login.tsx:225 renders as the title in every locale.
  • botStatusReasonMessage never returns undefined (it ends in detailsInLogs), so the ?? fallback at its callers is dead; the English message assembled at settings-ipc-helpers.ts:210-240 has no reader left.
  • Upgrade: a persisted lastError / readinessReason from before this PR is prose, the new presenter only knows codes, so an existing "last failure" shows the generic line until the next test. Acceptable, say it in the body.

Body corrections:

  • The problem statement says the bot settings pages guarded with /[㐀-鿿]/ sniffing. They do not on main: the bot pages branch on locale === 'zh-CN' (bot-chat-detail.tsx:308, bot-chat-overview.tsx:160, bot-onboarding-modal.tsx:265,273). The information loss for non-zh users is real either way; say what the code did.
  • "zh copy moved verbatim" is mostly true; send-failed and get-me-failed are rewrites.
  • isWechatIlinkChannel losing its token check (wechat-bridge.ts:539, bot-test.ts:158) is harmless (an iLink URL with an empty token now reports ilink_credentials_incomplete instead of bridge_url_invalid), but it is a behavior change unrelated to codes; one sentence in the body.
  • zh-CN users lose the platform's own text (Telegram me.description, WebSocket close reason) in "last failure"; it now only reaches console.warn. Fine, but user-visible, so it belongs in the body.
  • zh-TW: testErrors.connection_failed says 憑據 while the rest of the block says 憑證.

Ordering with #4551: the two PRs make the same deletion in settingsTestResultMessage's default branch and both touch the Permission Center reason path, so this should land first; #4551 then rebases. No symbol is defined in both.

Manual acceptance, three locales: a bot with a gateway-closed-4004 last failure on the bot page and in Permission Center, the allowlist warning with four invalid IDs, a failed test per provider, the "credentials saved but not connected" toast, and the WeChat QR failure dialog.

Evidence boundary: static read against main; no build, no suites, no bot connected.

AI-assisted review: drafted with Maka in two passes; I verified the Permission Center path, the duplicated tables, the pinning test and the main bot-page branches myself.

@orangeCatDeveloper

Copy link
Copy Markdown
Contributor Author

Thanks — addressed at 137dd48 (squashed to one commit). Per item:

  • Permission Center raw code (P2): capabilityReasonText resolves bot:* reasons through botStatusReasonCopy, a new catalog export that returns copy only for codes it knows; unknown values fall through to the existing prose path. The Health center's localizedSignalDetail does the same, and permission-center-bot-reason.test.ts pins that gateway-closed-4004 never reaches either page in any locale. No CJK sniff was added, so check:locale-hygiene stays flat; fix(i18n): emit settings expected-result codes from producers #4551 then codes the configuration prose.
  • onboarding.errors exhaustiveness (P2): satisfies Record<BotOnboardingErrorCode, string> in all three locales; the modal resolves through botOnboardingErrorMessage instead of Object.hasOwn. testHints is now only the two WechatBridgeQrHintCode entries.
  • Duplicated sentences: statusReasons.codes reads the five GeneralizedErrorClass sentences from GENERALIZED_ERROR_COPY. The eight remaining BotTestErrorCode sentences still live in both settings-bot-copy.ts and settings-test-result-copy.ts because catalogs may not runtime-import each other under the architecture check, so the pinning test stays; noted in the body.
  • details.hintCode with no reader: dropped, along with BotTestHintCode and the eight hint tables. The QR dialog keeps its WechatBridgeQrHintCode table.
  • WeChat QR failure title: the dialog renders the hint-code title; the English diagnostic goes to console.warn.
  • Dead ?? fallback / unread message: the ?? sites are gone except bot-chat-shared.tsx, where status.reason is optional and the undefined overload applies. SettingsTestResult.message stays as an English support-dump diagnostic and is never rendered; the body says so.
  • Body: the sniff claim, "verbatim", isWechatIlinkChannel, the zh-CN platform-text loss, and the upgrade note for persisted prose reasons are all corrected or added; zh-TW uses 憑證 throughout.

#4551 is rebased on this head and picks up botStatusReasonCopy for the same call sites.

@hqhq1025 hqhq1025 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 137dd482b2dc5b6e4271c45a6f5b259ce819558b. The change replaces bot status, test, and onboarding prose with stable codes and adds localized presenters, but two producer/presentation gaps remain, so I do not consider this ready to merge yet.

I found two P2 issues: initial onboarding failures discard the newly computed code before IPC, and shared generalized copy describes bot-channel failures as model-service failures. Local validation passed build:test, Desktop 2214/2214, Core 826/826, Runtime 3200 passed / 13 skipped, full typecheck, lint, format, ASF, diff, and renderer architecture 101/101. Hosted test is green. A current-main merge-tree was clean; native provider login flows were not exercised.

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.

Comment thread apps/desktop/src/main/bot-onboarding-main.ts Outdated
Comment thread packages/core/src/redaction.ts
@orangeCatDeveloper

orangeCatDeveloper commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

@hqhq1025 Both points hold; fixed at 1b16de3.

  • Onboarding start() discarded the code: the catch now returns the error snapshot (state: 'error', errorCode) instead of throwing, so settings:bots:onboarding:start hands the modal the same shape a failed poll produces and it renders botOnboardingErrorMessage(errorCode). bot-onboarding-main.test.ts covers a provider start() that throws HTTP 503: the snapshot carries provider_error and every locale renders the onboarding copy for it.
  • Model-service wording on bot surfaces: the bot catalog owns its five transport sentences (BOT_TRANSPORT_ERRORS, platform wording: "平台服务暂时不可用" / "The platform is temporarily unavailable"), spread into both statusReasons.codes and onboarding.errors. GENERALIZED_ERROR_COPY is no longer imported at runtime there; the classifier codes stay shared.

@hqhq1025 hqhq1025 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 1b16de367cf15ec4f510c5cc242c5c83058f28f0. I found no remaining P0-P3 issues in this revision.

The two findings from my previous review are fixed. BotOnboardingService.start() now returns its terminal snapshot with the classified errorCode, so the IPC success arm reaches botOnboardingErrorMessage() in every locale. The bot settings catalog also owns bot-transport-specific wording instead of reusing model-service copy, while retaining an exhaustive mapping for BotOnboardingErrorCode.

Local validation passed clean install, build:test, Desktop 2215/2215, Core 826/826, Runtime 3200 passed / 13 skipped, full typecheck, lint, format, locale hygiene, ASF headers, diff check, renderer architecture 101/101, and the architecture ratchet against current main on a clean patch-preserving synthetic merge. The hosted test check is currently red on the new quote-window-boundary Electron test from main; the focused test passed locally on both exact main and the PR+main synthetic merge, so I could not attribute that failure to this PR, but the required hosted gate still needs a green rerun before merge. I did not connect to a live bot provider.

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.

@hqhq1025 hqhq1025 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 exact head 12b1660c68454b04919d2fdf7eacaf74094b05da against current main (ef2a724537188aaafc4e3a8f2340b137109e6c5f).

The new commit is intentionally empty: its tree (77332766cbd18628b97714e22c5e8b62d11aa996) is byte-identical to the previously reviewed technical-GO head 1b16de367cf15ec4f510c5cc242c5c83058f28f0. The two earlier P2 issues therefore remain fixed: onboarding start failures preserve their stable producer error code, and Bot network/rate-limit/service copy no longer attributes Bot transport failures to the model provider.

I found no new P0-P3 issue in this exact head. The prior identical-tree validation covered clean install, build:test, Desktop 2215/2215, Core 826/826, Runtime 3200 passed / 13 skipped, full typecheck, lint/format/locale/ASF/diff checks, and renderer architecture 101/101. The required hosted test is now green on this exact head. A fresh merge-tree onto current main is clean and preserves the PR patch-id (d4eeef05).

I did not connect to a real Bot provider.

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.

@hqhq1025 hqhq1025 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 exact head c4bd455ae5fa5d611f780274426d9c4d5ef6ac49 against current main (d2d7efe645b6f1af2cb47ec9c85f28a85b32b5f9).

I found no new P0-P3 issue. The substantive author commit has the same stable patch ID (d4eeef05) as the previously reviewed technical-GO commit, and the tip commit is empty. The two earlier P2 fixes therefore remain intact: bot onboarding start failures preserve their stable error code, and bot transport failures use bot-specific localized copy.

The current head forms a clean merge tree with current main, and the merged tree preserves the same patch ID. The exact-head hosted test completed every preceding step successfully and failed only in the Storybook story product-composer-slash-menu--context-switch-starts-with-a-loading-catalog. I reproduced the identical visibility assertion on the PR's exact base ef2a724537188aaafc4e3a8f2340b137109e6c5f, so I do not attribute that failure to this PR; the required check nevertheless remains red. The previously reviewed identical patch passed clean install, build:test, Desktop 2215/2215, Core 826/826, Runtime 3200 passed with 13 skipped, full typecheck, lint, format, locale, ASF, diff, and renderer architecture checks. I did not connect a live bot provider or run native macOS/Windows behavior locally.

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.

Bot test, onboarding, and bridge producers emitted zh prose that reached
en users verbatim, and the renderer sniffed CJK to decide whether to
show it. Producers now return stable codes; the settings-bot catalog
maps each code per locale with an explicit unknown fallback, and each
locale owns its complete allowlist warning and help messages. The
Permission Center and Health center resolve bot reasons through the
same catalog, so no surface sniffs the text.

Generated-by: Claude Code
Generated-by: OpenCode

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

Re-reviewed at f1344fd against main (a5a99a633); the increment over 3e60c8c is 22 files, +367/−201, CI green, clean merge.

Both P2s are fixed at the owner. Permission Center now routes bot: capability reasons through botStatusReasonCopy before falling back to the snapshot text (permission-center-page.tsx:710-720), the same path Health Center takes, and the new test pins gateway-closed-4004 in three locales. onboarding.errors carries satisfies Record<BotOnboardingErrorCode, string> in all three locales (settings-bot-copy.ts:187/326/417) and the modal reads through botOnboardingErrorMessage. hqhq1025's two points are fixed the right way too: bot-onboarding-main.ts:206 returns the error snapshot instead of throwing, and the modal's polling effect only runs in waiting / scanned, so an error snapshot does not keep polling. hintCode is gone end to end, the ?? fallback reads are gone, and the duplicated tables went from ten to eight with the remaining ones pinned exhaustively.

P3s, all pure deletions this increment made possible:

  • GENERALIZED_ERROR_COPY is now exported from packages/core/src/redaction.ts:242 with no importer, and the comment says catalog authors spread it, which the bot catalog does not do (it defines BOT_TRANSPORT_ERRORS). Drop the export and the comment.
  • settings-health-copy.ts:256/288/317 rename the parameter from detail to signal and immediately do const detail = signal.detail; HealthSignalDetail at :24 becomes an unused import. Revert those six lines.
  • Nine copy entries lost their last reader when the fallbacks went: detail.latestFailureDetail, detail.savedButNotConnected, onboarding.connectedWarning in all three locales. Delete them.
  • bot-wechat-login.tsx:228-229 shows copy.readQrFailed as both title and description when there is no hintCode. Pass description only when there is something different to say.

Manual check before merge, three locales: a bot whose gateway is closed in Permission Center and Health Center; onboarding failing on the version handshake; the QR read failing.

Evidence boundary: static read against main; no build, no test run, no live bot provider.

AI-assisted review: drafted with Maka; I verified the Permission Center presenter, the three satisfies clauses and the dead exports myself.

@Astro-Han
Astro-Han merged commit ee88894 into apache:main Sep 6, 2026
1 check passed
@orangeCatDeveloper
orangeCatDeveloper deleted the fix/locale-bot-codes branch September 6, 2026 05:15
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.

3 participants