Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ e2e/.openfox/
e2e-playwright/.openfox/
e2e/.openfox-test/
e2e/**/.openfox-test/
.openfox/pr-description.md

# Logs
*.log
Expand Down
1 change: 1 addition & 0 deletions src/server/chat/agent-loop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ export interface TopLevelLoopConfig {
supportsVision?: boolean
chatTemplateKwargs?: Record<string, unknown>
queryParams?: Record<string, unknown>
omitParams?: string[]
}
signal?: AbortSignal | undefined
onMessage?: ((msg: ServerMessage) => void) | undefined
Expand Down
30 changes: 30 additions & 0 deletions src/server/chat/stream-pure.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,36 @@ describe('stream-pure', () => {
})
})

it('excludes omitted params from result.modelParams so stats reflect the wire request', async () => {
const client = createMockClient([
{ type: 'text_delta', content: 'hi' },
{
type: 'done',
response: {
id: 'resp-omit',
content: 'hi',
toolCalls: [],
finishReason: 'stop',
usage: { promptTokens: 5, completionTokens: 5, totalTokens: 10 },
},
},
])

const gen = streamLLMPure({
messageId: 'msg-omit',
systemPrompt: 'system',
llmClient: client,
messages: [{ role: 'user', content: 'hello' }],
modelSettings: { omitParams: ['temperature', 'max_tokens'] },
})

const result = await consumeStreamGenerator(gen, () => {})

expect(result.modelParams).not.toHaveProperty('temperature')
expect(result.modelParams).not.toHaveProperty('maxTokens')
expect(result.modelParams).toHaveProperty('topP')
})

it('streams partial arguments for run_command', async () => {
const client = createMockClient([
{ type: 'tool_call_delta', index: 0, name: 'run_command' },
Expand Down
14 changes: 11 additions & 3 deletions src/server/chat/stream-pure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ export interface PureStreamOptions {
toolChoice?: 'auto' | 'none' | 'required'
signal?: AbortSignal | undefined
reasoningEffort?: ReasoningEffort
/** User-configured model settings (temperature, topP, topK, maxTokens, supportsVision) */
modelSettings?: ModelParams & { supportsVision?: boolean }
/** User-configured model settings (temperature, topP, topK, maxTokens, supportsVision, omitParams) */
modelSettings?: ModelParams & { supportsVision?: boolean; omitParams?: string[] }
/** Retry patterns to check mid-stream */
retryPatterns?: RetryPatternConfig[]
/** Set of tool names that are sub-agent aliases (e.g. "explorer").
Expand Down Expand Up @@ -179,7 +179,15 @@ export async function* streamLLMPure(options: PureStreamOptions): AsyncGenerator
const maxTokens = userMaxTokens ?? profile.defaultMaxTokens
const topP = userTopP ?? profile.topP
const topK = userTopK ?? (backend.supportsTopK ? profile.topK : undefined)
const modelParams = buildModelParams({ temperature, topP, topK, maxTokens })
// Omitted params (stripped from the wire request in client-pure) must not be
// reported in stats/truncation-retry modelParams either.
const modelParams = buildModelParams({
temperature,
topP,
topK,
maxTokens,
...(options.modelSettings?.omitParams !== undefined && { omitParams: options.modelSettings.omitParams }),
})

// Log model settings for debugging
logger.debug('LLM request settings', {
Expand Down
3 changes: 1 addition & 2 deletions src/server/chat/stream-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ function buildStreamRequestObject(params: {
toolChoice?: LLMCompletionRequest['toolChoice']
reasoningEffort?: ReasoningEffort | undefined
signal?: AbortSignal | undefined
modelSettings?:
{ temperature?: number; topP?: number; topK?: number; maxTokens?: number; supportsVision?: boolean } | undefined
modelSettings?: LLMCompletionRequest['modelSettings']
}): LLMCompletionRequest {
const { messages, tools, toolChoice, reasoningEffort, signal, modelSettings } = params
return {
Expand Down
3 changes: 3 additions & 0 deletions src/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1817,6 +1817,7 @@ export async function createServerHandle(config: Config): Promise<ServerHandle>
...(m.nonThinkingExtraKwargs !== undefined && { nonThinkingExtraKwargs: m.nonThinkingExtraKwargs }),
...(m.thinkingQueryParams !== undefined && { thinkingQueryParams: m.thinkingQueryParams }),
...(m.nonThinkingQueryParams !== undefined && { nonThinkingQueryParams: m.nonThinkingQueryParams }),
...(m.omitParams !== undefined && { omitParams: m.omitParams }),
...(m.temperature !== undefined && { temperature: m.temperature }),
...(m.topP !== undefined && { topP: m.topP }),
...(m.topK !== undefined && { topK: m.topK }),
Expand Down Expand Up @@ -2046,6 +2047,7 @@ export async function createServerHandle(config: Config): Promise<ServerHandle>
nonThinkingEnabled?: boolean
thinkingQueryParams?: string
nonThinkingQueryParams?: string
omitParams?: string[]
}
}
if (!url) return res.status(400).json({ error: 'url is required' })
Expand Down Expand Up @@ -2086,6 +2088,7 @@ export async function createServerHandle(config: Config): Promise<ServerHandle>
if (modelConfig?.topK !== undefined) modelSettings['topK'] = modelConfig.topK
if (modelConfig?.maxTokens !== undefined) modelSettings['maxTokens'] = modelConfig.maxTokens
if (modelConfig?.supportsVision !== undefined) modelSettings['supportsVision'] = modelConfig.supportsVision
if (modelConfig?.omitParams !== undefined) modelSettings['omitParams'] = modelConfig.omitParams

const rawQP = mode === 'thinking' ? modelConfig?.thinkingQueryParams : modelConfig?.nonThinkingQueryParams
if (rawQP) {
Expand Down
116 changes: 116 additions & 0 deletions src/server/llm/client-pure.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -457,4 +457,120 @@ describe('llm client pure helpers', () => {
// chat_template_kwargs must NOT be here — the modelSettings don't request it
expect(result.params).not.toHaveProperty('chat_template_kwargs')
})

it('strips params listed in modelSettings.omitParams from the final request', async () => {
const profile = {
temperature: 0.7,
defaultMaxTokens: 4096,
topP: 0.9,
supportsVision: false,
}

// omitParams=['temperature'] removes temperature even though profile sets it
const result = await buildNonStreamingCreateParams({
model: 'claude-opus-5',
request: {
messages: [{ role: 'user' as const, content: 'hi' }],
modelSettings: { omitParams: ['temperature'] },
},
profile,
capabilities: { supportsTopK: false, supportsChatTemplateKwargs: false },
})
expect(result.params).not.toHaveProperty('temperature')
expect(result.params).toHaveProperty('top_p', 0.9)
expect(result.params).toHaveProperty('max_tokens', 4096)
})

it('strips top_p when listed in omitParams', async () => {
const profile = {
temperature: 0.7,
defaultMaxTokens: 4096,
topP: 0.9,
supportsVision: false,
}
const result = await buildNonStreamingCreateParams({
model: 'test-model',
request: {
messages: [{ role: 'user' as const, content: 'hi' }],
modelSettings: { omitParams: ['top_p'] },
},
profile,
capabilities: { supportsTopK: false, supportsChatTemplateKwargs: false },
})
expect(result.params).not.toHaveProperty('top_p')
expect(result.params).toHaveProperty('temperature', 0.7)
})

it('omitParams wins over queryParams additions (runs after merge)', async () => {
const profile = {
temperature: 0.7,
defaultMaxTokens: 4096,
topP: 0.9,
supportsVision: false,
}
// queryParams adds temperature: 0.2, but omitParams strips it
const result = await buildNonStreamingCreateParams({
model: 'test-model',
request: {
messages: [{ role: 'user' as const, content: 'hi' }],
modelSettings: {
queryParams: { temperature: 0.2, custom_param: true },
omitParams: ['temperature'],
},
},
profile,
capabilities: { supportsTopK: false, supportsChatTemplateKwargs: false },
})
expect(result.params).not.toHaveProperty('temperature')
expect(result.params).toHaveProperty('custom_param', true)
})

it('does not change params when omitParams is empty or undefined', async () => {
const profile = {
temperature: 0.7,
defaultMaxTokens: 4096,
topP: 0.9,
supportsVision: false,
}
const baseReq = { messages: [{ role: 'user' as const, content: 'hi' }] }

const withoutOmit = await buildNonStreamingCreateParams({
model: 'test-model',
request: baseReq,
profile,
capabilities: { supportsTopK: false, supportsChatTemplateKwargs: false },
})
expect(withoutOmit.params).toHaveProperty('temperature', 0.7)

const withEmpty = await buildNonStreamingCreateParams({
model: 'test-model',
request: { ...baseReq, modelSettings: { omitParams: [] } },
profile,
capabilities: { supportsTopK: false, supportsChatTemplateKwargs: false },
})
expect(withEmpty.params).toHaveProperty('temperature', 0.7)
})

it('omits stripped params from modelParams so stats and retries reflect the wire request', async () => {
const profile = {
temperature: 0.7,
defaultMaxTokens: 4096,
topP: 0.9,
supportsVision: false,
}
const result = await buildNonStreamingCreateParams({
model: 'test-model',
request: {
messages: [{ role: 'user' as const, content: 'hi' }],
modelSettings: { omitParams: ['temperature', 'max_tokens'] },
},
profile,
capabilities: { supportsTopK: false, supportsChatTemplateKwargs: false },
})
expect(result.params).not.toHaveProperty('temperature')
expect(result.params).not.toHaveProperty('max_tokens')
expect(result.modelParams).not.toHaveProperty('temperature')
expect(result.modelParams).not.toHaveProperty('maxTokens')
expect(result.modelParams).toHaveProperty('topP', 0.9)
})
})
31 changes: 26 additions & 5 deletions src/server/llm/client-pure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,15 @@ export function buildModelParams(params: {
topP?: number
topK?: number | undefined
maxTokens?: number
/** Sampling params stripped from the wire request — excluded from modelParams too. */
omitParams?: string[]
}): ModelParams {
const isOmitted = (key: string): boolean => params.omitParams?.includes(key) ?? false
return {
...(params.temperature !== undefined && { temperature: params.temperature }),
...(params.topP !== undefined && { topP: params.topP }),
...(params.topK !== undefined && { topK: params.topK }),
...(params.maxTokens !== undefined && { maxTokens: params.maxTokens }),
...(!isOmitted('temperature') && params.temperature !== undefined && { temperature: params.temperature }),
...(!isOmitted('top_p') && params.topP !== undefined && { topP: params.topP }),
...(!isOmitted('top_k') && params.topK !== undefined && { topK: params.topK }),
...(!isOmitted('max_tokens') && params.maxTokens !== undefined && { maxTokens: params.maxTokens }),
}
}

Expand Down Expand Up @@ -302,7 +305,25 @@ async function buildChatCompletionCreateParams(
}
}

const modelParams = buildModelParams({ temperature, topP, topK, maxTokens })
// Strip params the model rejects (some hosted models reject certain sampling params).
// Runs after all merges so it wins over queryParams additions.
const omitParams = request.modelSettings?.omitParams
if (omitParams && omitParams.length > 0) {
const paramRecord = params as unknown as Record<string, unknown>
for (const key of omitParams) {
delete paramRecord[key]
}
}

// modelParams feed stats and the truncation-retry budget — align them with
// the actual wire request so omitted params aren't reported as sent.
const modelParams = buildModelParams({
temperature,
topP,
topK,
maxTokens,
...(omitParams !== undefined && { omitParams }),
})

return { params, modelParams }
}
Expand Down
2 changes: 2 additions & 0 deletions src/server/llm/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ export interface LLMCompletionRequest {
supportsVision?: boolean
chatTemplateKwargs?: Record<string, unknown>
queryParams?: Record<string, unknown>
/** Top-level request body params to strip from outgoing requests. */
omitParams?: string[]
}
/** When true, include the raw API response body in the result */
returnRaw?: boolean
Expand Down
51 changes: 45 additions & 6 deletions src/server/provider-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -364,8 +364,8 @@ describe('ProviderManager - Model Selection', () => {
const transport = {
id: 'example-transport',
listModels: vi.fn(async () => [
{ id: 'gpt-5.4', contextWindow: 1050000, source: 'backend' as const },
{ id: 'gpt-5.5', contextWindow: 1050000, source: 'backend' as const },
{ id: 'catalog-a', contextWindow: 1050000, source: 'backend' as const },
{ id: 'catalog-b', contextWindow: 1050000, source: 'backend' as const },
]),
complete: vi.fn(),
stream: vi.fn(),
Expand All @@ -382,21 +382,21 @@ describe('ProviderManager - Model Selection', () => {
transportAdapter: 'example-transport',
models: [
{ id: 'model-large', contextWindow: 1050000, source: 'user' },
{ id: 'gpt-5.4', contextWindow: 900000, source: 'user' },
{ id: 'catalog-a', contextWindow: 900000, source: 'user' },
],
isActive: true,
createdAt: new Date().toISOString(),
},
],
defaultModelSelection: 'external/gpt-5.4',
defaultModelSelection: 'external/catalog-a',
}
const manager = createProviderManager(chatConfig, { adapters: adapters as never })

const result = await manager.refreshProviderModels('external')

expect(result).toEqual({ success: true })
const models = manager.getProviders()[0]!.models
expect(models.map((model) => model.id)).toEqual(['gpt-5.4', 'gpt-5.5'])
expect(models.map((model) => model.id)).toEqual(['catalog-a', 'catalog-b'])
expect(models[0]!.contextWindow).toBe(900000)
})

Expand Down Expand Up @@ -433,6 +433,23 @@ describe('ProviderManager - Model Selection', () => {
expect(result).toEqual({ success: false, error: 'No models returned from backend' })
})

it('preserves user models and stays unknown when backend returns empty', async () => {
// model-a becomes a user model after updateModelSettings
await providerManager.updateModelSettings('provider-1', 'model-a', { temperature: 0.5 })

mockFetch.mockResolvedValueOnce({
ok: true,
json: async () => ({ data: [] }),
})

const result = await providerManager.refreshProviderModels('provider-1')
expect(result).toEqual({ success: true })

const provider = providerManager.getProviders().find((p) => p.id === 'provider-1')
expect(provider?.status).toBe('unknown')
expect(provider?.models.map((m) => m.id)).toContain('model-a')
})

it('fetches OpenCode Go models from /zen/go/v1/models', async () => {
const opencodeProvider: Provider = {
id: 'provider-opencode',
Expand Down Expand Up @@ -571,6 +588,28 @@ describe('ProviderManager - Model Selection', () => {
const settings = providerManager.getModelSettings('provider-1', 'model-b')
expect(settings).toBeUndefined()
})

it('surfaces omitParams in modelSettings even without thinking config', async () => {
await providerManager.updateModelSettings('provider-1', 'model-a', {
omitParams: ['temperature'],
})

const settings = providerManager.getModelSettings('provider-1', 'model-a')
expect(settings).toBeDefined()
expect(settings?.omitParams).toEqual(['temperature'])
})

it('surfaces omitParams alongside queryParams', async () => {
await providerManager.updateModelSettings('provider-1', 'model-a', {
thinkingEnabled: true,
thinkingQueryParams: '{"reasoning_effort":"high"}',
omitParams: ['top_p'],
})

const settings = providerManager.getModelSettings('provider-1', 'model-a', 'thinking')
expect(settings?.queryParams).toEqual({ reasoning_effort: 'high' })
expect(settings?.omitParams).toEqual(['top_p'])
})
})

describe('automatic model resolution', () => {
Expand Down Expand Up @@ -656,7 +695,7 @@ describe('ProviderManager - Model Selection', () => {
backend: 'openai',
models: [
{ id: 'model-large', contextWindow: 1050000, source: 'backend' },
{ id: 'gpt-5.4', contextWindow: 1050000, source: 'backend' },
{ id: 'catalog-a', contextWindow: 1050000, source: 'backend' },
],
isActive: true,
createdAt: new Date().toISOString(),
Expand Down
Loading
Loading