fix(tutor): propagate chatStream bridge-not-ready as a rejection (stranded spinner)#173
Closed
heznpc wants to merge 2 commits into
Closed
fix(tutor): propagate chatStream bridge-not-ready as a rejection (stranded spinner)#173heznpc wants to merge 2 commits into
heznpc wants to merge 2 commits into
Conversation
…K prose A service-quality audit confirmed (reproduced from shipped data) that restoreProtectedTerms — an unanchored replaceAll with no CJK word boundary — corrupts CORRECT translations whenever a _protected "wrong form" is itself a common standalone word: 클라우드(Cloud) -> Claude "클라우드 컴퓨팅" -> "Claude 컴퓨팅" 인류(humanity) -> Anthropic "인류의 미래" -> "Anthropic의 미래" 기업(enterprise) -> Enterprise "대기업 환경" -> "대Enterprise 환경" 협업(collaborate)-> Cowork, etc. This hit all four CJK locales (ko/ja/zh-CN/zh-TW) live — worse than untranslated text, since it ships confident, brand-correct-looking but semantically wrong output. The code comment already documented the substring class; the data was triggering it broadly. Fix: removed the dangerous wrong-forms — common words whose meaning differs from the brand — from the _protected sections of all 4 CJK dictionaries, keeping the brand transliterations and same-concept forms (클로드, 플러그인, 技能, サブエージェント). Intended restorations still work (클로드->Claude, 서브에이전트->subagent verified); brand-English fidelity that's lost (e.g. Enterprise occasionally shown as 기업) is the Gemini-verify pass's job and is far preferable to corrupting real prose. Regenerated the companion plugin's bundled data accordingly. Tests: added a regression guard in protected-terms.test.js that runs the REAL src/data/*.json (not a fixture) and asserts ordinary CJK prose passes through untouched — it fails if a dangerous wrong-form is ever re-introduced. 492 tests, eslint, check:plugin, check:locales green. Note: the src/data diff is large because ja/zh _protected arrays were normalized from multi-line to single-line; the only semantic change is the removed forms.
… a string
Confirmed by a service-quality audit. chatStream's try/catch caught the
synchronous `throw new Error('Bridge not ready')` (Puter bridge handshake not
finished — reachable on cold start / slow or blocked CDN / non-trusted host) and
RESOLVED to a localized error string. But the sole caller (sidebar-chat.js:381)
discards chatStream's return value and relies on a thrown error to render the
error bubble + retry button. So on bridge-not-ready: the caller's await resolved
normally, its catch never ran, onChunk never fired — leaving the "thinking…"
spinner stranded forever with no error and no retry. The E2E helpers even
document the intended behavior as "chatStream throws 'Bridge not ready'".
Fix: re-throw from the catch instead of returning a string, so the synchronous
setup failure propagates as a promise rejection and the caller's existing
error+retry path handles it. Promise-path failures (timeout, abort,
success:false) already reject and are unaffected.
Added a regression test asserting chatStream rejects (not resolves) when
isReady is false. 493 tests, eslint, prettier green.
Owner
Author
|
Superseded. This branch was accidentally created on top of #172's protected-terms changes (the background 'checkout main' hadn't completed), so it bundled both fixes and failed the same protected-terms E2E. #172 carries the protected-terms fix + its E2E update; the chatStream fix (commit fc32537) will be re-opened cleanly from main as its own PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Confirmed bug
chatStream'stry/catch(translator.js) caught the synchronousthrow new Error('Bridge not ready')— raised when the Puter bridge handshake hasn't finished (reachable on cold start, slow/blocked CDN, or a non-trusted host) — and resolved to a localized error string.But the sole production caller (
sidebar-chat.js:381) discards chatStream's return value and relies on a thrown error to render the error bubble + retry button (catch atsidebar-chat.js:408). So on bridge-not-ready:awaitresolved normally → itscatchnever ran → no error bubble, no retry;onChunknever fired →startedstayed false → the bubble was never cleared;The E2E helpers (
tests/e2e/helpers/extension.js:412,tutor-chat.spec.js:58) even document the intended behavior as "chatStream throws 'Bridge not ready'" — the production code diverged from it.Fix
Re-throw from the catch instead of returning a string, so the synchronous setup failure propagates as a promise rejection and the caller's existing error+retry path handles it. Promise-path failures (timeout / abort /
success:false) already reject and are unaffected.Test
Added a regression test:
chatStreamrejects (not resolves) whenisReadyis false. 493 tests · eslint · prettier green.🤖 Generated with Claude Code