mirror of
https://github.com/stablyai/orca.git
synced 2026-10-21 00:03:46 +00:00
fix(speech): quantise polled model state so download progress stops churning the renderer
Adversarial round 1 found the renderer-side stabilisation was inert during a real download. The progress fan-out already coalesces to whole percent, but the renderer discards the event payload and re-polls getModelStates, and getModelState returned the cached downloading state verbatim - raw sub-percent progress. So every chunk produced a fresh object, resolveModelStates never matched, and all the real benefit came from the main-side coalescing alone. Quantise the polled reply to the precision the UI already renders (Math.round(progress * 100)), keeping the stored cache exact. Also from round 1: - the whole-status dedup swallowed downloadModel's already-downloaded branch, whose lone 'ready' is the only notification the requesting window ever gets - a dead click and a permanently stale pane with two windows open. Gate it on downloading -> downloading so every one-shot transition stays unconditional. - rename the storm test off '.react185.': it counts renders and asserts nothing about #185, and #185 could not be reproduced at IPC-realistic pacing.
This commit is contained in:
@@ -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<number | undefined>()
|
||||
|
||||
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 })
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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)
|
||||
) {
|
||||
|
||||
+4
-2
@@ -162,7 +162,9 @@ async function runDownloadProgressBurst(events: number): Promise<number> {
|
||||
|
||||
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<number> {
|
||||
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
|
||||
Reference in New Issue
Block a user