diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index 7bbdff6d..3a20ad14 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -928,36 +928,51 @@ Object.assign(CodemanApp.prototype, { /** * Polls llama-swap's own `/running` (via the read-only running-status route) until - * `modelId` reports `state: 'ready'`, showing a sticky toast the whole time so a slow + * `modelId` reports `state: 'ready'`, showing a sticky banner the whole time so a slow * unload/reload (measured well over a minute for a large model) reads as "loading", - * never as silence or a wrong answer from whatever was loaded before. Bounded at 2 - * minutes; still not ready by then gets a toast saying so rather than polling forever. + * never as silence or a wrong answer from whatever was loaded before. Checks immediately + * (a fast load, or a re-apply onto an already-ready model, shouldn't wait a full interval + * to say so), then every `pollIntervalMs`. Bounded at `maxWaitMs`; still not ready by then + * gets a toast saying so rather than polling forever — 5 minutes by default, since a large + * (20GB+) model reading from disk can genuinely take longer than the 2 minutes this used + * to allow. + * + * `_watchLlamaSwapGeneration` guards against two overlapping calls (a second launch + * started before the first one's loop finished) clobbering each other's banner: + * `_showCenterStatus` reuses one shared DOM node, so an older loop's `dismiss()`/message + * update firing after a newer one has already taken over the banner would otherwise hide + * or overwrite the WRONG one. Each call claims the counter as its own "generation" and + * checks it still owns it before touching the banner. * * `pollIntervalMs`/`maxWaitMs` exist to let a test drive this in milliseconds instead of * minutes — real callers never pass them, which is what keeps the defaults live here * rather than only in a test fixture. */ - async _watchLlamaSwapLoading(endpointId, modelId, pollIntervalMs = 3000, maxWaitMs = 120000) { + async _watchLlamaSwapLoading(endpointId, modelId, pollIntervalMs = 1000, maxWaitMs = 300000) { + const generation = (this._watchLlamaSwapGeneration = (this._watchLlamaSwapGeneration || 0) + 1); + const isCurrent = () => this._watchLlamaSwapGeneration === generation; // Prominent and screen-centred, not a corner toast — a real llama-swap model load can // sit on screen for well over a minute, easy to mistake for nothing happening there. const toast = this._showCenterStatus(`Loading ${modelId} on ${endpointId}… this can take a while`); const deadline = Date.now() + maxWaitMs; while (Date.now() < deadline) { - await new Promise((resolve) => setTimeout(resolve, pollIntervalMs)); const status = await this._apiJson(`/api/model-endpoints/${encodeURIComponent(endpointId)}/running-status`); - if (!status) continue; // transient failure — keep waiting rather than giving up early - if (!status.isLlamaSwap) { + if (!isCurrent()) return; // a newer launch took over the banner — this loop is done + if (!status) { + // transient failure — keep waiting rather than giving up early + } else if (!status.isLlamaSwap) { // Endpoint changed under us, or wasn't llama-swap after all — nothing more to // watch for, and not a failure worth a toast of its own. toast?.dismiss(); return; - } - if (status.running.some((r) => r.model === modelId && r.state === 'ready')) { + } else if (status.running.some((r) => r.model === modelId && r.state === 'ready')) { toast?.dismiss(); this.showToast(`${modelId} is ready`, 'success', { duration: 2500 }); return; } + await new Promise((resolve) => setTimeout(resolve, pollIntervalMs)); } + if (!isCurrent()) return; toast?.dismiss(); this.showToast(`Still waiting for ${modelId} to finish loading on ${endpointId} — check the llama-swap server`, 'warning'); }, diff --git a/test/custom-model-run-menu-ui.test.ts b/test/custom-model-run-menu-ui.test.ts index f1271c58..9fb0e496 100644 --- a/test/custom-model-run-menu-ui.test.ts +++ b/test/custom-model-run-menu-ui.test.ts @@ -676,6 +676,64 @@ describe('Custom Model Endpoint Profiles: _watchLlamaSwapLoading polling', () => expect(toastCalls.at(-1)).toMatch(/ready/i); }); + + it('checks immediately rather than waiting a full interval before the first check', async () => { + // A model that is already ready by the time this runs (a fast load, or a re-apply + // onto one that was already loaded) shouldn't sit on "Loading..." for a whole + // pollIntervalMs before saying so. + const { app } = bootApp({}); + app._showCenterStatus = () => ({ dismiss: () => {}, setMessage: () => {} }); + let calls = 0; + app._apiJson = async () => { + calls += 1; + return { isLlamaSwap: true, running: [{ model: 'qwen3', state: 'ready' }] }; + }; + + // A huge interval that would time the test out if the function actually waited for + // it before the first check. + await app._watchLlamaSwapLoading('llama-box', 'qwen3', 60000, 300000); + + expect(calls).toBe(1); + }); + + it('a newer call takes over the shared banner — the older one neither dismisses nor overwrites it', async () => { + const { app } = bootApp({}); + const bannerCalls: string[] = []; + const dismissCalls: string[] = []; + app._showCenterStatus = (message: string) => { + bannerCalls.push(message); + return { dismiss: () => dismissCalls.push(message), setMessage: () => {} }; + }; + app.showToast = () => {}; + // The FIRST call never sees its target model ready, so it would otherwise run all + // the way to its own timeout and dismiss/warn — but a second call starts first. + let firstResolveApiJson: (() => void) | undefined; + const firstNeverReady = new Promise((resolve) => { + firstResolveApiJson = resolve; + }); + app._apiJson = async () => { + await firstNeverReady; // block the first loop's very first check indefinitely + return { isLlamaSwap: true, running: [] }; + }; + const firstCall = app._watchLlamaSwapLoading('llama-box', 'model-a', 5, 50); + + // Second call, for a DIFFERENT model, starts while the first is still blocked on its + // very first status check — claims the banner as the newer generation. + app._apiJson = async () => ({ isLlamaSwap: true, running: [{ model: 'model-b', state: 'ready' }] }); + await app._watchLlamaSwapLoading('llama-box', 'model-b', 5, 200); + + // Now let the first call's blocked check resolve and run to completion. + firstResolveApiJson?.(); + await firstCall; + + expect(bannerCalls).toEqual([ + 'Loading model-a on llama-box… this can take a while', + 'Loading model-b on llama-box… this can take a while', + ]); + // Only the CURRENT (second) call's own dismiss ever ran — the stale first call's + // late resolution recognised it no longer owns the banner and touched nothing. + expect(dismissCalls).toEqual([bannerCalls[1]]); + }); }); describe('Custom Model Endpoint Profiles: _confirmModelSwap (in-app modal, replaces a native confirm() popup)', () => {