From 19ca49c915ad8ab21ae64cdd46a276760a024c77 Mon Sep 17 00:00:00 2001 From: Cato <41178744+catomean@users.noreply.github.com> Date: Mon, 7 Sep 2026 00:59:44 +0200 Subject: [PATCH] =?UTF-8?q?refactor(llm):=20complete()=20owns=20the=20requ?= =?UTF-8?q?est=20=E2=80=94=20and=20a=20test=20asserted=20the=20bug?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This file already walked ai-kit's chain but kept its own fetch, so the CHAIN came from the package and the REQUEST did not. That is where the remaining defects lived, and they are the fleet's most common ones (census 2026-09-06: 9 of 12 hand-rolled clients share them): - `data.choices[0]?.message?.content || ''` handed an EMPTY 200 to the user as the bot's reply, and the chain stopped there, satisfied. A reasoning model that spends its budget thinking returns exactly that. - a 429 became `throw new Error('Groq API error: ' + status)`: the body was read, logged and discarded. Capacity, request-too-large and daily quota share that status code and want opposite responses — retry shortly, send less, or come back tomorrow. - neither call had a DEADLINE. A vendor that accepted the connection and never answered held the chat open indefinitely, and the fallback beneath it was never reached. Also removes the last decision this file was still making: the `attempt` callback branched on `provider.id` to choose a key. Keys now resolve through a synthetic env, which is the seam complete() reads, so a BYOK caller's key and the server's own take one path instead of two. The two per-vendor call functions and both model loops are gone (-134 lines). Attribution is preserved deliberately. The OpenRouter call sends HTTP-Referer and X-Title, which OpenRouter ranks apps by; complete() had no way to send them, so ai-kit grew `extraHeaders` (0.10.0) rather than this adoption quietly dropping Botsmann off that list. Now tested. THREE TEST DEFECTS, all found by the conversion: - one test asserted THE BUG: "returns empty string when no content in response" expected `result.content === ''`. It is replaced by two tests — an empty 200 demotes to the next link, and an empty 200 at every link throws rather than answering with nothing. - the fetch mocks were `{ ok, json }` literals with no `text()`. That encoded an assumption about HOW the client reads a body and broke the moment it read the text first (to keep the vendor's body in the error). Real Response objects now. - `mockResolvedValue(new Response(...))` handed every link the SAME object, and a Response body can only be read once — so link two always failed with "Body has already been read", a fake failure standing in front of the real behaviour. Anything walking a chain needs a factory, not a value. The `vi.mock('@/lib/constants')` for the endpoint URLs is removed: the client takes them from the same ai-kit provider record that supplies the model list, so mocking them asserted a copy nothing reads. Verified the resulting URLs are byte-identical to the constants they replace. verify green: format, lint, typecheck, selfhost check, 277 tests, build. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NHDLTKGirKU7He8q2YR2y4 --- lib/llm-client.ts | 232 +++++++++++-------------- package.json | 2 +- pnpm-lock.yaml | 10 +- pnpm-workspace.yaml | 2 +- tests/__tests__/lib/llm-client.test.ts | 198 +++++++++++++-------- 5 files changed, 235 insertions(+), 209 deletions(-) diff --git a/lib/llm-client.ts b/lib/llm-client.ts index 45cd29bd..466ad852 100644 --- a/lib/llm-client.ts +++ b/lib/llm-client.ts @@ -7,27 +7,18 @@ * - Ollama (local) */ -import { freeChain, providerModels, usableChain, tryChain } from '@bitbaum/ai-kit'; -import { API_CONFIG } from '@/lib/constants'; +import { + complete, + freeChain, + providerModels, + usableChain, + linkId, + type Link, + type Provider, +} from '@bitbaum/ai-kit'; import { getServerEnv, getClientEnv } from '@/lib/config/env'; import { logger } from './logger'; -/** - * The models to try at each vendor, in order, from `ai-kit`. - * - * A list rather than a name, because the previous single ids were retired out - * from under this app and there was nothing between that and total failure: - * `generateWithBestProvider` picks ONE provider and calls it once. A retired id - * was a dead chatbot with a valid key. - * - * The lists cross models, not vendors — vendor selection above stays exactly as - * it was. That is the smaller half of the protection (a spent daily budget is - * org-wide, so every model at the same vendor dies together), but it is the - * half that covers rot, which is what actually happened here twice. - */ -const groqModels = () => providerModels(freeChain('BOTSMANN')[0]); -const openRouterModels = () => providerModels(freeChain('BOTSMANN')[1]); - export type ModelProvider = 'groq' | 'openrouter' | 'ollama'; interface LLMMessage { @@ -80,44 +71,71 @@ export async function generateLLMResponse( } /** - * One call, one model, at Groq. The single-shot primitive both - * `generateWithGroq`'s model loop and the ai-kit chain in - * `generateWithBestProvider` walk over — a 404 here means the id was - * retired, a 429 means this model is busy or spent, and either way the - * caller's job is to try the next link, not this function's. + * One vendor's chain, in `ai-kit`'s shape. + * + * The key travels through a synthetic env rather than being baked into a + * closure, because that is the seam `complete()` reads — so a BYOK caller's + * key and this server's own key take the same path instead of two. */ -async function callGroqModel( - model: string, +function vendorChain( + which: 0 | 1, key: string, + models?: string[], +): { chain: Link[]; env: Record } { + const provider = freeChain('BOTSMANN')[which] as Provider; + const ids = models?.length ? models : providerModels(provider); + return { + chain: ids.map((model) => ({ provider, model })), + env: { [provider.keyEnv]: key }, + }; +} + +/** + * Walk a chain and answer, or throw naming every link that failed. + * + * WHY THIS REPLACED TWO HAND-ROLLED LOOPS AND TWO `fetch` CALLS. + * + * The loops had the right shape and the wrong judgements — the fleet's most + * common AI defect (census 2026-09-06: 9 of 12 hand-rolled clients share it): + * + * - `data.choices[0]?.message?.content || ''` handed an EMPTY 200 to the + * user as the bot's reply. A reasoning model that spends its budget + * thinking returns exactly that, and so does a vendor having a moment. + * `complete()` calls it a failure and demotes to the next link. + * - a 429 became `throw new Error('Groq API error: ' + status)`: the body + * was read, logged, and discarded. The three kinds of 429 share that + * status code and want opposite responses. A DAILY cap condemns the whole + * vendor, because its other models draw on the same exhausted org-wide + * budget; a SIZE cap means demoting is strictly WORSE, since the next + * model's ceiling is smaller. + * - neither call had a DEADLINE. A vendor that accepted the connection and + * never answered held the chat open indefinitely, and the fallback beneath + * it was never reached — a chain that cannot time out is not a fallback + * for the outage it most needs to survive. + */ +async function completeOn( + { chain, env }: { chain: Link[]; env: Record }, messages: LLMMessage[], temperature: number, maxTokens: number, + extraHeaders?: Record, ): Promise { - const response = await fetch(API_CONFIG.GROQ_API_URL, { - method: 'POST', - headers: { - Authorization: `Bearer ${key}`, - 'Content-Type': 'application/json', + const result = await complete({ + chain, + env, + messages, + temperature, + maxTokens, + extraHeaders, + onLinkFailure: (link: Link, error: Error) => { + logger.warn(`[LLM] ${linkId(link)} failed, trying next`, { error: error.message }); }, - body: JSON.stringify({ - model, - messages, - temperature, - max_tokens: maxTokens, - }), }); - if (!response.ok) { - const text = await response.text(); - logger.error(`Groq API error: ${response.status} (model ${model})`, text); - throw new Error(`Groq API error: ${response.status}`); - } - - const data = await response.json(); return { - content: data.choices[0]?.message?.content || '', - provider: 'groq', - model, + content: result.text, + provider: result.link.provider.id as ModelProvider, + model: result.link.model, }; } @@ -138,59 +156,21 @@ async function generateWithGroq( throw new Error('Groq API key not configured'); } - const models = groqModels(); - let lastError: Error = new Error('Groq API error: no model attempted'); - - for (const model of models) { - try { - return await callGroqModel(model, key, messages, temperature, maxTokens); - } catch (error) { - lastError = error instanceof Error ? error : new Error(String(error)); - } - } - - throw new Error(`${lastError.message} — all ${models.length} model(s) failed`); + return completeOn(vendorChain(0, key), messages, temperature, maxTokens); } /** - * One call, one model, at OpenRouter — the single-shot primitive both - * `generateWithOpenRouter`'s model loop and the ai-kit chain walk over. - * Supports Claude, GPT-4, Gemini, Grok, Llama, Mistral, and more. + * OpenRouter reads these for app attribution in its public rankings. + * + * Carried through `complete`'s `extraHeaders` (ai-kit >= 0.10.0), which exists + * because of this call site: without it, adopting the shared engine would have + * dropped Botsmann off that list with no error, no log line, and nothing to + * notice — a silent downgrade disguised as a refactor. */ -async function callOpenRouterModel( - model: string, - apiKey: string, - messages: LLMMessage[], - temperature: number, - maxTokens: number, -): Promise { - const response = await fetch(API_CONFIG.OPENROUTER_API_URL, { - method: 'POST', - headers: { - Authorization: `Bearer ${apiKey}`, - 'Content-Type': 'application/json', - 'HTTP-Referer': getClientEnv().NEXT_PUBLIC_APP_URL, - 'X-Title': 'Botsmann', - }, - body: JSON.stringify({ - model, - messages, - temperature, - max_tokens: maxTokens, - }), - }); - - if (!response.ok) { - const text = await response.text(); - logger.error(`OpenRouter API error: ${response.status} (model ${model})`, text); - throw new Error(`OpenRouter API error: ${response.status}`); - } - - const data = await response.json(); +function openRouterAttribution(): Record { return { - content: data.choices[0]?.message?.content || '', - provider: 'openrouter', - model, + 'HTTP-Referer': getClientEnv().NEXT_PUBLIC_APP_URL, + 'X-Title': 'Botsmann', }; } @@ -211,18 +191,13 @@ async function generateWithOpenRouter( // An explicit caller override is honoured as-is and alone: if someone names a // model, silently answering from a different one is worse than failing. - const models = model ? [model] : openRouterModels(); - let lastError: Error = new Error('OpenRouter API error: no model attempted'); - - for (const selectedModel of models) { - try { - return await callOpenRouterModel(selectedModel, apiKey, messages, temperature, maxTokens); - } catch (error) { - lastError = error instanceof Error ? error : new Error(String(error)); - } - } - - throw new Error(`${lastError.message} — all ${models.length} model(s) failed`); + return completeOn( + vendorChain(1, apiKey, model ? [model] : undefined), + messages, + temperature, + maxTokens, + openRouterAttribution(), + ); } /** @@ -364,10 +339,13 @@ export async function getBestProvider(): Promise<{ * key look identical to having no provider at all \u2014 botsmann's Groq key * started returning 401 and the whole AI layer went down while an * OpenRouter key sat unused. Now it is ONE chain, built and walked by - * `ai-kit` (`usableChain`/`tryChain`): provider and model demote together, + * `ai-kit` (`usableChain` + `complete`): provider and model demote together, * in a single pass, and `ai-kit` owns the ordering so a fix to the chain * lands here without a matching edit in this file. * + * `complete` also owns the REQUEST now, which is where this file's remaining + * defects lived — see `completeOn` for what the hand-rolled version got wrong. + * * Ollama stays outside that chain and is tried first: its availability is a * live ping, not an API key, which does not fit `ai-kit`'s `Provider` shape. */ @@ -389,31 +367,31 @@ export async function generateWithBestProvider( } } - const chain = usableChain(freeChain('BOTSMANN'), { - GROQ_API_KEY: env.GROQ_API_KEY, - OPENROUTER_API_KEY: env.OPENROUTER_API_KEY, - }); + // One chain across BOTH vendors, so provider and model demote together in a + // single pass. The key for each link is resolved into a synthetic env, which + // is the seam `complete()` reads — the previous `attempt` callback had to + // branch on `provider.id` to pick a key, and that branch was the last place + // this file still made a decision the engine already owns. + const keys = { + GROQ_API_KEY: cleanApiKey(env.GROQ_API_KEY) ?? '', + OPENROUTER_API_KEY: env.OPENROUTER_API_KEY ?? '', + }; + const chain = usableChain(freeChain('BOTSMANN'), keys); if (chain.length === 0) { throw new Error('No LLM provider available. Start Ollama or configure API keys.'); } - const response = await tryChain(chain, { - attempt: ({ provider, model }) => { - if (provider.id === 'groq') { - const key = cleanApiKey(env.GROQ_API_KEY); - if (!key) throw new Error('Groq API key not configured'); - return callGroqModel(model, key, messages, temperature, maxTokens); - } - if (!env.OPENROUTER_API_KEY) throw new Error('OpenRouter API key required'); - return callOpenRouterModel(model, env.OPENROUTER_API_KEY, messages, temperature, maxTokens); - }, - onLinkFailure: (link, error) => { - logger.warn(`[LLM] ${link.provider.id}/${link.model} failed, trying next`, { - error: error instanceof Error ? error.message : String(error), - }); - }, - }); + const response = await completeOn( + { chain, env: keys }, + messages, + temperature, + maxTokens, + // Harmless at Groq, which ignores unknown headers, and required at + // OpenRouter — one chain means one header set, and losing attribution to + // avoid sending two extra headers to Groq would be the wrong trade. + openRouterAttribution(), + ); return { ...response, providerInfo: `${response.provider} (${response.model})` }; } diff --git a/package.json b/package.json index 83742d82..4786a44c 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,7 @@ "lint-staged": "lint-staged" }, "dependencies": { - "@bitbaum/ai-kit": "^0.6.2", + "@bitbaum/ai-kit": "^0.11.0", "@bitbaum/mail-kit": "^0.1.0", "@giscus/react": "^3.1.0", "@headlessui/react": "^2.2.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f935655d..9f2f2991 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -27,8 +27,8 @@ importers: .: dependencies: '@bitbaum/ai-kit': - specifier: ^0.6.2 - version: 0.6.2(react@19.2.8) + specifier: ^0.11.0 + version: 0.11.0(react@19.2.8) '@bitbaum/mail-kit': specifier: ^0.1.0 version: 0.1.0 @@ -245,8 +245,8 @@ packages: resolution: {integrity: sha512-4zBIxpPzowiZpusoFkyGVwakdRJUyuH5PxQ/PrqghfdFWWasvnCdPfQXHrenDai+gyLARulZjZowCOj6fjT4pA==} engines: {node: '>=6.9.0'} - '@bitbaum/ai-kit@0.6.2': - resolution: {integrity: sha512-974ollQp2pugPpsUgdJY/i6r+/v3CEwa3XqmfSdi+vM7w8eMVWufSKQ3mu9QLoEm7b0Gq/mKgLKql3lSawmCfw==} + '@bitbaum/ai-kit@0.11.0': + resolution: {integrity: sha512-75VUchdjZ3iAIZrqjBpWDmtgUyCx0lZ4KN8CYvq2p1sGJwocpIfWEtC2e5khtN6bxeLtanyepHUDZPXgm+fEsQ==} engines: {node: '>=20'} peerDependencies: react: '>=18' @@ -3875,7 +3875,7 @@ snapshots: '@babel/helper-string-parser': 7.29.7 '@babel/helper-validator-identifier': 7.29.7 - '@bitbaum/ai-kit@0.6.2(react@19.2.8)': + '@bitbaum/ai-kit@0.11.0(react@19.2.8)': dependencies: ai-forms: 0.1.2(react@19.2.8) optionalDependencies: diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 75a0a560..b66b4057 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -39,5 +39,5 @@ overrides: # should be deleted. Add lines only for versions we published ourselves — # never to hurry along a third-party package. minimumReleaseAgeExclude: - - '@bitbaum/ai-kit@0.6.2' + - '@bitbaum/ai-kit@0.6.2 || 0.11.0' - '@bitbaum/mail-kit' diff --git a/tests/__tests__/lib/llm-client.test.ts b/tests/__tests__/lib/llm-client.test.ts index d6002600..77c253f2 100644 --- a/tests/__tests__/lib/llm-client.test.ts +++ b/tests/__tests__/lib/llm-client.test.ts @@ -5,18 +5,15 @@ import { generateWithBestProvider, } from '@/lib/llm-client'; -// Mock dependencies // The model ids are deliberately absent: they come from `ai-kit` now, not from // this repo. Asserting them literally here is what made these tests agree with // a production outage — they mocked `llama-3.1-8b-instant` and passed happily // for as long as Groq had been refusing that id in production. -vi.mock('@/lib/constants', () => ({ - API_CONFIG: { - GROQ_API_URL: 'https://api.groq.com/openai/v1/chat/completions', - OPENROUTER_API_URL: 'https://openrouter.ai/api/v1/chat/completions', - }, -})); - +// +// The endpoint URLs are gone from here for the same reason. They used to be +// mocked out of `@/lib/constants`; the client now takes them from the same +// `ai-kit` provider record that supplies the model list, so a mock of them +// would assert a copy nothing reads. import { freeChain, providerModels } from '@bitbaum/ai-kit'; const GROQ_MODELS = providerModels(freeChain('BOTSMANN')[0]); @@ -51,6 +48,40 @@ if (!AbortSignal.timeout) { const mockFetch = vi.fn(); global.fetch = mockFetch; +/** + * Real `Response` objects, not `{ ok, json }` literals. + * + * These tests used to hand back hand-built objects with only `json()`. That + * quietly encoded an assumption about HOW the client reads a body, and it broke + * the moment the client started reading the text first (to keep the vendor's + * body in the error, which is the difference between "429" and "your daily + * budget is gone, it resets in 4h"). A fake that diverges from the contract it + * imitates is how a suite stays green over a client that cannot work. + */ +function jsonResponse(body: unknown, status = 200) { + return new Response(JSON.stringify(body), { + status, + headers: { 'content-type': 'application/json' }, + }); +} + +function completion(content: string) { + return jsonResponse({ choices: [{ message: { content } }] }); +} + +/** + * A fetch mock that returns a FRESH `Response` per call. + * + * `mockResolvedValue(new Response(...))` hands every link the same object, and + * a `Response` body can only be read once — so link two always failed with + * "Body has already been read", a fake failure standing in front of whatever + * the real behaviour was. Anything that walks a chain needs a factory, not a + * value. + */ +function alwaysRespond(make: () => Response) { + mockFetch.mockImplementation(async () => make()); +} + import { getServerEnv } from '@/lib/config/env'; import type { Mock } from 'vitest'; @@ -75,12 +106,11 @@ const testMessages = [ describe('generateLLMResponse', () => { describe('groq provider', () => { it('sends correct request to Groq API', async () => { - mockFetch.mockResolvedValue({ - ok: true, - json: async () => ({ + mockFetch.mockResolvedValue( + jsonResponse({ choices: [{ message: { content: 'Hello back!' } }], }), - }); + ); const result = await generateLLMResponse(testMessages, { provider: 'groq' }); @@ -89,7 +119,9 @@ describe('generateLLMResponse', () => { expect.objectContaining({ method: 'POST', headers: expect.objectContaining({ - Authorization: 'Bearer test-groq-key', + // lowercase: ai-kit's casing. HTTP header names are + // case-insensitive, so this is a spelling change, not a contract one. + authorization: 'Bearer test-groq-key', }), body: expect.stringContaining(`"model":"${GROQ_MODELS[0]}"`), }), @@ -109,15 +141,12 @@ describe('generateLLMResponse', () => { // Before this, `generateWithBestProvider` picked one provider and called // it once, so a retired id was simply a dead chatbot. mockFetch - .mockResolvedValueOnce({ - ok: false, - status: 404, - text: async () => '{"error":{"code":"model_not_found"}}', - }) - .mockResolvedValueOnce({ - ok: true, - json: async () => ({ choices: [{ message: { content: 'second model' } }] }), - }); + .mockResolvedValueOnce( + new Response('{"error":{"code":"model_not_found"}}', { status: 404 }), + ) + .mockResolvedValueOnce( + jsonResponse({ choices: [{ message: { content: 'second model' } }] }), + ); const result = await generateLLMResponse(testMessages, { provider: 'groq' }); @@ -128,25 +157,23 @@ describe('generateLLMResponse', () => { }); it('reports the whole list when every model fails', async () => { - mockFetch.mockResolvedValue({ - ok: false, - status: 429, - text: async () => 'rate limit exceeded', - }); + alwaysRespond(() => new Response('rate limit exceeded', { status: 429 })); + // Every link named, not just the last — a chain that reports only its + // final failure makes "the key is dead" and "one model rotted" read the + // same in a log. await expect(generateLLMResponse(testMessages, { provider: 'groq' })).rejects.toThrow( - /all \d+ model\(s\) failed/, + /All \d+ link\(s\) failed/, ); expect(mockFetch).toHaveBeenCalledTimes(GROQ_MODELS.length); }); it('uses provided API key over server key', async () => { - mockFetch.mockResolvedValue({ - ok: true, - json: async () => ({ + mockFetch.mockResolvedValue( + jsonResponse({ choices: [{ message: { content: 'ok' } }], }), - }); + ); await generateLLMResponse(testMessages, { provider: 'groq', apiKey: 'user-key' }); @@ -154,7 +181,7 @@ describe('generateLLMResponse', () => { expect.any(String), expect.objectContaining({ headers: expect.objectContaining({ - Authorization: 'Bearer user-key', + authorization: 'Bearer user-key', }), }), ); @@ -168,26 +195,39 @@ describe('generateLLMResponse', () => { ); }); - it('throws on non-ok response', async () => { - mockFetch.mockResolvedValue({ - ok: false, - status: 429, - text: async () => 'rate limited', - }); + it('a 429 says WHICH kind of limit, not just the status code', async () => { + alwaysRespond(() => new Response('rate limited', { status: 429 })); + // The old message was 'Groq API error: 429'. Capacity, request-too-large + // and daily-quota share that status code and want opposite responses: + // retry shortly, send less, or come back tomorrow. Only the body tells + // them apart, and it used to be read, logged and discarded. await expect(generateLLMResponse(testMessages, { provider: 'groq' })).rejects.toThrow( - 'Groq API error: 429', + /429 capacity/, ); }); - it('returns empty string when no content in response', async () => { - mockFetch.mockResolvedValue({ - ok: true, - json: async () => ({ choices: [{ message: {} }] }), - }); + it("an empty 200 is a FAILURE that demotes — it used to be the bot's reply", async () => { + // This test previously asserted `result.content === ''`: a 200 carrying + // no content was handed straight to the user as the assistant's answer, + // and the chain stopped, satisfied. A reasoning model that spends its + // whole budget thinking returns exactly this shape. + mockFetch + .mockResolvedValueOnce(jsonResponse({ choices: [{ message: {} }] })) + .mockResolvedValueOnce(completion('the next link had something to say')); const result = await generateLLMResponse(testMessages, { provider: 'groq' }); - expect(result.content).toBe(''); + + expect(result.content).toBe('the next link had something to say'); + expect(result.model).toBe(GROQ_MODELS[1]); + }); + + it('an empty 200 at EVERY link throws rather than answering with nothing', async () => { + alwaysRespond(() => jsonResponse({ choices: [{ message: { content: '' } }] })); + + await expect(generateLLMResponse(testMessages, { provider: 'groq' })).rejects.toThrow( + /empty content/, + ); }); }); @@ -198,13 +238,28 @@ describe('generateLLMResponse', () => { ); }); + it('sends the attribution headers OpenRouter ranks apps by', async () => { + alwaysRespond(() => completion('response')); + + await generateLLMResponse(testMessages, { provider: 'openrouter', apiKey: 'or-key' }); + + // OpenRouter reads these for app attribution in its public rankings. + // Dropping them breaks nothing, errors nowhere and logs nothing — + // Botsmann simply disappears from that list. Exactly the kind of loss + // that survives a refactor unnoticed unless a test names it, which is + // why ai-kit grew `extraHeaders` rather than this call losing them. + const [, init] = mockFetch.mock.calls[0]; + expect(init.headers['HTTP-Referer']).toBe('http://localhost:3000'); + expect(init.headers['X-Title']).toBe('Botsmann'); + expect(init.headers.authorization).toBe('Bearer or-key'); + }); + it('uses default model when none specified', async () => { - mockFetch.mockResolvedValue({ - ok: true, - json: async () => ({ + mockFetch.mockResolvedValue( + jsonResponse({ choices: [{ message: { content: 'response' } }], }), - }); + ); const result = await generateLLMResponse(testMessages, { provider: 'openrouter', @@ -226,12 +281,11 @@ describe('generateLLMResponse', () => { }); it('uses custom model when specified', async () => { - mockFetch.mockResolvedValue({ - ok: true, - json: async () => ({ + mockFetch.mockResolvedValue( + jsonResponse({ choices: [{ message: { content: 'response' } }], }), - }); + ); const result = await generateLLMResponse(testMessages, { provider: 'openrouter', @@ -245,12 +299,11 @@ describe('generateLLMResponse', () => { describe('ollama provider', () => { it('uses default URL when none provided', async () => { - mockFetch.mockResolvedValue({ - ok: true, - json: async () => ({ + mockFetch.mockResolvedValue( + jsonResponse({ message: { content: 'local response' }, }), - }); + ); const result = await generateLLMResponse(testMessages, { provider: 'ollama' }); @@ -265,12 +318,11 @@ describe('generateLLMResponse', () => { }); it('uses custom ollama URL', async () => { - mockFetch.mockResolvedValue({ - ok: true, - json: async () => ({ + mockFetch.mockResolvedValue( + jsonResponse({ message: { content: 'ok' }, }), - }); + ); await generateLLMResponse(testMessages, { provider: 'ollama', @@ -297,12 +349,11 @@ describe('generateLLMResponse', () => { }); it('passes temperature and maxTokens', async () => { - mockFetch.mockResolvedValue({ - ok: true, - json: async () => ({ + mockFetch.mockResolvedValue( + jsonResponse({ choices: [{ message: { content: 'ok' } }], }), - }); + ); await generateLLMResponse(testMessages, { provider: 'groq', @@ -318,7 +369,7 @@ describe('generateLLMResponse', () => { describe('isOllamaAvailable', () => { it('returns true when Ollama responds ok', async () => { - mockFetch.mockResolvedValue({ ok: true }); + mockFetch.mockResolvedValue(new Response('{}', { status: 200 })); const result = await isOllamaAvailable('http://localhost:11434'); expect(result).toBe(true); @@ -336,7 +387,7 @@ describe('isOllamaAvailable', () => { }); it('returns false when Ollama returns non-ok', async () => { - mockFetch.mockResolvedValue({ ok: false }); + mockFetch.mockResolvedValue(new Response('nope', { status: 500 })); const result = await isOllamaAvailable(); expect(result).toBe(false); @@ -345,7 +396,7 @@ describe('isOllamaAvailable', () => { describe('getBestProvider', () => { it('prefers Ollama when available', async () => { - mockFetch.mockResolvedValue({ ok: true }); + mockFetch.mockResolvedValue(new Response('{}', { status: 200 })); const result = await getBestProvider(); expect(result.provider).toBe('ollama'); @@ -412,12 +463,9 @@ describe('generateWithBestProvider — provider-level failover', () => { const target = String(url); if (target.includes('11434')) throw new Error('connection refused'); // no Ollama if (target.includes('groq.com')) { - return { ok: false, status: 401, text: async () => '{"error":{"code":"invalid_api_key"}}' }; + return new Response('{"error":{"code":"invalid_api_key"}}', { status: 401 }); } - return { - ok: true, - json: async () => ({ choices: [{ message: { content: 'from openrouter' } }] }), - }; + return jsonResponse({ choices: [{ message: { content: 'from openrouter' } }] }); }); const result = await generateWithBestProvider(messages); @@ -436,7 +484,7 @@ describe('generateWithBestProvider — provider-level failover', () => { mockFetch.mockImplementation(async (url: string) => { if (String(url).includes('11434')) throw new Error('connection refused'); - return { ok: false, status: 401, text: async () => 'invalid_api_key' }; + return new Response('invalid_api_key', { status: 401 }); }); // Wording moved from "provider(s)" to "link(s)" when the walk moved into