Uh oh!
There was an error while loading. Please reload this page.
fix(i18n): emit bot expected-result codes from producers - #4639
Conversation
1518efa to
560790eCompareae17f95 to
b87baf2Compare
Astro-Han
left a comment
There was a problem hiding this comment.
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
BotTestErrorCodesentences exist twice, insettings-bot-copy.ts(testErrors) andsettings-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_limitedhas three different sentences inside this PR (statusReasons.codes,onboarding.errors, and the fiveGeneralizedErrorClasssentencesredaction.tsalready owns and exposes throughgeneralizedErrorMessageForLocale); read those five fromredaction.ts. details.hintCodehas no renderer reader (the WeChat QR dialog reads a differenthintCodeonWechatBridgeQrCodeResult).main'sdetails.hinthad 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:333and:349still return English sentences thatbot-wechat-login.tsx:225renders as the title in every locale. botStatusReasonMessagenever returnsundefined(it ends indetailsInLogs), so the?? fallbackat its callers is dead; the Englishmessageassembled atsettings-ipc-helpers.ts:210-240has no reader left.- Upgrade: a persisted
lastError/readinessReasonfrom 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 onmain: the bot pages branch onlocale === '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-failedandget-me-failedare rewrites. isWechatIlinkChannellosing its token check (wechat-bridge.ts:539,bot-test.ts:158) is harmless (an iLink URL with an empty token now reportsilink_credentials_incompleteinstead ofbridge_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 reachesconsole.warn. Fine, but user-visible, so it belongs in the body. - zh-TW:
testErrors.connection_failedsays 憑據 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.
0cb9ff4 to
137dd48CompareorangeCatDeveloper
commented
Sep 5, 2026
Thanks — addressed at 137dd48 (squashed to one commit). Per item:
#4551 is rebased on this head and picks up |
hqhq1025
left a comment
There was a problem hiding this comment.
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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
137dd48 to
1b16de3Compare@hqhq1025 Both points hold; fixed at 1b16de3.
|
hqhq1025
left a comment
There was a problem hiding this comment.
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
left a comment
There was a problem hiding this comment.
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.
12b1660 to
c4bd455Compare
hqhq1025
left a comment
There was a problem hiding this comment.
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
c4bd455 to
f1344fdCompare
Astro-Han
left a comment
There was a problem hiding this comment.
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_COPYis now exported frompackages/core/src/redaction.ts:242with no importer, and the comment says catalog authors spread it, which the bot catalog does not do (it definesBOT_TRANSPORT_ERRORS). Drop the export and the comment.settings-health-copy.ts:256/288/317rename the parameter fromdetailtosignaland immediately doconst detail = signal.detail;HealthSignalDetailat:24becomes an unused import. Revert those six lines.- Nine copy entries lost their last reader when the fallbacks went:
detail.latestFailureDetail,detail.savedButNotConnected,onboarding.connectedWarningin all three locales. Delete them. bot-wechat-login.tsx:228-229showscopy.readQrFailedas both title and description when there is nohintCode. Passdescriptiononly 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.
Uh oh!
There was an error while loading. Please reload this page.
Summary
Bot producers (
bot-testerror/hint pairs,bot-eventsreplies and status reasons,wechat-bridgesetup hints, bot onboarding) emitted zh-only strings. The bot settings pages did not sniff with/[㐀-鿿]/regexes on main — they branch onlocale === '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-copymaps 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-failedandget-me-failedare 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.tsis restructured on the way: the keyword classifier becomesclassifyGeneralizedError() → GeneralizedErrorClass(bot onboarding emits the class as its code), and the two hand-mirrored en/zh message maps collapse into oneUiCatalog<Record<GeneralizedErrorClass, string>>behindgeneralizedErrorMessageForLocale(…, locale).generalizedErrorMessagestays 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):
gateway-closed-4004,stream-failed) in every locale. They callbotStatusReasonCopy, 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 asCapabilityReasonCode. Copy catalogs may not runtime-import each other (renderer architecture check), so the resolution lives at the page layer.onboarding.errorsandtestHintsare closed withsatisfiesover their producer unions, so a new code fails the typecheck instead of silently rendering asfailedat runtime; the modal resolves codes through the catalog'sbotOnboardingErrorMessage, keeping the version-skew fallback the rendering tests pin.BotTestHintCode, the eight per-locale hint tables no surface reads, and theSettingsTestResult.details.hintCodepassthrough (these hints were already dead on main — zh prose went intodetails.hintwith no reader). QR-dialog hints keep theirWechatBridgeQrHintCodetable.error.error) moves toconsole.warn.botTestFailurecollapses to code resolution; theSettingsTestResult.messagefield keeps a stable English diagnostic for support dumps and is never rendered.?? fallbackexpressions at the fivebotStatusReasonMessagecall sites are removed; the shared default gains a definite-string overload so only the undefined-reason path falls back.GeneralizedErrorClasssentences live once in the bot catalog (BOT_TRANSPORT_ERRORS, spread into bothstatusReasons.codesandonboarding.errors) with platform wording, not the sharedGENERALIZED_ERROR_COPY, whose sentences name the model service. The eight remainingBotTestErrorCodesentences still exist in bothsettings-bot-copy.tsandsettings-test-result-copy.tsbecause catalogs may not import each other, and the pinning test keeps them equal.start()failure during onboarding now returns the error snapshot with itserrorCodeinstead of throwing, so the modal rendersbotOnboardingErrorMessagefor it the same way it does for a failed poll; a test covers the producer-to-presenter path.User-visible behavior notes from review:
isWechatIlinkChannelno longer requires a token, so an iLink URL with an empty token now reportswechat_ilink_credentials_incompleteinstead of taking the bridge path; unrelated to codes but user-visible.me.description, WebSocket close reason) in "last failure"; producers log it viaconsole.warnand the presenter shows localized copy ordetailsInLogs.lastError/readinessReason(prose) degrades to the generic line until the next connection test.Refs #2672
Verification
The review follow-ups re-verified all of the above after the Permission Center / Health Center bot-reason resolution, the
satisfiescatalog closures, and the hint-plumbing removal. The newpermission-center-bot-reason.test.ts(4 tests) pins that a raw code likegateway-closed-4004never 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:
AI use
Select exactly one:
Tool(s) and scope: Claude Code — analysis, implementation, tests, and this description, under the contributor's direction; the commit carries a
Generated-by: Claude Codetrailer.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,
satisfiescatalog closures, hint-plumbing removal, WeChat QR localized titles); commits carry aGenerated-by: Claude Codetrailer.Checklist