diff --git a/CHANGELOG.md b/CHANGELOG.md index d0fcbcbb4..a7d72c658 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **AI streaming no longer appends fallback text after a provider emits a partial response:** + the partial-then-fail guard previously only covered Grok, so a non-Grok primary (OpenRouter, + OpenAI, Gemini, Anthropic, Ollama) that streamed visible text and then failed mid-response + could have a second, full fallback answer appended after it — now every provider attempt in + the fallback chain, including the OpenRouter-promoted-fallback attempt, terminates on failure + once it has emitted output. Part of #714. PR #770. - **Writer generations, Global Copilot replies, and Dashboard logline suggestions now reject stale results after a project-incarnation change:** Writer's generation history and Copilot's chat/reply state live in global Redux, so they survived a view unmount/remount across a project diff --git a/README.md b/README.md index 379b0cd39..2f52da652 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,7 @@ TypeScript native preview (tsgo) Tauri 2 19 locales — 2942 keys - 7927+ tests / 609 files + 7931+ tests / 609 files Codecov Coverage CI status MIT License @@ -809,7 +809,7 @@ The normal web PR pipeline is not the complete native release qualification surf | DOCX | `docx` + JSZip | Word-compatible export | | PWA | Service Worker + Web App Manifest | Offline shell/installability | | i18n | Custom React i18n context | 2942 keys × 19 locales | -| Testing | Vitest 4.x (7927+ tests / 609 files) + Playwright | Unit/integration/E2E | +| Testing | Vitest 4.x (7931+ tests / 609 files) + Playwright | Unit/integration/E2E | | Quality | Biome + tsgo + CodeQL/security tooling | Static and CI gates | | Desktop | Tauri 2 | Current native shell | @@ -886,7 +886,7 @@ WorldScript-Studio/ ├── locales/ # Source locale trees ├── public/ # PWA assets, manifest, SW, runtime locale bundles ├── tests/ -│ ├── unit/ # Vitest unit tests (7927+ tests; file count includes package test directories) +│ ├── unit/ # Vitest unit tests (7931+ tests; file count includes package test directories) │ └── e2e/ # Playwright ├── docs/ # Canonical product/engineering documentation + ADRs ├── src-tauri/ # Tauri v2 desktop shell / Rust @@ -1031,7 +1031,7 @@ Raw bundle-budget ceilings (KB per uncompressed asset): entry **2500 KB**, vendo Current source-synchronized README metrics: -- **7927+ unit tests** across **609 test files** +- **7931+ unit tests** across **609 test files** - i18n: **2942 keys × 19 locales** CI remains authoritative for actual pass/fail and live coverage. diff --git a/services/aiProviderService.ts b/services/aiProviderService.ts index 246863529..c0cb0bf46 100644 --- a/services/aiProviderService.ts +++ b/services/aiProviderService.ts @@ -94,12 +94,22 @@ function recordProviderSuccess(primary: AIProvider, provider: AIProvider, index: index > 0 ? `Primary provider ${primary} failed; fell back to ${provider}.` : ''; } -// QNBS-v3: retain Grok chunk tracking so partial output cannot be followed by fallback text. -function createGrokAttemptCallbacks( - callbacks: AIStreamCallbacks, - onChunk: (text: string) => void, -): AIStreamCallbacks { - return { ...callbacks, onChunk }; +// QNBS-v3: tracks whether an attempt emitted output, so a later failure on it can't be followed by fallback text appended to a truncated answer — applies to every provider, not just Grok. +function createAttemptEmittedTracker(callbacks: AIStreamCallbacks): { + callbacks: AIStreamCallbacks; + hasEmitted: () => boolean; +} { + let emitted = false; + return { + callbacks: { + ...callbacks, + onChunk: (text) => { + emitted = true; + callbacks.onChunk(text); + }, + }, + hasEmitted: () => emitted, + }; } // ─── Gemini Provider ────────────────────────────────────────────────────────── @@ -437,6 +447,154 @@ export async function generateImage( ); } +// QNBS-v3: extracted out of streamText's fallback loop to keep its cognitive complexity under the repo ceiling — the promoted-fallback attempt shares the same try/tracker/cancellation shape. +type AttemptOutcome = + | { kind: 'succeeded' } + | { kind: 'failed-emitted'; error: unknown } + | { kind: 'failed-silent'; error: unknown }; + +async function attemptChainProvider( + prompt: string, + creativity: AiCreativity, + mergedOpts: AIRequestOptions, + nextProvider: AIProvider, + guardedCallbacks: AIStreamCallbacks, + signal: AbortSignal | undefined, +): Promise { + // QNBS-v3: track partial output before fallback decisions — applies to every provider, not just Grok. + const attemptTracker = createAttemptEmittedTracker(guardedCallbacks); + try { + await streamProvider( + prompt, + creativity, + { ...mergedOpts, provider: nextProvider }, + attemptTracker.callbacks, + signal, + ); + throwIfRequestAborted(undefined, mergedOpts.signal, signal); + return { kind: 'succeeded' }; + } catch (error) { + // QNBS-v3: A user-cancelled request is NOT a provider failure — throwing here propagates the cancellation straight out of streamText instead of continuing the fallback loop. + throwIfRequestAborted(error, mergedOpts.signal, signal); + return attemptTracker.hasEmitted() + ? { kind: 'failed-emitted', error } + : { kind: 'failed-silent', error }; + } +} + +async function attemptPromotedOpenRouterFallback( + mergedOpts: AIRequestOptions, + fallback: string, + prompt: string, + creativity: AiCreativity, + guardedCallbacks: AIStreamCallbacks, + signal: AbortSignal | undefined, +): Promise { + const fallbackTracker = createAttemptEmittedTracker(guardedCallbacks); + try { + await attemptOpenRouterFallback(mergedOpts, fallback, (fallbackOpts) => + streamProvider(prompt, creativity, fallbackOpts, fallbackTracker.callbacks, signal), + ); + throwIfRequestAborted(undefined, mergedOpts.signal, signal); + return { kind: 'succeeded' }; + } catch (fallbackError) { + // QNBS-v3: mirrors the outer catch's cancellation guard — a cancel during the promoted fallback must not be treated as a provider failure either. + throwIfRequestAborted(fallbackError, mergedOpts.signal, signal); + return fallbackTracker.hasEmitted() + ? { kind: 'failed-emitted', error: fallbackError } + : { kind: 'failed-silent', error: fallbackError }; + } +} + +// QNBS-v3: after the chain is exhausted, deliver a registered heuristic result through the stream (onChunk + onDone) instead of erroring — so streaming features (Writer tools) stay useful offline. +function tryHeuristicStream( + mergedOpts: AIRequestOptions, + prompt: string, + guardedCallbacks: AIStreamCallbacks, +): boolean { + const heuristic = applyHeuristicFallback( + mergedOpts.heuristicTask, + mergedOpts.heuristicContext ?? { prompt, reasonKey: 'error.fallback.generic' }, + ); + if (!heuristic) return false; + guardedCallbacks.onChunk(heuristic.data); + guardedCallbacks.onDone?.(); + return true; +} + +// QNBS-v3: the whole fallback-chain walk, extracted so streamText itself stays a shallow coordinator (build guardedCallbacks, resolve chain, delegate) instead of owning every branch of the walk. +async function runProviderFallbackChain( + prompt: string, + creativity: AiCreativity, + mergedOpts: AIRequestOptions, + callbacks: AIStreamCallbacks, + guardedCallbacks: AIStreamCallbacks, + signal: AbortSignal | undefined, +): Promise { + const chain = resolveProviderFallbackChain(mergedOpts); + let lastError: unknown; + // QNBS-v3: tracks an OpenRouter-promoted fallback provider already attempted this call, so the outer loop doesn't invoke it a second time (and double-bill/duplicate chunks) if the chain also lists it later. + let attemptedOpenRouterFallback: string | undefined; + for (let i = 0; i < chain.length; i++) { + const nextProvider = chain[i]; + if (nextProvider === undefined || nextProvider === attemptedOpenRouterFallback) continue; + const outcome = await attemptChainProvider( + prompt, + creativity, + mergedOpts, + nextProvider, + guardedCallbacks, + signal, + ); + if (outcome.kind === 'succeeded') { + // QNBS-v3: mirrors generateText's fallback-reason bookkeeping — without this, a stale reason from an earlier failed/promoted request would keep showing in GpuMetricsPanel after this request's primary provider succeeds outright. + recordProviderSuccess(mergedOpts.provider, nextProvider, i); + return; + } + if (outcome.kind === 'failed-emitted') { + // QNBS-v3: A partial response from any provider must terminate rather than append fallback text to a truncated answer. + const terminal = + outcome.error instanceof Error ? outcome.error : new Error(String(outcome.error)); + callbacks.onError?.(terminal); + throw terminal; + } + const error = outcome.error; + lastError = error; + // QNBS-v3: mirrors generateText's OpenRouter rate-limit/circuit-open promotion — without this, a stream promoted to OpenRouter by resolvePositiveRoutingOpts would fail hard on a transient OpenRouter outage instead of falling back. + if (isOpenRouterTransientFailure(nextProvider, error)) { + const fallback = getOpenRouterFallbackProvider(); + attemptedOpenRouterFallback = fallback; + const promotedOutcome = await attemptPromotedOpenRouterFallback( + mergedOpts, + fallback, + prompt, + creativity, + guardedCallbacks, + signal, + ); + if (promotedOutcome.kind === 'succeeded') { + _lastFallbackReason = `OpenRouter rate-limited; fell back to ${fallback}.`; + return; + } + if (promotedOutcome.kind === 'failed-emitted') { + // QNBS-v3: same partial-response guard applies to the promoted OpenRouter fallback attempt itself. + const terminal = + promotedOutcome.error instanceof Error + ? promotedOutcome.error + : new Error(String(promotedOutcome.error)); + callbacks.onError?.(terminal); + throw terminal; + } + lastError = promotedOutcome.error; + } + } + // QNBS-v3: onError is owned by this orchestration layer — fire it exactly once, after the whole chain is exhausted, so no fallback provider still in flight gets a premature terminal error. + const terminal = lastError instanceof Error ? lastError : new Error(String(lastError)); + if (tryHeuristicStream(mergedOpts, prompt, guardedCallbacks)) return; + callbacks.onError?.(terminal); + throw terminal; +} + export async function streamText( prompt: string, creativity: AiCreativity, @@ -455,90 +613,14 @@ export async function streamText( if (!mergedOpts.signal?.aborted) callbacks.onDone?.(); }, }; - const chain = resolveProviderFallbackChain(mergedOpts); - let lastError: unknown; - // QNBS-v3: tracks an OpenRouter-promoted fallback provider already attempted this call, so the outer loop doesn't invoke it a second time (and double-bill/duplicate chunks) if the chain also lists it later. - let attemptedOpenRouterFallback: string | undefined; - // QNBS-v3: after the chain is exhausted, deliver a registered heuristic result through the stream - // (onChunk + onDone) instead of erroring — so streaming features (Writer tools) stay useful offline. - const tryHeuristicStream = (): boolean => { - const heuristic = applyHeuristicFallback( - mergedOpts.heuristicTask, - mergedOpts.heuristicContext ?? { prompt, reasonKey: 'error.fallback.generic' }, - ); - if (!heuristic) return false; - guardedCallbacks.onChunk(heuristic.data); - guardedCallbacks.onDone?.(); - return true; - }; - for (let i = 0; i < chain.length; i++) { - const nextProvider = chain[i]; - if (nextProvider === undefined || nextProvider === attemptedOpenRouterFallback) continue; - // QNBS-v3: track partial Grok output before fallback decisions. - let grokEmitted = false; - const callbacksForAttempt = - nextProvider === 'grok' - ? createGrokAttemptCallbacks(guardedCallbacks, (text) => { - grokEmitted = true; - guardedCallbacks.onChunk(text); - }) - : guardedCallbacks; - try { - await streamProvider( - prompt, - creativity, - { ...mergedOpts, provider: nextProvider }, - callbacksForAttempt, - signal, - ); - throwIfRequestAborted(undefined, mergedOpts.signal, signal); - // QNBS-v3: mirrors generateText's fallback-reason bookkeeping — without this, a stale reason from an earlier failed/promoted request would keep showing in GpuMetricsPanel after this request's primary provider succeeds outright. - recordProviderSuccess(mergedOpts.provider, nextProvider, i); - return; - } catch (error) { - // QNBS-v3: A user-cancelled request is NOT a provider failure. Don't fall back to the next - // provider and don't fire a terminal onError — surface the cancellation directly so callers - // run their silent cancel flow instead of an error path. - throwIfRequestAborted(error, mergedOpts.signal, signal); - if (nextProvider === 'grok' && grokEmitted) { - // QNBS-v3: A partial Grok response must terminate rather than append fallback text to a truncated answer. - const terminal = error instanceof Error ? error : new Error(String(error)); - callbacks.onError?.(terminal); - throw terminal; - } - lastError = error; - // QNBS-v3: mirrors generateText's OpenRouter rate-limit/circuit-open promotion — without this, a stream promoted to OpenRouter by resolvePositiveRoutingOpts would fail hard on a transient OpenRouter outage instead of falling back. - if (isOpenRouterTransientFailure(nextProvider, error)) { - const fallback = getOpenRouterFallbackProvider(); - attemptedOpenRouterFallback = fallback; - try { - await attemptOpenRouterFallback(mergedOpts, fallback, (fallbackOpts) => - streamProvider(prompt, creativity, fallbackOpts, guardedCallbacks, signal), - ); - throwIfRequestAborted(undefined, mergedOpts.signal, signal); - _lastFallbackReason = `OpenRouter rate-limited; fell back to ${fallback}.`; - return; - } catch (fallbackError) { - // QNBS-v3: mirrors the outer catch's cancellation guard — a cancel during the promoted fallback must not be treated as a provider failure either. - throwIfRequestAborted(fallbackError, mergedOpts.signal, signal); - lastError = fallbackError; - } - } - if (i === chain.length - 1) { - // QNBS-v3: onError is owned by this orchestration layer — fire it exactly once, after - // the whole fallback chain is exhausted, so a failing provider never surfaces a terminal - // error callback while a subsequent fallback provider is still about to succeed. - const terminal = lastError instanceof Error ? lastError : new Error(String(lastError)); - if (tryHeuristicStream()) return; - callbacks.onError?.(terminal); - throw terminal; - } - } - } - const terminal = lastError instanceof Error ? lastError : new Error(String(lastError)); - if (tryHeuristicStream()) return; - callbacks.onError?.(terminal); - throw terminal; + await runProviderFallbackChain( + prompt, + creativity, + mergedOpts, + callbacks, + guardedCallbacks, + signal, + ); }); } diff --git a/tests/unit/aiProviderService.test.ts b/tests/unit/aiProviderService.test.ts index 616551bc6..cc3db1934 100644 --- a/tests/unit/aiProviderService.test.ts +++ b/tests/unit/aiProviderService.test.ts @@ -48,6 +48,11 @@ vi.mock('@tauri-apps/plugin-http', () => ({ })); import { setActiveAiMode, setOpenRouterConfig } from '../../services/ai/aiModeService'; +import { + _clearHeuristicRegistry, + makeHeuristicResult, + registerHeuristicGenerator, +} from '../../services/ai/heuristicFallback'; import { streamOpenAiCompatibleLocal } from '../../services/ai/providers/localOpenAiCompatibleProvider'; import { consumeOpenAiCompatibleStream } from '../../services/ai/providers/openaiProvider'; import * as openrouterProvider from '../../services/ai/providers/openrouterProvider'; @@ -620,6 +625,125 @@ describe('streamText', () => { expect(geminiService.streamText).toHaveBeenCalledTimes(1); }); + // QNBS-v3 (#714): the partial-then-fail guard was previously Grok-only, so a non-Grok primary could emit visible text and then have a full fallback answer appended after it. + describe('partial-then-fail fallback guard applies to every provider (#714)', () => { + afterEach(() => { + _clearHeuristicRegistry(); + }); + + it('does not append fallback text after a non-Grok primary (OpenRouter) emits a partial response', async () => { + vi.mocked(storageService.getApiKey).mockResolvedValueOnce('or-key'); + vi.mocked(openrouterProvider.streamOpenRouter).mockImplementationOnce( + async (_prompt, _opts, callbacks) => { + callbacks.onChunk('partial openrouter text'); + throw new Error('stream ended before completion'); + }, + ); + const onChunk = vi.fn(); + const onError = vi.fn(); + await expect( + streamText( + 'prompt', + 'Balanced', + { + ...defaultOpts, + provider: 'openrouter', + hybridFallbackEnabled: true, + hybridFallbackChain: ['gemini'], + }, + { onChunk, onError }, + ), + ).rejects.toThrow('stream ended before completion'); + expect(geminiService.streamText).not.toHaveBeenCalled(); + expect(onChunk).toHaveBeenCalledTimes(1); + expect(onChunk).toHaveBeenCalledWith('partial openrouter text'); + expect(onError).toHaveBeenCalledTimes(1); + }); + + it('terminates without further fallback when the OpenRouter-promoted fallback attempt itself emits a partial response then fails', async () => { + // QNBS-v3 (cubic): a chain with nothing after the promoted gemini attempt made the guard unobservable, so add a third entry (ollama, left unconfigured) that the fixed code must never reach. + const { streamOllama } = await import('../../services/ollamaService'); + vi.mocked(storageService.getApiKey).mockResolvedValueOnce('or-key'); + vi.mocked(openrouterProvider.streamOpenRouter).mockRejectedValueOnce( + new Error('OPENROUTER_RATE_LIMITED: too many requests'), + ); + vi.mocked(geminiService.streamText).mockImplementationOnce(async (_p, _c, onChunk) => { + onChunk('partial gemini fallback text'); + throw new Error('gemini stream ended before completion'); + }); + const onChunk = vi.fn(); + const onError = vi.fn(); + await expect( + streamText( + 'prompt', + 'Balanced', + { + ...defaultOpts, + provider: 'openrouter', + hybridFallbackEnabled: true, + hybridFallbackChain: ['gemini', 'ollama'], + }, + { onChunk, onError }, + ), + ).rejects.toThrow('gemini stream ended before completion'); + expect(streamOllama).not.toHaveBeenCalled(); + expect(onChunk).toHaveBeenCalledTimes(1); + expect(onChunk).toHaveBeenCalledWith('partial gemini fallback text'); + expect(onError).toHaveBeenCalledTimes(1); + }); + + it('still delivers a registered heuristic result when the chain is exhausted with zero chunks emitted (unchanged behavior)', async () => { + const generator = vi + .fn() + .mockReturnValue( + makeHeuristicResult('heuristic answer', { confidence: 0.5, tier: 'basic' }), + ); + registerHeuristicGenerator('test.streamText.714', generator); + vi.mocked(geminiService.streamText).mockRejectedValueOnce(new Error('Gemini offline')); + const onChunk = vi.fn(); + const onDone = vi.fn(); + const onError = vi.fn(); + await streamText( + 'prompt', + 'Balanced', + { ...defaultOpts, heuristicTask: 'test.streamText.714' }, + { onChunk, onDone, onError }, + ); + expect(generator).toHaveBeenCalledTimes(1); + expect(onChunk).toHaveBeenCalledTimes(1); + expect(onChunk).toHaveBeenCalledWith('heuristic answer'); + expect(onDone).toHaveBeenCalledTimes(1); + expect(onError).not.toHaveBeenCalled(); + }); + + it('does not fall through to the heuristic generator once the failing provider already emitted a partial response', async () => { + const generator = vi + .fn() + .mockReturnValue( + makeHeuristicResult('heuristic answer', { confidence: 0.5, tier: 'basic' }), + ); + registerHeuristicGenerator('test.streamText.714b', generator); + vi.mocked(geminiService.streamText).mockImplementationOnce(async (_p, _c, onChunk) => { + onChunk('partial gemini text'); + throw new Error('Gemini offline mid-stream'); + }); + const onChunk = vi.fn(); + const onError = vi.fn(); + await expect( + streamText( + 'prompt', + 'Balanced', + { ...defaultOpts, heuristicTask: 'test.streamText.714b' }, + { onChunk, onError }, + ), + ).rejects.toThrow('Gemini offline mid-stream'); + expect(generator).not.toHaveBeenCalled(); + expect(onChunk).toHaveBeenCalledTimes(1); + expect(onChunk).toHaveBeenCalledWith('partial gemini text'); + expect(onError).toHaveBeenCalledTimes(1); + }); + }); + // QNBS-v3: Grok regression locks SSE request, delta, and completion contracts. it('uses xAI streamed chat completions and preserves the system prompt', async () => { const originalFetch = globalThis.fetch; @@ -805,7 +929,13 @@ describe('streamText', () => { streamText( 'user prompt', 'Balanced', - { provider: 'grok', model: 'grok-4.5', fallbackProviders: ['gemini'] }, + { + provider: 'grok', + model: 'grok-4.5', + // QNBS-v3: fallbackProviders only applies to local primaries — grok needs hybridFallbackEnabled + hybridFallbackChain, or the chain stays ['grok'] and the assertions below pass vacuously. + hybridFallbackEnabled: true, + hybridFallbackChain: ['gemini'], + }, { onChunk, onError }, ), ).rejects.toThrow('stream ended before completion');