From aba936daf714cda4bc4441ab2a6e573096d0d5b1 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 1 Aug 2026 05:22:53 -0700 Subject: [PATCH] fix(speech): coalesce model download progress churn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A model download emits one state update per HTTP chunk (thousands for a 500MB model). Each one costs the renderer an IPC round-trip and a forced re-render of the speech-model menu, which stays open by design while a download runs — the Radix portal/Presence tree all four page.settings React #185 reports crashed inside. Emit at whole-percent granularity (what the UI renders) and keep the modelStates array identity stable when a refresh changed nothing, so a no-op refresh no longer forces a commit. Adds a speech_model_state_churn breadcrumb, registered in both coalescing sets, because no bundle in the cluster carried any speech telemetry to confirm a download was in flight. --- src/main/ipc/crash-reporting.ts | 8 +- .../model-manager-progress-callback.test.ts | 24 +++ src/main/speech/model-manager.ts | 18 ++ ...-download-progress-storm.react185.test.tsx | 204 ++++++++++++++++++ src/renderer/src/store/slices/dictation.ts | 94 ++++++-- 5 files changed, 331 insertions(+), 17 deletions(-) create mode 100644 src/renderer/src/components/settings/voice-model-download-progress-storm.react185.test.tsx diff --git a/src/main/ipc/crash-reporting.ts b/src/main/ipc/crash-reporting.ts index f028c7bc3b3..3d5601ec7ee 100644 --- a/src/main/ipc/crash-reporting.ts +++ b/src/main/ipc/crash-reporting.ts @@ -332,6 +332,7 @@ const COALESCED_RENDERER_BREADCRUMB_NAMES = new Set([ 'renderer_unhandled_rejection', 'terminal_park_verdict_churn', 'terminal_safe_fit_retry_exhausted', + 'speech_model_state_churn', DUPLICATE_TAB_OWNER_BREADCRUMB, TERMINAL_WEBGL_DIAGNOSTIC_BREADCRUMB ]) @@ -345,9 +346,14 @@ const RENDERER_BREADCRUMB_COALESCE_MS = 30_000 // mounted pane within ~60ms. Windows crash F0BKR84AHEH lost 26-90% of its // 30-entry ring to two such bursts. `suppressedSinceLast` keeps the pane count // — the only signal these carry — in one slot. +// +// speech_model_state_churn: a model download that outruns its coalescing fires this +// once per 5s window for as long as the storm lasts. The rate is the signal, so one +// entry plus `suppressedSinceLast` preserves it without evicting the crash itself. const NAME_ONLY_COALESCED_BREADCRUMB_NAMES = new Set([ 'terminal_park_verdict_churn', - 'terminal_safe_fit_retry_exhausted' + 'terminal_safe_fit_retry_exhausted', + 'speech_model_state_churn' ]) function rendererBreadcrumbCoalesceKey( diff --git a/src/main/speech/model-manager-progress-callback.test.ts b/src/main/speech/model-manager-progress-callback.test.ts index 8b566abad70..044d8b5936d 100644 --- a/src/main/speech/model-manager-progress-callback.test.ts +++ b/src/main/speech/model-manager-progress-callback.test.ts @@ -45,4 +45,28 @@ describe('ModelManager progress callbacks', () => { rmSync(dir, { recursive: true, force: true }) } }) + + it('coalesces per-chunk download progress to whole percent', () => { + const dir = mkdtempSync(join(tmpdir(), 'orca-model-manager-')) + try { + const manager = new ModelManager(dir) + const internals = manager as unknown as ModelManagerInternals + const listener = vi.fn() + manager.setProgressCallback(listener) + + // A 500MB model over a 64KB chunk stream reports this many times. + for (let chunk = 0; chunk < 8_000; chunk += 1) { + internals.updateState('model-a', 'downloading', chunk / 8_000) + } + + expect(listener.mock.calls.length).toBeLessThanOrEqual(101) + // Status transitions must still reach the UI unconditionally. + const afterDownload = listener.mock.calls.length + internals.updateState('model-a', 'extracting') + internals.updateState('model-a', 'ready') + expect(listener.mock.calls.length).toBe(afterDownload + 2) + } finally { + rmSync(dir, { recursive: true, force: true }) + } + }) }) diff --git a/src/main/speech/model-manager.ts b/src/main/speech/model-manager.ts index 9125ffa4962..dc175eb11ae 100644 --- a/src/main/speech/model-manager.ts +++ b/src/main/speech/model-manager.ts @@ -47,6 +47,11 @@ type DownloadTotals = { } type ContentRange = { start: number; end: number; totalBytes?: number } +// Why: -1 stands in for "no progress reported" so undefined never equals 0%. +function toWholeProgressPercent(progress: number | undefined): number { + return progress === undefined ? -1 : Math.round(progress * 100) +} + const DOWNLOAD_IDLE_TIMEOUT_MS = 120_000 // Why: flaky networks/proxies often kill long CDN transfers near the end; Range-resume lets them finish. const DOWNLOAD_RETRY_DELAYS_MS = [1_000, 2_000, 4_000] @@ -387,10 +392,23 @@ export class ModelManager { progress?: number, error?: string ): void { + const previous = this.modelStates.get(modelId) const state: SpeechModelState = { id: modelId, status, progress, error } this.modelStates.set(modelId, state) // Why: notify on every state change (not just progress) so extracting/ready/error transitions reach the UI. const progressValue = progress ?? (status === 'extracting' ? 0.95 : -1) + // Why: downloads call this once per HTTP chunk (thousands per model), and each + // notification costs the renderer an IPC round-trip plus a full re-render of the + // open speech-model menu. The UI only ever shows whole percent, so emitting at + // that granularity keeps every visible transition and drops the rest. + if ( + previous !== undefined && + previous.status === status && + previous.error === error && + toWholeProgressPercent(previous.progress) === toWholeProgressPercent(progress) + ) { + return + } for (const callback of this.progressCallbacks) { callback(modelId, progressValue) } diff --git a/src/renderer/src/components/settings/voice-model-download-progress-storm.react185.test.tsx b/src/renderer/src/components/settings/voice-model-download-progress-storm.react185.test.tsx new file mode 100644 index 00000000000..d6c3e99ebb1 --- /dev/null +++ b/src/renderer/src/components/settings/voice-model-download-progress-storm.react185.test.tsx @@ -0,0 +1,204 @@ +/** @vitest-environment happy-dom */ +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { create, type StateCreator } from 'zustand' +import type { SpeechModelManifest, SpeechModelState } from '../../../../shared/speech-types' +import type { GlobalSettings } from '../../../../shared/types' +import { getDefaultVoiceSettings } from '../../../../shared/constants' +import { + createDictationSlice, + SPEECH_MODEL_STATE_CHURN_BREADCRUMB +} from '../../store/slices/dictation' +import type { AppState } from '../../store/types' + +;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + +type DictationTestStore = Pick< + AppState, + 'modelStates' | 'refreshModelStates' | 'setModelStates' | 'dictationState' +> + +// Why: drive the production dictation slice so modelStates identity churn is real, not modelled. +const dictationSlice = createDictationSlice as unknown as StateCreator +const useDictationTestStore = create(dictationSlice) +const initialDictationState = useDictationTestStore.getState() + +vi.mock('@/store', () => ({ + useAppStore: (selector: (state: DictationTestStore) => T): T => + useDictationTestStore(selector) +})) + +vi.mock('@/hooks/useShortcutLabel', () => ({ useShortcutLabel: () => 'Ctrl+Shift+Y' })) +vi.mock('sonner', () => ({ + toast: { error: vi.fn(), message: vi.fn(), success: vi.fn(), info: vi.fn() } +})) + +const MODEL_ID = 'whisper-small' +const CATALOG_ENTRY: SpeechModelManifest = { + id: MODEL_ID, + label: 'Whisper Small', + description: 'Local streaming model', + type: 'whisper', + provider: 'local', + language: 'en', + sizeBytes: 500_000_000, + sampleRate: 16_000, + streaming: true, + recommended: true +} + +let emitDownloadProgress: ((payload: { modelId: string; progress: number }) => void) | null = null +let reportedProgress = 0 + +function installWindowApi(): void { + Object.assign(window, { + api: { + developerPermissions: { request: vi.fn() }, + crashReports: { recordBreadcrumb: vi.fn() }, + speech: { + getCatalog: vi.fn(async () => [{ ...CATALOG_ENTRY }]), + getOpenAiApiKeyStatus: vi.fn(async () => ({ configured: false })), + saveOpenAiApiKey: vi.fn(async () => ({ configured: true })), + clearOpenAiApiKey: vi.fn(async () => ({ configured: false })), + // Why: every IPC reply is a fresh array, exactly like structured clone across the bridge. + getModelStates: vi.fn( + async (): Promise => [ + { id: MODEL_ID, status: 'downloading', progress: reportedProgress } + ] + ), + downloadModel: vi.fn(async () => {}), + deleteModel: vi.fn(async () => {}), + onDownloadProgress: vi.fn( + (callback: (payload: { modelId: string; progress: number }) => void) => { + emitDownloadProgress = callback + return () => { + emitDownloadProgress = null + } + } + ) + } + } + }) +} + +// Why: Radix Popper/DismissableLayer need DOM APIs happy-dom does not implement. +function installDomShims(): void { + vi.stubGlobal( + 'ResizeObserver', + class { + observe(): void {} + unobserve(): void {} + disconnect(): void {} + } + ) + Element.prototype.hasPointerCapture ??= (): boolean => false + Element.prototype.setPointerCapture ??= (): void => {} + Element.prototype.releasePointerCapture ??= (): void => {} + Element.prototype.scrollIntoView ??= (): void => {} +} + +let root: Root + +beforeEach(() => { + reportedProgress = 0 + emitDownloadProgress = null + useDictationTestStore.setState(initialDictationState, true) + installWindowApi() + installDomShims() +}) + +afterEach(() => { + act(() => root?.unmount()) + document.body.replaceChildren() + vi.unstubAllGlobals() +}) + +/** + * Renders the real VoicePane with the speech-model dropdown open — the state a + * download deliberately leaves the menu in — and replays a download-progress + * burst, counting how many times modelStates subscribers are forced to re-render. + */ +async function runDownloadProgressBurst(events: number): Promise { + const { VoicePane } = await import('./VoicePane') + let subscriberRenders = 0 + + function ModelStatesSubscriber(): null { + // Mirrors how VoicePane and the open menu read the store. + useDictationTestStore((state) => state.modelStates) + subscriberRenders += 1 + return null + } + + const settings = { + voice: { ...getDefaultVoiceSettings(), enabled: true, sttModel: '' } + } as GlobalSettings + + const container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + await act(async () => { + root.render( + <> + + + + ) + }) + + const trigger = container.querySelector('button[aria-haspopup="menu"]') + if (!trigger) { + throw new Error('speech model dropdown trigger was not rendered') + } + await act(async () => { + for (const type of ['pointerdown', 'mousedown', 'click']) { + trigger.dispatchEvent(new MouseEvent(type, { bubbles: true, button: 0 })) + } + await Promise.resolve() + }) + if (!document.querySelector('[role="menu"]')) { + throw new Error('speech model dropdown did not open') + } + + const rendersBeforeBurst = subscriberRenders + for (let index = 0; index < events; index += 1) { + // Whole-percent granularity: what the UI actually renders. + reportedProgress = Math.floor((index / events) * 100) / 100 + await act(async () => { + emitDownloadProgress?.({ modelId: MODEL_ID, progress: reportedProgress }) + await Promise.resolve() + await Promise.resolve() + }) + } + return subscriberRenders - rendersBeforeBurst +} + +describe('speech model download progress storm (React #185)', () => { + it('does not re-render the open speech-model menu once per download chunk', async () => { + // A 500MB model over a 64KB chunk stream emits thousands of these; the menu + // stays open by design while a download runs, so each one re-renders the + // whole Radix portal/Presence tree the #185 reports crashed inside. + const EVENTS = 200 + const renders = await runDownloadProgressBurst(EVENTS) + + // Progress is rendered as whole percent, so 200 chunks carry at most 100 + // distinct states — a refresh that changed nothing must not force a commit. + expect(renders).toBeLessThanOrEqual(100) + }) + + it('records the churn breadcrumb when refreshes outrun coalescing', async () => { + // Own store instance: the churn window is per-slice state the other tests advance. + const churnStore = create(dictationSlice) + for (let refresh = 0; refresh < 60; refresh += 1) { + await churnStore.getState().refreshModelStates() + } + + const recordBreadcrumb = window.api.crashReports.recordBreadcrumb as ReturnType + const churn = recordBreadcrumb.mock.calls + .map((call) => call[0] as { name: string; data?: Record }) + .find((entry) => entry.name === SPEECH_MODEL_STATE_CHURN_BREADCRUMB) + expect(churn?.data).toMatchObject({ refreshes: 60, noOpRefreshes: 59 }) + // Registered in COALESCED/NAME_ONLY sets in src/main/ipc/crash-reporting.ts. + expect(SPEECH_MODEL_STATE_CHURN_BREADCRUMB).toBe('speech_model_state_churn') + }) +}) diff --git a/src/renderer/src/store/slices/dictation.ts b/src/renderer/src/store/slices/dictation.ts index 7b89cb69e35..da3bed90902 100644 --- a/src/renderer/src/store/slices/dictation.ts +++ b/src/renderer/src/store/slices/dictation.ts @@ -1,6 +1,14 @@ import type { StateCreator } from 'zustand' import type { AppState } from '../types' import type { DictationState, SpeechModelState } from '../../../../shared/speech-types' +import { recordRendererCrashBreadcrumb } from '../../lib/crash-breadcrumb-recorder' + +export const SPEECH_MODEL_STATE_CHURN_BREADCRUMB = 'speech_model_state_churn' +// Why: a whole-percent download emits ~100 refreshes total; anything past this in +// one window means the coalescing upstream stopped holding and the renderer is +// being driven per HTTP chunk again — the shape that starves React's commit loop. +const CHURN_WINDOW_MS = 5_000 +const CHURN_REFRESH_THRESHOLD = 60 export type DictationSlice = { dictationState: DictationState @@ -14,23 +22,77 @@ export type DictationSlice = { refreshModelStates: () => Promise } -export const createDictationSlice: StateCreator = (set) => ({ - dictationState: 'idle', - partialTranscript: '', - activeModelId: null, - modelStates: [], +function sameSpeechModelState(a: SpeechModelState, b: SpeechModelState): boolean { + return a.id === b.id && a.status === b.status && a.progress === b.progress && a.error === b.error +} - setDictationState: (state) => set({ dictationState: state }), - setPartialTranscript: (text) => set({ partialTranscript: text }), - setActiveModelId: (id) => set({ activeModelId: id }), - setModelStates: (states) => set({ modelStates: states }), +// Why: every getModelStates reply is a fresh array, so without this each no-op +// refresh re-renders every subscriber — including the open speech-model menu. +function resolveModelStates( + previous: SpeechModelState[], + next: SpeechModelState[] +): SpeechModelState[] { + if (previous.length !== next.length) { + return next + } + return previous.every((state, index) => sameSpeechModelState(state, next[index])) + ? previous + : next +} - refreshModelStates: async () => { - try { - const states = await window.api.speech.getModelStates() - set({ modelStates: states }) - } catch (err) { - console.error('Failed to fetch model states:', err) +export const createDictationSlice: StateCreator = (set) => { + let windowStartedAt = 0 + let refreshes = 0 + let noOpRefreshes = 0 + + // Why: all four page.settings React #185 reports crashed inside the open speech-model + // menu with no speech telemetry in the bundle. Record the refresh rate so the next + // occurrence says whether model-state churn was driving the commit storm. + const noteRefresh = (changed: boolean): void => { + const now = Date.now() + if (now - windowStartedAt > CHURN_WINDOW_MS) { + windowStartedAt = now + refreshes = 0 + noOpRefreshes = 0 + } + refreshes += 1 + if (!changed) { + noOpRefreshes += 1 + } + if (refreshes === CHURN_REFRESH_THRESHOLD) { + recordRendererCrashBreadcrumb(SPEECH_MODEL_STATE_CHURN_BREADCRUMB, { + refreshes, + noOpRefreshes, + windowMs: now - windowStartedAt + }) } } -}) + + return { + dictationState: 'idle', + partialTranscript: '', + activeModelId: null, + modelStates: [], + + setDictationState: (state) => set({ dictationState: state }), + setPartialTranscript: (text) => set({ partialTranscript: text }), + setActiveModelId: (id) => set({ activeModelId: id }), + setModelStates: (states) => + set((prev) => ({ modelStates: resolveModelStates(prev.modelStates, states) })), + + refreshModelStates: async () => { + try { + const states = await window.api.speech.getModelStates() + let changed = false + set((prev) => { + const resolved = resolveModelStates(prev.modelStates, states) + changed = resolved !== prev.modelStates + return { modelStates: resolved } + }) + noteRefresh(changed) + } catch (err) { + console.error('Failed to fetch model states:', err) + } + } + } +}