diff --git a/src/main/speech/model-manager-progress-callback.test.ts b/src/main/speech/model-manager-progress-callback.test.ts index 044d8b5936d..5ba6b21927b 100644 --- a/src/main/speech/model-manager-progress-callback.test.ts +++ b/src/main/speech/model-manager-progress-callback.test.ts @@ -69,4 +69,43 @@ describe('ModelManager progress callbacks', () => { 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) + } + + expect(seen.size).toBeLessThanOrEqual(101) + } finally { + rmSync(dir, { recursive: true, force: true }) + } + }) }) diff --git a/src/main/speech/model-manager.ts b/src/main/speech/model-manager.ts index dc175eb11ae..f6aa219b802 100644 --- a/src/main/speech/model-manager.ts +++ b/src/main/speech/model-manager.ts @@ -52,6 +52,13 @@ 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] @@ -217,7 +224,10 @@ export class ModelManager { await this.migrationReady const cached = this.modelStates.get(modelId) if (cached && (cached.status === 'downloading' || cached.status === 'extracting')) { - return cached + // 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) } const manifest = getCatalogModel(modelId) @@ -401,9 +411,12 @@ export class ModelManager { // 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. if ( - previous !== undefined && - previous.status === status && + status === 'downloading' && + previous?.status === 'downloading' && previous.error === error && toWholeProgressPercent(previous.progress) === toWholeProgressPercent(progress) ) { 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.test.tsx similarity index 95% rename from src/renderer/src/components/settings/voice-model-download-progress-storm.react185.test.tsx rename to src/renderer/src/components/settings/voice-model-download-progress-storm.test.tsx index d6c3e99ebb1..280fc289ee9 100644 --- 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.test.tsx @@ -162,7 +162,9 @@ async function runDownloadProgressBurst(events: number): Promise { const rendersBeforeBurst = subscriberRenders for (let index = 0; index < events; index += 1) { - // Whole-percent granularity: what the UI actually renders. + // 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 }) @@ -173,7 +175,7 @@ async function runDownloadProgressBurst(events: number): Promise { return subscriberRenders - rendersBeforeBurst } -describe('speech model download progress storm (React #185)', () => { +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