refactor(llm): complete() owns the request — and a test asserted the bug - #181
Merged
Merged
Conversation
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NHDLTKGirKU7He8q2YR2y4
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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).content || ''throw new Error('Groq API error: ' + status)It also removes the last decision this file was still making: the
attemptcallback branched onprovider.idto pick a key. Keys now resolve through a synthetic env — the seamcomplete()reads — so a BYOK caller's key and the server's own take one path instead of two. Both per-vendor call functions and both model loops are gone.Attribution is preserved, deliberately
The OpenRouter call sends
HTTP-RefererandX-Title, which OpenRouter ranks apps by.complete()had no way to send them — so ai-kit grewextraHeaders(0.10.0) rather than this adoption quietly dropping Botsmann off that list. There is now a test naming it, because the loss produces no error, no log line and nothing to notice.Three test defects, all surfaced by the conversion
One test asserted the bug.
returns empty string when no content in responseexpectedresult.content === ''— it encoded the defect as the contract. 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 notext(). That quietly 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 — the difference between "429" and "your daily budget is gone, resets in 4h"). RealResponseobjects now.mockResolvedValue(new Response(...))handed every link the same object, and aResponsebody can only be read once. Link two therefore always failed withBody 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.The now-inert
vi.mock('@/lib/constants')for 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, so no endpoint moved.Verified
llm-client.ts🤖 Generated with Claude Code
https://claude.ai/code/session_01NHDLTKGirKU7He8q2YR2y4