From cddf47ab8182e11d9d938b78165620b3e64ba5d9 Mon Sep 17 00:00:00 2001 From: ajianaz Date: Fri, 28 Aug 2026 09:28:15 +0700 Subject: [PATCH] fix(llm): recover from empty responses caused by reasoning token budgets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reasoning models (e.g. GLM via Bifrost) can spend the entire max_tokens budget on chain-of-thought and return HTTP 200 with content: "" and finish_reason: "length". cora never read finish_reason or reasoning_content and silently fed an empty string to the JSON parser, surfacing the misleading 'EOF while parsing a value at line 1 column 0' — while the model's actual answer sat in the backend logs (cosy#63) (#536). Recovery now follows productivity over token frugality: - response parsing captures finish_reason and reasoning_content - empty content + finish_reason=length auto-retries with doubled budget (4096 -> ... -> 32768 ceiling) inside chat_completion - last resort: JSON-looking reasoning text is salvaged as the raw response; the parse layer still validates it - parse layer reports an explicit 'provider returned an EMPTY response' instead of serde EOF noise - default max_tokens raised 4096 -> 8192 for reasoning headroom Regression tests: budget escalation table, reasoning salvage (string/parts/fenced/prose), explicit empty-raw error. Signed-off-by: ajianaz --- CHANGELOG.md | 8 +++ src/config/schema.rs | 6 +- src/engine/llm.rs | 150 ++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 153 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0682144..1754ea9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **Empty LLM responses from reasoning models.** Models like GLM can spend the entire `max_tokens` budget on chain-of-thought and return `content: ""` with `finish_reason: "length"`, which previously surfaced as a misleading `EOF while parsing` error. Cora now reads `finish_reason`/`reasoning_content`, automatically retries with a doubled budget (up to 32768), salvages JSON from reasoning text as a last resort, and reports an explicit "EMPTY response" error when nothing is recoverable (#536). + +### Changed + +- **Default `max_tokens` raised from 4096 to 8192** to give reasoning models headroom above their chain-of-thought (#536). + ### Changed - **Relicensed from MIT to Apache-2.0.** All 18 CodeCoraDev repositories now diff --git a/src/config/schema.rs b/src/config/schema.rs index ffc96d7..56bdd67 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -150,7 +150,7 @@ impl Default for Config { scan_system_prompt_override: None, scan_system_prompt_file: None, temperature: 0.0, - max_tokens: 4096, + max_tokens: 8192, // #536: reasoning models need headroom above chain-of-thought max_tokens_param: "auto".to_string(), timeout: 600, cache_ttl: 1440, // 24h in minutes @@ -1340,7 +1340,7 @@ scan: #[test] fn config_default_max_tokens() { let cfg = Config::default(); - assert_eq!(cfg.max_tokens, 4096); + assert_eq!(cfg.max_tokens, 8192); } #[test] @@ -1402,7 +1402,7 @@ llm: cora.merge_into(&mut cfg).unwrap(); assert_eq!(cfg.temperature, 0.7); // Other LLM fields should remain at defaults - assert_eq!(cfg.max_tokens, 4096); + assert_eq!(cfg.max_tokens, 8192); assert_eq!(cfg.timeout, 600); assert_eq!(cfg.cache_ttl, 1440); } diff --git a/src/engine/llm.rs b/src/engine/llm.rs index 19f2692..fa74e7a 100644 --- a/src/engine/llm.rs +++ b/src/engine/llm.rs @@ -42,6 +42,55 @@ static SHARED_CLIENT: LazyLock = LazyLock::new(|| { }) }); +/// Cap for the empty-content budget escalation (#536). +const MAX_TOKENS_CEILING: u32 = 32_768; + +/// Next output budget when a response came back with empty content. +/// `finish_reason == "length"` means reasoning consumed the budget — double +/// it, capped at [`MAX_TOKENS_CEILING`]. Any other reason → give up (None). +fn next_budget_on_empty(finish_reason: Option<&str>, current: u32) -> Option { + if finish_reason != Some("length") { + return None; + } + let doubled = current.saturating_mul(2); + (doubled <= MAX_TOKENS_CEILING).then_some(doubled) +} + +/// Flatten a `reasoning_content` value (string or content-parts array) to text. +fn reasoning_text(v: &Value) -> Option { + match v { + Value::String(s) => Some(s.clone()), + Value::Array(parts) => { + let joined: Vec = parts + .iter() + .filter_map(|p| { + p.get("text") + .and_then(|t| t.as_str()) + .map(std::string::ToString::to_string) + }) + .collect(); + (!joined.is_empty()).then(|| { + joined.join( + " +", + ) + }) + } + _ => None, + } +} + +/// Last-resort raw response when `content` is empty: some models write the +/// final JSON inside their reasoning. Only accept when it plausibly contains +/// JSON — the parse layer still validates. +fn salvage_from_reasoning(reasoning: Option<&Value>) -> Option { + let text = reasoning_text(reasoning?)?; + let trimmed = text.trim(); + let plausible = + trimmed.starts_with('[') || trimmed.starts_with('{') || trimmed.contains("```json"); + plausible.then(|| trimmed.to_string()) +} + /// Return the shared `reqwest::Client` for LLM API requests. pub fn shared_client() -> reqwest::Client { SHARED_CLIENT.clone() @@ -80,7 +129,20 @@ struct ChatResponse { #[derive(Debug, Clone, Deserialize)] struct ChatChoice { - message: ChatMessage, + message: ResponseMessage, + #[serde(default)] + finish_reason: Option, +} + +/// Response-side message: `content` may be ABSENT or null when a reasoning +/// model spends the entire output budget on chain-of-thought (#536), and some +/// providers expose the thinking under `reasoning_content` (string or parts). +#[derive(Debug, Clone, Deserialize)] +struct ResponseMessage { + #[serde(default)] + content: Option, + #[serde(default)] + reasoning_content: Option, } /// Usage statistics from the LLM API response. @@ -342,18 +404,53 @@ async fn chat_completion( let parsed: ChatResponse = serde_json::from_str(&body).map_err(|e| CoraError::LlmParse(format!("{e}: {body}")))?; - let content = parsed - .choices - .first() - .map(|c| c.message.content.clone()) - .unwrap_or_default(); - let usage = parsed.usage.as_ref().and_then(parse_usage_value); debug!(tokens = ?usage, "LLM response received"); tracing::Span::current().record("tokens_used", usage.as_ref().map(|u| u.total_tokens)); - Ok((content, usage)) + let choice = parsed.choices.first(); + let finish_reason = choice.and_then(|c| c.finish_reason.clone()); + let reasoning = choice.and_then(|c| c.message.reasoning_content.clone()); + let content = choice + .and_then(|c| c.message.content.clone()) + .unwrap_or_default(); + + if !content.trim().is_empty() { + return Ok((content, usage)); + } + + // Empty content (#536): a reasoning model can spend the whole output + // budget on chain-of-thought. Recover instead of failing — first by + // raising the budget, then by salvaging JSON from the reasoning text. + if let Some(next) = next_budget_on_empty(finish_reason.as_deref(), config.max_tokens) { + tracing::warn!( + finish_reason = ?finish_reason, + from = config.max_tokens, + to = next, + "empty LLM content — retrying with raised max_tokens" + ); + let mut raised = config.clone(); + raised.max_tokens = next; + return Box::pin(chat_completion( + &raised, + system_prompt, + user_message, + spinner, + response_format, + )) + .await; + } + + if let Some(salvaged) = salvage_from_reasoning(reasoning.as_ref()) { + tracing::warn!("content empty — salvaged JSON from reasoning_content"); + return Ok((salvaged, usage)); + } + + Err(CoraError::LlmParse(format!( + "provider returned an EMPTY response (finish_reason={finish_reason:?}) after raising max_tokens to {}. Raise `max_tokens` in config or disable reasoning on the model.", + config.max_tokens + ))) } /// Create an animated spinner for LLM operations. @@ -883,6 +980,12 @@ pub(crate) fn parse_review_response( raw: &str, usage: Option<&Usage>, ) -> std::result::Result<(Vec, String, Option), CoraError> { + if raw.trim().is_empty() { + return Err(CoraError::LlmParse( + "provider returned an EMPTY response (no message content). Common cause: reasoning consumed the output budget — raise `max_tokens` in config." + .to_string(), + )); + } let (json_str, summary) = extract_json_and_summary(raw); // Strip markdown code fences if present @@ -1327,6 +1430,37 @@ fn strip_code_fences(s: &str) -> String { #[cfg(test)] mod tests { use super::*; + + #[test] + fn budget_doubles_only_on_length() { + assert_eq!(next_budget_on_empty(Some("length"), 4096), Some(8192)); + assert_eq!(next_budget_on_empty(Some("length"), 32768), None); + assert_eq!(next_budget_on_empty(Some("stop"), 4096), None); + assert_eq!(next_budget_on_empty(None, 4096), None); + } + + #[test] + fn salvage_accepts_only_jsonish_reasoning() { + let arr = Value::String("[{\"file\":\"a.rs\"}]".to_string()); + assert!(salvage_from_reasoning(Some(&arr)).is_some()); + + let fenced = Value::String("thinking... ```json\n[]\n```".to_string()); + assert!(salvage_from_reasoning(Some(&fenced)).is_some()); + + let parts = Value::Array(vec![serde_json::json!({"text": "{\"x\":1}"})]); + assert!(salvage_from_reasoning(Some(&parts)).is_some()); + + let prose = Value::String("the diff looks fine overall".to_string()); + assert!(salvage_from_reasoning(Some(&prose)).is_none()); + assert!(salvage_from_reasoning(None).is_none()); + } + + #[test] + fn empty_raw_is_explicit_not_eof() { + let err = parse_review_response("", None).unwrap_err(); + assert!(err.to_string().contains("EMPTY"), "got: {err}"); + } + use crate::engine::types::Severity; const SINGLE_ISSUE_JSON: &str = r#"[{"file":"src/main.rs","line":42,"severity":"critical","issue_type":"security","title":"SQL Injection","body":"User input is concatenated directly into SQL query.","suggested_fix":"Use parameterized queries."}]"#;