From a18fcb1820e7fb693a3e6c69e970bdbd63a204b3 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 2 Aug 2026 01:04:03 -0700 Subject: [PATCH] refactor(speech): narrow progress churn fix --- ...ash-reporting-renderer-breadcrumbs.test.ts | 21 -- src/main/ipc/crash-reporting.ts | 8 +- .../model-manager-progress-callback.test.ts | 50 +---- src/main/speech/model-manager.ts | 39 +--- ...ice-model-download-progress-storm.test.tsx | 212 ------------------ ...ictation-model-state-stabilisation.test.ts | 189 +++------------- src/renderer/src/store/slices/dictation.ts | 52 +---- 7 files changed, 55 insertions(+), 516 deletions(-) delete mode 100644 src/renderer/src/components/settings/voice-model-download-progress-storm.test.tsx diff --git a/src/main/ipc/crash-reporting-renderer-breadcrumbs.test.ts b/src/main/ipc/crash-reporting-renderer-breadcrumbs.test.ts index 76e59cca6fc..1fb1bb12d7b 100644 --- a/src/main/ipc/crash-reporting-renderer-breadcrumbs.test.ts +++ b/src/main/ipc/crash-reporting-renderer-breadcrumbs.test.ts @@ -275,27 +275,6 @@ describe('renderer breadcrumb IPC routing', () => { } }) - // Why: each window emits its own churn notice with a different payload, so - // coalescing by name is what keeps a download storm to one ring slot. Without - // the registration these carry no message and fall through uncoalesced — - // evicting the pre-crash trail this breadcrumb exists to sit beside. - it('coalesces speech model-state churn by name across windows', () => { - emitRendererBreadcrumb({ - name: 'speech_model_state_churn', - data: { refreshes: 250, noOpRefreshes: 249, windowMs: 4200 } - }) - emitRendererBreadcrumb({ - name: 'speech_model_state_churn', - data: { refreshes: 250, noOpRefreshes: 12, windowMs: 3100 } - }) - - expect(recordCrashBreadcrumbMock).not.toHaveBeenCalled() - expect(recordCoalescedCrashBreadcrumbMock).toHaveBeenCalledTimes(2) - for (const call of recordCoalescedCrashBreadcrumbMock.mock.calls) { - expect(call[0]).toMatchObject({ coalesceKey: 'speech_model_state_churn' }) - } - }) - // Why kind-scoped: a routine post-wake atlas reset must never suppress the // context-loss crumb that says the driver gave up on this renderer. it('coalesces WebGL diagnostics per kind so one kind cannot mask another', () => { diff --git a/src/main/ipc/crash-reporting.ts b/src/main/ipc/crash-reporting.ts index 3d5601ec7ee..f028c7bc3b3 100644 --- a/src/main/ipc/crash-reporting.ts +++ b/src/main/ipc/crash-reporting.ts @@ -332,7 +332,6 @@ 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 ]) @@ -346,14 +345,9 @@ 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', - 'speech_model_state_churn' + 'terminal_safe_fit_retry_exhausted' ]) 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 0a946906e46..1c8633188e7 100644 --- a/src/main/speech/model-manager-progress-callback.test.ts +++ b/src/main/speech/model-manager-progress-callback.test.ts @@ -59,58 +59,14 @@ describe('ModelManager progress callbacks', () => { internals.updateState('model-a', 'downloading', chunk / 8_000) } - // Exact, not a ceiling: a bound alone passes when coalescing swallows every - // step after the first and the progress bar sits at 0% for the whole download. - expect(listener.mock.calls.map(([, progress]) => Math.round(progress * 100))).toEqual( - Array.from({ length: 101 }, (_unused, percent) => percent) + expect(listener.mock.calls.map(([, progress]) => progress)).toEqual( + Array.from({ length: 101 }, (_unused, percent) => percent / 100) ) - // 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 }) - } - }) - - it('re-emits a repeated ready so a stale pane still resyncs', () => { - 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) - - // downloadModel's already-downloaded branch: this lone 'ready' is the only - // notification the requesting window receives before the handler returns. internals.updateState('model-a', 'ready') - internals.updateState('model-a', 'ready') - - expect(listener).toHaveBeenCalledTimes(2) - } finally { - rmSync(dir, { recursive: true, force: true }) - } - }) - - // Why this matters: the renderer discards the progress event payload and re-polls - // getModelState, so coalescing only the fan-out leaves the polled path raw. - it('reports whole-percent progress to pollers during a download', async () => { - const dir = mkdtempSync(join(tmpdir(), 'orca-model-manager-')) - try { - const manager = new ModelManager(dir) - const internals = manager as unknown as ModelManagerInternals - const seen = new Set() - - for (let chunk = 0; chunk < 8000; chunk += 1) { - internals.updateState('whisper-tiny', 'downloading', chunk / 8000) - seen.add((await manager.getModelState('whisper-tiny')).progress) - } - - // Exact, not a ceiling: a bound alone passes when every poll reports 0%. - expect([...seen].sort((a, b) => (a ?? 0) - (b ?? 0))).toEqual( - Array.from({ length: 101 }, (_unused, percent) => percent / 100) - ) + expect(listener.mock.calls.length).toBe(afterDownload + 3) } finally { rmSync(dir, { recursive: true, force: true }) } diff --git a/src/main/speech/model-manager.ts b/src/main/speech/model-manager.ts index f6aa219b802..f183805cddd 100644 --- a/src/main/speech/model-manager.ts +++ b/src/main/speech/model-manager.ts @@ -47,18 +47,6 @@ 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) -} - -/** The precision the UI actually renders, so polled replies match the fan-out. */ -function toWholePercentState(state: SpeechModelState): SpeechModelState { - return state.progress === undefined - ? state - : { ...state, progress: toWholeProgressPercent(state.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] @@ -224,10 +212,7 @@ export class ModelManager { await this.migrationReady const cached = this.modelStates.get(modelId) if (cached && (cached.status === 'downloading' || cached.status === 'extracting')) { - // Why quantise here too: the renderer ignores the progress event payload and - // re-polls, so returning the raw sub-percent cache would hand it a new value - // per chunk and defeat the whole-percent coalescing the fan-out already does. - return toWholePercentState(cached) + return cached } const manifest = getCatalogModel(modelId) @@ -403,25 +388,23 @@ export class ModelManager { 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. - // Why only 'downloading': every other transition is one-shot, and the - // already-ready branch of downloadModel is the sole notification that window - // gets — deduplicating it strands a stale pane on a dead click. + // Whole-percent state matches the UI and prevents chunk-level IPC/poll churn. + const reportedProgress = + status === 'downloading' && progress !== undefined + ? Math.round(progress * 100) / 100 + : progress if ( status === 'downloading' && previous?.status === 'downloading' && previous.error === error && - toWholeProgressPercent(previous.progress) === toWholeProgressPercent(progress) + previous.progress === reportedProgress ) { return } + const state: SpeechModelState = { id: modelId, status, progress: reportedProgress, error } + this.modelStates.set(modelId, state) + // Repeated non-download states can be the requesting window's only resync signal. + const progressValue = reportedProgress ?? (status === 'extracting' ? 0.95 : -1) for (const callback of this.progressCallbacks) { callback(modelId, progressValue) } diff --git a/src/renderer/src/components/settings/voice-model-download-progress-storm.test.tsx b/src/renderer/src/components/settings/voice-model-download-progress-storm.test.tsx deleted file mode 100644 index a482ca54e2b..00000000000 --- a/src/renderer/src/components/settings/voice-model-download-progress-storm.test.tsx +++ /dev/null @@ -1,212 +0,0 @@ -/** @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() - vi.restoreAllMocks() -}) - -/** - * 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) { - // Mirrors what main now returns: getModelState quantises the polled reply, so - // raw sub-percent progress never reaches the renderer. Without that, this mock - // would flatter the renderer-side stabilisation it is meant to measure. - 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 (render pressure)', () => { - 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) - - // 200 chunks carry 100 distinct whole percents and the mount refresh already - // applied the first, so exactly 99 of these are real work. Asserting the floor - // too: over-holding freezes the progress bar, which a ceiling alone calls a pass. - expect(renders).toBe(99) - expect(useDictationTestStore.getState().modelStates[0]?.progress).toBe(reportedProgress) - }) - - it('records the churn breadcrumb when refreshes outrun coalescing', async () => { - // Why pin Date.now: the threshold is per 5s window, so a loaded machine that - // takes longer than one window to run the loop would measure itself, not the rule. - vi.spyOn(Date, 'now').mockReturnValue(1_000_000) - // Own store instance: the churn window is per-slice state the other tests advance. - const churnStore = create(dictationSlice) - for (let refresh = 0; refresh < 250; 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: 250, noOpRefreshes: 249 }) - // 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-model-state-stabilisation.test.ts b/src/renderer/src/store/slices/dictation-model-state-stabilisation.test.ts index c0a8d07bd52..3f8ad8eb59f 100644 --- a/src/renderer/src/store/slices/dictation-model-state-stabilisation.test.ts +++ b/src/renderer/src/store/slices/dictation-model-state-stabilisation.test.ts @@ -1,185 +1,62 @@ /** @vitest-environment happy-dom */ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { create, type StateCreator, type StoreApi, type UseBoundStore } from 'zustand' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { create, type StateCreator } from 'zustand' import type { SpeechModelState } from '../../../../shared/speech-types' import type { AppState } from '../types' -import { createDictationSlice, SPEECH_MODEL_STATE_CHURN_BREADCRUMB } from './dictation' +import { createDictationSlice } from './dictation' type DictationTestStore = Pick const dictationSlice = createDictationSlice as unknown as StateCreator -let reply: SpeechModelState[] = [] -let recordBreadcrumb: ReturnType - -// Mirrors CHURN_WINDOW_MS in ./dictation; enough to roll the window. -const PAST_CHURN_WINDOW_MS = 5_001 - -// Why a fresh store per test: the churn window is per-slice closure state. -function newStore(): UseBoundStore> { - return create(dictationSlice) -} - -// Why pin Date.now: the churn rule is "per window", so a test that lets real time -// pass measures the machine, not the rule. -function pinClock(): { advance: (ms: number) => void } { - let now = 1_000_000 - vi.spyOn(Date, 'now').mockImplementation(() => now) - return { - advance: (ms) => { - now += ms - } - } -} - -function churnBreadcrumbs(): { name: string; data?: Record }[] { - return recordBreadcrumb.mock.calls - .map((call) => call[0] as { name: string; data?: Record }) - .filter((entry) => entry.name === SPEECH_MODEL_STATE_CHURN_BREADCRUMB) -} +let reply: SpeechModelState[] beforeEach(() => { - recordBreadcrumb = vi.fn() + reply = [] Object.assign(window, { api: { - crashReports: { recordBreadcrumb }, - // Why a fresh clone per call: structured clone across the bridge never - // preserves identity, which is the condition the stabilisation exists for. speech: { getModelStates: vi.fn(async () => reply.map((state) => ({ ...state }))) } } }) }) -afterEach(() => { - vi.restoreAllMocks() -}) - describe('dictation model-state stabilisation', () => { - it('keeps the previous array when every field matches', async () => { + it('does not publish an unchanged reply', async () => { reply = [{ id: 'whisper-tiny', status: 'downloading', progress: 0.42 }] - const store = newStore() + const store = create(dictationSlice) await store.getState().refreshModelStates() - const first = store.getState().modelStates + const previous = store.getState() + const subscriber = vi.fn() + store.subscribe(subscriber) + await store.getState().refreshModelStates() - expect(store.getState().modelStates).toBe(first) + expect(store.getState()).toBe(previous) + expect(subscriber).not.toHaveBeenCalled() }) - // Guards the failure mode the stabilisation itself creates: over-holding leaves the - // Voice pane frozen on a stale download, and no render-pressure ceiling catches it. it.each([ - ['progress', { id: 'whisper-tiny', status: 'downloading', progress: 0.43 }], - ['status', { id: 'whisper-tiny', status: 'ready', progress: 0.42 }], - ['error', { id: 'whisper-tiny', status: 'downloading', progress: 0.42, error: 'boom' }], - ['id', { id: 'parakeet-tdt-0.6b-v3-int8', status: 'downloading', progress: 0.42 }] - ] as [string, SpeechModelState][])('adopts a reply whose %s changed', async (_, changed) => { - reply = [{ id: 'whisper-tiny', status: 'downloading', progress: 0.42 }] - const store = newStore() - await store.getState().refreshModelStates() - const first = store.getState().modelStates + { changed: [{ id: 'whisper-tiny', status: 'downloading', progress: 0.43 }] }, + { changed: [{ id: 'whisper-tiny', status: 'ready' }] }, + { changed: [{ id: 'whisper-tiny', status: 'error', error: 'boom' }] }, + { + changed: [{ id: 'parakeet-tdt-0.6b-v3-int8', status: 'downloading', progress: 0.42 }] + }, + { + changed: [ + { id: 'whisper-tiny', status: 'downloading', progress: 0.42 }, + { id: 'parakeet-tdt-0.6b-v3-int8', status: 'not-downloaded' } + ] + } + ] as { changed: SpeechModelState[] }[])('publishes a changed reply', async ({ changed }) => { + const store = create(dictationSlice) + store.getState().setModelStates([{ id: 'whisper-tiny', status: 'downloading', progress: 0.42 }]) + const subscriber = vi.fn() + store.subscribe(subscriber) - reply = [changed] + reply = changed await store.getState().refreshModelStates() - expect(store.getState().modelStates).not.toBe(first) - expect(store.getState().modelStates).toEqual([changed]) - }) - - it('adopts a reply that added a model', async () => { - reply = [{ id: 'whisper-tiny', status: 'ready' }] - const store = newStore() - await store.getState().refreshModelStates() - - reply = [ - { id: 'whisper-tiny', status: 'ready' }, - { id: 'parakeet-tdt-0.6b-v3-int8', status: 'not-downloaded' } - ] - await store.getState().refreshModelStates() - - expect(store.getState().modelStates).toHaveLength(2) - }) - - it('applies every step of a whole-percent download', async () => { - const store = newStore() - const rendered: (number | undefined)[] = [] - for (let percent = 0; percent <= 90; percent += 1) { - reply = [{ id: 'whisper-tiny', status: 'downloading', progress: percent / 100 }] - await store.getState().refreshModelStates() - rendered.push(store.getState().modelStates[0]?.progress) - } - - expect(rendered).toEqual(Array.from({ length: 91 }, (_unused, percent) => percent / 100)) - }) - - // Main clamps progress at 0.9 and emits per whole percent, so a fast 56MB download - // puts its whole ~91-refresh run in one window. Firing there would be a false alarm - // and would evict real evidence from the 30-entry breadcrumb ring. - it('does not report churn for one healthy download inside a single window', async () => { - const store = newStore() - for (let percent = 0; percent <= 90; percent += 1) { - reply = [{ id: 'whisper-tiny', status: 'downloading', progress: percent / 100 }] - await store.getState().refreshModelStates() - } - - expect(recordBreadcrumb).not.toHaveBeenCalled() - }) - - it('reports churn once the renderer is driven far past whole-percent pacing', async () => { - pinClock() - const store = newStore() - reply = [{ id: 'whisper-tiny', status: 'downloading', progress: 0.42 }] - for (let refresh = 0; refresh < 250; refresh += 1) { - await store.getState().refreshModelStates() - } - - const churn = churnBreadcrumbs() - expect(churn).toHaveLength(1) - expect(churn[0].data).toMatchObject({ refreshes: 250, noOpRefreshes: 249 }) - }) - - // Why spread across the window: a burst-only test passes for any window length, - // including one too short for a real storm to ever accumulate the threshold. The - // window has to be wide enough to hold a sustained storm, not just an instant one. - it('still reports a storm that is spread across most of one window', async () => { - const clock = pinClock() - const store = newStore() - reply = [{ id: 'whisper-tiny', status: 'downloading', progress: 0.42 }] - for (let refresh = 0; refresh < 250; refresh += 1) { - await store.getState().refreshModelStates() - clock.advance(19) - } - - const churn = churnBreadcrumbs() - expect(churn).toHaveLength(1) - expect(churn[0].data).toMatchObject({ refreshes: 250, windowMs: 249 * 19 }) - }) - - // Stopping at exactly the threshold cannot tell "fires once" from "fires on every - // refresh after", and the second floods the 30-entry ring this breadcrumb is - // coalesced into to protect. - it('records the churn breadcrumb at most once per window', async () => { - pinClock() - const store = newStore() - reply = [{ id: 'whisper-tiny', status: 'downloading', progress: 0.42 }] - for (let refresh = 0; refresh < 400; refresh += 1) { - await store.getState().refreshModelStates() - } - - expect(churnBreadcrumbs()).toHaveLength(1) - }) - - // The threshold is a rate, not a lifetime total. Without a per-window reset three - // healthy downloads (3 x 91) cross 250 and the breadcrumb cries wolf again. - it('counts per window, so healthy downloads never accumulate into a false alarm', async () => { - const clock = pinClock() - const store = newStore() - reply = [{ id: 'whisper-tiny', status: 'downloading', progress: 0.42 }] - for (let download = 0; download < 4; download += 1) { - for (let refresh = 0; refresh <= 90; refresh += 1) { - await store.getState().refreshModelStates() - } - clock.advance(PAST_CHURN_WINDOW_MS) - } - - expect(churnBreadcrumbs()).toEqual([]) + expect(store.getState().modelStates).toEqual(changed) + expect(subscriber).toHaveBeenCalledTimes(1) }) }) diff --git a/src/renderer/src/store/slices/dictation.ts b/src/renderer/src/store/slices/dictation.ts index 25bdf351490..e5cd28914c5 100644 --- a/src/renderer/src/store/slices/dictation.ts +++ b/src/renderer/src/store/slices/dictation.ts @@ -1,16 +1,6 @@ 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' -const CHURN_WINDOW_MS = 5_000 -// Why this high: main clamps download progress at 0.9 and emits on whole-percent -// change, so one healthy download is capped at ~91 refreshes and a 56MB model on a -// fast link lands all of them inside a single window. Anything past 50/s is the -// renderer being driven per HTTP chunk again — the shape that starves React's -// commit loop. `noOpRefreshes` in the payload separates the two causes. -const CHURN_REFRESH_THRESHOLD = 250 export type DictationSlice = { dictationState: DictationState @@ -43,31 +33,11 @@ function resolveModelStates( } 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 - }) - } + const setModelStates = (states: SpeechModelState[]): void => { + set((prev) => { + const modelStates = resolveModelStates(prev.modelStates, states) + return modelStates === prev.modelStates ? prev : { modelStates } + }) } return { @@ -79,19 +49,11 @@ export const createDictationSlice: StateCreator set({ dictationState: state }), setPartialTranscript: (text) => set({ partialTranscript: text }), setActiveModelId: (id) => set({ activeModelId: id }), - setModelStates: (states) => - set((prev) => ({ modelStates: resolveModelStates(prev.modelStates, states) })), + setModelStates, 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) + setModelStates(await window.api.speech.getModelStates()) } catch (err) { console.error('Failed to fetch model states:', err) }