Skip to content

Commit 6594289

Browse files
committed
fix(providers): cover the option payload, drop the unused discovery constant
1 parent bbc5853 commit 6594289

2 files changed

Lines changed: 58 additions & 32 deletions

File tree

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,65 @@
11
/**
22
* @vitest-environment node
33
*
4-
* These pin transport policy against the vendored SDK rather than against a number
5-
* typed from memory: if an `openai` bump moves `DEFAULT_TIMEOUT` or the retry default,
6-
* this fails and the divergence is a decision rather than a surprise.
4+
* Pinned against the vendored SDKs rather than against numbers typed from memory: if an
5+
* SDK bump moves a default, these fail and the divergence becomes a decision instead of
6+
* a surprise.
77
*/
8+
import Cerebras from '@cerebras/cerebras_cloud_sdk'
9+
import Groq from 'groq-sdk'
10+
import OpenAI from 'openai'
811
import { describe, expect, it } from 'vitest'
912
import {
1013
openAICompatTransport,
11-
PROVIDER_DISCOVERY_TIMEOUT_MS,
1214
PROVIDER_HEADERS_TIMEOUT_MS,
1315
PROVIDER_MAX_RETRIES,
1416
} from '@/providers/transport'
1517

1618
describe('provider transport policy', () => {
17-
it('pins the headers budget to the vendored OpenAI client default', async () => {
18-
const { default: OpenAI } = await import('openai')
19-
expect(PROVIDER_HEADERS_TIMEOUT_MS).toBe(
20-
(OpenAI as unknown as { DEFAULT_TIMEOUT: number }).DEFAULT_TIMEOUT
21-
)
22-
expect(PROVIDER_HEADERS_TIMEOUT_MS).toBe(600_000)
19+
it('pins the headers budget to the vendored OpenAI client default', () => {
20+
expect(PROVIDER_HEADERS_TIMEOUT_MS).toBe(OpenAI.DEFAULT_TIMEOUT)
2321
})
2422

2523
/**
26-
* Not lowered to 0 on purpose. A chat completion is non-idempotent and carries no
27-
* idempotency key, and on the non-streaming path the response exists only once the
28-
* generation is already billed — so a replay re-bills completed work.
24+
* Not lowered to 0 in favour of a hand-rolled loop: a chat completion is
25+
* non-idempotent and carries no idempotency key, and on the non-streaming path the
26+
* response exists only once the generation is already billed, so a replay re-bills
27+
* completed work.
2928
*/
3029
it('keeps the vendor retry default rather than hand-rolling one', () => {
31-
expect(PROVIDER_MAX_RETRIES).toBe(2)
30+
expect(PROVIDER_MAX_RETRIES).toBe(OpenAI.DEFAULT_MAX_RETRIES ?? 2)
3231
})
3332

34-
it('stamps only constructor-safe options, since clients are memoised', () => {
35-
expect(openAICompatTransport()).toEqual({ timeout: 600_000, maxRetries: 2 })
33+
/**
34+
* The assertion that matters: object spread gets no excess-property checking, so a
35+
* renamed option in either SDK would become a silent no-op with a green typecheck.
36+
* These read the value back off a constructed client.
37+
*/
38+
it('actually reaches the Groq client, which defaults to 60s', () => {
39+
const client = new Groq({ apiKey: 'test', ...openAICompatTransport() })
40+
expect(client.timeout).toBe(PROVIDER_HEADERS_TIMEOUT_MS)
41+
expect(client.maxRetries).toBe(PROVIDER_MAX_RETRIES)
42+
})
43+
44+
it('actually reaches the Cerebras client, which defaults to 60s', () => {
45+
const client = new Cerebras({ apiKey: 'test', ...openAICompatTransport() })
46+
expect(client.timeout).toBe(PROVIDER_HEADERS_TIMEOUT_MS)
47+
expect(client.maxRetries).toBe(PROVIDER_MAX_RETRIES)
48+
})
49+
50+
it('actually reaches an OpenAI-compatible client', () => {
51+
const client = new OpenAI({
52+
apiKey: 'test',
53+
baseURL: 'https://example.invalid',
54+
...openAICompatTransport(),
55+
})
56+
expect(client.timeout).toBe(PROVIDER_HEADERS_TIMEOUT_MS)
57+
expect(client.maxRetries).toBe(PROVIDER_MAX_RETRIES)
3658
})
3759

38-
/** A catalog probe must not inherit a generation-sized budget. */
39-
it('bounds discovery far below a generation', () => {
40-
expect(PROVIDER_DISCOVERY_TIMEOUT_MS).toBeLessThan(PROVIDER_HEADERS_TIMEOUT_MS / 10)
60+
/** Stamping a `fetch` wrapper here would convert a header-only timer into a total
61+
* deadline that truncates live streams, so the shape is pinned too. */
62+
it('stamps only constructor-safe options', () => {
63+
expect(Object.keys(openAICompatTransport()).sort()).toEqual(['maxRetries', 'timeout'])
4164
})
4265
})

apps/sim/providers/transport.ts

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,27 @@
11
/**
22
* Transport policy for provider requests, in one place.
33
*
4-
* The values here mostly *pin* what the vendor SDKs already default to, so an SDK
5-
* bump cannot silently move production behaviour. Two are deliberate divergences,
6-
* marked below.
4+
* These pin what the vendor SDKs already default to, so an SDK bump cannot silently
5+
* move production behaviour. For 16 of 18 providers this is a no-op. Groq and Cerebras
6+
* are the exception and are marked at {@link PROVIDER_HEADERS_TIMEOUT_MS}.
77
*
8-
* What these deliberately do NOT do: bound a stalled stream. Under Bun a `fetch`
9-
* carries a socket-scoped idle wall (~300s, reduced by however long a pooled socket
10-
* sat idle before reuse) that no SDK, undici `Agent`, or `RequestInit` option can
11-
* reach. Raising a timeout number therefore cannot rescue a long silent generation —
12-
* only keeping bytes on the socket can. That work belongs in the stream pump.
8+
* What these deliberately do NOT do: bound a stalled stream. Bun's `fetch` is native
9+
* and appears to impose a socket-scoped idle wall of roughly 300s, reduced by however
10+
* long a pooled socket sat idle before reuse. That figure is observed, not documented,
11+
* and is not something these constants can override — raising a number above it changes
12+
* nothing. Only keeping bytes on the socket can rescue a long silent generation, and
13+
* that work belongs in the stream pump.
1314
*/
1415

1516
/**
1617
* Time-to-headers budget for a single attempt, matching `openai@7`'s own
17-
* `DEFAULT_TIMEOUT` so this is behaviour-preserving for every provider already on
18-
* that client.
18+
* `DEFAULT_TIMEOUT`.
19+
*
20+
* Behaviour-preserving for the 16 providers already on that client. It is a deliberate
21+
* divergence for **Groq and Cerebras**, whose SDKs default to 60s: on a non-streaming
22+
* call headers do not arrive until the generation completes, so 60s caps every
23+
* generation at a minute and then retries it twice, re-billing. The cost of the raise is
24+
* that a genuinely hung call now fails slower — see the PR for the worst-case numbers.
1925
*
2026
* It must stay generous: `deepseek-reasoner`, `kimi-k3`, `grok-4.5-reasoning` and
2127
* every dynamic-catalog provider can legitimately generate for minutes with zero
@@ -34,9 +40,6 @@ export const PROVIDER_HEADERS_TIMEOUT_MS = 600_000
3440
*/
3541
export const PROVIDER_MAX_RETRIES = 2
3642

37-
/** Liveness probe for model catalogs, never a generation. Failure degrades gracefully. */
38-
export const PROVIDER_DISCOVERY_TIMEOUT_MS = 15_000
39-
4043
export interface OpenAICompatTransport {
4144
timeout: number
4245
maxRetries: number

0 commit comments

Comments
 (0)