diff --git a/src/utils/ai.js b/src/utils/ai.js index 8c0b6f79..af50aefc 100644 --- a/src/utils/ai.js +++ b/src/utils/ai.js @@ -361,8 +361,10 @@ async function callBedrock(messages, model, temperature, maxTokens) { // hit the limit with the whole budget spent on reasoning and nothing written: // a 200, finish_reason=length, content ''. Every caller then saw an empty // reply — the take-home sheet was the one the user noticed. The retry gives -// the same request room to finish and asks for low reasoning effort; it fires -// only for that signature, never for a genuinely empty answer. +// the same request room to finish; it does not ask for a different reasoning +// effort, because on DeepSeek 'low' means thinking on — which is how a starved +// call came back starved with four times the budget. It fires only for that +// signature, never for a genuinely empty answer. var REASONING_RETRY_FLOOR = 6000; async function callLiteLLM(messages, model, temperature, maxTokens, generation) { @@ -389,7 +391,7 @@ async function callLiteLLM(messages, model, temperature, maxTokens, generation) var retryBudget = Math.max(REASONING_RETRY_FLOOR, (maxTokens || 0) * 4); console.warn('[ai] reasoning consumed the budget; retrying ' + model + ' with max_tokens=' + retryBudget); return callLiteLLM(messages, model, temperature, retryBudget, - Object.assign({}, generation || {}, { reasoningEffort: 'low', reasoningRetried: true })); + Object.assign({}, generation || {}, { reasoningRetried: true })); } } return { diff --git a/src/utils/generationOptions.js b/src/utils/generationOptions.js index ac1a7bff..ece50a3b 100644 --- a/src/utils/generationOptions.js +++ b/src/utils/generationOptions.js @@ -12,10 +12,18 @@ function addReasoningOptions(request, generation) { // DeepSeek models think by default: measured on ds-deepseek-v4.1-flash, a // three-sentence clinical answer spent 301 reasoning tokens and 2.9 s before // writing, and gave the same answer in 0.9 s with thinking switched off. The - // switch is DeepSeek's own field, which LiteLLM passes through; an effort of - // 'none' turns it off and anything else leaves the provider default alone. + // switch is DeepSeek's own field, which LiteLLM passes through. + // + // Off unless a caller asks for thinking. It used to be opt-in and the callers + // that said nothing were the ones that paid: a 16,000-token deck reasoned its + // whole budget away and wrote nothing (reasoning_chars=51573), the four + // 2,000-token reviews of that deck did the same, and a revision reasoned for + // a minute and a half. An explicit effort still means what it says, so a task + // that wants reasoning asks for it. if (/deepseek/i.test(String(request.model || ''))) { - if (generation.reasoningEffort === 'none') request.thinking = { type: 'disabled' }; + if (generation.reasoningEffort == null || generation.reasoningEffort === 'none') { + request.thinking = { type: 'disabled' }; + } return request; } // These OpenAI-compatible fields are rejected by ordinary chat models such as GPT-4.1. diff --git a/test/clinical-generation-options.test.js b/test/clinical-generation-options.test.js index a1abf55e..6033b521 100644 --- a/test/clinical-generation-options.test.js +++ b/test/clinical-generation-options.test.js @@ -1,5 +1,7 @@ const test = require('node:test'); const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); const { assistantGenerationOptions } = require('../src/utils/clinicalAnswer'); const { resolveGenerationOptions, addReasoningOptions } = require('../src/utils/generationOptions'); @@ -35,6 +37,33 @@ test('DeepSeek thinks unless told not to: only an effort of none sends the switc { model: 'ds-deepseek-r1', thinking: { type: 'disabled' } }); }); +test('saying nothing now means no thinking on DeepSeek, and an explicit effort still overrides it', () => { + // The switch was opt-in, and the callers that did not opt in were the ones + // that paid for it: a deck reasoned its whole 16,000-token budget away and + // wrote nothing, and so did the reviews and the revision after it. Every + // clinical route passes no reasoning option at all, which is what this is for. + assert.deepEqual(addReasoningOptions({ model: 'ds-deepseek-v4.1-flash' }, {}), + { model: 'ds-deepseek-v4.1-flash', thinking: { type: 'disabled' } }); + assert.deepEqual(addReasoningOptions({ model: 'openrouter-deepseek-v4.1-flash' }, {}), + { model: 'openrouter-deepseek-v4.1-flash', thinking: { type: 'disabled' } }); + assert.deepEqual(addReasoningOptions({ model: 'ds-deepseek-r1' }, { reasoningEffort: 'high' }), + { model: 'ds-deepseek-r1' }, 'a task that wants reasoning still gets it'); + // And nothing changes for models this rule was never about. + assert.deepEqual(addReasoningOptions({ model: 'openai-gpt-4.1' }, {}), { model: 'openai-gpt-4.1' }); + assert.deepEqual(addReasoningOptions({ model: 'groq-qwen3.8-27b' }, {}), { model: 'groq-qwen3.8-27b' }); +}); + +test('the starvation retry adds room, and does not turn thinking back on', () => { + // It used to retry with reasoningEffort 'low', which on DeepSeek is thinking + // on: the retry of a call that had just reasoned its budget away asked it to + // reason again with four times the budget. More room is the fix. + const src = fs.readFileSync(path.join(__dirname, '..', 'src/utils/ai.js'), 'utf8'); + const retry = src.slice(src.indexOf('var starved ='), src.indexOf('var starved =') + 700); + assert.match(retry, /reasoningRetried: true/); + assert.doesNotMatch(retry, /reasoningEffort/, 'the retry must not ask for thinking'); + assert.match(retry, /reasoning consumed the budget; retrying/); +}); + test('the assistant profile takes its effort from the environment, and only a known value', () => { const { assistantReasoningEffort } = require('../src/utils/clinicalAnswer'); assert.equal(assistantReasoningEffort({}), 'low');