diff --git a/.gitignore b/.gitignore index 5590afdf..5a78b273 100644 --- a/.gitignore +++ b/.gitignore @@ -44,6 +44,7 @@ e2e/.openfox/ e2e-playwright/.openfox/ e2e/.openfox-test/ e2e/**/.openfox-test/ +.openfox/pr-description.md # Logs *.log diff --git a/src/server/chat/agent-loop.ts b/src/server/chat/agent-loop.ts index af65b467..66b8a20e 100644 --- a/src/server/chat/agent-loop.ts +++ b/src/server/chat/agent-loop.ts @@ -127,6 +127,7 @@ export interface TopLevelLoopConfig { supportsVision?: boolean chatTemplateKwargs?: Record queryParams?: Record + omitParams?: string[] } signal?: AbortSignal | undefined onMessage?: ((msg: ServerMessage) => void) | undefined diff --git a/src/server/chat/stream-pure.test.ts b/src/server/chat/stream-pure.test.ts index 889471e5..483a492b 100644 --- a/src/server/chat/stream-pure.test.ts +++ b/src/server/chat/stream-pure.test.ts @@ -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' }, diff --git a/src/server/chat/stream-pure.ts b/src/server/chat/stream-pure.ts index d8f54b6b..3c544da8 100644 --- a/src/server/chat/stream-pure.ts +++ b/src/server/chat/stream-pure.ts @@ -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"). @@ -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', { diff --git a/src/server/chat/stream-utils.ts b/src/server/chat/stream-utils.ts index 9d84ee9f..dbcc4b71 100644 --- a/src/server/chat/stream-utils.ts +++ b/src/server/chat/stream-utils.ts @@ -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 { diff --git a/src/server/index.ts b/src/server/index.ts index a0621f07..5b541710 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -1817,6 +1817,7 @@ export async function createServerHandle(config: Config): Promise ...(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 }), @@ -2046,6 +2047,7 @@ export async function createServerHandle(config: Config): Promise nonThinkingEnabled?: boolean thinkingQueryParams?: string nonThinkingQueryParams?: string + omitParams?: string[] } } if (!url) return res.status(400).json({ error: 'url is required' }) @@ -2086,6 +2088,7 @@ export async function createServerHandle(config: Config): Promise 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) { diff --git a/src/server/llm/client-pure.test.ts b/src/server/llm/client-pure.test.ts index c17043f2..d92e5192 100644 --- a/src/server/llm/client-pure.test.ts +++ b/src/server/llm/client-pure.test.ts @@ -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) + }) }) diff --git a/src/server/llm/client-pure.ts b/src/server/llm/client-pure.ts index 75c3708a..aa2f8278 100644 --- a/src/server/llm/client-pure.ts +++ b/src/server/llm/client-pure.ts @@ -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 }), } } @@ -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 + 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 } } diff --git a/src/server/llm/types.ts b/src/server/llm/types.ts index baaa0d33..eb2ff92a 100644 --- a/src/server/llm/types.ts +++ b/src/server/llm/types.ts @@ -39,6 +39,8 @@ export interface LLMCompletionRequest { supportsVision?: boolean chatTemplateKwargs?: Record queryParams?: Record + /** 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 diff --git a/src/server/provider-manager.test.ts b/src/server/provider-manager.test.ts index 1f1c63ac..fd95cdb5 100644 --- a/src/server/provider-manager.test.ts +++ b/src/server/provider-manager.test.ts @@ -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(), @@ -382,13 +382,13 @@ 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 }) @@ -396,7 +396,7 @@ describe('ProviderManager - Model Selection', () => { 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) }) @@ -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', @@ -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', () => { @@ -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(), diff --git a/src/server/provider-manager.ts b/src/server/provider-manager.ts index 6bfac299..535fe101 100644 --- a/src/server/provider-manager.ts +++ b/src/server/provider-manager.ts @@ -260,6 +260,7 @@ export interface ModelSettingsUpdate { nonThinkingExtraKwargs?: string thinkingQueryParams?: string nonThinkingQueryParams?: string + omitParams?: string[] } export interface ProviderManager { @@ -302,6 +303,7 @@ export interface ProviderManager { supportsVision?: boolean chatTemplateKwargs?: Record queryParams?: Record + omitParams?: string[] } | undefined } @@ -789,6 +791,11 @@ export function createProviderManager(config: Config, options: ProviderManagerOp : existingModel?.nonThinkingQueryParams !== undefined ? { nonThinkingQueryParams: existingModel.nonThinkingQueryParams } : {}), + ...(settings.omitParams !== undefined + ? { omitParams: settings.omitParams } + : existingModel?.omitParams !== undefined + ? { omitParams: existingModel.omitParams } + : {}), }) if (existingModel) { @@ -814,6 +821,7 @@ export function createProviderManager(config: Config, options: ProviderManagerOp if (model['topK'] !== undefined) baseSettings['topK'] = model['topK'] if (model['maxTokens'] !== undefined) baseSettings['maxTokens'] = model['maxTokens'] if (model['supportsVision'] !== undefined) baseSettings['supportsVision'] = model['supportsVision'] + if (model['omitParams'] !== undefined) baseSettings['omitParams'] = model['omitParams'] // User-configured queryParams take priority const rawQueryParams = mode === 'thinking' ? model.thinkingQueryParams : model.nonThinkingQueryParams @@ -836,6 +844,9 @@ export function createProviderManager(config: Config, options: ProviderManagerOp return { ...baseSettings, queryParams: JSON.parse(fallbackRawQP) as Record } } + // Return base settings only when omitParams is configured (no thinking config) + if (model['omitParams'] !== undefined) return baseSettings + return undefined }, @@ -863,9 +874,12 @@ export function createProviderManager(config: Config, options: ProviderManagerOp }) if (modelsWithContext.length === 0) { - providerStatus.set(providerId, 'disconnected') - // Keep existing user models when backend is unavailable + // When the backend doesn't expose /v1/models but the user has + // configured models manually, keep them and stay 'unknown' instead + // of marking the provider 'disconnected' — chat/completions may + // still work fine. if (userModels.length > 0) { + providerStatus.set(providerId, 'unknown') logger.debug('Backend unavailable, preserving user models', { providerId, userModels: userModels.map((m) => ({ id: m.id, contextWindow: m.contextWindow })), @@ -873,6 +887,7 @@ export function createProviderManager(config: Config, options: ProviderManagerOp providers = providers.map((p) => (p.id === providerId ? { ...p, models: userModels } : p)) return { success: true } } + providerStatus.set(providerId, 'disconnected') return { success: false, error: 'No models returned from backend' } } diff --git a/src/server/providers/auto-config.test.ts b/src/server/providers/auto-config.test.ts index c762eea6..e7962146 100644 --- a/src/server/providers/auto-config.test.ts +++ b/src/server/providers/auto-config.test.ts @@ -246,3 +246,243 @@ describe('Reasoning message compatibility', () => { expect(result.models[0]?.sendReasoningInMessages).toBeUndefined() }) }) + +describe('Rejected-params probing', () => { + it('detects rejected params from HTTP 400 mentioning the param name', async () => { + const { autoConfig } = await import('./auto-config.js') + + const chatCalls: Array> = [] + const originalFetch = globalThis.fetch + try { + globalThis.fetch = (async (input: string | URL, init?: RequestInit) => { + const url = typeof input === 'string' ? input : input.toString() + // /models endpoint — return empty so detectModelInfo falls back to default + if (url.endsWith('/models')) { + return new Response(JSON.stringify({ data: [] }), { status: 200 }) + } + + // /chat/completions endpoint + const body = init?.body ? (JSON.parse(init.body as string) as Record) : {} + chatCalls.push(body) + + // The rejection-probe baseline sends tools + all standard params. + // Combo probes don't send tools. + const isRejectionProbe = Array.isArray(body['tools']) + + if (isRejectionProbe && 'temperature' in body) { + return new Response( + JSON.stringify({ + error_code: 'BAD_REQUEST', + message: 'BAD_REQUEST: Model does not support the temperature parameter.', + }), + { status: 400 }, + ) + } + // All other requests (combo probes, rejection retry without temperature) → 200 + return new Response( + JSON.stringify({ + choices: [{ message: { content: 'hi' } }], + }), + { status: 200 }, + ) + }) as typeof globalThis.fetch + + const result = await autoConfig({ + url: 'http://localhost:8000/v1', + backend: 'unknown', + models: [{ id: 'test-model' }], + }) + + const model = result.models[0]! + expect(model.rejectedParams).toEqual(['temperature']) + // The retry call (after dropping temperature) should NOT include temperature + const retryCall = chatCalls.find( + (c) => 'top_p' in c && 'max_tokens' in c && 'top_k' in c && !('temperature' in c), + ) + expect(retryCall).toBeDefined() + expect(retryCall).not.toHaveProperty('temperature') + } finally { + globalThis.fetch = originalFetch + } + }) + + it('returns no rejectedParams when baseline succeeds', async () => { + const { autoConfig } = await import('./auto-config.js') + + const originalFetch = globalThis.fetch + try { + globalThis.fetch = (async (input: string | URL) => { + const url = typeof input === 'string' ? input : input.toString() + if (url.endsWith('/models')) { + return new Response(JSON.stringify({ data: [] }), { status: 200 }) + } + return new Response( + JSON.stringify({ + choices: [{ message: { content: 'hi' } }], + }), + { status: 200 }, + ) + }) as typeof globalThis.fetch + + const result = await autoConfig({ + url: 'http://localhost:8000/v1', + backend: 'unknown', + models: [{ id: 'test-model' }], + }) + + const model = result.models[0]! + expect(model.rejectedParams).toBeUndefined() + } finally { + globalThis.fetch = originalFetch + } + }) + + it('detects reasoning_effort rejection when tools are present', async () => { + const { autoConfig } = await import('./auto-config.js') + + const originalFetch = globalThis.fetch + try { + globalThis.fetch = (async (input: string | URL, init?: RequestInit) => { + const url = typeof input === 'string' ? input : input.toString() + if (url.endsWith('/models')) { + return new Response(JSON.stringify({ data: [] }), { status: 200 }) + } + + const body = init?.body ? (JSON.parse(init.body as string) as Record) : {} + const isRejectionProbe = Array.isArray(body['tools']) + + // Rejection probe with reasoning_effort + tools → 400 + if (isRejectionProbe && 'reasoning_effort' in body) { + return new Response( + JSON.stringify({ + error: { + message: + 'Function tools with reasoning_effort are not supported for this model in /v1/chat/completions.', + type: 'invalid_request_error', + param: 'reasoning_effort', + }, + }), + { status: 400 }, + ) + } + // Everything else → 200 + return new Response( + JSON.stringify({ + choices: [{ message: { content: 'hi' } }], + }), + { status: 200 }, + ) + }) as typeof globalThis.fetch + + const result = await autoConfig({ + url: 'http://localhost:8000/v1', + backend: 'unknown', + models: [{ id: 'probe-model' }], + }) + + const model = result.models[0]! + expect(model.rejectedParams).toContain('reasoning_effort') + } finally { + globalThis.fetch = originalFetch + } + }) + + it('skips rejected-params detection when the control probe fails (structural rejection)', async () => { + const { autoConfig } = await import('./auto-config.js') + + const originalFetch = globalThis.fetch + try { + globalThis.fetch = (async (input: string | URL, init?: RequestInit) => { + const url = typeof input === 'string' ? input : input.toString() + if (url.endsWith('/models')) { + return new Response(JSON.stringify({ data: [] }), { status: 200 }) + } + + const body = init?.body ? (JSON.parse(init.body as string) as Record) : {} + const isRejectionProbe = Array.isArray(body['tools']) + const hasSamplingParams = ['temperature', 'top_p', 'max_tokens', 'top_k', 'reasoning_effort'].some( + (p) => p in body, + ) + + // Control probe: tools but no sampling params → structural 400 (e.g. no tool support) + if (isRejectionProbe && !hasSamplingParams) { + return new Response(JSON.stringify({ error: { message: 'This model does not support tools.' } }), { + status: 400, + }) + } + // A param-attributed 400 that would otherwise trigger stripping must be ignored + if (isRejectionProbe && 'temperature' in body) { + return new Response(JSON.stringify({ error: { message: 'unsupported parameter: temperature' } }), { + status: 400, + }) + } + return new Response( + JSON.stringify({ + choices: [{ message: { content: 'hi' } }], + }), + { status: 200 }, + ) + }) as typeof globalThis.fetch + + const result = await autoConfig({ + url: 'http://localhost:8000/v1', + backend: 'unknown', + models: [{ id: 'probe-model' }], + }) + + const model = result.models[0]! + expect(model.rejectedParams).toBeUndefined() + } finally { + globalThis.fetch = originalFetch + } + }) + + it('matches rejected param names on word boundaries to avoid prefix false positives', async () => { + const { autoConfig } = await import('./auto-config.js') + + const originalFetch = globalThis.fetch + try { + globalThis.fetch = (async (input: string | URL, init?: RequestInit) => { + const url = typeof input === 'string' ? input : input.toString() + if (url.endsWith('/models')) { + return new Response(JSON.stringify({ data: [] }), { status: 200 }) + } + + const body = init?.body ? (JSON.parse(init.body as string) as Record) : {} + const isRejectionProbe = Array.isArray(body['tools']) + const hasSamplingParams = ['temperature', 'top_p', 'max_tokens', 'top_k', 'reasoning_effort'].some( + (p) => p in body, + ) + + if (isRejectionProbe && !hasSamplingParams) { + return new Response(JSON.stringify({ choices: [{ message: { content: 'hi' } }] }), { status: 200 }) + } + // Message mentions a DIFFERENT param (max_tokens_budget) whose prefix + // contains 'max_tokens' — must not be attributed to max_tokens. + if (isRejectionProbe && 'max_tokens' in body) { + return new Response( + JSON.stringify({ error: { message: 'max_tokens_budget exceeds server limit of 512000.' } }), + { status: 400 }, + ) + } + return new Response( + JSON.stringify({ + choices: [{ message: { content: 'hi' } }], + }), + { status: 200 }, + ) + }) as typeof globalThis.fetch + + const result = await autoConfig({ + url: 'http://localhost:8000/v1', + backend: 'unknown', + models: [{ id: 'probe-model' }], + }) + + const model = result.models[0]! + expect(model.rejectedParams).toBeUndefined() + } finally { + globalThis.fetch = originalFetch + } + }) +}) diff --git a/src/server/providers/auto-config.ts b/src/server/providers/auto-config.ts index a474cfde..2f7d2496 100644 --- a/src/server/providers/auto-config.ts +++ b/src/server/providers/auto-config.ts @@ -20,6 +20,8 @@ export interface ModelProbeResult { nonThinkingConfig: Record | null /** Set to false when the provider rejects reasoning in assistant history. */ sendReasoningInMessages?: boolean + /** Top-level request body params rejected by the model (to be stripped at request time). */ + rejectedParams?: string[] } export interface AutoConfigInput { @@ -302,6 +304,144 @@ async function probeReasoningInMessages( } } +// ============================================================================ +// Rejected-params probing +// ============================================================================ + +/** Standard top-level sampling params that some models reject. */ +const STANDARD_PARAMS = ['temperature', 'top_p', 'max_tokens', 'top_k', 'reasoning_effort'] + +/** Sampling values used when probing whether a param is accepted. */ +const PARAM_VALUES: Record = { + temperature: 0.7, + top_p: 0.9, + max_tokens: 50, + top_k: 40, + reasoning_effort: 'high', +} + +const escapeRegExp = (value: string): string => value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + +const REJECTED_PARAM_PATTERNS = STANDARD_PARAMS.map((param) => ({ + param, + pattern: new RegExp(`\\b${escapeRegExp(param)}\\b`, 'i'), +})) + +/** Extract the rejected param name from a 400 error message, if any. + * Word-boundary matching avoids attributing errors about e.g. + * "max_tokens_budget" to "max_tokens". */ +function extractRejectedParam(errorText: string): string | undefined { + for (const { param, pattern } of REJECTED_PARAM_PATTERNS) { + if (pattern.test(errorText)) return param + } + return undefined +} + +/** Dummy tool matching the agentic loop's tool schema, so rejection probing + * catches params that are only rejected when tools are present (e.g. some + * models reject reasoning_effort with function tools). */ +const DUMMY_TOOL = { + type: 'function', + function: { + name: 'noop', + description: 'No-op tool for probing', + parameters: { type: 'object', properties: {}, required: [] }, + }, +} + +/** Send a single rejection-probe request with the given sampling params and + * the agentic loop's tool payload. Returns status + error text for parsing. */ +async function probeChatCompletions( + baseUrl: string, + headers: Record, + model: string, + samplingParams: string[], +): Promise<{ ok: boolean; status: number; errorText: string }> { + const body: Record = { + model, + messages: [{ role: 'user', content: 'say hi in one word' }], + tools: [DUMMY_TOOL], + tool_choice: 'auto', + } + for (const param of samplingParams) { + body[param] = PARAM_VALUES[param] + } + try { + const response = await fetch(`${ensureVersionPrefix(baseUrl)}/chat/completions`, { + method: 'POST', + headers, + body: JSON.stringify(body), + signal: AbortSignal.timeout(15000), + }) + if (response.ok) return { ok: true, status: response.status, errorText: '' } + return { ok: false, status: response.status, errorText: await response.text() } + } catch { + return { ok: false, status: 0, errorText: '' } + } +} + +/** + * Probe a model with a baseline request containing all standard sampling params + * and a dummy tool (matching the agentic loop). On HTTP 400 mentioning a param, + * drop it and retry. Returns the list of rejected params so the builder can + * strip them at request time. + */ +async function probeRejectedParams( + baseUrl: string, + apiKey: string | undefined, + model: string, + backend: string, +): Promise { + const headers: Record = { 'Content-Type': 'application/json' } + if (apiKey) headers['Authorization'] = `Bearer ${apiKey}` + + // Start from all standard params; drop top_k for backends that don't support it. + const candidates = [...STANDARD_PARAMS] + if (['openai', 'anthropic', 'ollama'].includes(backend)) { + const idx = candidates.indexOf('top_k') + if (idx !== -1) candidates.splice(idx, 1) + } + + // Control probe: same agentic-loop shape but without any sampling params. + // If the backend rejects even that (e.g. no function-tool support), 400s + // cannot be attributed to specific params — bail out instead of letting + // substring matches strip healthy params off unrelated errors. + const control = await probeChatCompletions(baseUrl, headers, model, []) + if (!control.ok) { + logger.debug('Auto-config: rejection-probe control failed, skipping param detection', { + model, + status: control.status, + }) + return [] + } + + const rejected: string[] = [] + const remaining = [...candidates] + + while (remaining.length > 0) { + const probe = await probeChatCompletions(baseUrl, headers, model, remaining) + if (probe.ok) break + if (probe.status === 400) { + const rejectedParam = extractRejectedParam(probe.errorText) + if (rejectedParam && remaining.includes(rejectedParam)) { + rejected.push(rejectedParam) + remaining.splice(remaining.indexOf(rejectedParam), 1) + logger.debug('Auto-config: param rejected, retrying without', { model, rejectedParam }) + continue + } + // Unknown 400 — stop probing to avoid loops. + break + } + // Non-400 error — stop probing. + break + } + + if (rejected.length > 0) { + logger.info('Auto-config: detected rejected params', { model, rejected }) + } + return rejected +} + // ============================================================================ // Main entry point // ============================================================================ @@ -321,9 +461,10 @@ export async function autoConfig(input: AutoConfigInput): Promise 0 ? { rejectedParams } : {}), }) } diff --git a/src/shared/types.ts b/src/shared/types.ts index fbee422b..1b619920 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -660,6 +660,8 @@ export interface ModelConfig { nonThinkingExtraKwargs?: string // JSON string for non-thinking mode kwargs thinkingQueryParams?: string // JSON string of extra top-level request body params for thinking mode nonThinkingQueryParams?: string // JSON string of extra top-level request body params for non-thinking mode + /** Top-level request body params to strip from outgoing requests (e.g. ["temperature"]). */ + omitParams?: string[] // Profile defaults for transparency defaultTemperature?: number defaultTopP?: number diff --git a/web/src/components/shared/ProviderModal.test.tsx b/web/src/components/shared/ProviderModal.test.tsx index f3540da3..6f266c9b 100644 --- a/web/src/components/shared/ProviderModal.test.tsx +++ b/web/src/components/shared/ProviderModal.test.tsx @@ -576,3 +576,170 @@ describe('ProviderModal - thinkingLevel persistence', () => { expect(savedData.sendReasoningInMessages).toBe(false) }) }) + +describe('ProviderModal - sampling param Send checkboxes', () => { + let container: HTMLElement + let root: ReturnType + let onSaveMock: ReturnType + + beforeEach(() => { + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + onSaveMock = vi.fn() + }) + + afterEach(() => { + root.unmount() + document.body.removeChild(container) + }) + + async function renderModal(models: Array>, editModelId?: string) { + await new Promise((resolve) => { + root.render( + void} + initialStep={2} + editProvider={{ + id: 'test-provider', + name: 'Test Provider', + url: 'http://localhost:8000/v1', + backend: 'vllm' as const, + models: models as never, + }} + editModelId={editModelId} + />, + ) + setTimeout(resolve, 200) + }) + } + + function getSendCheckbox(paramKey: string): HTMLInputElement | null { + return container.querySelector(`input[data-testid="send-${paramKey}"]`) as HTMLInputElement | null + } + + function getParamInput(paramKey: string): HTMLInputElement | null { + return container.querySelector(`input[data-testid="param-${paramKey}"]`) as HTMLInputElement | null + } + + function save() { + const saveButton = container.querySelector('[data-testid="provider-modal-save"]') as HTMLButtonElement | null + saveButton?.click() + } + + it('renders Send checkboxes for Temperature, Top P, Top K, Max tokens checked by default', async () => { + await renderModal([{ id: 'test-model', contextWindow: 200000 }], 'test-model') + + for (const key of ['temperature', 'top_p', 'top_k', 'max_tokens']) { + const cb = getSendCheckbox(key) + expect(cb, `checkbox for ${key} should exist`).toBeTruthy() + expect(cb?.checked, `checkbox for ${key} should be checked by default`).toBe(true) + } + }) + + it('unchecking Temperature adds temperature to omitParams and dims the input', async () => { + await renderModal([{ id: 'test-model', contextWindow: 200000, temperature: 0.7 }], 'test-model') + + const cb = getSendCheckbox('temperature') + expect(cb).toBeTruthy() + cb!.click() + + const input = getParamInput('temperature') + expect(input?.disabled).toBe(true) + expect(input?.value).toBe('') + + save() + const savedData: ProviderFormData = onSaveMock.mock.calls[0]![0]! + const savedModel = savedData.models.find((m) => m.id === 'test-model') + expect(savedModel?.omitParams).toEqual(['temperature']) + }) + + it('re-checking Temperature removes it from omitParams', async () => { + await renderModal([{ id: 'test-model', contextWindow: 200000 }], 'test-model') + + const cb = getSendCheckbox('temperature')! + cb.click() + cb.click() + + save() + const savedData: ProviderFormData = onSaveMock.mock.calls[0]![0]! + const savedModel = savedData.models.find((m) => m.id === 'test-model') + expect(savedModel?.omitParams).toBeUndefined() + }) + + it('preserves pre-existing omitParams entries (e.g. reasoning_effort) when toggling Temperature', async () => { + await renderModal([{ id: 'test-model', contextWindow: 200000, omitParams: ['reasoning_effort'] }], 'test-model') + + const tempCb = getSendCheckbox('temperature')! + expect(tempCb.checked).toBe(true) + tempCb.click() + + save() + const savedData: ProviderFormData = onSaveMock.mock.calls[0]![0]! + const savedModel = savedData.models.find((m) => m.id === 'test-model') + expect(savedModel?.omitParams).toEqual(expect.arrayContaining(['reasoning_effort', 'temperature'])) + expect(savedModel?.omitParams).toHaveLength(2) + }) + + it('reflects auto-config omitParams as unchecked boxes on modal open', async () => { + await renderModal( + [{ id: 'test-model', contextWindow: 200000, omitParams: ['temperature', 'top_p', 'top_k', 'reasoning_effort'] }], + 'test-model', + ) + + expect(getSendCheckbox('temperature')?.checked).toBe(false) + expect(getSendCheckbox('top_p')?.checked).toBe(false) + expect(getSendCheckbox('top_k')?.checked).toBe(false) + expect(getSendCheckbox('max_tokens')?.checked).toBe(true) + + const tempInput = getParamInput('temperature') + expect(tempInput?.disabled).toBe(true) + + save() + const savedData: ProviderFormData = onSaveMock.mock.calls[0]![0]! + const savedModel = savedData.models.find((m) => m.id === 'test-model') + expect(savedModel?.omitParams).toEqual(['temperature', 'top_p', 'top_k', 'reasoning_effort']) + }) + + it('shows a re-enable checkbox for reasoning_effort when auto-config omitted it, and re-checking removes it', async () => { + await renderModal( + [{ id: 'test-model', contextWindow: 200000, thinkingEnabled: true, omitParams: ['reasoning_effort'] }], + 'test-model', + ) + + const reEnableCb = container.querySelector( + 'input[data-testid="re-enable-reasoning_effort"]', + ) as HTMLInputElement | null + expect(reEnableCb).toBeTruthy() + expect(reEnableCb?.checked).toBe(false) + + reEnableCb!.click() + + save() + const savedData: ProviderFormData = onSaveMock.mock.calls[0]![0]! + const savedModel = savedData.models.find((m) => m.id === 'test-model') + expect(savedModel?.omitParams).toBeUndefined() + }) + + it('shows re-enable reasoning_effort checkbox even when thinking is disabled', async () => { + await renderModal( + [{ id: 'test-model', contextWindow: 200000, thinkingEnabled: false, omitParams: ['reasoning_effort'] }], + 'test-model', + ) + + const reEnableCb = container.querySelector( + 'input[data-testid="re-enable-reasoning_effort"]', + ) as HTMLInputElement | null + expect(reEnableCb).toBeTruthy() + expect(reEnableCb?.checked).toBe(false) + + reEnableCb!.click() + + save() + const savedData: ProviderFormData = onSaveMock.mock.calls[0]![0]! + const savedModel = savedData.models.find((m) => m.id === 'test-model') + expect(savedModel?.omitParams).toBeUndefined() + }) +}) diff --git a/web/src/components/shared/ProviderModal.tsx b/web/src/components/shared/ProviderModal.tsx index e95f5b0b..308fd265 100644 --- a/web/src/components/shared/ProviderModal.tsx +++ b/web/src/components/shared/ProviderModal.tsx @@ -41,6 +41,7 @@ interface ModelConfig { nonThinkingExtraKwargs?: string thinkingQueryParams?: string nonThinkingQueryParams?: string + omitParams?: string[] temperature?: number topP?: number topK?: number @@ -121,6 +122,13 @@ function ModelConfigPanel({ onTestParams: (id: string, mode: 'thinking' | 'non-thinking') => void onShowRaw: (data: string) => void }) { + function toggleOmitParam(modelId: string, paramKey: string) { + const current = modelConfigs[modelId]?.omitParams ?? [] + const isOmitted = current.includes(paramKey) + const next = isOmitted ? current.filter((p) => p !== paramKey) : [...current, paramKey] + onUpdateConfig(modelId, { omitParams: next.length > 0 ? next : undefined }) + } + return (
@@ -279,83 +287,75 @@ function ModelConfigPanel({ />
)} + {(modelConfigs[model.id]?.omitParams ?? []).includes('reasoning_effort') && ( + + )}

Sampling parameters

-
- - - onUpdateConfig(model.id, { - temperature: e.target.value ? parseFloat(e.target.value) : undefined, - }) - } - placeholder={modelConfigs[model.id]?.defaultTemperature?.toString() ?? 'Using default'} - className="w-full px-2 py-1 bg-bg-tertiary border border-border rounded text-xs text-text-primary" - /> - {modelConfigs[model.id]?.defaultTemperature !== undefined && ( -

default: {modelConfigs[model.id]?.defaultTemperature}

- )} -
-
- - - onUpdateConfig(model.id, { - topP: e.target.value ? parseFloat(e.target.value) : undefined, - }) - } - placeholder={modelConfigs[model.id]?.defaultTopP?.toString() ?? 'Using default'} - className="w-full px-2 py-1 bg-bg-tertiary border border-border rounded text-xs text-text-primary" - /> - {modelConfigs[model.id]?.defaultTopP !== undefined && ( -

default: {modelConfigs[model.id]?.defaultTopP}

- )} -
+ (v ? parseFloat(v) : undefined)} + valueField="temperature" + /> + (v ? parseFloat(v) : undefined)} + valueField="topP" + />
-
- - - onUpdateConfig(model.id, { - topK: e.target.value ? parseInt(e.target.value) : undefined, - }) - } - placeholder={modelConfigs[model.id]?.defaultTopK?.toString() ?? 'Using default'} - className="w-full px-2 py-1 bg-bg-tertiary border border-border rounded text-xs text-text-primary" - /> - {modelConfigs[model.id]?.defaultTopK !== undefined && ( -

default: {modelConfigs[model.id]?.defaultTopK}

- )} -
-
- - - onUpdateConfig(model.id, { - maxTokens: e.target.value ? parseInt(e.target.value) : undefined, - }) - } - placeholder={modelConfigs[model.id]?.defaultMaxTokens?.toString() ?? 'Using default'} - className="w-full px-2 py-1 bg-bg-tertiary border border-border rounded text-xs text-text-primary" - /> - {modelConfigs[model.id]?.defaultMaxTokens !== undefined && ( -

default: {modelConfigs[model.id]?.defaultMaxTokens}

- )} -
+ (v ? parseInt(v) : undefined)} + valueField="topK" + /> + (v ? parseInt(v) : undefined)} + valueField="maxTokens" + />
@@ -370,6 +370,64 @@ function ModelConfigPanel({ ) } +function SamplingParamField({ + modelId, + paramKey, + label, + value, + defaultValue, + omitParams, + onUpdateConfig, + onToggleOmit, + parseValue, + valueField, + step, +}: { + modelId: string + paramKey: string + label: string + value: number | undefined + defaultValue: number | undefined + omitParams: string[] | undefined + onUpdateConfig: (id: string, partial: Partial) => void + onToggleOmit: (modelId: string, paramKey: string) => void + parseValue: (v: string) => number | undefined + valueField: keyof ModelConfig + step?: number +}) { + const isOmitted = omitParams?.includes(paramKey) ?? false + return ( +
+
+ + +
+ onUpdateConfig(modelId, { [valueField]: parseValue(e.target.value) } as Partial)} + placeholder={isOmitted ? 'Not sent' : (defaultValue?.toString() ?? 'Using default')} + className="w-full px-2 py-1 bg-bg-tertiary border border-border rounded text-xs text-text-primary disabled:opacity-40" + /> + {!isOmitted && defaultValue !== undefined && ( +

default: {defaultValue}

+ )} +
+ ) +} + function AutoCompactionField({ value, maxTokens, @@ -620,6 +678,7 @@ export function ProviderModal({ nonThinkingEnabled: m.nonThinkingEnabled, thinkingQueryParams: m.thinkingQueryParams, nonThinkingQueryParams: m.nonThinkingQueryParams, + omitParams: m.omitParams, defaultTemperature: m.defaultTemperature, defaultTopP: m.defaultTopP, defaultTopK: m.defaultTopK, @@ -845,6 +904,7 @@ export function ProviderModal({ thinkingConfig: Record | null nonThinkingConfig: Record | null sendReasoningInMessages?: boolean + rejectedParams?: string[] }> } for (const m of data.models) { @@ -865,6 +925,9 @@ export function ProviderModal({ if (m.sendReasoningInMessages === false) { setSendReasoningInMessages(false) } + if (m.rejectedParams && m.rejectedParams.length > 0) { + config.omitParams = m.rejectedParams + } updateModelConfig(m.id, config) setAutoConfigState((prev) => ({ ...prev, @@ -910,6 +973,7 @@ export function ProviderModal({ nonThinkingEnabled: config?.nonThinkingEnabled, thinkingQueryParams: config?.thinkingQueryParams, nonThinkingQueryParams: config?.nonThinkingQueryParams, + omitParams: config?.omitParams, }, }), }) @@ -979,6 +1043,7 @@ export function ProviderModal({ nonThinkingEnabled: modelConfigs[m.id]?.nonThinkingEnabled, thinkingQueryParams: modelConfigs[m.id]?.thinkingQueryParams, nonThinkingQueryParams: modelConfigs[m.id]?.nonThinkingQueryParams, + omitParams: modelConfigs[m.id]?.omitParams, temperature: modelConfigs[m.id]?.temperature, topP: modelConfigs[m.id]?.topP, topK: modelConfigs[m.id]?.topK,