ReviewOS
Reload

also looking at this

stacks/buddy

fix(test): defuse the suite's landmines, and cover the dossier's untested modules

#1436
Merged glennmichael123 wants to merge fix/test-landmines into main
11 files +617 -12

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

test/ai-provider.test.tsmodified+100-0
@@ -376,6 +376,106 @@ describe('anthropic provider', () => {
376376
377377 expect(response.usage.cachedInputTokens).toBe(900)
378378 })
379
380 it('edge case - the remaining stop reasons land in the normalized set', async () => {
381 process.env.ANTHROPIC_API_KEY = 'sk-ant-test'
382 const cases: Array<[string, string]> = [
383 ['stop_sequence', 'end'],
384 ['max_tokens', 'max_tokens'],
385 ['something_new', 'other'],
386 ]
387
388 for (const [wire, normalized] of cases) {
389 stubFetch({
390 id: 'msg',
391 type: 'message',
392 role: 'assistant',
393 model: 'claude-opus-5',
394 content: [{ type: 'text', text: 'x' }],
395 stop_reason: wire,
396 usage: { input_tokens: 1, output_tokens: 1 },
397 })
398
399 const response = await createAiClient({})!.complete({ messages: [{ role: 'user', content: 'hi' }] })
400 expect(response.stopReason).toBe(normalized as never)
401 }
402 })
403
404 it('success case - parses json only for schema-constrained requests', async () => {
405 process.env.ANTHROPIC_API_KEY = 'sk-ant-test'
406 stubFetch({
407 id: 'msg',
408 type: 'message',
409 role: 'assistant',
410 model: 'claude-opus-5',
411 content: [{ type: 'text', text: '{"answer":42}' }],
412 stop_reason: 'end_turn',
413 usage: { input_tokens: 1, output_tokens: 1 },
414 })
415
416 const unconstrained = await createAiClient({})!.complete({ messages: [{ role: 'user', content: 'hi' }] })
417 expect(unconstrained.json).toBeUndefined()
418
419 const constrained = await createAiClient({})!.complete({
420 messages: [{ role: 'user', content: 'hi' }],
421 jsonSchema: { type: 'object' },
422 })
423 expect(constrained.json).toEqual({ answer: 42 })
424 })
425
426 it('failure case - malformed json for a schema-constrained request throws', async () => {
427 process.env.ANTHROPIC_API_KEY = 'sk-ant-test'
428 stubFetch({
429 id: 'msg',
430 type: 'message',
431 role: 'assistant',
432 model: 'claude-opus-5',
433 content: [{ type: 'text', text: 'not json' }],
434 stop_reason: 'end_turn',
435 usage: { input_tokens: 1, output_tokens: 1 },
436 })
437
438 await expect(createAiClient({})!.complete({
439 messages: [{ role: 'user', content: 'hi' }],
440 jsonSchema: { type: 'object' },
441 })).rejects.toBeInstanceOf(AiProviderError)
442 })
443
444 it('success case - sends system, tools and the output config on the wire', async () => {
445 // Asserted at the fetch layer, through the real SDK — a second
446 // `mock.module('@anthropic-ai/sdk')` owner would clobber this suite's
447 // stubs process-wide, which is the same landmine the node:fs mock was.
448 process.env.ANTHROPIC_API_KEY = 'sk-ant-test'
449 let body: any
450 globalThis.fetch = (async (input: unknown, init?: { body?: unknown }) => {
451 const raw = init?.body ?? (input instanceof Request ? await input.clone().text() : undefined)
452 body = raw ? JSON.parse(String(raw)) : undefined
453 return new Response(JSON.stringify({
454 id: 'msg',
455 type: 'message',
456 role: 'assistant',
457 model: 'claude-opus-5',
458 content: [{ type: 'text', text: '{}' }],
459 stop_reason: 'end_turn',
460 usage: { input_tokens: 1, output_tokens: 1 },
461 }), { status: 200, headers: { 'Content-Type': 'application/json' } })
462 }) as unknown as typeof fetch
463
464 await createAiClient({})!.complete({
465 system: 'be brief',
466 effort: 'low',
467 jsonSchema: { type: 'object' },
468 tools: [{ name: 'grep', description: 'search', parameters: { type: 'object' } }],
469 messages: [{ role: 'user', content: 'hi' }],
470 })
471
472 expect(body.model).toBe('claude-opus-5')
473 expect(body.system).toBe('be brief')
474 // The default ceiling is sized to stay under the SDK's HTTP timeout.
475 expect(body.max_tokens).toBe(16000)
476 expect(body.tools).toEqual([{ name: 'grep', description: 'search', input_schema: { type: 'object' } }])
477 expect(body.output_config).toEqual({ effort: 'low', format: { type: 'json_schema', schema: { type: 'object' } } })
478 })
379479})
380480
381481describe('token budget', () => {
test/anthropic-provider.test.tsdeleted+0-191
@@ -1,191 +0,0 @@
1import type { AiCompletionRequest } from '../src/ai/types'
2import { beforeEach, describe, expect, it, mock } from 'bun:test'
3import { createAnthropicProvider } from '../src/ai/providers/anthropic'
4import { AiProviderError } from '../src/ai/types'
5
6/**
7 * The google and openai providers were tested directly; this one — the
8 * default when several keys are present — was not. The SDK is faked at the
9 * module seam so `complete` runs its real mapping code: block collection,
10 * stop-reason normalisation, usage mapping, and the lazy client.
11 *
12 * `mock.module` is process-global; that is safe here because nothing else in
13 * the suite imports `@anthropic-ai/sdk` (it is an optional peer the provider
14 * itself loads lazily). The fake delegates nowhere — there is no real network
15 * client any test should ever construct.
16 */
17interface FakeResponse {
18 content: Array<{ type: 'text', text: string } | { type: 'tool_use', id: string, name: string, input: unknown }>
19 stop_reason: string | null
20 usage: { input_tokens: number, output_tokens: number, cache_read_input_tokens?: number }
21 model: string
22}
23
24const OK_RESPONSE: FakeResponse = {
25 content: [{ type: 'text', text: 'hello' }],
26 stop_reason: 'end_turn',
27 usage: { input_tokens: 10, output_tokens: 5 },
28 model: 'claude-test-1',
29}
30
31let respondWith: () => FakeResponse = () => OK_RESPONSE
32let createCalls: any[] = []
33let constructorCalls: any[] = []
34
35class FakeAnthropic {
36 messages = {
37 create: async (params: unknown): Promise<FakeResponse> => {
38 createCalls.push(params)
39 return respondWith()
40 },
41 }
42
43 constructor(options: unknown) {
44 constructorCalls.push(options)
45 }
46}
47
48mock.module('@anthropic-ai/sdk', () => ({ default: FakeAnthropic }))
49
50/** A minimal request; tests spread overrides onto it. */
51function request(overrides: Partial<AiCompletionRequest> = {}): AiCompletionRequest {
52 return { messages: [{ role: 'user', content: 'hi' }], ...overrides } as AiCompletionRequest
53}
54
55beforeEach(() => {
56 respondWith = () => OK_RESPONSE
57 createCalls = []
58 constructorCalls = []
59})
60
61describe('anthropic provider', () => {
62 describe('response mapping', () => {
63 it('success case - concatenates text blocks and collects tool calls', async () => {
64 respondWith = () => ({
65 ...OK_RESPONSE,
66 content: [
67 { type: 'text', text: 'part one ' },
68 { type: 'tool_use', id: 't1', name: 'read_file', input: { path: 'a.ts' } },
69 { type: 'text', text: 'part two' },
70 ],
71 stop_reason: 'tool_use',
72 })
73
74 const provider = createAnthropicProvider({ apiKey: 'sk-test' })
75 const response = await provider.complete(request(), 'claude-test-1')
76
77 expect(response.text).toBe('part one part two')
78 expect(response.toolCalls).toEqual([{ id: 't1', name: 'read_file', input: { path: 'a.ts' } }])
79 expect(response.stopReason).toBe('tool_use')
80 })
81
82 it('success case - maps usage, including the cache read count only when present', async () => {
83 const provider = createAnthropicProvider({ apiKey: 'sk-test' })
84
85 const plain = await provider.complete(request(), 'claude-test-1')
86 expect(plain.usage).toEqual({ inputTokens: 10, outputTokens: 5 })
87
88 respondWith = () => ({ ...OK_RESPONSE, usage: { input_tokens: 10, output_tokens: 5, cache_read_input_tokens: 7 } })
89 const cached = await provider.complete(request(), 'claude-test-1')
90 expect(cached.usage.cachedInputTokens).toBe(7)
91 })
92
93 it('edge case - every stop reason lands in the normalized set', async () => {
94 // `refusal` in particular must survive: it arrives as a successful HTTP
95 // response with empty content, and a caller that cannot tell would read
96 // the empty text as a valid answer.
97 const cases: Array<[string | null, string]> = [
98 ['end_turn', 'end'],
99 ['stop_sequence', 'end'],
100 ['tool_use', 'tool_use'],
101 ['max_tokens', 'max_tokens'],
102 ['refusal', 'refusal'],
103 ['something_new', 'other'],
104 [null, 'other'],
105 ]
106
107 const provider = createAnthropicProvider({ apiKey: 'sk-test' })
108 for (const [wire, normalized] of cases) {
109 respondWith = () => ({ ...OK_RESPONSE, stop_reason: wire })
110 const response = await provider.complete(request(), 'claude-test-1')
111 expect(response.stopReason).toBe(normalized as typeof response.stopReason)
112 }
113 })
114
115 it('success case - parses json only for schema-constrained requests', async () => {
116 respondWith = () => ({ ...OK_RESPONSE, content: [{ type: 'text', text: '{"answer":42}' }] })
117 const provider = createAnthropicProvider({ apiKey: 'sk-test' })
118
119 const unconstrained = await provider.complete(request(), 'claude-test-1')
120 expect(unconstrained.json).toBeUndefined()
121
122 const constrained = await provider.complete(request({ jsonSchema: { type: 'object' } }), 'claude-test-1')
123 expect(constrained.json).toEqual({ answer: 42 })
124 })
125
126 it('failure case - malformed json for a schema-constrained request throws', async () => {
127 respondWith = () => ({ ...OK_RESPONSE, content: [{ type: 'text', text: 'not json' }] })
128 const provider = createAnthropicProvider({ apiKey: 'sk-test' })
129
130 await expect(provider.complete(request({ jsonSchema: { type: 'object' } }), 'claude-test-1'))
131 .rejects.toBeInstanceOf(AiProviderError)
132 })
133 })
134
135 describe('request mapping', () => {
136 it('success case - passes model, system, tools and output config through', async () => {
137 // The schema-constrained request parses the reply as JSON on the way out.
138 respondWith = () => ({ ...OK_RESPONSE, content: [{ type: 'text', text: '{}' }] })
139 const provider = createAnthropicProvider({ apiKey: 'sk-test' })
140 await provider.complete(request({
141 system: 'be brief',
142 effort: 'low',
143 jsonSchema: { type: 'object' },
144 tools: [{ name: 'grep', description: 'search', parameters: { type: 'object' } }],
145 }), 'claude-test-1')
146
147 const params = createCalls[0]
148 expect(params.model).toBe('claude-test-1')
149 expect(params.system).toBe('be brief')
150 expect(params.tools).toEqual([{ name: 'grep', description: 'search', input_schema: { type: 'object' } }])
151 expect(params.output_config).toEqual({ effort: 'low', format: { type: 'json_schema', schema: { type: 'object' } } })
152 })
153
154 it('edge case - omits the optional keys rather than sending undefined', async () => {
155 const provider = createAnthropicProvider({ apiKey: 'sk-test' })
156 await provider.complete(request(), 'claude-test-1')
157
158 const params = createCalls[0]
159 expect('system' in params).toBe(false)
160 expect('tools' in params).toBe(false)
161 expect('output_config' in params).toBe(false)
162 // The default ceiling is sized to stay under the SDK's HTTP timeout.
163 expect(params.max_tokens).toBe(16000)
164 })
165
166 it('success case - an explicit maxTokens overrides the default', async () => {
167 const provider = createAnthropicProvider({ apiKey: 'sk-test' })
168 await provider.complete(request({ maxTokens: 512 }), 'claude-test-1')
169
170 expect(createCalls[0].max_tokens).toBe(512)
171 })
172 })
173
174 describe('client construction', () => {
175 it('success case - the client is built once and reused', async () => {
176 const provider = createAnthropicProvider({ apiKey: 'sk-test' })
177 await provider.complete(request(), 'claude-test-1')
178 await provider.complete(request(), 'claude-test-1')
179
180 expect(constructorCalls).toHaveLength(1)
181 expect(constructorCalls[0]).toEqual({ apiKey: 'sk-test' })
182 })
183
184 it('success case - a gateway base URL reaches the SDK', async () => {
185 const provider = createAnthropicProvider({ apiKey: 'sk-test', baseUrl: 'https://gateway.example' })
186 await provider.complete(request(), 'claude-test-1')
187
188 expect(constructorCalls[0]).toEqual({ apiKey: 'sk-test', baseURL: 'https://gateway.example' })
189 })
190 })
191})