diff --git a/docs/custom-model-endpoints.md b/docs/custom-model-endpoints.md index 9d7997b5..b608fa2e 100644 --- a/docs/custom-model-endpoints.md +++ b/docs/custom-model-endpoints.md @@ -97,6 +97,16 @@ against any real endpoint's actual hardware/storage) and to scale that same banner's own give-up timeout for a very large model; never anything a server-side check relies on. +**The loading banner shows a live countdown against that same timeout, and +treats a real timeout as a failure, not a shrug.** It checks llama-swap's +own `/running` every second (`GET /api/model-endpoints/:id/running-status`) +and counts down against the size-scaled (or flat 5-minute) timeout live; if +the countdown reaches zero with the target model still not ready, the +banner turns into a sticky error naming the llama-swap server's own logs as +where to look, and the session the load was for is closed automatically — +a console left open and pointed at a model that never finished loading is +worse than no console at all. + `defaultModelId` names which discovered model the picker pre-marks for that endpoint — the settings panel's Edit form exposes it as a select populated from the endpoint's own discovered `models`, and the route refuses a value diff --git a/docs/wiki/Custom-Model-Endpoints.md b/docs/wiki/Custom-Model-Endpoints.md index 96eb69d3..a6eed35f 100644 --- a/docs/wiki/Custom-Model-Endpoints.md +++ b/docs/wiki/Custom-Model-Endpoints.md @@ -78,10 +78,16 @@ waiting on your first prompt to do it.** llama-swap has no "switch model" button — the only thing that starts a swap is a real request naming the model, and confirmed live: just applying a selection never reached llama-swap's own logs at all until something asked it to load. Picking an entry now also sends the smallest real request that will trigger -that load, in the background, the moment the target model isn't already loaded and ready — -which is what the prominent **"Loading ``… this can take a while"** banner -(centred on screen, not a corner toast — a real load can take well over a minute) is -actually watching for. +that load, in the background, the moment the target model isn't already loaded and ready. + +**The centred loading banner shows a live countdown, and a real timeout is an error, not a +shrug.** When it knows the model's discovered file size (its GB figure, when llama-swap +states one), it shows both a rough expected-time estimate and a live countdown against it — +e.g. "Loading qwen3.8-27b (16.4 GB, typically ~1–3 min) on llama-swap — 47s remaining". If +the countdown reaches zero and the model still isn't ready, the banner turns into a sticky +error telling you to check the llama-swap server's own logs, and **the session that load was +for is closed automatically** — a console left open and pointed at a model that never +finished loading would just be confusing to leave sitting there. **Claude Code specifically gets two extra fixes applied automatically:** diff --git a/src/web/public/panels-ui.js b/src/web/public/panels-ui.js index eda7557d..5fc16d21 100644 --- a/src/web/public/panels-ui.js +++ b/src/web/public/panels-ui.js @@ -5556,37 +5556,59 @@ Object.assign(CodemanApp.prototype, { * genuinely worth interrupting the eye for rather than living in the corner with every * other toast (currently: a custom-model session's "switching backends" and "loading * model" states, both of which can sit on screen for well over a minute and are easy to - * mistake for nothing happening). Non-blocking (`pointer-events: none`, no backdrop) — - * this is informational, never a gate the user has to dismiss to keep working. Only one - * is ever shown at a time (the DOM node is created once and reused), which matches every - * current caller: each hands off to the next rather than stacking. + * mistake for nothing happening). Non-blocking (`pointer-events: none` on the wrapper, + * restored only on the card) — an info banner is never a gate the user has to dismiss to + * keep working. Only one is ever shown at a time (the DOM node is created once and + * reused), which matches every current caller: each hands off to the next rather than + * stacking. + * + * `opts.type` — `'info'` (default, spinner, no close button — a caller ends it itself via + * `dismiss()`) or `'error'` (no spinner — nothing is in progress once this shows — with a + * close button, since a sticky error the user cannot dismiss would just sit there). The + * DOM is rebuilt fresh each call rather than patched, since which children exist differs + * by type; `setMessage` still only ever touches the text node afterwards. */ - _showCenterStatus(message) { + _showCenterStatus(message, opts = {}) { + const { type = 'info' } = opts; let el = document.getElementById('customModelCenterStatus'); if (!el) { el = document.createElement('div'); el.id = 'customModelCenterStatus'; - el.className = 'center-status-banner'; + document.body.appendChild(el); + } + el.className = `center-status-banner center-status-${type}`; + el.innerHTML = ''; + const dismiss = () => { + el.classList.remove('show'); + setTimeout(() => { + el.hidden = true; + }, 200); + }; + if (type !== 'error') { const spinner = document.createElement('span'); spinner.className = 'center-status-spinner'; spinner.setAttribute('aria-hidden', 'true'); - const text = document.createElement('span'); - text.className = 'center-status-text'; el.appendChild(spinner); - el.appendChild(text); - document.body.appendChild(el); } - const textEl = el.querySelector('.center-status-text'); - if (textEl) textEl.textContent = message; + const text = document.createElement('span'); + text.className = 'center-status-text'; + text.textContent = message; + el.appendChild(text); + if (type === 'error') { + const closeBtn = document.createElement('button'); + closeBtn.className = 'center-status-close'; + closeBtn.textContent = '×'; + closeBtn.setAttribute('aria-label', 'Dismiss'); + closeBtn.onclick = (e) => { + e.stopPropagation(); + dismiss(); + }; + el.appendChild(closeBtn); + } el.hidden = false; requestAnimationFrame(() => el.classList.add('show')); return { - dismiss: () => { - el.classList.remove('show'); - setTimeout(() => { - el.hidden = true; - }, 200); - }, + dismiss, setMessage: (next) => { const t = el.querySelector('.center-status-text'); if (t) t.textContent = next; diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index 5e493ad1..753618cc 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -765,7 +765,7 @@ Object.assign(CodemanApp.prototype, { const result = this._lastCustomModelLaunchResult; this._lastCustomModelLaunchResult = undefined; if (result?.modelSwapInProgress) { - void this._watchLlamaSwapLoading(endpointId, modelId); + void this._watchLlamaSwapLoading(endpointId, modelId, result.sessionId); } }, @@ -906,7 +906,7 @@ Object.assign(CodemanApp.prototype, { // Hand off to its own sticky toast rather than stacking a second one on top. if (payload?.modelSwapInProgress) { switchingToast?.dismiss(); - void this._watchLlamaSwapLoading(endpointId, modelId); + void this._watchLlamaSwapLoading(endpointId, modelId, sessionId); return; } @@ -967,29 +967,43 @@ Object.assign(CodemanApp.prototype, { return this._MODEL_LOAD_TIME_MATRIX.find((bracket) => sizeGB <= bracket.maxGB) ?? null; }, + /** `ms` -> `"1m 08s remaining"` / `"8s remaining"`, for the loading banner's live countdown. */ + _formatRemaining(ms) { + const totalSec = Math.max(0, Math.ceil(ms / 1000)); + const mins = Math.floor(totalSec / 60); + const secs = totalSec % 60; + return mins > 0 ? `${mins}m ${String(secs).padStart(2, '0')}s remaining` : `${secs}s remaining`; + }, + /** * Polls llama-swap's own `/running` (via the read-only running-status route) until - * `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. 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` — defaults to a rough, - * size-scaled estimate (`_estimateModelLoad`) when the model's discovered size is known, - * falling back to a flat 5 minutes when it isn't; still not ready by then gets a toast - * saying so rather than polling forever. + * `modelId` reports `state: 'ready'`, showing a sticky banner with a live countdown the + * whole time so a slow unload/reload (measured well over a minute for a large model) + * reads as "loading, N seconds left", 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` — defaults to a rough, size-scaled estimate + * (`_estimateModelLoad`) when the model's discovered size is known, falling back to a + * flat 5 minutes when it isn't. + * + * If the countdown reaches zero with the model still not ready, this is a real failure, + * not a "keep waiting" — the banner turns into a sticky error naming the llama-swap + * server's own logs as where to look, and `sessionId` (the session this was launched + * for) is closed automatically: a console left open and pointed at a model that never + * finished loading is worse than no console at all. * * `_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. + * or overwrite the WRONG one, or close the WRONG session. Each call claims the counter + * as its own "generation" and checks it still owns it before touching either. * * `pollIntervalMs`/`maxWaitMs` exist to let a test drive this in milliseconds instead of * minutes — real callers never pass `maxWaitMs`, which is what keeps the size-scaled * default live here rather than only in a test fixture. */ - async _watchLlamaSwapLoading(endpointId, modelId, pollIntervalMs = 1000, maxWaitMs) { + async _watchLlamaSwapLoading(endpointId, modelId, sessionId, pollIntervalMs = 1000, maxWaitMs) { const generation = (this._watchLlamaSwapGeneration = (this._watchLlamaSwapGeneration || 0) + 1); const isCurrent = () => this._watchLlamaSwapGeneration === generation; const sizeGB = await this._lookupModelSizeGB(endpointId, modelId); @@ -999,10 +1013,11 @@ Object.assign(CodemanApp.prototype, { const sizeSuffix = sizeGB ? ` (${sizeGB.toFixed(1)} GB${estimate ? `, typically ${estimate.label}` : ''})` : ''; + const baseMessage = `Loading ${modelId}${sizeSuffix} on ${endpointId} —`; // 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}${sizeSuffix} on ${endpointId}… this can take a while`); const deadline = Date.now() + effectiveMaxWaitMs; + const toast = this._showCenterStatus(`${baseMessage} ${this._formatRemaining(deadline - Date.now())}`); while (Date.now() < deadline) { const status = await this._apiJson(`/api/model-endpoints/${encodeURIComponent(endpointId)}/running-status`); if (!isCurrent()) return; // a newer launch took over the banner — this loop is done @@ -1018,11 +1033,24 @@ Object.assign(CodemanApp.prototype, { this.showToast(`${modelId} is ready`, 'success', { duration: 2500 }); return; } + if (!isCurrent()) return; + toast?.setMessage(`${baseMessage} ${this._formatRemaining(deadline - Date.now())}`); 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'); + this._showCenterStatus( + `${modelId} did not finish loading on ${endpointId} within the expected time. ` + + `Check the llama-swap server logs for details.` + + (sessionId ? ' The session has been closed.' : ''), + { type: 'error' } + ); + if (sessionId) { + try { + await this.closeSession(sessionId); + } catch { + // closeSession already reports its own failure via toast — nothing more to do here + } + } }, /** diff --git a/src/web/public/styles.css b/src/web/public/styles.css index adf9491c..df9e79ef 100644 --- a/src/web/public/styles.css +++ b/src/web/public/styles.css @@ -8577,11 +8577,37 @@ kbd { } .center-status-text { + flex: 1; pointer-events: auto; white-space: pre-wrap; word-break: break-word; } +/* Error variant: the load didn't finish in time — nothing is "in progress" anymore (no + spinner), and since this one doesn't dismiss itself, it needs a close button the user + can actually click, so pointer-events is restored here too (see the wrapper's own + comment on why that's `none` by default). */ +.center-status-error { + border-color: rgba(239, 68, 68, 0.5); +} + +.center-status-close { + flex-shrink: 0; + pointer-events: auto; + background: none; + border: none; + color: inherit; + opacity: 0.6; + font-size: 1.2rem; + line-height: 1; + padding: 0 0.15rem; + cursor: pointer; +} + +.center-status-close:hover { + opacity: 1; +} + .toast-success { border-color: rgba(34, 197, 94, 0.4); } .toast-error { border-color: rgba(239, 68, 68, 0.4); } .toast-warning { border-color: rgba(234, 179, 8, 0.4); } diff --git a/test/custom-model-one-shot-launch.test.ts b/test/custom-model-one-shot-launch.test.ts index d965606f..c4ba8598 100644 --- a/test/custom-model-one-shot-launch.test.ts +++ b/test/custom-model-one-shot-launch.test.ts @@ -108,17 +108,17 @@ describe('_runCustomModelEntryOneShot', () => { expect(app._pendingCustomModelForLaunch).toBeUndefined(); }); - it('starts the loading watcher when the launch reports modelSwapInProgress', async () => { + it('starts the loading watcher when the launch reports modelSwapInProgress, passing the new session id', async () => { const { app } = bootApp(); app.run = async () => { - app._lastCustomModelLaunchResult = { modelSwapInProgress: true }; + app._lastCustomModelLaunchResult = { modelSwapInProgress: true, sessionId: 'new-session' }; }; let watched: unknown[] | null = null; app._watchLlamaSwapLoading = async (...args: unknown[]) => { watched = args; }; await app._runCustomModelEntryOneShot('codex', 'llama-box', 'qwen3'); - expect(watched).toEqual(['llama-box', 'qwen3']); + expect(watched).toEqual(['llama-box', 'qwen3', 'new-session']); }); it('never starts the watcher when no swap was needed', async () => { diff --git a/test/custom-model-run-menu-ui.test.ts b/test/custom-model-run-menu-ui.test.ts index c3f073b1..9dd6868c 100644 --- a/test/custom-model-run-menu-ui.test.ts +++ b/test/custom-model-run-menu-ui.test.ts @@ -577,7 +577,7 @@ describe('Custom Model Endpoint Profiles: llama-swap model-swap confirmation and await app.runCustomModelEntry('claude', 'llama-box', 'qwen3'); - expect(watched).toEqual(['llama-box', 'qwen3']); + expect(watched).toEqual(['llama-box', 'qwen3', 'new-session']); }); it('a successful apply with no swap needed never starts the loading watcher', async () => { @@ -616,25 +616,48 @@ describe('Custom Model Endpoint Profiles: _watchLlamaSwapLoading polling', () => }; app._apiJson = async () => ({ isLlamaSwap: true, running: [{ model: 'qwen3', state: 'ready' }] }); - await app._watchLlamaSwapLoading('llama-box', 'qwen3', 5, 200); + await app._watchLlamaSwapLoading('llama-box', 'qwen3', 'sess-1', 5, 200); expect(bannerMessages[0]).toMatch(/loading qwen3/i); expect(dismissed).toContain(bannerMessages[0]); expect(toastCalls.at(-1)).toMatch(/ready/i); }); - it('gives up after the bounded wait and warns instead of polling forever', async () => { + it('gives up after the bounded wait, turns the banner into a sticky error, and closes the session', async () => { const { app } = bootApp({}); - app._showCenterStatus = () => ({ dismiss: () => {}, setMessage: () => {} }); - const toastCalls: string[] = []; - app.showToast = (message: string) => { - toastCalls.push(message); + const banners: Array<{ message: string; opts: unknown }> = []; + app._showCenterStatus = (message: string, opts: unknown) => { + banners.push({ message, opts }); + return { dismiss: () => {}, setMessage: () => {} }; + }; + app.showToast = () => {}; + let closedSessionId: string | undefined; + app.closeSession = async (id: string) => { + closedSessionId = id; }; app._apiJson = async () => ({ isLlamaSwap: true, running: [{ model: 'something-else', state: 'ready' }] }); - await app._watchLlamaSwapLoading('llama-box', 'qwen3', 5, 30); + await app._watchLlamaSwapLoading('llama-box', 'qwen3', 'sess-1', 5, 30); - expect(toastCalls.at(-1)).toMatch(/still waiting/i); + const errorBanner = banners.find((b) => (b.opts as { type?: string } | undefined)?.type === 'error'); + expect(errorBanner?.message).toMatch(/did not finish loading/i); + expect(errorBanner?.message).toMatch(/llama-swap server logs/i); + expect(closedSessionId).toBe('sess-1'); + }); + + it('never closes anything when no sessionId was given (a caller that has none to close)', async () => { + const { app } = bootApp({}); + app._showCenterStatus = () => ({ dismiss: () => {}, setMessage: () => {} }); + app.showToast = () => {}; + let closeCalled = false; + app.closeSession = async () => { + closeCalled = true; + }; + app._apiJson = async () => ({ isLlamaSwap: true, running: [{ model: 'something-else', state: 'ready' }] }); + + await app._watchLlamaSwapLoading('llama-box', 'qwen3', undefined, 5, 30); + + expect(closeCalled).toBe(false); }); it('stops polling (without a warning) once the endpoint no longer reads as llama-swap', async () => { @@ -652,7 +675,7 @@ describe('Custom Model Endpoint Profiles: _watchLlamaSwapLoading polling', () => }; app._apiJson = async () => ({ isLlamaSwap: false, running: [] }); - await app._watchLlamaSwapLoading('llama-box', 'qwen3', 5, 200); + await app._watchLlamaSwapLoading('llama-box', 'qwen3', 'sess-1', 5, 200); expect(bannerDismissed).toBe(true); expect(toastCalls).toHaveLength(0); // no follow-up warning toast @@ -672,7 +695,7 @@ describe('Custom Model Endpoint Profiles: _watchLlamaSwapLoading polling', () => return { isLlamaSwap: true, running: [{ model: 'qwen3', state: 'ready' }] }; }; - await app._watchLlamaSwapLoading('llama-box', 'qwen3', 5, 200); + await app._watchLlamaSwapLoading('llama-box', 'qwen3', 'sess-1', 5, 200); expect(toastCalls.at(-1)).toMatch(/ready/i); }); @@ -692,7 +715,7 @@ describe('Custom Model Endpoint Profiles: _watchLlamaSwapLoading polling', () => // 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); + await app._watchLlamaSwapLoading('llama-box', 'qwen3', 'sess-1', 60000, 300000); expect(calls).toBe(1); }); @@ -706,12 +729,13 @@ describe('Custom Model Endpoint Profiles: _watchLlamaSwapLoading polling', () => }); app.showToast = () => {}; // The FIRST call never sees its own target model ready, so left alone it would run all - // the way to its own timeout and dismiss/warn. + // the way to its own timeout and (now) turn into an error + close its session — but no + // sessionId is passed, so there is nothing for it to close even if it does get there. app._apiJson = async (path: string) => { if (path === '/api/model-endpoints') return []; return { isLlamaSwap: true, running: [] }; }; - const firstCall = app._watchLlamaSwapLoading('llama-box', 'model-a', 5, 30); + const firstCall = app._watchLlamaSwapLoading('llama-box', 'model-a', undefined, 5, 30); // Second call, for a DIFFERENT model that IS ready right away, takes over the banner // before the first call's own bounded wait has elapsed. @@ -719,15 +743,16 @@ describe('Custom Model Endpoint Profiles: _watchLlamaSwapLoading polling', () => if (path === '/api/model-endpoints') return []; return { isLlamaSwap: true, running: [{ model: 'model-b', state: 'ready' }] }; }; - await app._watchLlamaSwapLoading('llama-box', 'model-b', 5, 200); + await app._watchLlamaSwapLoading('llama-box', 'model-b', undefined, 5, 200); // Let the stale first call run out its own bounded wait and finish. await firstCall; // Whatever the first call did or didn't show along the way, its own eventual // completion (a timeout, in this case) must never touch a banner state that belongs - // to the newer, still-current call — exactly one dismiss (model-b's own) is the tell. - expect(dismissCalls).toEqual(['Loading model-b on llama-box… this can take a while']); + // to the newer, still-current call — exactly one dismiss, for model-b, is the tell. + expect(dismissCalls).toHaveLength(1); + expect(dismissCalls[0]).toContain('model-b'); }); }); @@ -787,10 +812,10 @@ describe('Custom Model Endpoint Profiles: model-size load-time estimate', () => return { isLlamaSwap: true, running: [{ model: 'qwen3.8-27b-ud-q4_k_xl', state: 'ready' }] }; }; - await app._watchLlamaSwapLoading('llama-box', 'qwen3.8-27b-ud-q4_k_xl', 5); + await app._watchLlamaSwapLoading('llama-box', 'qwen3.8-27b-ud-q4_k_xl', undefined, 5); - expect(bannerMessages[0]).toBe( - 'Loading qwen3.8-27b-ud-q4_k_xl (16.4 GB, typically ~1–3 min) on llama-box… this can take a while' + expect(bannerMessages[0]).toMatch( + /^Loading qwen3\.8-27b-ud-q4_k_xl \(16\.4 GB, typically ~1–3 min\) on llama-box — .+ remaining$/ ); }); @@ -807,9 +832,9 @@ describe('Custom Model Endpoint Profiles: model-size load-time estimate', () => return { isLlamaSwap: true, running: [{ model: 'big', state: 'ready' }] }; }; - await app._watchLlamaSwapLoading('llama-box', 'big', 5); + await app._watchLlamaSwapLoading('llama-box', 'big', undefined, 5); - expect(bannerMessages[0]).toBe('Loading big on llama-box… this can take a while'); + expect(bannerMessages[0]).toMatch(/^Loading big on llama-box — .+ remaining$/); }); it('uses the size-scaled estimate as the default timeout when maxWaitMs is not passed', async () => { @@ -828,7 +853,7 @@ describe('Custom Model Endpoint Profiles: model-size load-time estimate', () => }; // pollIntervalMs only — maxWaitMs omitted, so it must fall back to the size estimate. - await app._watchLlamaSwapLoading('llama-box', 'huge', 5); + await app._watchLlamaSwapLoading('llama-box', 'huge', undefined, 5); expect(calls).toBe(3); });