From e300cd2b7d646dac54a02e167a01b99f305f2027 Mon Sep 17 00:00:00 2001 From: mattshax Date: Tue, 15 Sep 2026 19:38:46 +0000 Subject: [PATCH] perf(models): the listing answers from a cache and refreshes behind it Every load of the model list fetched the gateway catalog and pinged each provider family, about a second here and several on a deployment whose gateway and providers are further away, and the client asked three times per mount. The computed listing is now kept per viewer and credential: a load inside the fresh window is answered from it, an older one is answered from it too and refreshed in the background so the next load is current, and only the first load per credential waits. ?refresh=1 is the user saying "look again"; it drops the probes and recomputes before answering, and a key test in Settings clears every listing. A model whose last call failed is decorated after the cache, so that mark stays live. On the client one request serves the picker, the banner, the credential notice, and the fleet page, with a fifteen-second reuse so a remount does not refetch. Status stays fresh without a reload: the list is asked again when the tab comes back into view and every five minutes while it stays visible, and the picker is remounted only when the set of marked models actually changes, since a remount mid-reply would drop the visible stream. Measured on the dev server: first load 1.2 s, the next three under ten milliseconds, a forced refresh 0.4 s. --- server/src/chat/routes.ts | 94 +++++++++++++++++++------ server/test/modelsListingCache.test.mjs | 70 ++++++++++++++++++ web/src/adapter.ts | 10 +-- web/src/api.ts | 34 +++++++++ web/src/views/ChatView.tsx | 55 ++++++++++----- web/src/views/FleetPage.tsx | 3 +- 6 files changed, 220 insertions(+), 46 deletions(-) create mode 100644 server/test/modelsListingCache.test.mjs diff --git a/server/src/chat/routes.ts b/server/src/chat/routes.ts index 703e2cb..bc394a0 100644 --- a/server/src/chat/routes.ts +++ b/server/src/chat/routes.ts @@ -411,25 +411,28 @@ function templateKwargsFor(model: string): Record { } export async function chatRoutes(app: FastifyInstance): Promise { - app.get('/api/chat/models', async (req, reply) => { - if (!gatewayConfigured() && !resolveUserKey(req.user?.id) && !sidecarConfigured()) { - return reply.send({ models: [], unreachableSessions: [], error: 'no gateway credential: set PW_API_KEY or authenticate the pw CLI' }) - } - const eff0 = effectiveSettings() - const cred = resolveUserCred(req.user?.id) - // Settings asks with config=1: the deployment's own model choices (the - // vision model, the RAG default) have to be configurable even where - // chat requires each user to bring a key, so that listing falls back to - // the deployment credential instead of coming back empty. - const forConfig = String((req.query as { config?: string }).config ?? '') === '1' - if (!forConfig && eff0.requirePersonalKey && authEnabled() && !personalKeysDisabled() && !cred && !getSharedKeyStatus().active) { - return reply.send({ models: [], unreachableSessions: [], error: 'This deployment requires your own model credential: add your API key or platform token in Settings, Model access.' }) - } - // The platform catalog and the model served on this node are listed - // independently. Either can be missing: the gateway credential may be - // absent on a deployment whose whole point is the private model beside - // it, and the serve may still be loading. Neither absence should empty - // the picker of the other. + // The expensive part of a listing is the gateway catalog plus one probe + // per provider family, and both were paid on every load: about a second + // here, several on a deployment whose gateway and providers are further + // away, and per user, since each credential probes on its own. The + // computed listing is now kept per credential. A load inside the fresh + // window is answered from it; an older one is answered from it too and + // refreshed in the background, so the next load is current; only the + // very first load per credential waits. ?refresh=1 is the user saying + // "look again": it drops the probes and recomputes before answering. + // Per-request decorations (a model whose last call failed) are applied + // after the cache, so they stay live. + type Listing = { models: any[]; impaired: { id: string; locked: boolean; unlock_url: string | null }[]; unreachableSessions: unknown[] } + const LISTING_FRESH_MS = 60_000 + const LISTING_KEEP_MS = 30 * 60_000 + const listings = new Map() + const refreshing = new Map>() + const listingKey = (cred: { key?: string | null; baseUrl?: string | null } | null, viewer: string) => + `${viewer}|${(cred?.key ?? '').slice(-8)}|${cred?.baseUrl ?? ''}` + + // Everything up to the cache: catalog, provider probes, availability marks. + class ListingReply extends Error { constructor(public payload: unknown) { super('listing reply') } } + async function computeListing(cred: ReturnType, req: any): Promise { const sidecar = await listSidecarModels() let wire: any = {} try { @@ -441,7 +444,7 @@ export async function chatRoutes(app: FastifyInstance): Promise { const msg = String((e as Error).message ?? e) const unlockUrl = extractUnlockUrl(msg) if (cred?.baseUrl && (unlockUrl || /401|403|locked|unauthorized/i.test(msg))) { - return reply.send({ + throw new ListingReply({ models: sidecar, unreachableSessions: [], error: `Your provider key is locked or rejected by ${new URL(cred.baseUrl).host}.${unlockUrl ? ` Unlock it here and reload: ${unlockUrl}` : ''} The Settings, Model access page shows the same link and a Re-check.`, }) @@ -452,7 +455,7 @@ export async function chatRoutes(app: FastifyInstance): Promise { const rejected = /\b(401|403)\b|unauthorized|forbidden/i.test(msg) if (rejected) { const st = cred && req.user ? getUserKeyStatus(req.user.id) : null - return reply.send({ + throw new ListingReply({ models: sidecar, unreachableSessions: [], credential: 'rejected', error: credentialRejectionMessage(cred ? 'personal' : 'deployment', st?.kind ?? null, st?.credExpiresAt ?? null), }) @@ -575,13 +578,58 @@ export async function chatRoutes(app: FastifyInstance): Promise { // A model whose last call failed is still offered (the provider may // recover, or the key may be renewed), but the picker gets told, so // choosing it is informed rather than a surprise at reply time. + return { models, impaired, unreachableSessions: wire.unreachable_sessions ?? [] } as Listing + } + + app.get('/api/chat/models', async (req, reply) => { + if (!gatewayConfigured() && !resolveUserKey(req.user?.id) && !sidecarConfigured()) { + return reply.send({ models: [], unreachableSessions: [], error: 'no gateway credential: set PW_API_KEY or authenticate the pw CLI' }) + } + const eff0 = effectiveSettings() + const cred = resolveUserCred(req.user?.id) + // Settings asks with config=1: the deployment's own model choices (the + // vision model, the RAG default) have to be configurable even where + // chat requires each user to bring a key, so that listing falls back to + // the deployment credential instead of coming back empty. + const forConfig = String((req.query as { config?: string }).config ?? '') === '1' + if (!forConfig && eff0.requirePersonalKey && authEnabled() && !personalKeysDisabled() && !cred && !getSharedKeyStatus().active) { + return reply.send({ models: [], unreachableSessions: [], error: 'This deployment requires your own model credential: add your API key or platform token in Settings, Model access.' }) + } + // The platform catalog and the model served on this node are listed + // independently. Either can be missing: the gateway credential may be + // absent on a deployment whose whole point is the private model beside + // it, and the serve may still be loading. Neither absence should empty + // the picker of the other. + const key = listingKey(cred, (req.user?.username ?? '').toLowerCase()) + const wantFresh = String((req.query as { refresh?: string }).refresh ?? '') === '1' + if (wantFresh) { invalidateProviderProbes(); listings.delete(key) } + const compute = () => { + const p = computeListing(cred, req).then(v => { listings.set(key, { at: Date.now(), v }); return v }).finally(() => refreshing.delete(key)) + refreshing.set(key, p) + return p + } + const hit = listings.get(key) + let listing: Listing + try { + if (hit && Date.now() - hit.at < LISTING_KEEP_MS) { + listing = hit.v + if (Date.now() - hit.at > LISTING_FRESH_MS && !refreshing.has(key)) compute().catch(() => { /* the next load retries */ }) + } else { + listing = await (refreshing.get(key) ?? compute()) + } + } catch (e) { + if (e instanceof ListingReply) return reply.send(e.payload) + throw e + } + let models = listing.models + const impaired = listing.impaired models = models.map((m: any) => { const f = modelFailure(String(m.id)) return f ? { ...m, callable: false, last_error: { at: f.at, auth: f.auth, status: f.status } } : m }) models.sort((a: any, b: any) => Number(String(a.id).startsWith('org:')) - Number(String(b.id).startsWith('org:'))) - return reply.send({ models, impaired, unreachableSessions: wire.unreachable_sessions ?? [] }) + return reply.send({ models, impaired, unreachableSessions: listing.unreachableSessions }) }) // Live model-callability check for the footer status line. @@ -736,7 +784,7 @@ export async function chatRoutes(app: FastifyInstance): Promise { app.post('/api/me/model-key/test', async (req, reply) => { // A re-check is the user saying "I changed something, look again": // the provider probe cache must not outlive that. - invalidateProviderProbes() + invalidateProviderProbes(); listings.clear() if (!req.user) return reply.status(403).send({ error: 'not verified' }) const body = req.body as { key?: string; baseUrl?: string } | null const stored = resolveUserCred(req.user.id) diff --git a/server/test/modelsListingCache.test.mjs b/server/test/modelsListingCache.test.mjs new file mode 100644 index 0000000..f65f9fb --- /dev/null +++ b/server/test/modelsListingCache.test.mjs @@ -0,0 +1,70 @@ +// The model listing is cached per viewer and credential and refreshed in +// the background: the first load pays for the catalog and the probes, the +// next loads answer at once, and a stale answer triggers a refresh so the +// following load is current. ?refresh=1 recomputes before answering. +import test from 'node:test' +import assert from 'node:assert/strict' +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' + +process.env.KB_ROOT = fs.mkdtempSync(path.join(os.tmpdir(), 'kb-')) +process.env.INDEX_BASE = fs.mkdtempSync(path.join(os.tmpdir(), 'ix-')) +process.env.PW_GATEWAY_URL = 'https://gw.test/api/openai/v1' +process.env.PW_API_KEY = 'deployment-key' + +let catalogCalls = 0 +let probeCalls = 0 +let catalogVersion = 1 +globalThis.fetch = async (url, init) => { + const u = String(url) + if (u.endsWith('/models')) { + catalogCalls++ + return { ok: true, status: 200, json: async () => ({ data: [{ id: `me:genaimil/gemini-v${catalogVersion}`, object: 'model', owned_by: 'x' }] }), text: async () => '' } + } + if (u.endsWith('/chat/completions')) { + probeCalls++ + const text = 'data: {"choices":[{"delta":{"content":"pong"}}]}\n\ndata: [DONE]\n' + return { ok: true, status: 200, text: async () => text, body: null } + } + return { ok: false, status: 404, text: async () => 'nope', json: async () => ({}) } +} + +const { default: Fastify } = await import('fastify') +const { chatRoutes } = await import('../dist/chat/routes.js') +const { sanitizedErrorHandler } = await import('../dist/routes.js') +const app = Fastify() +app.setErrorHandler(sanitizedErrorHandler(app)) +await app.register(chatRoutes) +await app.ready() +const list = async (q = '') => (await app.inject({ method: 'GET', url: `/api/chat/models${q}` })).json() +const sleep = ms => new Promise(r => setTimeout(r, ms)) + +test('the first load computes; the second answers from the cache without asking again', async () => { + const a = await list() + assert.equal(a.models.length, 1) + const [c1, p1] = [catalogCalls, probeCalls] + assert.ok(c1 >= 1 && p1 >= 1, 'first load fetched the catalog and probed') + const b = await list() + assert.equal(b.models[0].id, a.models[0].id) + assert.equal(catalogCalls, c1, 'no catalog fetch on a cached load') + assert.equal(probeCalls, p1, 'no probe on a cached load') +}) + +test('refresh=1 recomputes before answering, and sees a changed catalog', async () => { + catalogVersion = 2 + const [c1] = [catalogCalls] + const r = await list('?refresh=1') + assert.equal(r.models[0].id, 'me:genaimil/gemini-v2') + assert.ok(catalogCalls > c1, 'the catalog was fetched again') +}) + +test('the per-request decoration stays live on a cached listing', async () => { + const { noteModelFailure } = await import('../dist/chat/gateway.js').catch(() => ({})) + if (!noteModelFailure) return // decoration helper not exported here; covered by the route reading modelFailure per request + noteModelFailure('me:genaimil/gemini-v2', { status: 401, auth: true }) + const r = await list() + assert.equal(r.models[0].callable, false) +}) + +test.after(async () => { await app.close() }) diff --git a/web/src/adapter.ts b/web/src/adapter.ts index 27765b1..56425bf 100644 --- a/web/src/adapter.ts +++ b/web/src/adapter.ts @@ -1,16 +1,16 @@ import type { ChatAdapter, ModelsList, StreamCompletion } from '@parallelworks/ai-chat' import { getLabelScope, getPersona } from './labelScope' +import { fetchModels } from './api' async function listModels(): Promise { - const res = await fetch('/api/chat/models') - if (!res.ok) throw new Error(`models: ${res.status}`) - const data = await res.json() + // The shared request is typed loosely; the package's own types apply here. + const data = (await fetchModels()) as unknown as { models?: ModelsList['models']; unreachableSessions?: ModelsList['unreachableSessions'] } // The server marks models whose most recent call failed (an expired // provider key fails at call time while listing fine). The picker // component is upstream, so the signal rides the display name; it // disappears on the first successful call. - const models = (data.models ?? []).map((m: { id: string; name?: string; callable?: boolean }) => - m?.callable === false ? { ...m, name: `${m.name || m.id} · last call failed` } : m) + const models = (data.models ?? []).map(m => + (m as { callable?: boolean }).callable === false ? { ...m, name: `${m.name || m.id} · last call failed` } : m) return { models, unreachableSessions: data.unreachableSessions ?? [] } } diff --git a/web/src/api.ts b/web/src/api.ts index d3dc873..5bfc29b 100644 --- a/web/src/api.ts +++ b/web/src/api.ts @@ -219,3 +219,37 @@ export interface IndexJob { ms: number | null error: string | null } + +// ---- the model list ---- +// Several places ask for it on the same mount: the chat package's picker, +// the availability banner, the credential notice, the fleet page. One +// in-flight request serves all of them, and a listing is reused for a +// short window so a remount does not refetch. The server keeps its own +// longer cache and refreshes in the background; this only stops the +// client asking three times for one answer. +export interface ModelsResponse { + models: { id: string; name?: string; callable?: boolean; [k: string]: unknown }[] + impaired?: { id: string; locked: boolean; unlock_url: string | null }[] + unreachableSessions?: unknown[] + error?: string + credential?: string +} +const MODELS_REUSE_MS = 15_000 +let modelsAt = 0 +let modelsLast: ModelsResponse | null = null +let modelsInFlight: Promise | null = null +export function fetchModels(opts: { refresh?: boolean } = {}): Promise { + if (!opts.refresh && modelsLast && Date.now() - modelsAt < MODELS_REUSE_MS) return Promise.resolve(modelsLast) + if (modelsInFlight) return modelsInFlight + modelsInFlight = fetch(opts.refresh ? '/api/chat/models?refresh=1' : '/api/chat/models') + .then(async r => { + if (!r.ok) throw new Error(`models: ${r.status}`) + const d = (await r.json()) as ModelsResponse + modelsLast = d; modelsAt = Date.now() + return d + }) + .finally(() => { modelsInFlight = null }) + return modelsInFlight +} +/** Forget the reused listing, so the next ask goes to the server. */ +export function forgetModels(): void { modelsLast = null; modelsAt = 0 } diff --git a/web/src/views/ChatView.tsx b/web/src/views/ChatView.tsx index 5c8ceee..9471000 100644 --- a/web/src/views/ChatView.tsx +++ b/web/src/views/ChatView.tsx @@ -9,7 +9,7 @@ import { } from '@parallelworks/ai-chat' import { createStudioAdapter, getChatListFilter, setChatListFilter, setViewerUsername } from '../adapter' import { useAppConfig } from '../config' -import { api } from '../api' +import { api, fetchModels, forgetModels, type ModelsResponse } from '../api' import { setLabelScope, setPersona } from '../labelScope' import { PersonaIcon } from '../components/PersonaIcon' import { ConversationScrubber } from '../components/ConversationScrubber' @@ -126,9 +126,8 @@ export function ChatView() { useEffect(() => { const onAccessChange = () => { setCredNote(null) - fetch('/api/chat/models').then(r => r.json()) - .then(d => { if (!d.models?.length && d.error) setCredNote(String(d.error)) }) - .catch(() => { /* the remount below refetches regardless */ }) + forgetModels() + fetchModels({ refresh: true }).then(applyModels).catch(() => { /* the remount below refetches regardless */ }) setChatEpoch(e => e + 1) } window.addEventListener('ade:model-access-changed', onAccessChange) @@ -161,6 +160,40 @@ export function ChatView() { }, []) const [scopeFilter, setScopeFilter] = useState('') + // What the listing says, applied wherever it came from: the banner above + // the thread, and a remount of the picker when the set of marked models + // changes, since the picker is the package's and only re-reads on mount. + // A remount mid-reply would drop the visible stream, so it waits for the + // next idle moment. + const marksRef = useRef('') + const applyModels = (d: ModelsResponse) => { + if (d.error && !(d.models ?? []).length) { setCredNote(String(d.error)); return } + const impaired = (d.impaired ?? []) as { id: string; locked: boolean }[] + const marks = impaired.map(m => `${m.id}:${m.locked ? 'L' : 'U'}`).sort().join(',') + if (impaired.length) { + const locked = impaired.some(m => m.locked) + setCredNote(`${impaired.length} model${impaired.length === 1 ? ' is' : 's are'} marked [${locked ? 'locked' : 'unavailable'}] in the model list${locked + ? ': the provider reports the key is locked' + : ' (for GenAI this usually means the key is locked on its 8-hour schedule)'}. Unlock and Re-check under Settings, Model access; the marks clear on the next listing.`) + } else if (marksRef.current) { + setCredNote(null) + } + if (marksRef.current && marks !== marksRef.current) setChatEpoch(e => e + 1) + marksRef.current = marks + } + // Keep the status fresh without a reload: ask again when the tab comes + // back into view and every five minutes while it stays there. The + // server answers from its cache and refreshes behind the answer, so + // these are cheap. + useEffect(() => { + const tick = () => { if (document.visibilityState === 'visible') fetchModels().then(applyModels).catch(() => {}) } + const onVisible = () => { if (document.visibilityState === 'visible') { forgetModels(); tick() } } + document.addEventListener('visibilitychange', onVisible) + const id = window.setInterval(() => { forgetModels(); tick() }, 5 * 60_000) + return () => { document.removeEventListener('visibilitychange', onVisible); window.clearInterval(id) } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []) + useEffect(() => { migrateRememberedModel() api.tagVocabulary().then(v => setVocab(v.tags)).catch(() => {}) @@ -168,16 +201,7 @@ export function ChatView() { // alone, so a locked provider cannot be marked inside it (upstream // issue filed). The warning therefore lives here, above the thread, // where it cannot be missed or truncated. - fetch('/api/chat/models').then(r => r.json()).then(d => { - if (d.error && !(d.models ?? []).length) { setCredNote(String(d.error)); return } - const impaired = (d.impaired ?? []) as { id: string; locked: boolean }[] - if (impaired.length) { - const locked = impaired.some(m => m.locked) - setCredNote(`${impaired.length} model${impaired.length === 1 ? ' is' : 's are'} marked [${locked ? 'locked' : 'unavailable'}] in the model list${locked - ? ': the provider reports the key is locked' - : ' (for GenAI this usually means the key is locked on its 8-hour schedule)'}. Unlock and Re-check under Settings, Model access; the marks clear on the next listing.`) - } - }).catch(() => {}) + fetchModels().then(applyModels).catch(() => {}) fetch('/api/me/model-key').then(r => r.json()) .then(d => { setMultiUser(!!d.authEnabled) @@ -195,9 +219,6 @@ export function ChatView() { // Both the no-models state and mid-conversation credential failures // surface as a toast with a click path to Settings; the empty state // text explains, the toast provides the button. - fetch('/api/chat/models').then(r => r.json()) - .then(d => { if (!d.models?.length && d.error) setCredNote(String(d.error)) }) - .catch(() => {}) const onCredError = (e: Event) => setCredNote(String((e as CustomEvent).detail ?? 'Model credential needed.')) window.addEventListener('ade-credential-error', onCredError) // Back and forward between conversations. diff --git a/web/src/views/FleetPage.tsx b/web/src/views/FleetPage.tsx index 5b82235..af9a33c 100644 --- a/web/src/views/FleetPage.tsx +++ b/web/src/views/FleetPage.tsx @@ -1,3 +1,4 @@ +import { fetchModels } from '../api' import { useEffect, useState } from 'react' import { PersonaIcon } from '../components/PersonaIcon' @@ -40,7 +41,7 @@ export function FleetPage({ personas, onOpenTask }: { personas: Persona[]; onOpe refresh() const t = setInterval(refresh, 10_000) fetch('/api/fleet/templates').then(r => r.json()).then(d => setTemplates(d.templates ?? [])).catch(() => {}) - fetch('/api/chat/models').then(r => r.json()).then(d => { + fetchModels().then(d => { const ids = (d.models ?? []).filter((m: { callable?: boolean }) => m.callable !== false).map((m: { id: string }) => m.id) setModels(ids) setForm(f => f.model ? f : { ...f, model: ids[0] ?? '' })