From ba99b079917eb90cfa1eeb5c1684f627b835ac39 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Jul 2026 18:48:55 +0000 Subject: [PATCH] fix(ai-import): adapt to models that reject an explicit temperature OpenAI GPT-5.x / reasoning models reject a non-default `temperature` with a 400, which broke AI bean import for those models (#1151). Instead of hard-coding a model-name list, send `temperature` best-effort: if a request is rejected specifically because of temperature, drop it, retry once, and remember the provider+model so later requests skip it. This keeps the deterministic low temperature on every model that honors it (Anthropic, Gemini, Mistral, gpt-4o, ...) and self-heals for current and future models that do not. Callers that already know a model's capabilities (e.g. from OpenRouter's supported_parameters) can set `supportsTemperature: false` to skip even the first failed attempt. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01DKKv2cbKhcgE2cTbkhvzjd --- .../cloud-llm-communication.service.spec.ts | 142 +++++++++++++++++ .../cloud-llm-communication.service.ts | 144 +++++++++++++++--- 2 files changed, 266 insertions(+), 20 deletions(-) diff --git a/src/services/aiBeanImport/__tests__/cloud-llm-communication.service.spec.ts b/src/services/aiBeanImport/__tests__/cloud-llm-communication.service.spec.ts index dfffcfcd5..ebfb5beae 100644 --- a/src/services/aiBeanImport/__tests__/cloud-llm-communication.service.spec.ts +++ b/src/services/aiBeanImport/__tests__/cloud-llm-communication.service.spec.ts @@ -2,6 +2,7 @@ import { AI_PROVIDER_ENUM } from '../../../enums/settings/aiProvider'; import { CloudLLMConfig, CloudLLMMessage, + resetTemperatureSupportCache, sendCloudLLMPrompt, } from '../cloud-llm-communication.service'; @@ -38,8 +39,19 @@ describe('cloud-llm-communication.service', () => { } as unknown as Response; } + function mockErrorResponse(status: number, text: string): Response { + return { + ok: false, + status, + text: () => Promise.resolve(text), + } as unknown as Response; + } + beforeEach(() => { fetchSpy = spyOn(globalThis, 'fetch'); + // The service learns (per session) which models reject an explicit + // temperature. Reset that between tests so cases stay independent. + resetTemperatureSupportCache(); }); // ── Request building per provider ────────────────────────────────── @@ -417,4 +429,134 @@ describe('cloud-llm-communication.service', () => { ); }); }); + + // ── Temperature capability adaptation ────────────────────────────── + // + // Some models (e.g. OpenAI GPT-5.x reasoning models) reject an explicit + // temperature with a 400. We send it best-effort, and when it is + // rejected we drop it, retry once, and remember the model so later + // requests skip it. Models that accept temperature keep the + // deterministic low value. + + describe('temperature capability adaptation', () => { + const openaiSuccess = () => + mockFetchResponse({ + choices: [{ message: { content: 'ok' } }], + model: 'gpt-5.6-terra', + }); + + it('retries without temperature when the model rejects it', async () => { + // Arrange + const config = createConfig({ + provider: AI_PROVIDER_ENUM.OPENAI, + model: 'gpt-5.6-terra', + }); + fetchSpy.and.returnValues( + Promise.resolve( + mockErrorResponse( + 400, + "Unsupported value: 'temperature' does not support 0.1 with " + + 'this model. Only the default (1) value is supported.', + ), + ), + Promise.resolve(openaiSuccess()), + ); + + // Act + const result = await sendCloudLLMPrompt(config, messages); + + // Assert + expect(result.content).toBe('ok'); + expect(fetchSpy.calls.count()).toBe(2); + const firstBody = JSON.parse(fetchSpy.calls.argsFor(0)[1].body); + const secondBody = JSON.parse(fetchSpy.calls.argsFor(1)[1].body); + expect(firstBody.temperature).toBe(0.1); + expect('temperature' in secondBody).toBe(false); + }); + + it('remembers the rejection and omits temperature on later calls for the same model', async () => { + // Arrange: prime the cache with a reject-then-retry cycle. + const config = createConfig({ + provider: AI_PROVIDER_ENUM.OPENAI, + model: 'gpt-5.6-terra', + }); + fetchSpy.and.returnValues( + Promise.resolve(mockErrorResponse(400, "unsupported 'temperature'")), + Promise.resolve(openaiSuccess()), + ); + await sendCloudLLMPrompt(config, messages); + + // Act: a fresh call for the same model should skip temperature outright. + fetchSpy.calls.reset(); + fetchSpy.and.returnValue(Promise.resolve(openaiSuccess())); + await sendCloudLLMPrompt(config, messages); + + // Assert + expect(fetchSpy.calls.count()).toBe(1); + const body = JSON.parse(fetchSpy.calls.mostRecent().args[1].body); + expect('temperature' in body).toBe(false); + }); + + it('does not retry when a 400 is unrelated to temperature', async () => { + // Arrange + const config = createConfig({ + provider: AI_PROVIDER_ENUM.OPENAI, + model: 'gpt-4o', + }); + fetchSpy.and.returnValue( + Promise.resolve(mockErrorResponse(400, 'Invalid request: bad input')), + ); + + // Act & Assert + await expectAsync( + sendCloudLLMPrompt(config, messages), + ).toBeRejectedWithError(/Cloud LLM API error \(400\)/); + expect(fetchSpy.calls.count()).toBe(1); + }); + + it('omits temperature from the first request when supportsTemperature is false', async () => { + // WHY: OpenRouter exposes supported_parameters, so a caller can tell us + // up front and skip even the first failed attempt. + + // Arrange + const config = createConfig({ + provider: AI_PROVIDER_ENUM.OPENROUTER, + model: 'openai/gpt-5.6-terra', + supportsTemperature: false, + }); + fetchSpy.and.returnValue(Promise.resolve(openaiSuccess())); + + // Act + await sendCloudLLMPrompt(config, messages); + + // Assert + expect(fetchSpy.calls.count()).toBe(1); + const body = JSON.parse(fetchSpy.calls.mostRecent().args[1].body); + expect('temperature' in body).toBe(false); + }); + + it('keeps the low temperature for models that accept it', async () => { + // Arrange + const config = createConfig({ + provider: AI_PROVIDER_ENUM.ANTHROPIC, + model: 'claude-sonnet-4-20250514', + }); + fetchSpy.and.returnValue( + Promise.resolve( + mockFetchResponse({ + content: [{ text: 'ok' }], + model: 'claude-sonnet-4-20250514', + }), + ), + ); + + // Act + await sendCloudLLMPrompt(config, messages); + + // Assert + const body = JSON.parse(fetchSpy.calls.mostRecent().args[1].body); + expect(body.temperature).toBe(0.1); + expect(fetchSpy.calls.count()).toBe(1); + }); + }); }); diff --git a/src/services/aiBeanImport/cloud-llm-communication.service.ts b/src/services/aiBeanImport/cloud-llm-communication.service.ts index 3b4e5d09b..cd242f51e 100644 --- a/src/services/aiBeanImport/cloud-llm-communication.service.ts +++ b/src/services/aiBeanImport/cloud-llm-communication.service.ts @@ -5,6 +5,10 @@ export interface CloudLLMConfig { apiKey: string; model: string; baseUrl?: string; // for CUSTOM provider + // Set to false when the caller already knows the model rejects an explicit + // temperature (e.g. from OpenRouter's supported_parameters), so we skip + // sending it up front instead of learning from a failed request. + supportsTemperature?: boolean; } export interface CloudLLMMessage { @@ -24,11 +28,26 @@ export interface CloudLLMResponse { // response parsing. The shared sendCloudLLMPrompt function handles // fetch, timeout, and error handling — protocol details stay here. +// Low temperature keeps label extraction deterministic. It is sent +// best-effort: models that reject an explicit temperature fall back to their +// default (see sendCloudLLMPrompt). +const DEFAULT_TEMPERATURE = 0.1; + +/** Options that influence how a request body is built. */ +interface BuildOptions { + /** Whether to include the temperature parameter in the request body. */ + includeTemperature: boolean; +} + /** Protocol-level config that describes how to talk to a specific LLM API. */ interface ProviderProtocol { readonly url: string; readonly headers: Record; - buildRequestBody(model: string, messages: CloudLLMMessage[]): object; + buildRequestBody( + model: string, + messages: CloudLLMMessage[], + options: BuildOptions, + ): object; parseResponse(body: unknown): CloudLLMResponse; } @@ -65,11 +84,15 @@ class OpenAICompatibleProtocol implements ProviderProtocol { }; } - buildRequestBody(model: string, messages: CloudLLMMessage[]): object { + buildRequestBody( + model: string, + messages: CloudLLMMessage[], + { includeTemperature }: BuildOptions, + ): object { return { model, messages: messages.map((m) => ({ role: m.role, content: m.content })), - temperature: 0.1, + ...(includeTemperature ? { temperature: DEFAULT_TEMPERATURE } : {}), }; } @@ -105,13 +128,17 @@ class AnthropicProtocol implements ProviderProtocol { }; } - buildRequestBody(model: string, messages: CloudLLMMessage[]): object { + buildRequestBody( + model: string, + messages: CloudLLMMessage[], + { includeTemperature }: BuildOptions, + ): object { const systemMsg = messages.find((m) => m.role === 'system'); const userMsgs = messages.filter((m) => m.role !== 'system'); return { model, max_tokens: 4096, - temperature: 0.1, + ...(includeTemperature ? { temperature: DEFAULT_TEMPERATURE } : {}), system: systemMsg?.content ?? '', messages: userMsgs.map((m) => ({ role: m.role, content: m.content })), }; @@ -177,44 +204,121 @@ function createProtocol(config: CloudLLMConfig): ProviderProtocol { } } -// ── Public API ─────────────────────────────────────────────────────── +// ── Temperature capability adaptation ──────────────────────────────── +// +// A few models (notably OpenAI's GPT-5.x / reasoning family) reject an +// explicit temperature with a 400 and only accept their default value. +// Rather than maintain a model-name list, we send temperature best-effort: +// if a request is rejected specifically because of temperature, we drop it, +// retry once, and remember the model+provider so later requests skip it. +// This keeps the deterministic low temperature on every model that honors +// it and self-heals for models (present and future) that do not. + +const temperatureUnsupported = new Set(); + +function temperatureCacheKey(config: CloudLLMConfig): string { + return `${config.provider}::${config.baseUrl ?? ''}::${config.model}`; +} + +/** True when a response was rejected specifically because of temperature. */ +function isTemperatureRejection(status: number, errorBody: string): boolean { + return status === 400 && /temperature/i.test(errorBody); +} /** - * Send a prompt to a cloud LLM provider and return the response. + * Reset the learned temperature-support cache. * - * Protocol details (URL, headers, body format, response parsing) are - * handled by provider-specific config classes. This function handles - * only fetch, timeout, and error handling. + * The cache is a session-lifetime optimization; this is primarily useful for + * tests, but also lets callers clear it if a model's capabilities change. */ -export async function sendCloudLLMPrompt( - config: CloudLLMConfig, +export function resetTemperatureSupportCache(): void { + temperatureUnsupported.clear(); +} + +// ── Public API ─────────────────────────────────────────────────────── + +/** Perform a single request with its own 30s timeout. */ +async function performRequest( + protocol: ProviderProtocol, + model: string, messages: CloudLLMMessage[], -): Promise { - const protocol = createProtocol(config); - const requestBody = protocol.buildRequestBody(config.model, messages); + includeTemperature: boolean, +): Promise { + const requestBody = protocol.buildRequestBody(model, messages, { + includeTemperature, + }); const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), 30000); - try { - const response = await fetch(protocol.url, { + return await fetch(protocol.url, { method: 'POST', headers: protocol.headers, body: JSON.stringify(requestBody), signal: controller.signal, }); + } finally { clearTimeout(timeout); + } +} + +/** + * Send a prompt to a cloud LLM provider and return the response. + * + * Protocol details (URL, headers, body format, response parsing) are + * handled by provider-specific config classes. This function handles + * fetch, timeout, error handling, and the temperature fallback described + * above. + */ +export async function sendCloudLLMPrompt( + config: CloudLLMConfig, + messages: CloudLLMMessage[], +): Promise { + const protocol = createProtocol(config); + const cacheKey = temperatureCacheKey(config); + const includeTemperature = + config.supportsTemperature !== false && + !temperatureUnsupported.has(cacheKey); + + try { + let response = await performRequest( + protocol, + config.model, + messages, + includeTemperature, + ); if (!response.ok) { const errorBody = await response.text().catch(() => ''); - throw new Error(`Cloud LLM API error (${response.status}): ${errorBody}`); + + // Retry once without temperature when that is what was rejected. + if ( + includeTemperature && + isTemperatureRejection(response.status, errorBody) + ) { + temperatureUnsupported.add(cacheKey); + response = await performRequest( + protocol, + config.model, + messages, + false, + ); + if (!response.ok) { + const retryErrorBody = await response.text().catch(() => ''); + throw new Error( + `Cloud LLM API error (${response.status}): ${retryErrorBody}`, + ); + } + } else { + throw new Error( + `Cloud LLM API error (${response.status}): ${errorBody}`, + ); + } } const body: unknown = await response.json(); return protocol.parseResponse(body); } catch (error) { - clearTimeout(timeout); - if (error.name === 'AbortError') { throw new Error('Cloud LLM request timed out after 30 seconds'); }