From 8928b7a4e7559b812434d804d7b7276099988798 Mon Sep 17 00:00:00 2001 From: Tashfeen Date: Sun, 26 Jul 2026 12:33:02 +0100 Subject: [PATCH 1/5] fix(config): skip missing-key entries at boot instead of crashing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A declarative config written when a platform was still keyless (pollinations lost `keyless: true` in #573) carries a `keys` entry with no `key`. applyDeclarativeConfigFromEnv() runs inside main(), and upsertApiKey threw `key is required for `, so main().catch exited 1 — a stale config bricked the install at startup. Boot-time apply now degrades that shape to a per-entry skip: any non-keyless platform entry without a key logs a warning naming the platform (with a remediation hint — pollinations points at enter.pollinations.ai) and is skipped, while the rest of the config still applies. The result gains a `warnings` array and the boot summary reports the skip count. Genuinely malformed config (unparseable JSON, schema violations, unknown platforms, custom entries without baseUrl) still fails loudly, and the write-time API path in routes/keys.ts is untouched. Fixes the boot-crash from #600 (legacy keyless pollinations config). Co-Authored-By: Claude Opus 5 --- .../services/declarative-config.test.ts | 88 ++++++++++++++++++- server/src/services/declarative-config.ts | 33 ++++++- 2 files changed, 118 insertions(+), 3 deletions(-) diff --git a/server/src/__tests__/services/declarative-config.test.ts b/server/src/__tests__/services/declarative-config.test.ts index d806c43a1..d03c1620d 100644 --- a/server/src/__tests__/services/declarative-config.test.ts +++ b/server/src/__tests__/services/declarative-config.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeAll, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeAll, beforeEach, afterEach, vi } from 'vitest'; import { initDb, getDb } from '../../db/index.js'; import { applyDeclarativeConfig, applyDeclarativeConfigFromEnv } from '../../services/declarative-config.js'; import { getRoutingStrategy } from '../../services/router.js'; @@ -104,4 +104,90 @@ describe('declarative config import', () => { expect(applyDeclarativeConfigFromEnv().applied).toBe(true); expect((getDb().prepare("SELECT COUNT(*) AS n FROM api_keys WHERE platform = 'groq' AND label = 'config'").get() as { n: number }).n).toBe(1); }); + + // #600: pollinations lost `keyless: true` in #573, so a legacy declarative + // config with a keyless-era pollinations entry (no `key`) used to throw at + // boot and brick the install. Missing-key entries must now degrade to a + // per-entry skip + warning instead of killing the apply. + it('skips a legacy keyless entry without a key, warns, and still applies the rest', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const result = applyDeclarativeConfig({ + keys: [ + { platform: 'pollinations' }, + { platform: 'groq', key: 'gsk_config_key', label: 'config' }, + ], + }); + expect(result.applied).toBe(true); + expect(result.keys).toBe(1); + expect(result.warnings).toHaveLength(1); + expect(result.warnings[0]).toContain('pollinations'); + expect(result.warnings[0]).toContain('enter.pollinations.ai'); + expect(result.warnings[0]).toContain('entry skipped'); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('pollinations')); + expect((getDb().prepare("SELECT COUNT(*) AS n FROM api_keys WHERE platform = 'pollinations'").get() as { n: number }).n).toBe(0); + expect((getDb().prepare("SELECT COUNT(*) AS n FROM api_keys WHERE platform = 'groq'").get() as { n: number }).n).toBe(1); + } finally { + warn.mockRestore(); + } + }); + + it('boot apply (FromEnv) survives a legacy keyless pollinations entry', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + process.env.FREEAPI_CONFIG_JSON = JSON.stringify({ + keys: [ + { platform: 'pollinations', label: 'legacy' }, + { platform: 'groq', key: 'gsk_config_key', label: 'config' }, + ], + }); + expect(() => applyDeclarativeConfigFromEnv()).not.toThrow(); + expect((getDb().prepare("SELECT COUNT(*) AS n FROM api_keys WHERE platform = 'groq'").get() as { n: number }).n).toBe(1); + expect((getDb().prepare("SELECT COUNT(*) AS n FROM api_keys WHERE platform = 'pollinations'").get() as { n: number }).n).toBe(0); + } finally { + warn.mockRestore(); + } + }); + + it('still applies a pollinations entry that has a key', () => { + const result = applyDeclarativeConfig({ + keys: [{ platform: 'pollinations', key: 'pk_live_abc', label: 'config' }], + }); + expect(result.keys).toBe(1); + expect(result.warnings).toHaveLength(0); + expect((getDb().prepare("SELECT COUNT(*) AS n FROM api_keys WHERE platform = 'pollinations'").get() as { n: number }).n).toBe(1); + }); + + it('generic missing-key skip covers any non-keyless platform, with a platform-named warning', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const result = applyDeclarativeConfig({ + keys: [{ platform: 'groq' }], + }); + expect(result.keys).toBe(0); + expect(result.warnings).toHaveLength(1); + expect(result.warnings[0]).toContain('groq'); + expect(result.warnings[0]).toContain('entry skipped'); + } finally { + warn.mockRestore(); + } + }); + + it('a keyless platform entry without a key still applies its sentinel row', () => { + const result = applyDeclarativeConfig({ + keys: [{ platform: 'kilo', label: 'config' }], + }); + expect(result.keys).toBe(1); + expect(result.warnings).toHaveLength(0); + expect((getDb().prepare("SELECT COUNT(*) AS n FROM api_keys WHERE platform = 'kilo'").get() as { n: number }).n).toBe(1); + }); + + it('genuinely malformed config still fails loudly', () => { + expect(() => applyDeclarativeConfig({ keys: [{ platform: 'groq' }], nonsense: true })) + .toThrow(/invalid declarative config/); + expect(() => applyDeclarativeConfig({ keys: [{ platform: 'unknown-platform', key: 'x' }] })) + .toThrow(/unknown provider platform/); + process.env.FREEAPI_CONFIG_JSON = '{ not json'; + expect(() => applyDeclarativeConfigFromEnv()).toThrow(); + }); }); diff --git a/server/src/services/declarative-config.ts b/server/src/services/declarative-config.ts index 2a0828104..c364e732e 100644 --- a/server/src/services/declarative-config.ts +++ b/server/src/services/declarative-config.ts @@ -95,6 +95,8 @@ export interface DeclarativeConfigResult { models: number; fallback: number; routing: boolean; + /** Per-entry problems that were degraded to a skip instead of failing the apply (#600). */ + warnings: string[]; } interface NormalizedCustomModel { @@ -125,6 +127,25 @@ function encryptedKey(raw: string) { return { encrypted, iv, authTag }; } +// Boot-time skip guard for `keys` entries (#600). When a platform stops being +// keyless (pollinations lost `keyless: true` in #573), a legacy declarative +// config still carries an entry with no `key` — applyDeclarativeConfigFromEnv() +// runs in main(), so throwing here used to brick the whole install at startup. +// Such entries degrade to a warning + skip; the rest of the config still +// applies. Platform-specific remediation hints live here. +const MISSING_KEY_HINTS: Record = { + pollinations: 'pollinations now requires an API key — get one at enter.pollinations.ai', +}; + +function missingKeyWarning(input: z.infer): string | null { + const platform = input.platform.trim(); + if (platform === 'custom' || input.key?.trim()) return null; + const provider = resolveProvider(platform as never); + if (!provider || provider.keyless) return null; + const hint = MISSING_KEY_HINTS[platform] ?? `${platform} requires an API key — add "key" to this entry or remove it`; + return `${hint}; entry skipped`; +} + function upsertApiKey(db: Db, input: z.infer): number { const platform = input.platform.trim(); const enabled = input.enabled === false ? 0 : 1; @@ -361,10 +382,17 @@ export function applyDeclarativeConfig(input: unknown, source = 'inline'): Decla models: 0, fallback: 0, routing: false, + warnings: [], }; const apply = db.transaction(() => { for (const key of parsed.data.keys ?? []) { + const warning = missingKeyWarning(key); + if (warning) { + result.warnings.push(warning); + console.warn(`[config] ${warning}`); + continue; + } upsertApiKey(db, key); result.keys++; } @@ -391,12 +419,13 @@ export function applyDeclarativeConfig(input: unknown, source = 'inline'): Decla export function applyDeclarativeConfigFromEnv(): DeclarativeConfigResult { const loaded = readConfigFromEnv(); if (!loaded) { - return { applied: false, keys: 0, customModels: 0, models: 0, fallback: 0, routing: false }; + return { applied: false, keys: 0, customModels: 0, models: 0, fallback: 0, routing: false, warnings: [] }; } const result = applyDeclarativeConfig(loaded.value, loaded.source); console.log( `[config] applied ${loaded.source}: ${result.keys} keys, ${result.customModels} custom models, ` + - `${result.models} model edits, ${result.fallback} fallback rows${result.routing ? ', routing' : ''}`, + `${result.models} model edits, ${result.fallback} fallback rows${result.routing ? ', routing' : ''}` + + `${result.warnings.length > 0 ? `, ${result.warnings.length} entries skipped` : ''}`, ); return result; } From 33599ab8dc449591248aceb8c2c7c5a82d1331f9 Mon Sep 17 00:00:00 2001 From: Tashfeen Date: Sun, 26 Jul 2026 12:35:44 +0100 Subject: [PATCH 2/5] docs(readme): credit issue reporters; document auto:* routing strategies Credit reporters and a proposer from adopted issues: - @NirvanaCh7 (#549 GitHub Models retirement heads-up; #335 routing/ observability write-up that led to same-model unification in #341) - @Mohamed3nan (#335 comment proposing same-model-first failover) - @Arman-Espiar (#592 local Ollama routes hitting false 429 cooldowns) - @MetaMysteries8 (#600 Pollinations community models + keyless boot crash) - @lujun880726 (#499 disabled models stuck in Fusion slots) @LoneRifle (#534 Kilo auto-route aliasing) is already on the wall for #179/#161, so no new avatar. Also document the previously undocumented routing strategy suffixes in "Using the API": auto:smart | auto:fast | auto:cheap | auto:reliable | auto:balanced, their synonyms and case-insensitivity, and auto: for routing through a named profile (server/src/services/router.ts GLOBAL_SORT_ALIASES + resolveRoutingChain). Co-Authored-By: Claude Opus 5 --- README.md | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/README.md b/README.md index e3a479f16..5e758d0a6 100644 --- a/README.md +++ b/README.md @@ -482,6 +482,42 @@ curl http://localhost:3001/v1/chat/completions \ }' ``` +**Routing strategies (`auto:*`)** + +Plain `auto` follows your active fallback chain. Add a suffix to steer a single request instead — no dashboard changes needed: + +- `auto:smart` — favor the highest-intelligence models +- `auto:fast` — favor measured speed (throughput and time-to-first-byte) +- `auto:cheap` — budget-leaning; currently the same blend as `balanced` (everything in the pool is already free) +- `auto:reliable` — favor recent success rate +- `auto:balanced` — the default blend (reliability first, speed and intelligence split the rest) + +These rank **every enabled model**, ignoring your chain order. Common synonyms resolve too (`auto:fastest`, `auto:speed`, `auto:smartest`, `auto:cheapest`, `auto:budget`, …), and the whole model string is case-insensitive. + +```bash +curl http://localhost:3001/v1/chat/completions \ + -H "Authorization: Bearer freellmapi-your-unified-key" \ + -H "Content-Type: application/json" \ + -d '{ + "model": "auto:fast", + "messages": [{"role": "user", "content": "hi"}] + }' +``` + +`auto:` routes through a named profile's chain instead of the active one, so different tools can use different chains through the same key: + +```bash +curl http://localhost:3001/v1/chat/completions \ + -H "Authorization: Bearer freellmapi-your-unified-key" \ + -H "Content-Type: application/json" \ + -d '{ + "model": "auto:coding", + "messages": [{"role": "user", "content": "Write a binary search in Rust."}] + }' +``` + +An unknown profile name returns a clear `400` rather than silently falling back. Profiles are the named fallback chains from **Model profiles** (see [Features](#features)) — create and switch them from the dashboard; whichever is active is what plain `auto` uses. + **Streaming** ```python @@ -867,6 +903,11 @@ See [CONTRIBUTING.md](./CONTRIBUTING.md) for the full migration CLI and workflow @noobix @nandukmelath @coffcoe +@NirvanaCh7 +@Mohamed3nan +@Arman-Espiar +@MetaMysteries8 +@lujun880726 ## Terms of Service review From 9f10f73c74dac56cc902c8590a41cb04bbfd9573 Mon Sep 17 00:00:00 2001 From: Tashfeen Date: Sun, 26 Jul 2026 12:38:53 +0100 Subject: [PATCH 3/5] Stop benching local Ollama routes with cloud-quota cooldowns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Local inference routes (custom-platform Ollama/llama.cpp/LM Studio) were getting fake 429s: a slow generation timed out, classified retryable, and fed the null-limits exhaustion heuristic — 90s transient bench, then the 2m -> 10m -> 1h -> 24h ladder — stranding what is usually the user's ONLY route and rendering "All models exhausted". Three changes at the existing choke points: - Error-kind gating: the null-limits "effectively daily-exhausted" heuristic in getCooldownDecisionForLimit now only fires on an actual quota signal (new isRateLimitSignal: structured 429 or rate-limit wording). Timeouts and 5xx keep the short transient bench and no longer record heuristic hits. cooldownDecisionForError and fusion's decision sites thread the error kind through; the Ollama-Cloud protection (real 429s on opaque-quota providers escalating) is preserved and now tested. - Locality exemption: a key whose base_url points at loopback/RFC1918 (new sync isLoopbackOrPrivateUrl in url-guard — literal IPs and 'localhost' only, no DNS on the hot path) is capped at a 5s bench with source 'heuristic', never the ladder, even on a literal 429 or a Retry-After. Verdict cached per key id (base_url is immutable per row). - Ladder decay: cooldownHits now clears on a successful request, like nullLimitHits — a served request proves the quota is alive, so the next failure starts the ladder over instead of inheriting up-to-24h steps from stale hits. Escalation while a quota is genuinely spent is untouched (no successes can occur then), and the documented ladder contract keeps its coverage. 17 new tests (red-green via patch method); full suite 1369 passing. Fixes #592. Co-Authored-By: Claude Opus 5 --- server/src/__tests__/lib/url-guard.test.ts | 33 +++++- .../ratelimit-cooldown-error-kind.test.ts | 109 ++++++++++++++++++ .../services/ratelimit-local-endpoint.test.ts | 96 +++++++++++++++ server/src/lib/error-classify.ts | 20 ++++ server/src/lib/fallback-loop.ts | 5 + server/src/lib/url-guard.ts | 29 +++++ server/src/services/fusion.ts | 6 +- server/src/services/ratelimit.ts | 79 ++++++++++++- 8 files changed, 371 insertions(+), 6 deletions(-) create mode 100644 server/src/__tests__/services/ratelimit-cooldown-error-kind.test.ts create mode 100644 server/src/__tests__/services/ratelimit-local-endpoint.test.ts diff --git a/server/src/__tests__/lib/url-guard.test.ts b/server/src/__tests__/lib/url-guard.test.ts index 05bdc46d8..e8f0d9c88 100644 --- a/server/src/__tests__/lib/url-guard.test.ts +++ b/server/src/__tests__/lib/url-guard.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, afterEach } from 'vitest'; -import { classifyIp, assessProviderUrl } from '../../lib/url-guard.js'; +import { classifyIp, assessProviderUrl, isLoopbackOrPrivateUrl } from '../../lib/url-guard.js'; // SSRF guard for user-supplied custom-provider base URLs (#440). Metadata and // link-local targets must never be reachable; loopback/private stay allowed by @@ -140,3 +140,34 @@ describe('assessProviderUrl', () => { expect(verdict.allowed).toBe(true); }); }); + +describe('isLoopbackOrPrivateUrl (#592 local-endpoint cooldown exemption)', () => { + it('true for loopback and localhost forms', () => { + expect(isLoopbackOrPrivateUrl('http://127.0.0.1:11434/v1')).toBe(true); + expect(isLoopbackOrPrivateUrl('http://localhost:11434/v1')).toBe(true); + expect(isLoopbackOrPrivateUrl('http://ollama.localhost/v1')).toBe(true); + expect(isLoopbackOrPrivateUrl('http://[::1]:8080/v1')).toBe(true); + expect(isLoopbackOrPrivateUrl('http://0.0.0.0:8080/v1')).toBe(true); + }); + + it('true for RFC1918 / ULA literal addresses', () => { + expect(isLoopbackOrPrivateUrl('http://192.168.1.50:8080/v1')).toBe(true); + expect(isLoopbackOrPrivateUrl('http://10.0.0.7:11434/v1')).toBe(true); + expect(isLoopbackOrPrivateUrl('http://172.16.4.2:5000/v1')).toBe(true); + expect(isLoopbackOrPrivateUrl('http://[fd12:3456::1]:8080/v1')).toBe(true); + }); + + it('false for public addresses and hostnames (no DNS — pragmatically non-local)', () => { + expect(isLoopbackOrPrivateUrl('http://203.0.113.10/v1')).toBe(false); + expect(isLoopbackOrPrivateUrl('https://api.example.com/v1')).toBe(false); + expect(isLoopbackOrPrivateUrl('http://my-lan-box.local:11434/v1')).toBe(false); // mDNS: undecidable without DNS + expect(isLoopbackOrPrivateUrl('https://ollama.com/v1')).toBe(false); // Ollama CLOUD stays protected + }); + + it('false for null/empty/garbage input', () => { + expect(isLoopbackOrPrivateUrl(null)).toBe(false); + expect(isLoopbackOrPrivateUrl(undefined)).toBe(false); + expect(isLoopbackOrPrivateUrl('')).toBe(false); + expect(isLoopbackOrPrivateUrl('not a url')).toBe(false); + }); +}); diff --git a/server/src/__tests__/services/ratelimit-cooldown-error-kind.test.ts b/server/src/__tests__/services/ratelimit-cooldown-error-kind.test.ts new file mode 100644 index 000000000..8a94dd053 --- /dev/null +++ b/server/src/__tests__/services/ratelimit-cooldown-error-kind.test.ts @@ -0,0 +1,109 @@ +import { describe, it, expect, beforeAll, beforeEach } from 'vitest'; + +// #592: the null-limits exhaustion heuristic (getCooldownDecisionForLimit) must +// only fire on an actual provider quota signal (a real 429 / rate-limit-classified +// error). Before the fix, ANY retryable failure — a timeout on a slow local +// generation, a 5xx blip — fed the "2+ hits in an hour = daily-exhausted" +// counter and escalated the bench 2m→10m→1h→24h, 429-ing every request on what +// is often the user's only route. Also covers the cooldownHits ladder decay on +// success: two back-to-back failures used to keep their ladder step for 24h +// even after successful requests proved the quota alive. + +import { initDb } from '../../db/index.js'; +import { cooldownDecisionForError } from '../../lib/fallback-loop.js'; +import { + getNextCooldownDuration, + recordRequest, + recentHitCount, +} from '../../services/ratelimit.js'; +import type { RouteResult } from '../../services/router.js'; + +const MINUTE = 60_000; +const TRANSIENT = 90_000; + +// Distinct keyId AND modelDbId per fake route: the cooldown/ladder maps are +// module-global, so shared ids would leak state across tests. +let keySeq = 640_000; +function nullLimitRoute(): RouteResult { + const n = ++keySeq; + return { + provider: {} as any, modelId: `null-limit-model-${n}`, modelDbId: 592_000 + n, + apiKey: 'k', keyId: n, platform: 'ollama', displayName: 'Null-Limit Model', + rpdLimit: null, tpdLimit: null, + }; +} + +const timeoutErr = () => Object.assign( + new Error('Request timeout after 120000ms'), { name: 'TimeoutError' }, +); +const serverErr = () => Object.assign( + new Error('Ollama API error 500: Internal Server Error'), { status: 500 }, +); +const real429 = () => Object.assign( + new Error('429 Too Many Requests'), { status: 429 }, +); + +beforeAll(() => { + process.env.ENCRYPTION_KEY = '0'.repeat(64); + initDb(':memory:'); +}); + +let route: RouteResult; +beforeEach(() => { route = nullLimitRoute(); }); + +describe('null-limits heuristic only fires on quota signals (#592)', () => { + it('repeated timeouts stay on the short transient bench — no ladder', () => { + for (let i = 0; i < 4; i++) { + const decision = cooldownDecisionForError(route, timeoutErr()); + expect(decision.durationMs).toBe(TRANSIENT); + expect(decision.source).toBe('heuristic'); + } + // No hits recorded → the heuristic counter never sees a timeout. + expect(recentHitCount(route.platform, route.modelId, route.keyId, Date.now())).toBe(0); + }); + + it('repeated 5xx stay on the short transient bench — no ladder', () => { + for (let i = 0; i < 4; i++) { + expect(cooldownDecisionForError(route, serverErr()).durationMs).toBe(TRANSIENT); + } + expect(recentHitCount(route.platform, route.modelId, route.keyId, Date.now())).toBe(0); + }); + + it('a timeout does not inherit escalation started by real 429s', () => { + expect(cooldownDecisionForError(route, real429()).durationMs).toBe(TRANSIENT); + // 2nd real 429 crosses the heuristic threshold → ladder (2m). + expect(cooldownDecisionForError(route, real429()).durationMs).toBe(2 * MINUTE); + // A timeout right after must NOT climb to the next ladder step (10m): + // it is not a quota signal, so it neither records a hit nor escalates. + expect(cooldownDecisionForError(route, timeoutErr()).durationMs).toBe(TRANSIENT); + // Hit count still only reflects the two genuine 429s. + expect(recentHitCount(route.platform, route.modelId, route.keyId, Date.now())).toBe(2); + }); + + it('real 429s still escalate — the Ollama-Cloud opaque-quota protection holds', () => { + // 1st 429 → transient (no signal yet), then the ladder: 2m, 10m, 1h. + expect(cooldownDecisionForError(route, real429()).durationMs).toBe(TRANSIENT); + expect(cooldownDecisionForError(route, real429()).durationMs).toBe(2 * MINUTE); + expect(cooldownDecisionForError(route, real429()).durationMs).toBe(10 * MINUTE); + expect(cooldownDecisionForError(route, real429()).durationMs).toBe(60 * MINUTE); + }); +}); + +describe('cooldownHits ladder decays on success (#592)', () => { + it('a successful request resets the escalation ladder', () => { + const { platform, modelId, keyId } = nullLimitRoute(); + expect(getNextCooldownDuration(platform, modelId, keyId)).toBe(2 * MINUTE); + expect(getNextCooldownDuration(platform, modelId, keyId)).toBe(10 * MINUTE); + // A served request proves the quota is alive — ladder starts over. + recordRequest(platform, modelId, keyId); + expect(getNextCooldownDuration(platform, modelId, keyId)).toBe(2 * MINUTE); + }); + + it('without a success the ladder keeps escalating (documented contract intact)', () => { + const { platform, modelId, keyId } = nullLimitRoute(); + expect(getNextCooldownDuration(platform, modelId, keyId)).toBe(2 * MINUTE); + expect(getNextCooldownDuration(platform, modelId, keyId)).toBe(10 * MINUTE); + expect(getNextCooldownDuration(platform, modelId, keyId)).toBe(60 * MINUTE); + expect(getNextCooldownDuration(platform, modelId, keyId)).toBe(24 * 60 * MINUTE); + }); +}); diff --git a/server/src/__tests__/services/ratelimit-local-endpoint.test.ts b/server/src/__tests__/services/ratelimit-local-endpoint.test.ts new file mode 100644 index 000000000..4761339e7 --- /dev/null +++ b/server/src/__tests__/services/ratelimit-local-endpoint.test.ts @@ -0,0 +1,96 @@ +import { describe, it, expect, beforeAll } from 'vitest'; + +// #592 locality exemption: a key whose base_url points at loopback or an +// RFC1918/private address is a LOCAL inference server (Ollama / llama.cpp / +// LM Studio registered through the 'custom' platform). It has no provider +// quota, so neither the 90s transient bench nor the 2m→24h escalation ladder +// ever applies — the bench is capped at a few seconds, even for a literal 429 +// (local servers emit those under concurrent load) and even when a Retry-After +// is attached. Non-local keys (public IPs/hostnames, built-in platforms with +// base_url NULL) keep the full pre-existing behavior. + +import { initDb, getDb } from '../../db/index.js'; +import { cooldownDecisionForError } from '../../lib/fallback-loop.js'; +import type { RouteResult } from '../../services/router.js'; + +const MINUTE = 60_000; +const TRANSIENT = 90_000; +const LOCAL_CAP_MS = 5_000; + +beforeAll(() => { + process.env.ENCRYPTION_KEY = '0'.repeat(64); + initDb(':memory:'); +}); + +// Register a custom-platform key row carrying the endpoint, like +// routes/keys.ts POST /custom does, and return its real DB id. +function insertKey(baseUrl: string | null): number { + const r = getDb().prepare(` + INSERT INTO api_keys (platform, label, encrypted_key, iv, auth_tag, status, enabled, base_url) + VALUES ('custom', 'test-endpoint', 'enc', 'iv', 'tag', 'valid', 1, ?) + `).run(baseUrl); + return Number(r.lastInsertRowid); +} + +let modelSeq = 0; +function routeForKey(keyId: number): RouteResult { + const n = ++modelSeq; + return { + provider: {} as any, modelId: `local-model-${n}`, modelDbId: 593_000 + n, + apiKey: 'k', keyId, platform: 'custom', displayName: 'Local Model', + rpdLimit: null, tpdLimit: null, + }; +} + +const real429 = () => Object.assign(new Error('429 Too Many Requests'), { status: 429 }); +const timeoutErr = () => Object.assign(new Error('Request timeout after 120000ms'), { name: 'TimeoutError' }); + +describe('local-endpoint cooldown cap (#592)', () => { + it('loopback base_url: capped short bench even on repeated real 429s', () => { + const route = routeForKey(insertKey('http://127.0.0.1:11434/v1')); + for (let i = 0; i < 4; i++) { + const decision = cooldownDecisionForError(route, real429()); + expect(decision.durationMs).toBe(LOCAL_CAP_MS); + expect(decision.source).toBe('heuristic'); + } + }); + + it('localhost hostname counts as loopback without DNS', () => { + const route = routeForKey(insertKey('http://localhost:11434/v1')); + expect(cooldownDecisionForError(route, real429()).durationMs).toBe(LOCAL_CAP_MS); + expect(cooldownDecisionForError(route, timeoutErr()).durationMs).toBe(LOCAL_CAP_MS); + }); + + it('RFC1918 base_url (LAN box): capped the same way', () => { + for (const url of ['http://192.168.1.50:8080/v1', 'http://10.0.0.7:11434/v1', 'http://172.16.4.2:5000/v1']) { + const route = routeForKey(insertKey(url)); + expect(cooldownDecisionForError(route, real429()).durationMs).toBe(LOCAL_CAP_MS); + expect(cooldownDecisionForError(route, real429()).durationMs).toBe(LOCAL_CAP_MS); + } + }); + + it('ignores an upstream Retry-After on a local endpoint', () => { + const route = routeForKey(insertKey('http://127.0.0.1:8080/v1')); + const err = Object.assign(new Error('429 Too Many Requests'), { status: 429, retryAfterMs: 5 * MINUTE }); + expect(cooldownDecisionForError(route, err).durationMs).toBe(LOCAL_CAP_MS); + }); + + it('public-IP base_url is NOT exempt: transient then ladder on real 429s', () => { + const route = routeForKey(insertKey('http://203.0.113.10/v1')); + expect(cooldownDecisionForError(route, real429()).durationMs).toBe(TRANSIENT); + expect(cooldownDecisionForError(route, real429()).durationMs).toBe(2 * MINUTE); + expect(cooldownDecisionForError(route, real429()).durationMs).toBe(10 * MINUTE); + }); + + it('non-literal hostname is treated as non-local (no DNS on the hot path)', () => { + const route = routeForKey(insertKey('https://inference.example.com/v1')); + expect(cooldownDecisionForError(route, real429()).durationMs).toBe(TRANSIENT); + expect(cooldownDecisionForError(route, real429()).durationMs).toBe(2 * MINUTE); + }); + + it('a key with no base_url (built-in platform shape) keeps full behavior', () => { + const route = routeForKey(insertKey(null)); + expect(cooldownDecisionForError(route, real429()).durationMs).toBe(TRANSIENT); + expect(cooldownDecisionForError(route, real429()).durationMs).toBe(2 * MINUTE); + }); +}); diff --git a/server/src/lib/error-classify.ts b/server/src/lib/error-classify.ts index 6208ebfb3..b1c020057 100644 --- a/server/src/lib/error-classify.ts +++ b/server/src/lib/error-classify.ts @@ -96,6 +96,26 @@ export function isRetryableError(err: any): boolean { || msg.includes('unparseable inline tool-call dialect'); } +// A genuine provider QUOTA signal: a structured 429 or rate-limit/quota wording. +// Distinct from the much broader isRetryableError: timeouts, 5xx, transport +// failures and dead-turn classes are retryable but say nothing about quotas. +// The null-limits exhaustion heuristic in getCooldownDecisionForLimit (services/ +// ratelimit.ts) keys off this: only an actual quota signal may feed its +// "effectively daily-exhausted" escalation ladder. Before this split, a slow +// local Ollama route timing out twice classified retryable → recorded as a +// null-limit "429" → escalated 2m→10m→1h→24h and 429'd every request while the +// server was merely busy generating (#592). +// Mirrors the other classifiers: trust the structured status when the adapter +// attached one (providerHttpError sets err.status everywhere); fall back to +// message markers only when there is no status. +export function isRateLimitSignal(err: any): boolean { + const status = typeof err?.status === 'number' ? err.status : 0; + if (status !== 0) return status === 429; + const msg = (err?.message ?? '').toLowerCase(); + return msg.includes('429') || msg.includes('rate limit') || msg.includes('too many requests') + || msg.includes('quota') || msg.includes('resource_exhausted'); +} + // ── Client-caused aborts ───────────────────────────────────────────────────── // When the gateway's OWN client hangs up, the proxy surfaces abort the composed // fetch signal with this marked error, and undici rejects the in-flight fetch / diff --git a/server/src/lib/fallback-loop.ts b/server/src/lib/fallback-loop.ts index f02b90b6d..748d1be10 100644 --- a/server/src/lib/fallback-loop.ts +++ b/server/src/lib/fallback-loop.ts @@ -30,6 +30,7 @@ import { } from '../services/ratelimit.js'; import { isRetryableError, + isRateLimitSignal, isKeyAuthError, isClientAbortError, isDailyQuotaExhaustedError, @@ -142,6 +143,10 @@ export function cooldownDecisionForError(route: RouteResult, err: any): Cooldown route.keyId, { rpd: route.rpdLimit, tpd: route.tpdLimit }, err?.retryAfterMs, + // Only a real 429/rate-limit error may feed the null-limits exhaustion + // heuristic; a timeout or 5xx is retryable but says nothing about quota, + // so it stays on the short transient bench instead of the ladder (#592). + { quotaSignal: isRateLimitSignal(err) }, ); } diff --git a/server/src/lib/url-guard.ts b/server/src/lib/url-guard.ts index 6dd24d56b..df41f0325 100644 --- a/server/src/lib/url-guard.ts +++ b/server/src/lib/url-guard.ts @@ -131,6 +131,35 @@ export function classifyIp(ip: string): AddressClass { return 'public'; } +/** + * Synchronous locality check for a stored provider base_url: true when the URL + * points at THIS machine or the LAN (loopback, RFC1918/ULA private, 'localhost'). + * Used by the rate limiter to exempt local inference servers (Ollama/llama.cpp/ + * LM Studio via the 'custom' platform) from cloud-quota cooldown ladders (#592): + * a local box has no quota, so long benches only strand the user's one route. + * + * Deliberately DNS-free so it can sit on the cooldown hot path: literal IPs and + * 'localhost'/'*.localhost' are decidable synchronously; any other hostname + * (LAN mDNS names included) is pragmatically treated as NON-local — the worst + * case there is the pre-existing conservative bench, never a wrong exemption. + */ +export function isLoopbackOrPrivateUrl(rawUrl: string | null | undefined): boolean { + if (!rawUrl) return false; + let url: URL; + try { + url = new URL(rawUrl); + } catch { + return false; + } + const hostname = url.hostname.replace(/^\[|\]$/g, '').toLowerCase(); + if (hostname === 'localhost' || hostname.endsWith('.localhost')) return true; + if (net.isIP(hostname)) { + const cls = classifyIp(hostname); + return cls === 'loopback' || cls === 'private'; + } + return false; +} + export interface UrlAssessment { allowed: boolean; reason?: string; diff --git a/server/src/services/fusion.ts b/server/src/services/fusion.ts index 8cb2627c2..d543baf86 100644 --- a/server/src/services/fusion.ts +++ b/server/src/services/fusion.ts @@ -11,7 +11,7 @@ import { } from './ratelimit.js'; import { logRequest } from '../lib/request-log.js'; import { - isRetryableError, isPaymentRequiredError, + isRetryableError, isRateLimitSignal, isPaymentRequiredError, isModelNotFoundError, isModelAccessForbiddenError, } from '../lib/error-classify.js'; import { contentToString } from '../lib/content.js'; @@ -265,7 +265,7 @@ async function runModelCall( ? { durationMs: PAYMENT_REQUIRED_COOLDOWN_MS, source: 'credit' as const } : isModelAccessForbiddenError(err) ? { durationMs: MODEL_FORBIDDEN_COOLDOWN_MS, source: 'tier' as const } - : getCooldownDecisionForLimit(route.platform, route.modelId, route.keyId, { rpd: route.rpdLimit, tpd: route.tpdLimit }, err.retryAfterMs); + : getCooldownDecisionForLimit(route.platform, route.modelId, route.keyId, { rpd: route.rpdLimit, tpd: route.tpdLimit }, err.retryAfterMs, { quotaSignal: isRateLimitSignal(err) }); setCooldown(route.platform, route.modelId, route.keyId, decision.durationMs, decision.source); recordRateLimitHit(route.modelDbId); continue; @@ -355,7 +355,7 @@ async function runJudgeStreaming( ? { durationMs: PAYMENT_REQUIRED_COOLDOWN_MS, source: 'credit' as const } : isModelAccessForbiddenError(err) ? { durationMs: MODEL_FORBIDDEN_COOLDOWN_MS, source: 'tier' as const } - : getCooldownDecisionForLimit(route.platform, route.modelId, route.keyId, { rpd: route.rpdLimit, tpd: route.tpdLimit }, err.retryAfterMs); + : getCooldownDecisionForLimit(route.platform, route.modelId, route.keyId, { rpd: route.rpdLimit, tpd: route.tpdLimit }, err.retryAfterMs, { quotaSignal: isRateLimitSignal(err) }); setCooldown(route.platform, route.modelId, route.keyId, decision.durationMs, decision.source); recordRateLimitHit(route.modelDbId); continue; diff --git a/server/src/services/ratelimit.ts b/server/src/services/ratelimit.ts index b8fc48bc5..aead6fd7a 100644 --- a/server/src/services/ratelimit.ts +++ b/server/src/services/ratelimit.ts @@ -1,6 +1,7 @@ // Sliding window rate limit tracker with SQLite persistence. import { getDb } from '../db/index.js'; +import { isLoopbackOrPrivateUrl } from '../lib/url-guard.js'; interface Window { timestamps: number[]; @@ -578,6 +579,7 @@ export function recordRequest(platform: string, modelId: string, keyId: number) recordUsage(platform, modelId, keyId, 'request', 0, now); clearNullLimitHits(platform, modelId, keyId); + clearCooldownHits(platform, modelId, keyId); } export function recordTokens( @@ -621,6 +623,12 @@ export type CooldownSource = 'heuristic' | 'authoritative' | 'credit' | 'tier'; // the 2-minute cooldown 20 times per request and consuming every fallback slot. // In-memory only — state resets on restart, which is fine (a clean restart // will re-escalate on the next 429 if the quota is genuinely exhausted). +// A successful request clears the counter (recordRequest → clearCooldownHits), +// same reversibility contract as nullLimitHits: a served request proves the +// quota is NOT exhausted right now, so the next failure starts the ladder over +// instead of inheriting up-to-24h steps from stale hits. While a daily quota is +// genuinely spent no successes can occur (the counters/cooldowns gate routing), +// so the intended escalation-across-the-day behavior is untouched. const cooldownHits = new Map(); // key -> timestamps of recent cooldown set events const HOUR = 60 * MINUTE; const COOLDOWN_DURATIONS = [ @@ -640,6 +648,10 @@ export function getNextCooldownDuration(platform: string, modelId: string, keyId return COOLDOWN_DURATIONS[idx]!; } +function clearCooldownHits(platform: string, modelId: string, keyId: number): void { + cooldownHits.delete(`${platform}:${modelId}:${keyId}`); +} + // Short cooldown for a transient (per-minute) 429 — recovers within ~one window. const TRANSIENT_COOLDOWN_MS = 90 * 1000; @@ -719,14 +731,63 @@ export interface CooldownDecision { source: CooldownSource; } +// ── Local-endpoint exemption (#592) ────────────────────────────────────────── +// A key whose base_url points at this machine or the LAN (custom-platform +// Ollama / llama.cpp / LM Studio) has no provider quota to protect: every +// cooldown longer than a few seconds just strands what is usually the user's +// ONLY route to that model, turning a slow local generation (timeout) into an +// "All models exhausted" 429. Such keys are capped at this short bench and +// never enter the escalation ladder. Source stays 'heuristic' so the +// cooldown-probe recovery job may clear even that early. +export const LOCAL_ENDPOINT_COOLDOWN_MS = 5_000; + +// base_url is immutable per key id (routes/keys.ts matches custom rows ON +// base_url — a new endpoint gets a new row, #212), so the verdict can be +// cached for the life of the process. Built-in platforms have base_url NULL +// → non-local. A DB that isn't ready yet is NOT cached, so a later call can +// still decide properly. +const keyLocality = new Map(); // keyId -> is local endpoint + +export function isLocalEndpointKey(keyId: number): boolean { + const cached = keyLocality.get(keyId); + if (cached !== undefined) return cached; + const baseUrl = withDb(db => { + const row = db.prepare('SELECT base_url FROM api_keys WHERE id = ?') + .get(keyId) as { base_url: string | null } | undefined; + return row?.base_url ?? ''; + }); + if (baseUrl === undefined) return false; // DB not ready — don't cache + const local = isLoopbackOrPrivateUrl(baseUrl || null); + keyLocality.set(keyId, local); + return local; +} + +/** Test hook: the locality verdict is cached per key id for the process + * lifetime (see above); tests that rewrite api_keys rows need a reset. */ +export function resetKeyLocalityCache(): void { + keyLocality.clear(); +} + +export interface CooldownLimitOptions { + /** Whether the triggering failure is an actual provider quota signal (a real + * 429 / rate-limit-classified error — see isRateLimitSignal in + * lib/error-classify.ts). Timeouts, 5xx and transport errors are retryable + * but carry no quota information, so they must not feed the null-limits + * exhaustion heuristic below (#592) — they get the short transient bench. + * Defaults to true: legacy callers without error context (fusion's + * empty-completion benches) keep their pre-#592 behavior. */ + quotaSignal?: boolean; +} + export function getCooldownDurationForLimit( platform: string, modelId: string, keyId: number, limits: { rpd: number | null; tpd: number | null }, retryAfterMs?: number | null, + opts?: CooldownLimitOptions, ): number { - return getCooldownDecisionForLimit(platform, modelId, keyId, limits, retryAfterMs).durationMs; + return getCooldownDecisionForLimit(platform, modelId, keyId, limits, retryAfterMs, opts).durationMs; } /** @@ -743,7 +804,18 @@ export function getCooldownDecisionForLimit( keyId: number, limits: { rpd: number | null; tpd: number | null }, retryAfterMs?: number | null, + opts?: CooldownLimitOptions, ): CooldownDecision { + // Local inference endpoint (loopback/RFC1918 base_url): no quota exists, so + // neither the transient 90s bench nor the escalation ladder applies — a + // failure there means "busy right now", and a few seconds is all the pool + // needs before retrying the user's (usually only) local route. Applies even + // to a literal 429 (local servers emit them under concurrent load) and + // ignores any Retry-After: both describe momentary busyness, not a quota. + if (isLocalEndpointKey(keyId)) { + return { durationMs: LOCAL_ENDPOINT_COOLDOWN_MS, source: 'heuristic' }; + } + const quotaSignal = opts?.quotaSignal ?? true; const now = Date.now(); const rpdExhausted = limits.rpd !== null && requestCount(platform, modelId, keyId, DAY, now) >= limits.rpd; @@ -753,9 +825,12 @@ export function getCooldownDecisionForLimit( // last hour is treated as effectively daily-exhausted. This unsticks // providers that publish no daily cap (ollama, cloudflare, etc.) from the // 90s-cooldown-loop without requiring operator-side limit seeding. + // Gated on quotaSignal (#592): only a REAL 429/rate-limit error is evidence + // of quota exhaustion. Timeouts and 5xx used to feed this counter too, so + // two slow generations on a null-limits route escalated 2m→10m→…→24h. const unknownLimits = limits.rpd === null && limits.tpd === null; let heuristicallyExhausted = false; - if (unknownLimits) { + if (unknownLimits && quotaSignal) { // The current hit is recorded first so the threshold can be reached across // consecutive 429s, but only for providers where counters cannot decide. recordNullLimitHit(platform, modelId, keyId, now); From 9367691f4d3769923d163e3545fe8b4e40e6918b Mon Sep 17 00:00:00 2001 From: Tashfeen Date: Sun, 26 Jul 2026 12:39:31 +0100 Subject: [PATCH 4/5] Served-model observability guard: detect upstream model drift (#534) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The proxy overwrites the upstream `model` field with the routed id before the caller sees it (the gateway contract — providers like Reka return placeholders such as "default"). That overwrite also destroyed the only evidence when a provider silently serves a DIFFERENT model than requested. Nothing in the codebase detected served != requested. - Capture the raw upstream model BEFORE the overwrite, on both the non-streaming path (result.model) and the streaming path (first frame that carries one). Covers openai-compat, cohere, and cloudflare, whose adapters pass the upstream body through; google/aihorde synthesize their responses with the routed id, so they carry no upstream signal. - lib/served-model.ts compares served vs routed ignoring cosmetic spelling (case, provider/org prefix, ":free"-style tier suffix, separator runs) and placeholders ("default"), warn-logs `[ServedModel] platform=... requested=... served=...` once per (platform, requested, served) tuple per process, and returns the raw served id for persistence. - New nullable requests.served_model column (migration 20260726_000005_request_served_model) written via lib/request-log.ts only when the served model genuinely differs — NULL in the healthy case keeps the table small and drifted rows trivially queryable. - Per-request analytics detail payload now includes servedModel. The response contract is unchanged: clients still see the routed model in bodies and streamed frames. Tests: 17 new (unit normalization/log-cap + integration for both paths, cosmetic equality, NULL column, contract); full suite 1369 passing, zero regressions from the 1352 baseline. Served-model drift detection for #534. Co-Authored-By: Claude Opus 5 --- .../__tests__/db/migrate/roundtrip.test.ts | 2 + server/src/__tests__/lib/served-model.test.ts | 99 ++++++++ .../routes/proxy-served-model.test.ts | 220 ++++++++++++++++++ server/src/db/migrate/defaults.ts | 3 + .../20260726_000005_request_served_model.ts | 35 +++ server/src/lib/request-log.ts | 11 +- server/src/lib/served-model.ts | Bin 0 -> 4219 bytes server/src/routes/analytics.ts | 5 +- server/src/routes/proxy.ts | 24 +- 9 files changed, 393 insertions(+), 6 deletions(-) create mode 100644 server/src/__tests__/lib/served-model.test.ts create mode 100644 server/src/__tests__/routes/proxy-served-model.test.ts create mode 100644 server/src/db/migrations/20260726_000005_request_served_model.ts create mode 100644 server/src/lib/served-model.ts diff --git a/server/src/__tests__/db/migrate/roundtrip.test.ts b/server/src/__tests__/db/migrate/roundtrip.test.ts index 990b6b15a..c1a93c0dc 100644 --- a/server/src/__tests__/db/migrate/roundtrip.test.ts +++ b/server/src/__tests__/db/migrate/roundtrip.test.ts @@ -17,6 +17,7 @@ const COOLDOWN_PROBE_PROVENANCE_FILENAME = '20260726_000001_cooldown_probe_prove const REQUEST_ATTEMPTS_FILENAME = '20260726_000002_request_attempts.ts'; const MODEL_SOURCE_PROVENANCE_FILENAME = '20260726_000003_model_source_provenance.ts'; const MEDIA_MODEL_META_FILENAME = '20260726_000004_media_model_meta.ts'; +const REQUEST_SERVED_MODEL_FILENAME = '20260726_000005_request_served_model.ts'; interface SchemaRow { type: string; @@ -80,6 +81,7 @@ describe('migration round trip', () => { REQUEST_ATTEMPTS_FILENAME, MODEL_SOURCE_PROVENANCE_FILENAME, MEDIA_MODEL_META_FILENAME, + REQUEST_SERVED_MODEL_FILENAME, ]); } finally { db.close(); diff --git a/server/src/__tests__/lib/served-model.test.ts b/server/src/__tests__/lib/served-model.test.ts new file mode 100644 index 000000000..2011a89ec --- /dev/null +++ b/server/src/__tests__/lib/served-model.test.ts @@ -0,0 +1,99 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { + normalizeModelIdForDrift, + observeServedModel, + resetServedModelObservations, +} from '../../lib/served-model.js'; + +// Served-model drift guard (#534 follow-up): the proxy overwrites the +// upstream `model` field before the caller sees it, which also destroys the +// only evidence when a provider silently serves a DIFFERENT model than the +// one requested (kilo-auto/small serving Gemma, etc.). This module compares +// the captured raw upstream value against the routed id, ignoring cosmetic +// spelling differences, and reports genuine drift once per tuple. + +describe('normalizeModelIdForDrift', () => { + it('ignores case differences', () => { + expect(normalizeModelIdForDrift('Llama-3.3-70B-Versatile')) + .toBe(normalizeModelIdForDrift('llama-3.3-70b-versatile')); + }); + + it('ignores a provider/org prefix', () => { + expect(normalizeModelIdForDrift('meta-llama/llama-3.3-70b-versatile')) + .toBe(normalizeModelIdForDrift('llama-3.3-70b-versatile')); + // Multi-segment namespaces too (@cf/meta/...). + expect(normalizeModelIdForDrift('@cf/meta/llama-3.1-8b-instruct')) + .toBe(normalizeModelIdForDrift('llama-3.1-8b-instruct')); + }); + + it('ignores a ":free"-style pricing-tier suffix', () => { + expect(normalizeModelIdForDrift('gemma-3-27b-it:free')) + .toBe(normalizeModelIdForDrift('gemma-3-27b-it')); + }); + + it('ignores separator spelling (underscore vs hyphen vs space)', () => { + expect(normalizeModelIdForDrift('qwen3_coder 480b')) + .toBe(normalizeModelIdForDrift('qwen3-coder-480b')); + }); + + it('keeps genuinely different ids distinct', () => { + expect(normalizeModelIdForDrift('kilo-auto/small')) + .not.toBe(normalizeModelIdForDrift('google/gemma-3-27b-it')); + expect(normalizeModelIdForDrift('llama-3.3-70b-versatile')) + .not.toBe(normalizeModelIdForDrift('llama-3.1-8b-instant')); + }); +}); + +describe('observeServedModel', () => { + beforeEach(() => { + resetServedModelObservations(); + vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('returns the raw served id and warns on genuine drift', () => { + const served = observeServedModel({ + platform: 'kilo', requestedModel: 'kilo-auto/small', servedModel: 'google/gemma-3-27b-it', + }); + expect(served).toBe('google/gemma-3-27b-it'); + expect(console.warn).toHaveBeenCalledTimes(1); + expect(vi.mocked(console.warn).mock.calls[0][0]) + .toContain('[ServedModel] platform=kilo requested=kilo-auto/small served=google/gemma-3-27b-it'); + }); + + it('warns only once per (platform, requested, served) tuple', () => { + for (let i = 0; i < 3; i++) { + observeServedModel({ platform: 'kilo', requestedModel: 'kilo-auto/small', servedModel: 'google/gemma-3-27b-it' }); + } + expect(console.warn).toHaveBeenCalledTimes(1); + // A different tuple gets its own single warn. + observeServedModel({ platform: 'kilo', requestedModel: 'kilo-auto/free', servedModel: 'google/gemma-3-27b-it' }); + expect(console.warn).toHaveBeenCalledTimes(2); + }); + + it('returns null for cosmetically-equal ids (case / prefix / tier suffix)', () => { + expect(observeServedModel({ + platform: 'groq', requestedModel: 'llama-3.3-70b-versatile', servedModel: 'Meta-Llama/Llama-3.3-70B-Versatile:free', + })).toBeNull(); + expect(console.warn).not.toHaveBeenCalled(); + }); + + it('returns null for identical ids', () => { + expect(observeServedModel({ + platform: 'groq', requestedModel: 'llama-3.3-70b-versatile', servedModel: 'llama-3.3-70b-versatile', + })).toBeNull(); + expect(console.warn).not.toHaveBeenCalled(); + }); + + it('returns null for missing or placeholder upstream values', () => { + expect(observeServedModel({ platform: 'reka', requestedModel: 'reka-flash-3', servedModel: null })).toBeNull(); + expect(observeServedModel({ platform: 'reka', requestedModel: 'reka-flash-3', servedModel: undefined })).toBeNull(); + // Reka returns the literal placeholder "default" for every concrete model + // (#568) — it names no model, so it is no evidence of drift. + expect(observeServedModel({ platform: 'reka', requestedModel: 'reka-flash-3', servedModel: 'default' })).toBeNull(); + expect(observeServedModel({ platform: 'reka', requestedModel: 'reka-flash-3', servedModel: ' ' })).toBeNull(); + expect(console.warn).not.toHaveBeenCalled(); + }); +}); diff --git a/server/src/__tests__/routes/proxy-served-model.test.ts b/server/src/__tests__/routes/proxy-served-model.test.ts new file mode 100644 index 000000000..1ad78c266 --- /dev/null +++ b/server/src/__tests__/routes/proxy-served-model.test.ts @@ -0,0 +1,220 @@ +import { describe, it, expect, beforeAll, beforeEach, afterEach, vi } from 'vitest'; +import type { Express } from 'express'; +import { createApp } from '../../app.js'; +import { initDb, getDb, getUnifiedApiKey } from '../../db/index.js'; +import { resetServedModelObservations } from '../../lib/served-model.js'; +import { mintDashboardToken, isGatedApiPath } from '../helpers/auth.js'; + +// Served-model drift guard (#534 follow-up): the proxy overwrites the +// upstream `model` field with the routed id before the caller sees it (the +// gateway contract — unchanged here). These tests assert the raw upstream +// value is captured BEFORE that overwrite, compared against the routed id, +// warn-logged once per tuple, and persisted to requests.served_model when it +// genuinely differs (NULL otherwise), on both the non-streaming and the +// streaming path. + +let dashToken = ''; + +async function request(app: Express, method: string, path: string, body?: any, headers: Record = {}) { + const server = app.listen(0); + const addr = server.address() as any; + const res = await fetch(`http://127.0.0.1:${addr.port}${path}`, { + method, + headers: { + ...(body ? { 'Content-Type': 'application/json' } : {}), + ...(isGatedApiPath(path) && !('Authorization' in headers) ? { Authorization: `Bearer ${dashToken}` } : {}), + ...headers, + }, + body: body ? JSON.stringify(body) : undefined, + }); + const text = await res.text(); + server.close(); + let json: any = null; + try { json = JSON.parse(text); } catch { /* SSE body */ } + return { status: res.status, headers: res.headers, text, body: json }; +} + +function authHeaders() { + return { Authorization: `Bearer ${getUnifiedApiKey()}` }; +} + +/** Parse an SSE response body into JSON frames (excluding [DONE]). */ +function frames(text: string): any[] { + return text.split('\n') + .filter(l => l.startsWith('data: ') && l.trim() !== 'data: [DONE]') + .map(l => JSON.parse(l.slice(6))); +} + +const sse = (...payloads: (object | string)[]) => + payloads.map(p => `data: ${typeof p === 'string' ? p : JSON.stringify(p)}\n\n`).join(''); + +/** Mock the groq upstream. `upstreamModel` is what the provider CLAIMS it served. */ +function mockUpstream(opts: { upstreamModel: string; stream?: boolean }) { + const origFetch = global.fetch; + vi.spyOn(global, 'fetch').mockImplementation(async (url, init) => { + const urlStr = typeof url === 'string' ? url : url.toString(); + if (!urlStr.includes('api.groq.com')) return origFetch(url as any, init as any); + if (opts.stream) { + const chunk = (delta: object, finish: string | null) => ({ + id: 'c1', object: 'chat.completion.chunk', created: 1, model: opts.upstreamModel, + choices: [{ index: 0, delta, finish_reason: finish }], + }); + return new Response( + sse(chunk({ role: 'assistant' }, null), chunk({ content: 'streamed hello' }, null), chunk({}, 'stop'), '[DONE]'), + { status: 200, headers: { 'Content-Type': 'text/event-stream' } }, + ); + } + return new Response(JSON.stringify({ + id: 'chatcmpl-served', object: 'chat.completion', created: 1, model: opts.upstreamModel, + choices: [{ index: 0, message: { role: 'assistant', content: 'hello' }, finish_reason: 'stop' }], + usage: { prompt_tokens: 2, completion_tokens: 1, total_tokens: 3 }, + }), { status: 200, headers: { 'Content-Type': 'application/json' } }); + }); +} + +function lastRequestRow(): any { + return getDb().prepare( + 'SELECT id, model_id, served_model, status FROM requests ORDER BY id DESC LIMIT 1' + ).get(); +} + +function servedModelWarns(): string[] { + return vi.mocked(console.warn).mock.calls + .map(c => String(c[0])) + .filter(m => m.startsWith('[ServedModel]')); +} + +describe('served-model drift guard (#534)', () => { + let app: Express; + let groqModelId: string; + + beforeAll(() => { + process.env.ENCRYPTION_KEY = '0'.repeat(64); + initDb(':memory:'); + app = createApp(); + dashToken = mintDashboardToken(); + groqModelId = (getDb().prepare(` + SELECT m.model_id FROM models m + JOIN fallback_config fc ON fc.model_db_id = m.id + WHERE m.platform = 'groq' AND m.enabled = 1 + ORDER BY fc.priority LIMIT 1 + `).get() as { model_id: string }).model_id; + }); + + beforeEach(async () => { + resetServedModelObservations(); + const db = getDb(); + db.prepare('DELETE FROM api_keys').run(); + db.prepare('DELETE FROM requests').run(); + db.prepare('DELETE FROM rate_limit_cooldowns').run(); + db.prepare('DELETE FROM rate_limit_usage').run(); + const addKey = await request(app, 'POST', '/api/keys', + { platform: 'groq', key: 'gsk_served_model_guard_test', label: 'served-model' }); + expect(addKey.status).toBe(201); + vi.spyOn(console, 'warn'); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('non-streaming: captures, logs, and persists a genuinely different served model — contract unchanged', async () => { + mockUpstream({ upstreamModel: 'gemma-3-27b-it' }); + const { status, body } = await request(app, 'POST', '/v1/chat/completions', { + model: groqModelId, messages: [{ role: 'user', content: 'drift test' }], + }, authHeaders()); + expect(status).toBe(200); + // Contract unchanged: the caller still sees the ROUTED model. + expect(body.model).toBe(groqModelId); + + const row = lastRequestRow(); + expect(row.status).toBe('success'); + expect(row.model_id).toBe(groqModelId); + expect(row.served_model).toBe('gemma-3-27b-it'); + + const warns = servedModelWarns(); + expect(warns).toHaveLength(1); + expect(warns[0]).toContain(`platform=groq requested=${groqModelId} served=gemma-3-27b-it`); + }); + + it('non-streaming: warns once per tuple but persists every drifted row', async () => { + mockUpstream({ upstreamModel: 'gemma-3-27b-it' }); + for (let i = 0; i < 2; i++) { + const { status } = await request(app, 'POST', '/v1/chat/completions', { + model: groqModelId, messages: [{ role: 'user', content: `drift repeat ${i}` }], + }, authHeaders()); + expect(status).toBe(200); + } + expect(servedModelWarns()).toHaveLength(1); + const rows = getDb().prepare( + "SELECT served_model FROM requests WHERE status = 'success' ORDER BY id" + ).all() as any[]; + expect(rows).toHaveLength(2); + for (const r of rows) expect(r.served_model).toBe('gemma-3-27b-it'); + }); + + it('non-streaming: cosmetically-equal id (case + org prefix + :free suffix) does not flag', async () => { + mockUpstream({ upstreamModel: `some-org/${groqModelId}:free`.toUpperCase() }); + const { status, body } = await request(app, 'POST', '/v1/chat/completions', { + model: groqModelId, messages: [{ role: 'user', content: 'cosmetic test' }], + }, authHeaders()); + expect(status).toBe(200); + expect(body.model).toBe(groqModelId); + expect(lastRequestRow().served_model).toBeNull(); + expect(servedModelWarns()).toHaveLength(0); + }); + + it('non-streaming: identical served id stores NULL', async () => { + mockUpstream({ upstreamModel: groqModelId }); + const { status } = await request(app, 'POST', '/v1/chat/completions', { + model: groqModelId, messages: [{ role: 'user', content: 'same id test' }], + }, authHeaders()); + expect(status).toBe(200); + expect(lastRequestRow().served_model).toBeNull(); + expect(servedModelWarns()).toHaveLength(0); + }); + + it('streaming: captures, logs, and persists drift — streamed frames still show the routed model', async () => { + mockUpstream({ upstreamModel: 'mystery-substitute-model', stream: true }); + const r = await request(app, 'POST', '/v1/chat/completions', { + model: groqModelId, stream: true, messages: [{ role: 'user', content: 'stream drift test' }], + }, authHeaders()); + expect(r.status).toBe(200); + const fs = frames(r.text); + expect(fs.length).toBeGreaterThan(0); + // Contract unchanged: every outbound frame carries the routed model. + for (const f of fs) expect(f.model).toBe(groqModelId); + expect(fs.some(f => f.choices?.[0]?.delta?.content?.includes('streamed hello'))).toBe(true); + + const row = lastRequestRow(); + expect(row.status).toBe('success'); + expect(row.served_model).toBe('mystery-substitute-model'); + const warns = servedModelWarns(); + expect(warns).toHaveLength(1); + expect(warns[0]).toContain(`platform=groq requested=${groqModelId} served=mystery-substitute-model`); + }); + + it('streaming: identical served id stores NULL and does not warn', async () => { + mockUpstream({ upstreamModel: groqModelId, stream: true }); + const r = await request(app, 'POST', '/v1/chat/completions', { + model: groqModelId, stream: true, messages: [{ role: 'user', content: 'stream same test' }], + }, authHeaders()); + expect(r.status).toBe(200); + expect(lastRequestRow().served_model).toBeNull(); + expect(servedModelWarns()).toHaveLength(0); + }); + + it('exposes servedModel in the per-request analytics detail payload', async () => { + mockUpstream({ upstreamModel: 'gemma-3-27b-it' }); + const { status } = await request(app, 'POST', '/v1/chat/completions', { + model: groqModelId, messages: [{ role: 'user', content: 'detail test' }], + }, authHeaders()); + expect(status).toBe(200); + const row = lastRequestRow(); + + const detail = await request(app, 'GET', `/api/analytics/requests/${row.id}`); + expect(detail.status).toBe(200); + expect(detail.body.servedModel).toBe('gemma-3-27b-it'); + expect(detail.body.modelId).toBe(groqModelId); + }); +}); diff --git a/server/src/db/migrate/defaults.ts b/server/src/db/migrate/defaults.ts index 633cde823..0cc37b4e6 100644 --- a/server/src/db/migrate/defaults.ts +++ b/server/src/db/migrate/defaults.ts @@ -12,6 +12,7 @@ import * as cooldownProbeProvenance from '../migrations/20260726_000001_cooldown import * as requestAttempts from '../migrations/20260726_000002_request_attempts.js'; import * as modelSourceProvenance from '../migrations/20260726_000003_model_source_provenance.js'; import * as mediaModelMeta from '../migrations/20260726_000004_media_model_meta.js'; +import * as requestServedModel from '../migrations/20260726_000005_request_served_model.js'; export interface MigrationModule { up(db: Db): void; @@ -36,6 +37,7 @@ export const COOLDOWN_PROBE_PROVENANCE_FILENAME = '20260726_000001_cooldown_prob export const REQUEST_ATTEMPTS_FILENAME = '20260726_000002_request_attempts.ts'; export const MODEL_SOURCE_PROVENANCE_FILENAME = '20260726_000003_model_source_provenance.ts'; export const MEDIA_MODEL_META_FILENAME = '20260726_000004_media_model_meta.ts'; +export const REQUEST_SERVED_MODEL_FILENAME = '20260726_000005_request_served_model.ts'; export const DEFAULT_MIGRATIONS: readonly DefaultMigration[] = [ { filename: LEGACY_BASELINE_FILENAME, module: legacyBaseline }, @@ -51,4 +53,5 @@ export const DEFAULT_MIGRATIONS: readonly DefaultMigration[] = [ { filename: REQUEST_ATTEMPTS_FILENAME, module: requestAttempts }, { filename: MODEL_SOURCE_PROVENANCE_FILENAME, module: modelSourceProvenance }, { filename: MEDIA_MODEL_META_FILENAME, module: mediaModelMeta }, + { filename: REQUEST_SERVED_MODEL_FILENAME, module: requestServedModel }, ]; diff --git a/server/src/db/migrations/20260726_000005_request_served_model.ts b/server/src/db/migrations/20260726_000005_request_served_model.ts new file mode 100644 index 000000000..69113b443 --- /dev/null +++ b/server/src/db/migrations/20260726_000005_request_served_model.ts @@ -0,0 +1,35 @@ +// Migration: requests.served_model — upstream-reported model when it drifts +// Created: 2026-07-26 +// +// DOWN: reversible +// +// The proxy overwrites the upstream `model` field with the routed id before +// the caller sees it (the gateway contract), which also destroyed the only +// evidence when a provider silently serves a DIFFERENT model than requested +// (#534 — verified live: kilo-auto/small serving Gemma). The served-model +// guard (lib/served-model.ts) now captures the raw upstream value before the +// overwrite; this column persists it — but ONLY when it genuinely differs +// from the routed model id after cosmetic normalization (case, provider +// prefix, ":free"-style tier suffixes). NULL means "matched (or upstream +// reported nothing usable)", which keeps the column empty in the healthy case +// and makes drifted rows trivially queryable: +// SELECT ... FROM requests WHERE served_model IS NOT NULL; + +import type { Db } from '../types.js'; + +function hasColumn(db: Db, table: string, column: string): boolean { + const columns = db.prepare(`PRAGMA table_info(${table})`).all() as { name: string }[]; + return columns.some((candidate) => candidate.name === column); +} + +export function up(db: Db): void { + if (!hasColumn(db, 'requests', 'served_model')) { + db.prepare('ALTER TABLE requests ADD COLUMN served_model TEXT').run(); + } +} + +export function down(db: Db): void { + if (hasColumn(db, 'requests', 'served_model')) { + db.prepare('ALTER TABLE requests DROP COLUMN served_model').run(); + } +} diff --git a/server/src/lib/request-log.ts b/server/src/lib/request-log.ts index a108e1b7e..03f687edb 100644 --- a/server/src/lib/request-log.ts +++ b/server/src/lib/request-log.ts @@ -55,6 +55,11 @@ export function logRequest( // analytics split pinned vs auto traffic and detect failover overrides // (requested_model set but != model_id). requestedModel: string | null = null, + // The model the UPSTREAM claims it served, ONLY when it genuinely differs + // from the routed model_id after cosmetic normalization (#534 — see + // lib/served-model.ts). NULL when it matches or the provider reported + // nothing usable, so the column stays empty in the healthy case. + servedModel: string | null = null, ) { try { const db = getDb(); @@ -63,9 +68,9 @@ export function logRequest( const client = getClientContext(); const tx = db.transaction(() => { const insert = db.prepare(` - INSERT INTO requests (platform, model_id, key_id, status, input_tokens, output_tokens, latency_ms, error, ttfb_ms, requested_model, client_ip, client_user_agent) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) - `).run(platform, modelId, keyId, status, inputTokens, outputTokens, latencyMs, error, ttfbMs, requestedModel, client.ip, client.userAgent); + INSERT INTO requests (platform, model_id, key_id, status, input_tokens, output_tokens, latency_ms, error, ttfb_ms, requested_model, served_model, client_ip, client_user_agent) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + `).run(platform, modelId, keyId, status, inputTokens, outputTokens, latencyMs, error, ttfbMs, requestedModel, servedModel, client.ip, client.userAgent); // Report the row id back to the fallback loop's attempt trace (if one is // active): the LAST id noted during a loop run is the terminal row the diff --git a/server/src/lib/served-model.ts b/server/src/lib/served-model.ts new file mode 100644 index 0000000000000000000000000000000000000000..aaa83d6a26875957285b3a575f1c06bfae6bda1d GIT binary patch literal 4219 zcmZ`+e{b7H61~6uDW+~vQI179m-g=9)GeG?NsT&D;UC2va)G@fSJJ{Hm$$pLqQ(aL zA?_32C%HGX6eZhn5Lh<3J3IU4y*H!bkj}Kb(fOdXxh^PoW*TVLD3??6?GG=%r>QLp zyBIWeKiwJ*@t@A;nr15Kg<2B!n2C1yt5}k~(Qe^P(4GR;G_?;-tCFrH_7zQyE^=C! zFz5Fi%GoC9oZ{Igr-`0gr)77hib6Z`8qrLk|NZ%2d0aP>!eo@$Dmaw|(w}SVos-_6rz{hbm88+#7yx*m7fB4%!`YDh?9vvziPZiz*Gyr3l z5hYszIn7yBnWn{DSESZ?dQ<2sV1?%@dAvG0e*1QGI{I*a@46L|{!fFrKr=xZ46x3T z`HkMEYg5>PYJwg55}-;bQ_bj|E=%=_!>m_Koz_SaWy?`qCIiCYc3N%&umErapq$P$ zg2yX3wdjh?;cisXXsKAHYCwq)z=5Fz{-2jLu}zgLx1`sjx8u{1yy;Mi`)pY&CzTh} zX3A7PhG;1RvY1(A9bY7~9`yt(e65RupX6pb)s7KGF;tZ!-`ct$C7tQ2F_mtq9u^i% zS^9lGhiCwVQJf}=XUhuM_;iJpuA;)ub_Elv!Gr{~2p~bZyDq_Q3`VhPd7na4 z12kR^1dtT60uK~T8o!2FZbq4^DjRSW2uIjwXHp~1jsIq_g$w{77xp2nH>f=G$8<#S zXL$sKiY);o&*Ri6REt;|-UDjdAG5O%RRz;XEk)Q_~ zILbMvqkU~FNUKw%C@Ry;)Upqj;NAC}B5+o|#~f zHrtJX1%Qn!)#z+)+vprFUYk8dW!EVkR?9Gtufav%xErA_7z9n2KU6TbLQ(1U`1}|0 zP$RSk-uPgOg2Tn6U_}53>R?&GUqm5-1<{LCpc#Ovy7EZ#Mz@9gYl_HlHK&ZQvvwv! zDnl-~q+~#hpk|b8@3R262RWANn(R zkn0@3RtE6k(+6yXpZD8rUZ$PB46U0{rApLLaYk~Y7!sGLP>@`2*c*5~E8#-m8-B90 z2uZqJT7qq}rw)ojm1-zI2QSl?gTL=hwuwuFP22Y$uM+w`1u-CZ6=0>Eg!%HWsXW;! z(-TbvIqD&t19ZyJCH!!{tYMcZ6j}xV?h;x@OG@LM1^rkH2ErRkvV$3*uYDRktq7ps z@uOe$vK2!YOx1PK-k3a~LQ*JAfQuUs=`SOG5CnH37iij4nUJZA{GQF@IDc#15gTe^ zAPaa5@b0gEp)Xqma)X$3Kv+OO|4hAJKgFst!G}Gu3+>)Oc?nR1uV9%^VQSoxlHq>B zw7F#t+^!gKEA_was{d@b%OQC;q>9RNdC%};F0uHpV_^GvJWWt@S!bq{tXKVXKzsf6 zWRD)KH{Ly`7qTOE&dt<5{@Y&;K0P18GJAvWMz6N+MDrJ`5kXo8(+c8bLZU9Mg7JWx z7Skbrz+@R>3V<*rLL9TZT@#m-nDin>9>dD0ZWRzl!DtK3!LKQMDtFij9Vg z?+aVkOVQuq@j#9ahGzsnH>FRvB8b189KIR-GCnyPonF2lAB|2fe?L6A7@YyrN-r>5 zh2&$ehr74)0{_|`+4}^@_@pt6djJz3#4aH{CR4D_lwNbEWE)j9x9mlFIC1tGGeS39 z31p_a!407Vjt4HFs2~ydo6euh1~uo-0`4yWGyw7-98Y7g7mNyJJ7v`@d)7aiH!Va} z+!#0-w;6^%PKgt2^TF1n@|>S0Y?#>S;t+>6Im6?=03lcFP_7=?_qi}v=8Eg%AcIX^453sSsRCnarZFemphnK-H%G%W;M zZRz^_s*8!ouf`_G3$Dl8xDz~79>+;gW_YZO5G=&?pwLd8P_E3+GGJm#q$=49?@ywH4VYUSjKjw#emL4Q4BoW$WvvWU2J#n+Wv1$8;-5 z;)7K<#n70e*lW{nU-Mbf4Zd_|ZVZDG67$}gU~~sUbDo?Ln`~It8f|WK$u|cF)Qe>A z^^rDzBav))PdqIyXZK!*Zac;*@Kg4b&XrH%w)@Sc|L82v@5xqnAI?4JbiGdwt#riUX8Bk`6@K)tuy=1MpX9| z2UEO!b4MSDb2?9zFd8_9e1V4=H=>|mcM%zSkyQL5-o}$t`mb8= e$M-+$(3zh6!jWF>$_->JxWlij(LlJ$p80>5O05k5 literal 0 HcmV?d00001 diff --git a/server/src/routes/analytics.ts b/server/src/routes/analytics.ts index 6a0000e2c..4c907dee8 100644 --- a/server/src/routes/analytics.ts +++ b/server/src/routes/analytics.ts @@ -524,7 +524,7 @@ analyticsRouter.get('/requests/:id', (req: Request, res: Response) => { const db = getDb(); const r = db.prepare(` - SELECT id, platform, model_id, requested_model, request_type, status, + SELECT id, platform, model_id, requested_model, served_model, request_type, status, input_tokens, output_tokens, latency_ms, ttfb_ms, error, client_ip, client_user_agent, strftime('%Y-%m-%dT%H:%M:%SZ', created_at) as created_at_iso @@ -548,6 +548,9 @@ analyticsRouter.get('/requests/:id', (req: Request, res: Response) => { platform: r.platform, modelId: r.model_id, requestedModel: r.requested_model, + // Upstream-reported model when it genuinely differed from the routed + // model_id (#534 served-model drift guard); null in the healthy case. + servedModel: r.served_model, requestType: r.request_type, status: r.status, inputTokens: r.input_tokens, diff --git a/server/src/routes/proxy.ts b/server/src/routes/proxy.ts index e3f21b6e8..ab919fee6 100644 --- a/server/src/routes/proxy.ts +++ b/server/src/routes/proxy.ts @@ -17,6 +17,7 @@ import { getContextHandoffMode, recordIncomingMessages, maybeInjectContextHandof import { isFusionModel, runFusion, fusionConfigSchema, FusionError, FUSION_MODEL_ID } from '../services/fusion.js'; import { isRetryableError, isPaymentRequiredError, isModelNotFoundError, isModelAccessForbiddenError, isClientAbortError, newClientAbortError } from '../lib/error-classify.js'; import { logRequest } from '../lib/request-log.js'; +import { observeServedModel } from '../lib/served-model.js'; import { parseCacheDirective, cacheActive, isCacheableTemperature, computeCacheKey, getCachedResponse, storeCachedResponse } from '../services/cache.js'; import { runFallbackLoop, newFallbackState, recordUpstreamSuccess, exhaustedRetryError, setFallbackHeaders, exhaustionErrorPayload, setExhaustionHeaders, type AttemptRecord } from '../lib/fallback-loop.js'; import { applyTokenBudget, tokenBudgetMessage } from '../lib/guardrails.js'; @@ -1699,6 +1700,11 @@ proxyRouter.post('/chat/completions', async (req: Request, res: Response) => { let upstreamFinish: string | null = null; let usageChunk: unknown = null; let lastMeta: { id?: string; model?: string; created?: number } = {}; + // Raw upstream-reported model, captured off the first frame that + // carries one — BEFORE the per-frame overwrite below destroys it. + // Only evidence when a provider serves a different model than routed + // (#534); compared/persisted on success via observeServedModel. + let upstreamModel: string | null = null; const flushHeaders = () => { if (headerSent) return; @@ -1735,6 +1741,10 @@ proxyRouter.post('/chat/completions', async (req: Request, res: Response) => { // the literal model name "default" even when a concrete model was // requested. Normalize every streamed frame at the proxy boundary // so clients consistently see the model that was actually routed. + const rawChunkModel = (chunk as Record).model; + if (upstreamModel == null && typeof rawChunkModel === 'string' && rawChunkModel.length > 0) { + upstreamModel = rawChunkModel; + } const anyChunk: Record = { ...(chunk as Record), model: route.modelId }; // In-band upstream error frame (observed live: Groq emits @@ -1912,7 +1922,8 @@ proxyRouter.post('/chat/completions', async (req: Request, res: Response) => { inputTokens: estimatedInputTokens + injectedHandoffTokens, outputTokens: totalOutputTokens, }); - logRequest(route.platform, route.modelId, route.keyId, 'success', estimatedInputTokens + injectedHandoffTokens, totalOutputTokens, Date.now() - start, null, ttfbMs, pinnedModelId); + logRequest(route.platform, route.modelId, route.keyId, 'success', estimatedInputTokens + injectedHandoffTokens, totalOutputTokens, Date.now() - start, null, ttfbMs, pinnedModelId, + observeServedModel({ platform: route.platform, requestedModel: route.modelId, servedModel: upstreamModel })); return 'done'; } catch (streamErr: any) { // Client abort mid-stream: the pump's own `if (clientGone) break` @@ -1953,6 +1964,14 @@ proxyRouter.post('/chat/completions', async (req: Request, res: Response) => { quotaContextForRoute(route, 'chat/completions'), ); + // Raw upstream-reported model, captured BEFORE the contract overwrite + // below destroys it — the only evidence when a provider silently + // serves a different model than requested (#534). The OpenAI-compat, + // cohere, and cloudflare adapters pass the upstream body through, so + // result.model here is still the provider's own claim; google/aihorde + // synthesize their responses with the routed id (no upstream signal). + const upstreamModel = typeof result.model === 'string' ? result.model : null; + // Upstream `model` fields are provider-controlled and can be a generic // placeholder such as Reka's "default". The gateway contract exposes // the concrete routed model, consistently across every provider. @@ -2091,7 +2110,8 @@ proxyRouter.post('/chat/completions', async (req: Request, res: Response) => { inputTokens: promptTokens, outputTokens: completionTokens, }); - logRequest(route.platform, route.modelId, route.keyId, 'success', promptTokens, completionTokens, Date.now() - start, null, null, pinnedModelId); + logRequest(route.platform, route.modelId, route.keyId, 'success', promptTokens, completionTokens, Date.now() - start, null, null, pinnedModelId, + observeServedModel({ platform: route.platform, requestedModel: route.modelId, servedModel: upstreamModel })); return 'done'; } }, From abed05a83dd27cc259bc9fbc02080c9ded251ae7 Mon Sep 17 00:00:00 2001 From: Tashfeen Date: Sun, 26 Jul 2026 12:43:06 +0100 Subject: [PATCH 5/5] feat(analytics): failover-ladder drill-down + provider stats UI (#335) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Server: - GET /api/analytics/requests accepts optional status ('success'|'error') and platform query filters; both validated (400 on bad input), applied as bound parameters, and absent filters keep the behavior byte-identical. - New migration 20260726_000005_attempt_error_summary: nullable request_attempts.error_summary. Populated by the fallback loop for every attempt that ended on an error — sanitized through lib/error-redaction.ts (same secret/URL scrubbing as the parent row's error text) and capped at 200 chars via the new summarizeAttemptError(). NULL for ok/committed hops and pre-existing rows. GET /api/analytics/requests/:id now returns errorSummary per attempt. Client (Analytics page): - Recent calls: renders the previously-dropped attemptCount, adds status (All/Success/Errors) + provider filters in the panel header, and makes rows clickable. - Clicking a row opens a request-detail dialog (the app's dialog idiom): parent-row fields plus the failover ladder — ordinal, platform (with its routing.ts color), model, key ordinal, outcome badge, start-offset + duration, and the redacted per-hop error summary when present. - New per-provider breakdown table surfacing the successRate and errorCount the by-platform endpoint already returned (plus p95/TTFT/tok-s/tokens). - All strings added to all six locales (en, es, fr, it, pt-BR, zh-CN). Tests: +7 (filter validation/behavior, error_summary persistence with redaction + cap, detail payload, summarizeAttemptError); full suite 1359 passing, client build clean. Failover-ladder drill-down + provider stats UI — closes the remaining slice of #335. Co-Authored-By: Claude Opus 5 --- client/src/i18n/locales/en.json | 15 +- client/src/i18n/locales/es.json | 15 +- client/src/i18n/locales/fr.json | 15 +- client/src/i18n/locales/it.json | 15 +- client/src/i18n/locales/pt-BR.json | 15 +- client/src/i18n/locales/zh-CN.json | 15 +- client/src/pages/AnalyticsPage.tsx | 300 +++++++++++++++++- .../__tests__/db/migrate/roundtrip.test.ts | 2 + .../src/__tests__/lib/error-redaction.test.ts | 24 +- .../__tests__/lib/request-attempts.test.ts | 36 ++- .../routes/analytics-request-attempts.test.ts | 20 +- .../routes/analytics-requests.test.ts | 46 ++- server/src/db/migrate/defaults.ts | 3 + .../20260726_000005_attempt_error_summary.ts | 33 ++ server/src/lib/attempt-trace.ts | 4 + server/src/lib/error-redaction.ts | 17 + server/src/lib/fallback-loop.ts | 18 +- server/src/lib/request-log.ts | 6 +- server/src/routes/analytics.ts | 36 ++- 19 files changed, 592 insertions(+), 43 deletions(-) create mode 100644 server/src/db/migrations/20260726_000005_attempt_error_summary.ts diff --git a/client/src/i18n/locales/en.json b/client/src/i18n/locales/en.json index 5076013a8..39726d038 100644 --- a/client/src/i18n/locales/en.json +++ b/client/src/i18n/locales/en.json @@ -409,7 +409,20 @@ "recentCalls": "Recent calls", "clientIp": "Client IP", "clientAgent": "Client app", - "requestedModelHint": "Requested {model}, served by fallback" + "requestedModelHint": "Requested {model}, served by fallback", + "attempts": "Attempts", + "filterAll": "All", + "providerBreakdown": "Per-provider breakdown", + "tokensPerSec": "Tok/s", + "allProviders": "All providers", + "requestDetailTitle": "Request #{id}", + "failoverLadder": "Failover ladder", + "failoverLadderHint": "Each hop this request took through the fallback chain, in order.", + "noAttemptTrace": "No attempt trace was recorded for this request.", + "keyOrdinal": "key #{n}", + "attemptTimingHint": "Start offset from request start · attempt duration", + "requestedModel": "Requested model", + "ttft": "TTFT" }, "playground": { "title": "Playground", diff --git a/client/src/i18n/locales/es.json b/client/src/i18n/locales/es.json index 6839c7bf1..0eee2e018 100644 --- a/client/src/i18n/locales/es.json +++ b/client/src/i18n/locales/es.json @@ -392,7 +392,20 @@ "recentCalls": "Llamadas recientes", "clientIp": "IP del cliente", "clientAgent": "Aplicación cliente", - "requestedModelHint": "Se solicitó {model}, servido por respaldo" + "requestedModelHint": "Se solicitó {model}, servido por respaldo", + "attempts": "Intentos", + "filterAll": "Todas", + "providerBreakdown": "Desglose por proveedor", + "tokensPerSec": "Tok/s", + "allProviders": "Todos los proveedores", + "requestDetailTitle": "Solicitud #{id}", + "failoverLadder": "Escalera de failover", + "failoverLadderHint": "Cada salto que esta solicitud dio por la cadena de respaldo, en orden.", + "noAttemptTrace": "No se registró ninguna traza de intentos para esta solicitud.", + "keyOrdinal": "clave #{n}", + "attemptTimingHint": "Desfase desde el inicio de la solicitud · duración del intento", + "requestedModel": "Modelo solicitado", + "ttft": "TTFT" }, "playground": { "title": "Entorno de pruebas", diff --git a/client/src/i18n/locales/fr.json b/client/src/i18n/locales/fr.json index 49aa0fcf3..aa51f18bd 100644 --- a/client/src/i18n/locales/fr.json +++ b/client/src/i18n/locales/fr.json @@ -392,7 +392,20 @@ "recentCalls": "Appels récents", "clientIp": "IP du client", "clientAgent": "Application cliente", - "requestedModelHint": "{model} demandé, servi par repli" + "requestedModelHint": "{model} demandé, servi par repli", + "attempts": "Tentatives", + "filterAll": "Toutes", + "providerBreakdown": "Détail par fournisseur", + "tokensPerSec": "Tok/s", + "allProviders": "Tous les fournisseurs", + "requestDetailTitle": "Requête #{id}", + "failoverLadder": "Échelle de failover", + "failoverLadderHint": "Chaque étape parcourue par cette requête dans la chaîne de repli, dans l'ordre.", + "noAttemptTrace": "Aucune trace de tentative n'a été enregistrée pour cette requête.", + "keyOrdinal": "clé #{n}", + "attemptTimingHint": "Décalage depuis le début de la requête · durée de la tentative", + "requestedModel": "Modèle demandé", + "ttft": "TTFT" }, "playground": { "title": "Bac à sable", diff --git a/client/src/i18n/locales/it.json b/client/src/i18n/locales/it.json index 98863ea79..71b52bb20 100644 --- a/client/src/i18n/locales/it.json +++ b/client/src/i18n/locales/it.json @@ -392,7 +392,20 @@ "recentCalls": "Chiamate recenti", "clientIp": "IP del client", "clientAgent": "App client", - "requestedModelHint": "Richiesto {model}, servito dal fallback" + "requestedModelHint": "Richiesto {model}, servito dal fallback", + "attempts": "Tentativi", + "filterAll": "Tutte", + "providerBreakdown": "Dettaglio per provider", + "tokensPerSec": "Tok/s", + "allProviders": "Tutti i provider", + "requestDetailTitle": "Richiesta #{id}", + "failoverLadder": "Scala di failover", + "failoverLadderHint": "Ogni passaggio di questa richiesta lungo la catena di fallback, in ordine.", + "noAttemptTrace": "Nessuna traccia dei tentativi registrata per questa richiesta.", + "keyOrdinal": "chiave #{n}", + "attemptTimingHint": "Offset dall'inizio della richiesta · durata del tentativo", + "requestedModel": "Modello richiesto", + "ttft": "TTFT" }, "playground": { "title": "Playground", diff --git a/client/src/i18n/locales/pt-BR.json b/client/src/i18n/locales/pt-BR.json index 8d39142a5..840c920e9 100644 --- a/client/src/i18n/locales/pt-BR.json +++ b/client/src/i18n/locales/pt-BR.json @@ -392,7 +392,20 @@ "recentCalls": "Chamadas recentes", "clientIp": "IP do cliente", "clientAgent": "Aplicativo cliente", - "requestedModelHint": "Solicitado {model}, servido por fallback" + "requestedModelHint": "Solicitado {model}, servido por fallback", + "attempts": "Tentativas", + "filterAll": "Todas", + "providerBreakdown": "Detalhamento por provedor", + "tokensPerSec": "Tok/s", + "allProviders": "Todos os provedores", + "requestDetailTitle": "Requisição #{id}", + "failoverLadder": "Escada de failover", + "failoverLadderHint": "Cada salto que esta requisição deu pela cadeia de fallback, em ordem.", + "noAttemptTrace": "Nenhum rastro de tentativas foi registrado para esta requisição.", + "keyOrdinal": "chave #{n}", + "attemptTimingHint": "Deslocamento desde o início da requisição · duração da tentativa", + "requestedModel": "Modelo solicitado", + "ttft": "TTFT" }, "playground": { "title": "Playground", diff --git a/client/src/i18n/locales/zh-CN.json b/client/src/i18n/locales/zh-CN.json index 7bae3ea82..7861c76b8 100644 --- a/client/src/i18n/locales/zh-CN.json +++ b/client/src/i18n/locales/zh-CN.json @@ -392,7 +392,20 @@ "recentCalls": "最近调用", "clientIp": "客户端 IP", "clientAgent": "客户端应用", - "requestedModelHint": "请求了 {model},由回退模型提供" + "requestedModelHint": "请求了 {model},由回退模型提供", + "attempts": "尝试次数", + "filterAll": "全部", + "providerBreakdown": "按提供商明细", + "tokensPerSec": "Tok/s", + "allProviders": "全部提供商", + "requestDetailTitle": "请求 #{id}", + "failoverLadder": "故障转移阶梯", + "failoverLadderHint": "该请求沿回退链经过的每一跳,按顺序排列。", + "noAttemptTrace": "此请求未记录尝试轨迹。", + "keyOrdinal": "密钥 #{n}", + "attemptTimingHint": "自请求开始的偏移 · 尝试耗时", + "requestedModel": "请求的模型", + "ttft": "TTFT" }, "playground": { "title": "试玩台", diff --git a/client/src/pages/AnalyticsPage.tsx b/client/src/pages/AnalyticsPage.tsx index eda91b84a..64cb56613 100644 --- a/client/src/pages/AnalyticsPage.tsx +++ b/client/src/pages/AnalyticsPage.tsx @@ -4,13 +4,18 @@ import { BarChart, Bar, XAxis, YAxis, CartesianGrid, Tooltip, ResponsiveContainer, LineChart, Line, Legend, } from 'recharts' +import { X } from 'lucide-react' import { apiFetch } from '@/lib/api' import { SegmentedControl } from '@/components/ui/segmented-control' import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from '@/components/ui/table' +import { Badge } from '@/components/ui/badge' +import { Dialog, DialogClose, DialogPopup, DialogTitle } from '@/components/ui/dialog' +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select' import { PageHeader } from '@/components/page-header' import { Skeleton } from '@/components/ui/skeleton' import { Tooltip as HoverTooltip } from '@/components/tooltip' import { formatSqliteUtcToLocalTime } from '@/lib/utils' +import { platformColors } from '@/lib/routing' import { useI18n } from '@/i18n' type TimeRange = '24h' | '7d' | '30d' | '90d' @@ -109,6 +114,9 @@ interface RecentCallRow { clientIp: string | null clientUserAgent: string | null createdAt: string + // Failover-ladder length: attempts hang off the TERMINAL row of a proxied + // request, so mid-ladder failure rows report 0. + attemptCount: number } interface RecentCallsResponse { @@ -116,6 +124,25 @@ interface RecentCallsResponse { rows: RecentCallRow[] } +// One hop of the failover ladder, from GET /api/analytics/requests/:id. +interface RequestAttempt { + ordinal: number + platform: string + modelId: string + keyOrdinal: number + outcome: string + startOffsetMs: number + durationMs: number + errorSummary: string | null +} + +interface RequestDetail extends Omit { + ttfbMs: number | null + attempts: RequestAttempt[] +} + +type StatusFilter = 'all' | 'success' | 'error' + // First product token of the UA ("python-requests/2.32.3", "curl/8.6.0", …) // is enough to tell callers apart in a narrow cell; full string on hover. function shortUserAgent(ua: string | null): string { @@ -143,17 +170,163 @@ function Stat({ label, value, hint, className }: { label: string; value: string return hint ? {card} : card } -function Panel({ title, children }: { title: string; children: React.ReactNode }) { +function Panel({ title, actions, children }: { title: string; actions?: React.ReactNode; children: React.ReactNode }) { return (
-
+

{title}

+ {actions}
{children}
) } +// Platform swatch shared by the ladder and the per-provider table; same color +// source as the token-usage legend (lib/routing.ts), same gray fallback. +function PlatformDot({ platform }: { platform: string }) { + return ( + + ) +} + +// Compact ms rendering for ladder timings: sub-second stays in ms, longer +// spans read as seconds ("38.8 s") like the issue reports do. +function formatMs(ms: number): string { + if (ms >= 1000) return `${(ms / 1000).toFixed(1)} s` + return `${ms} ms` +} + +// Key/value line of the request-detail summary grid. +function DetailField({ label, value, mono }: { label: string; value: React.ReactNode; mono?: boolean }) { + return ( +
+

{label}

+

{value}

+
+ ) +} + +// Per-request drill-down: the parent row's fields plus the failover ladder — +// one entry per dispatched attempt (ordinal → provider/model → key ordinal → +// outcome → timing, with the redacted per-hop error when one was recorded). +// A dialog (the app's detail-popup idiom, cf. keys/export-keys-dialog) rather +// than a routed page so the list's range/filter context stays put behind it. +function RequestDetailDialog({ requestId, onClose }: { requestId: number | null; onClose: () => void }) { + const { t } = useI18n() + + const { data: detail, isLoading } = useQuery({ + queryKey: ['analytics', 'request-detail', requestId], + queryFn: () => apiFetch(`/api/analytics/requests/${requestId}`), + enabled: requestId != null, + }) + + return ( + { if (!open) onClose() }}> + +
+ {t('analytics.requestDetailTitle', { id: requestId ?? '' })} + + + +
+ + {isLoading || !detail ? ( +
+ + +
+ ) : ( +
+
+ + {detail.status} + } + /> + + + {detail.platform} + + } + /> + + {detail.requestedModel && detail.requestedModel !== detail.modelId && ( + + )} + + + + + +
+ + {detail.error && ( +
+

{t('analytics.message')}

+

{detail.error}

+
+ )} + +
+

{t('analytics.failoverLadder')}

+

{t('analytics.failoverLadderHint')}

+ {detail.attempts.length === 0 ? ( +

{t('analytics.noAttemptTrace')}

+ ) : ( +
    + {detail.attempts.map((a) => ( +
  1. +
    + #{a.ordinal + 1} + + {a.platform} + {a.modelId} + {t('analytics.keyOrdinal', { n: a.keyOrdinal })} + + {a.outcome} + + + +{formatMs(a.startOffsetMs)} · {formatMs(a.durationMs)} + +
    + {a.errorSummary && ( +

    {a.errorSummary}

    + )} +
  2. + ))} +
+ )} +
+
+ )} +
+
+ ) +} + const axisStyle = { fontSize: 11, fill: 'var(--muted-foreground)' } as const const gridStyle = 'var(--border)' const primaryFill = 'var(--foreground)' @@ -214,9 +387,21 @@ export default function AnalyticsPage() { queryFn: () => apiFetch(`/api/analytics/error-distribution?range=${range}`), }) + // Recent-calls list filters (status/platform) + the row opened in the + // drill-down dialog. Filters ride the query key so react-query refetches + // (and caches) each combination on its own. + const [statusFilter, setStatusFilter] = useState('all') + const [platformFilter, setPlatformFilter] = useState('all') + const [detailId, setDetailId] = useState(null) + const { data: recentCalls } = useQuery({ - queryKey: ['analytics', 'requests', range], - queryFn: () => apiFetch(`/api/analytics/requests?range=${range}&limit=100`), + queryKey: ['analytics', 'requests', range, statusFilter, platformFilter], + queryFn: () => { + const params = new URLSearchParams({ range, limit: '100' }) + if (statusFilter !== 'all') params.set('status', statusFilter) + if (platformFilter !== 'all') params.set('platform', platformFilter) + return apiFetch(`/api/analytics/requests?${params}`) + }, }) // Savings card shows ONE stable monthly figure regardless of the selected @@ -479,9 +664,45 @@ export default function AnalyticsPage() { {/* Recent calls: one line per proxied request with the caller's IP + user agent. All local clients share the unified key, so this is - the only view that answers "who is hitting the router". */} + the only view that answers "who is hitting the router". Rows open + the failover-ladder drill-down; the header hosts status/provider + filters (server-side, so total reflects the filtered set). */}
- + + + +
+ } + > {!recentCalls?.rows?.length ? (

{t('common.noData')}

) : ( @@ -495,6 +716,7 @@ export default function AnalyticsPage() { {t('common.model')} {t('common.provider')} {t('common.status')} + {t('analytics.attempts')} {t('analytics.inTokens')} {t('analytics.outTokens')} {t('analytics.latency')} @@ -502,7 +724,11 @@ export default function AnalyticsPage() { {recentCalls.rows.map((r) => ( - + setDetailId(r.id)} + className="cursor-pointer" + > {formatSqliteUtcToLocalTime(r.createdAt, { month: 'short', day: 'numeric', hour: '2-digit', minute: '2-digit', second: '2-digit' })} @@ -518,6 +744,11 @@ export default function AnalyticsPage() { {r.status} + {/* >1 = the request burned failover hops; that is the + row worth drilling into, so give it weight. */} + 1 ? 'font-medium' : 'text-muted-foreground'}`}> + {r.attemptCount > 0 ? r.attemptCount : '—'} + {formatTokens(r.inputTokens)} {formatTokens(r.outputTokens)} {r.latencyMs} ms @@ -530,6 +761,59 @@ export default function AnalyticsPage() {
+ {/* Per-provider breakdown: the tabular face of the by-platform data — + the charts above show volume/latency, this row surfaces the + success-rate and error-count numbers (#335). */} +
+ + {byPlatform.length === 0 ? ( +

{t('common.noData')}

+ ) : ( +
+ + + + {t('common.provider')} + {t('analytics.requests')} + {t('common.success')} + {t('analytics.errors')} + {t('analytics.avgLatency')} + {t('analytics.p95Latency')} + {t('analytics.avgTtft')} + {t('analytics.tokensPerSec')} + {t('analytics.inTokens')} + {t('analytics.outTokens')} + + + + {byPlatform.map((p) => ( + + + + + {p.platform} + + + {p.requests} + {p.successRate}% + 0 ? 'text-destructive' : ''}`}> + {p.errorCount > 0 ? p.errorCount : '—'} + + {p.avgLatencyMs} ms + {p.p95LatencyMs != null ? `${p.p95LatencyMs} ms` : '—'} + {p.avgTtfbMs != null ? `${p.avgTtfbMs} ms` : '—'} + {p.avgTokensPerSecond != null ? p.avgTokensPerSecond : '—'} + {formatTokens(p.totalInputTokens)} + {formatTokens(p.totalOutputTokens)} + + ))} + +
+
+ )} +
+
+
{byModel.length === 0 ? ( @@ -610,6 +894,8 @@ export default function AnalyticsPage() { )}
+ + setDetailId(null)} /> ) } diff --git a/server/src/__tests__/db/migrate/roundtrip.test.ts b/server/src/__tests__/db/migrate/roundtrip.test.ts index 990b6b15a..21fb34708 100644 --- a/server/src/__tests__/db/migrate/roundtrip.test.ts +++ b/server/src/__tests__/db/migrate/roundtrip.test.ts @@ -17,6 +17,7 @@ const COOLDOWN_PROBE_PROVENANCE_FILENAME = '20260726_000001_cooldown_probe_prove const REQUEST_ATTEMPTS_FILENAME = '20260726_000002_request_attempts.ts'; const MODEL_SOURCE_PROVENANCE_FILENAME = '20260726_000003_model_source_provenance.ts'; const MEDIA_MODEL_META_FILENAME = '20260726_000004_media_model_meta.ts'; +const ATTEMPT_ERROR_SUMMARY_FILENAME = '20260726_000005_attempt_error_summary.ts'; interface SchemaRow { type: string; @@ -80,6 +81,7 @@ describe('migration round trip', () => { REQUEST_ATTEMPTS_FILENAME, MODEL_SOURCE_PROVENANCE_FILENAME, MEDIA_MODEL_META_FILENAME, + ATTEMPT_ERROR_SUMMARY_FILENAME, ]); } finally { db.close(); diff --git a/server/src/__tests__/lib/error-redaction.test.ts b/server/src/__tests__/lib/error-redaction.test.ts index 755c521c9..93d4bc0a4 100644 --- a/server/src/__tests__/lib/error-redaction.test.ts +++ b/server/src/__tests__/lib/error-redaction.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { sanitizeProviderErrorMessage } from '../../lib/error-redaction.js'; +import { sanitizeProviderErrorMessage, summarizeAttemptError } from '../../lib/error-redaction.js'; describe('sanitizeProviderErrorMessage', () => { it('redacts Bearer tokens', () => { @@ -42,3 +42,25 @@ describe('sanitizeProviderErrorMessage', () => { expect(out.length).toBeLessThanOrEqual(240); }); }); + +// The short per-attempt error summary stored in request_attempts.error_summary: +// same redactions as sanitizeProviderErrorMessage, tighter cap (200 chars). +describe('summarizeAttemptError', () => { + it('applies the same secret redactions', () => { + const out = summarizeAttemptError('401 rejected: Bearer sk-abc.def-12345 for key gsk_A1b2C3d4E5f6G7h8'); + expect(out).not.toContain('sk-abc.def-12345'); + expect(out).not.toContain('gsk_A1b2C3d4E5f6G7h8'); + expect(out).toContain('Bearer [redacted]'); + }); + + it('caps the summary at 200 characters with an ellipsis', () => { + const out = summarizeAttemptError('upstream exploded because ' + 'y '.repeat(300)); + expect(out.length).toBeLessThanOrEqual(200); + expect(out.endsWith('...')).toBe(true); + }); + + it('leaves ordinary provider prose intact', () => { + const msg = '429 Too Many Requests: rate limit reached for llama-3.3-70b, retry in 7m12s'; + expect(summarizeAttemptError(msg)).toBe(msg); + }); +}); diff --git a/server/src/__tests__/lib/request-attempts.test.ts b/server/src/__tests__/lib/request-attempts.test.ts index b106f5922..7f95ae3f5 100644 --- a/server/src/__tests__/lib/request-attempts.test.ts +++ b/server/src/__tests__/lib/request-attempts.test.ts @@ -54,11 +54,12 @@ interface AttemptRow { outcome: string; start_offset_ms: number; duration_ms: number; + error_summary: string | null; } function allAttempts(): AttemptRow[] { return getDb().prepare( - 'SELECT request_id, ordinal, platform, model_id, key_ordinal, outcome, start_offset_ms, duration_ms FROM request_attempts ORDER BY ordinal', + 'SELECT request_id, ordinal, platform, model_id, key_ordinal, outcome, start_offset_ms, duration_ms, error_summary FROM request_attempts ORDER BY ordinal', ).all() as AttemptRow[]; } @@ -126,6 +127,39 @@ describe('per-attempt routing traces', () => { } }); + it('persists a redacted, capped error_summary per failed attempt and null for the successful one', async () => { + const longTail = ' then the provider rambled on'.repeat(20); + await runFallbackLoop(hooksSkeleton({ + route: () => fakeRoute({ platform: 'groq', modelId: 'llama-x' }), + dispatch: async (route, attempt) => { + if (attempt === 0) { + throw Object.assign( + new Error(`429 rejected: Bearer sk-secret.token-12345 over quota${longTail}`), + { status: 429 }, + ); + } + logRequest(route.platform, route.modelId, route.keyId, 'success', 10, 5, 42, null); + return 'done'; + }, + logFailure: (route, err) => + logRequest(route.platform, route.modelId, route.keyId, 'error', 10, 0, 5, err.message), + })); + + const rows = allAttempts(); + expect(rows).toHaveLength(2); + expect(rows.map(r => r.outcome)).toEqual(['rate_limited', 'ok']); + + // Failed hop: short, redacted, capped summary — never the raw secret. + const summary = rows[0].error_summary; + expect(summary).toBeTruthy(); + expect(summary!).toContain('429'); + expect(summary!).not.toContain('sk-secret.token-12345'); + expect(summary!.length).toBeLessThanOrEqual(200); + + // Successful hop: no error, no summary. + expect(rows[1].error_summary).toBeNull(); + }); + it("persists exactly one 'ok' attempt for a single-attempt success", async () => { const route = fakeRoute({ platform: 'groq', modelId: 'solo-model' }); await runFallbackLoop(hooksSkeleton({ diff --git a/server/src/__tests__/routes/analytics-request-attempts.test.ts b/server/src/__tests__/routes/analytics-request-attempts.test.ts index 24706bf51..7fe86125a 100644 --- a/server/src/__tests__/routes/analytics-request-attempts.test.ts +++ b/server/src/__tests__/routes/analytics-request-attempts.test.ts @@ -32,11 +32,11 @@ function insertRequest(status: string, createdAt: string): number { return Number(insert.lastInsertRowid); } -function insertAttempt(requestId: number, ordinal: number, platform: string, modelId: string, keyOrdinal: number, outcome: string, startOffsetMs: number, durationMs: number) { +function insertAttempt(requestId: number, ordinal: number, platform: string, modelId: string, keyOrdinal: number, outcome: string, startOffsetMs: number, durationMs: number, errorSummary: string | null = null) { getDb().prepare(` - INSERT INTO request_attempts (request_id, ordinal, platform, model_id, key_ordinal, outcome, start_offset_ms, duration_ms) - VALUES (?, ?, ?, ?, ?, ?, ?, ?) - `).run(requestId, ordinal, platform, modelId, keyOrdinal, outcome, startOffsetMs, durationMs); + INSERT INTO request_attempts (request_id, ordinal, platform, model_id, key_ordinal, outcome, start_offset_ms, duration_ms, error_summary) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) + `).run(requestId, ordinal, platform, modelId, keyOrdinal, outcome, startOffsetMs, durationMs, errorSummary); } function recentSql(hour: number): string { @@ -72,10 +72,10 @@ describe('per-attempt traces in the analytics requests API', () => { expect(body.rows.map((r: any) => r.attemptCount)).toEqual([0, 3]); }); - it('returns the ordered attempts ladder in the per-request detail payload', async () => { + it('returns the ordered attempts ladder — with per-hop error summaries — in the per-request detail payload', async () => { const id = insertRequest('success', recentSql(12)); - insertAttempt(id, 0, 'groq', 'llama-x', 1, 'rate_limited', 0, 210); - insertAttempt(id, 1, 'google', 'gemini-y', 2, 'timeout', 215, 10000); + insertAttempt(id, 0, 'groq', 'llama-x', 1, 'rate_limited', 0, 210, '429 rate limit reached, retry in 7m12s'); + insertAttempt(id, 1, 'google', 'gemini-y', 2, 'timeout', 215, 10000, 'upstream timeout after 10000ms'); insertAttempt(id, 2, 'cerebras', 'qwen-z', 3, 'ok', 10220, 900); const { status, body } = await request(app, `/api/analytics/requests/${id}`); @@ -88,9 +88,9 @@ describe('per-attempt traces in the analytics requests API', () => { latencyMs: 1234, }); expect(body.attempts).toEqual([ - { ordinal: 0, platform: 'groq', modelId: 'llama-x', keyOrdinal: 1, outcome: 'rate_limited', startOffsetMs: 0, durationMs: 210 }, - { ordinal: 1, platform: 'google', modelId: 'gemini-y', keyOrdinal: 2, outcome: 'timeout', startOffsetMs: 215, durationMs: 10000 }, - { ordinal: 2, platform: 'cerebras', modelId: 'qwen-z', keyOrdinal: 3, outcome: 'ok', startOffsetMs: 10220, durationMs: 900 }, + { ordinal: 0, platform: 'groq', modelId: 'llama-x', keyOrdinal: 1, outcome: 'rate_limited', startOffsetMs: 0, durationMs: 210, errorSummary: '429 rate limit reached, retry in 7m12s' }, + { ordinal: 1, platform: 'google', modelId: 'gemini-y', keyOrdinal: 2, outcome: 'timeout', startOffsetMs: 215, durationMs: 10000, errorSummary: 'upstream timeout after 10000ms' }, + { ordinal: 2, platform: 'cerebras', modelId: 'qwen-z', keyOrdinal: 3, outcome: 'ok', startOffsetMs: 10220, durationMs: 900, errorSummary: null }, ]); }); diff --git a/server/src/__tests__/routes/analytics-requests.test.ts b/server/src/__tests__/routes/analytics-requests.test.ts index 1cc0e97c3..d47fc1cde 100644 --- a/server/src/__tests__/routes/analytics-requests.test.ts +++ b/server/src/__tests__/routes/analytics-requests.test.ts @@ -21,11 +21,11 @@ async function request(app: Express, path: string) { return { status: res.status, body: data }; } -function insertCall(createdAt: string, clientIp: string | null, clientUserAgent: string | null, status = 'success') { +function insertCall(createdAt: string, clientIp: string | null, clientUserAgent: string | null, status = 'success', platform = 'test') { getDb().prepare(` INSERT INTO requests (platform, model_id, status, input_tokens, output_tokens, latency_ms, error, created_at, client_ip, client_user_agent) - VALUES ('test', 'test-model', ?, 10, 5, 42, NULL, ?, ?, ?) - `).run(status, createdAt, clientIp, clientUserAgent); + VALUES (?, 'test-model', ?, 10, 5, 42, NULL, ?, ?, ?) + `).run(platform, status, createdAt, clientIp, clientUserAgent); } function recentUtcTimestamp(hour: number) { @@ -86,6 +86,46 @@ describe('GET /api/analytics/requests', () => { expect(clamped.body.rows).toHaveLength(5); }); + it('filters by status, keeping total consistent with the filter', async () => { + insertCall(recentUtcTimestamp(8).sql, '10.0.0.1', 'ua', 'success'); + insertCall(recentUtcTimestamp(9).sql, '10.0.0.1', 'ua', 'error'); + insertCall(recentUtcTimestamp(10).sql, '10.0.0.1', 'ua', 'error'); + + const errors = await request(app, '/api/analytics/requests?range=7d&status=error'); + expect(errors.status).toBe(200); + expect(errors.body.total).toBe(2); + expect(errors.body.rows.map((r: any) => r.status)).toEqual(['error', 'error']); + + const successes = await request(app, '/api/analytics/requests?range=7d&status=success'); + expect(successes.body.total).toBe(1); + expect(successes.body.rows.map((r: any) => r.status)).toEqual(['success']); + }); + + it('filters by platform, and combines with the status filter', async () => { + insertCall(recentUtcTimestamp(8).sql, '10.0.0.1', 'ua', 'success', 'groq'); + insertCall(recentUtcTimestamp(9).sql, '10.0.0.1', 'ua', 'error', 'groq'); + insertCall(recentUtcTimestamp(10).sql, '10.0.0.1', 'ua', 'error', 'google'); + + const groq = await request(app, '/api/analytics/requests?range=7d&platform=groq'); + expect(groq.status).toBe(200); + expect(groq.body.total).toBe(2); + expect(groq.body.rows.map((r: any) => r.platform)).toEqual(['groq', 'groq']); + + const combined = await request(app, '/api/analytics/requests?range=7d&platform=groq&status=error'); + expect(combined.body.total).toBe(1); + expect(combined.body.rows).toHaveLength(1); + expect(combined.body.rows[0]).toMatchObject({ platform: 'groq', status: 'error' }); + }); + + it('rejects invalid status/platform filter values with 400', async () => { + insertCall(recentUtcTimestamp(8).sql, '10.0.0.1', 'ua'); + + expect((await request(app, '/api/analytics/requests?range=7d&status=weird')).status).toBe(400); + expect((await request(app, '/api/analytics/requests?range=7d&platform=' + encodeURIComponent('a b;--'))).status).toBe(400); + // Absent filters keep the default behavior. + expect((await request(app, '/api/analytics/requests?range=7d')).status).toBe(200); + }); + it('records the request-scoped caller identity through logRequest', async () => { const { clientContextMiddleware } = await import('../../lib/client-context.js'); const req = { headers: { 'user-agent': 'vitest-client/1.0' }, socket: { remoteAddress: '192.168.0.99' } } as any; diff --git a/server/src/db/migrate/defaults.ts b/server/src/db/migrate/defaults.ts index 633cde823..8d477d6d2 100644 --- a/server/src/db/migrate/defaults.ts +++ b/server/src/db/migrate/defaults.ts @@ -12,6 +12,7 @@ import * as cooldownProbeProvenance from '../migrations/20260726_000001_cooldown import * as requestAttempts from '../migrations/20260726_000002_request_attempts.js'; import * as modelSourceProvenance from '../migrations/20260726_000003_model_source_provenance.js'; import * as mediaModelMeta from '../migrations/20260726_000004_media_model_meta.js'; +import * as attemptErrorSummary from '../migrations/20260726_000005_attempt_error_summary.js'; export interface MigrationModule { up(db: Db): void; @@ -36,6 +37,7 @@ export const COOLDOWN_PROBE_PROVENANCE_FILENAME = '20260726_000001_cooldown_prob export const REQUEST_ATTEMPTS_FILENAME = '20260726_000002_request_attempts.ts'; export const MODEL_SOURCE_PROVENANCE_FILENAME = '20260726_000003_model_source_provenance.ts'; export const MEDIA_MODEL_META_FILENAME = '20260726_000004_media_model_meta.ts'; +export const ATTEMPT_ERROR_SUMMARY_FILENAME = '20260726_000005_attempt_error_summary.ts'; export const DEFAULT_MIGRATIONS: readonly DefaultMigration[] = [ { filename: LEGACY_BASELINE_FILENAME, module: legacyBaseline }, @@ -51,4 +53,5 @@ export const DEFAULT_MIGRATIONS: readonly DefaultMigration[] = [ { filename: REQUEST_ATTEMPTS_FILENAME, module: requestAttempts }, { filename: MODEL_SOURCE_PROVENANCE_FILENAME, module: modelSourceProvenance }, { filename: MEDIA_MODEL_META_FILENAME, module: mediaModelMeta }, + { filename: ATTEMPT_ERROR_SUMMARY_FILENAME, module: attemptErrorSummary }, ]; diff --git a/server/src/db/migrations/20260726_000005_attempt_error_summary.ts b/server/src/db/migrations/20260726_000005_attempt_error_summary.ts new file mode 100644 index 000000000..598d1792b --- /dev/null +++ b/server/src/db/migrations/20260726_000005_attempt_error_summary.ts @@ -0,0 +1,33 @@ +// Migration: request_attempts.error_summary — per-hop error text for the ladder +// Created: 2026-07-26 +// +// DOWN: reversible +// +// The failover-ladder drill-down (issue #335) renders WHY each hop failed, but +// the `outcome` class alone loses the provider's actual words ("rate limit +// reached for llama-3.3-70b, retry in 7m12s" collapses to 'rate_limited'). +// This nullable column stores a short, REDACTED summary of the per-attempt +// error — sanitized through lib/error-redaction.ts (same rules as the error +// text on the parent `requests` row: keys/tokens/URLs scrubbed) and capped at +// 200 chars at write time (lib/fallback-loop.ts). NULL for successful hops +// ('ok'/'committed') and for every pre-existing row, where the detail is +// simply unknown. + +import type { Db } from '../types.js'; + +function hasColumn(db: Db, table: string, column: string): boolean { + const columns = db.prepare(`PRAGMA table_info(${table})`).all() as { name: string }[]; + return columns.some((candidate) => candidate.name === column); +} + +export function up(db: Db): void { + if (!hasColumn(db, 'request_attempts', 'error_summary')) { + db.prepare('ALTER TABLE request_attempts ADD COLUMN error_summary TEXT').run(); + } +} + +export function down(db: Db): void { + if (hasColumn(db, 'request_attempts', 'error_summary')) { + db.prepare('ALTER TABLE request_attempts DROP COLUMN error_summary').run(); + } +} diff --git a/server/src/lib/attempt-trace.ts b/server/src/lib/attempt-trace.ts index c84360d37..adcfcfdff 100644 --- a/server/src/lib/attempt-trace.ts +++ b/server/src/lib/attempt-trace.ts @@ -40,6 +40,10 @@ export interface AttemptTraceRecord { // Milliseconds this attempt ran (for 'ok' streams: until the response // finished, i.e. including streaming time). durationMs: number; + // Short, REDACTED summary of the error that ended this attempt (see + // lib/error-redaction.ts summarizeAttemptError — secrets scrubbed, capped at + // 200 chars). Null for successful hops ('ok'/'committed'). + errorSummary: string | null; } export interface RequestTrace { diff --git a/server/src/lib/error-redaction.ts b/server/src/lib/error-redaction.ts index c66a5df05..4ed3b26ce 100644 --- a/server/src/lib/error-redaction.ts +++ b/server/src/lib/error-redaction.ts @@ -32,3 +32,20 @@ export function sanitizeProviderErrorMessage(message: unknown): string { return sanitized; } + +// Cap for the per-hop `request_attempts.error_summary` column: one short line +// per failover attempt, tighter than the parent row's error text. +const MAX_ATTEMPT_ERROR_SUMMARY_LENGTH = 200; + +/** + * The short per-attempt error summary the failover ladder stores per hop: + * the same secret/URL redactions as sanitizeProviderErrorMessage, re-capped + * at 200 chars so the drill-down stays one line per attempt. + */ +export function summarizeAttemptError(message: unknown): string { + let summary = sanitizeProviderErrorMessage(message); + if (summary.length > MAX_ATTEMPT_ERROR_SUMMARY_LENGTH) { + summary = `${summary.slice(0, MAX_ATTEMPT_ERROR_SUMMARY_LENGTH - 3).trimEnd()}...`; + } + return summary; +} diff --git a/server/src/lib/fallback-loop.ts b/server/src/lib/fallback-loop.ts index f02b90b6d..e041e8050 100644 --- a/server/src/lib/fallback-loop.ts +++ b/server/src/lib/fallback-loop.ts @@ -40,7 +40,7 @@ import { isProviderDegradedError, isContextTooLargeError, } from './error-classify.js'; -import { sanitizeProviderErrorMessage } from './error-redaction.js'; +import { sanitizeProviderErrorMessage, summarizeAttemptError } from './error-redaction.js'; import { checkKeyHealth, markKeyHealthyFromRequest } from '../services/health.js'; import { getSetting } from '../db/index.js'; import { newBreaker, recordBreakerFailure } from './guardrails.js'; @@ -813,9 +813,12 @@ async function runFallbackLoopAttempts(hooks: FallbackHooks, trace: RequestTrace // Per-attempt trace record: pushed exactly once per dispatched attempt, on // whichever exit the attempt takes. startOffsetMs/durationMs bracket the // dispatch (for a successful stream, durationMs runs until the response - // finished — that IS the attempt). + // finished — that IS the attempt). When the attempt ended on an error, a + // short REDACTED summary of it rides along — the outcome class alone loses + // the provider's actual words, which is exactly what the dashboard's + // drill-down needs to answer "why did this hop fail". const attemptStartedAt = Date.now(); - const traceAttempt = (outcome: AttemptOutcome): void => { + const traceAttempt = (outcome: AttemptOutcome, err?: any): void => { trace.records.push({ ordinal: trace.records.length, platform: route.platform, @@ -824,6 +827,7 @@ async function runFallbackLoopAttempts(hooks: FallbackHooks, trace: RequestTrace outcome, startOffsetMs: attemptStartedAt - startedAt, durationMs: Date.now() - attemptStartedAt, + errorSummary: err != null ? summarizeAttemptError(err?.message) : null, }); }; @@ -858,7 +862,7 @@ async function runFallbackLoopAttempts(hooks: FallbackHooks, trace: RequestTrace // it immediately instead of 502-ing while healthy routes sit idle. recordAuthFailure(route, hooks.state); attempts.push({ platform: route.platform, modelId: route.modelId, keyOrdinal: keyOrdinal(route), errorClass: 'auth' }); - traceAttempt('auth'); + traceAttempt('auth', err); lastError = err; if (stopIfBreakerTripped()) return; continue; @@ -867,7 +871,7 @@ async function runFallbackLoopAttempts(hooks: FallbackHooks, trace: RequestTrace recordRetryableFailure(route, err, hooks.state); const errorClass = classifyAttemptError(err); attempts.push({ platform: route.platform, modelId: route.modelId, keyOrdinal: keyOrdinal(route), errorClass }); - traceAttempt(errorClass); + traceAttempt(errorClass, err); lastError = err; // skipBench failures (format ignored, hidden-reasoning truncation) are // model behavior, not provider health — recordRetryableFailure already @@ -877,7 +881,7 @@ async function runFallbackLoopAttempts(hooks: FallbackHooks, trace: RequestTrace if (err?.skipBench !== true && stopIfBreakerTripped()) return; continue; } - traceAttempt(classifyAttemptError(err)); + traceAttempt(classifyAttemptError(err), err); hooks.onFatal(route, err, attempt); return; } @@ -896,7 +900,7 @@ async function runFallbackLoopAttempts(hooks: FallbackHooks, trace: RequestTrace ); console.error('[FallbackLoop]', violation.message); hooks.logFailure(route, violation, attempt); - traceAttempt('error'); + traceAttempt('error', violation); hooks.onFatal(route, violation, attempt); return; } diff --git a/server/src/lib/request-log.ts b/server/src/lib/request-log.ts index a108e1b7e..09c48261e 100644 --- a/server/src/lib/request-log.ts +++ b/server/src/lib/request-log.ts @@ -116,12 +116,12 @@ export function persistRequestAttempts(trace: RequestTrace): void { try { const db = getDb(); const insert = db.prepare(` - INSERT INTO request_attempts (request_id, ordinal, platform, model_id, key_ordinal, outcome, start_offset_ms, duration_ms) - VALUES (?, ?, ?, ?, ?, ?, ?, ?) + INSERT INTO request_attempts (request_id, ordinal, platform, model_id, key_ordinal, outcome, start_offset_ms, duration_ms, error_summary) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) `); const tx = db.transaction(() => { for (const r of trace.records) { - insert.run(trace.lastRequestRowId, r.ordinal, r.platform, r.modelId, r.keyOrdinal, r.outcome, r.startOffsetMs, r.durationMs); + insert.run(trace.lastRequestRowId, r.ordinal, r.platform, r.modelId, r.keyOrdinal, r.outcome, r.startOffsetMs, r.durationMs, r.errorSummary); } }); tx(); diff --git a/server/src/routes/analytics.ts b/server/src/routes/analytics.ts index 6a0000e2c..7170f2f48 100644 --- a/server/src/routes/analytics.ts +++ b/server/src/routes/analytics.ts @@ -467,11 +467,34 @@ analyticsRouter.get('/requests', (req: Request, res: Response) => { const since = getSinceTimestamp(range); const limit = Math.min(Math.max(parseInt(req.query.limit as string, 10) || 100, 1), 500); const offset = Math.max(parseInt(req.query.offset as string, 10) || 0, 0); + + // Optional filters. Both are validated (whitelist / shape) and applied as + // bound parameters; absent filters keep the default behavior identical. + const status = req.query.status as string | undefined; + if (status !== undefined && status !== 'success' && status !== 'error') { + res.status(400).json({ error: "invalid status filter (expected 'success' or 'error')" }); + return; + } + // Platform ids are short slugs ('groq', 'pt-custom_1'); anything else is a + // client bug, not a filter. + const platform = req.query.platform as string | undefined; + if (platform !== undefined && !/^[A-Za-z0-9_-]{1,64}$/.test(platform)) { + res.status(400).json({ error: 'invalid platform filter' }); + return; + } const db = getDb(); + const filterSql = + (status !== undefined ? ' AND status = ?' : '') + + (platform !== undefined ? ' AND platform = ?' : ''); + const filterParams = [ + ...(status !== undefined ? [status] : []), + ...(platform !== undefined ? [platform] : []), + ]; + const total = (db.prepare( - 'SELECT COUNT(*) as c FROM requests WHERE created_at >= ?' - ).get(since) as { c: number }).c; + `SELECT COUNT(*) as c FROM requests WHERE created_at >= ?${filterSql}` + ).get(since, ...filterParams) as { c: number }).c; const rows = db.prepare(` SELECT id, platform, model_id, requested_model, request_type, status, @@ -480,10 +503,10 @@ analyticsRouter.get('/requests', (req: Request, res: Response) => { strftime('%Y-%m-%dT%H:%M:%SZ', created_at) as created_at_iso, (SELECT COUNT(*) FROM request_attempts a WHERE a.request_id = requests.id) as attempt_count FROM requests - WHERE created_at >= ? + WHERE created_at >= ?${filterSql} ORDER BY created_at DESC, id DESC LIMIT ? OFFSET ? - `).all(since, limit, offset) as any[]; + `).all(since, ...filterParams, limit, offset) as any[]; res.json({ total, @@ -537,7 +560,7 @@ analyticsRouter.get('/requests/:id', (req: Request, res: Response) => { } const attempts = db.prepare(` - SELECT ordinal, platform, model_id, key_ordinal, outcome, start_offset_ms, duration_ms + SELECT ordinal, platform, model_id, key_ordinal, outcome, start_offset_ms, duration_ms, error_summary FROM request_attempts WHERE request_id = ? ORDER BY ordinal ASC @@ -566,6 +589,9 @@ analyticsRouter.get('/requests/:id', (req: Request, res: Response) => { outcome: a.outcome, startOffsetMs: a.start_offset_ms, durationMs: a.duration_ms, + // Short, redacted per-hop error text (null for successful hops and for + // rows written before the error_summary migration). + errorSummary: a.error_summary ?? null, })), }); });