diff --git a/src/llm.rs b/src/llm.rs index b4320bd..10e1e36 100644 --- a/src/llm.rs +++ b/src/llm.rs @@ -3244,6 +3244,13 @@ impl LlmClient { .into_iter() .next() .ok_or_else(|| anyhow::Error::new(ModelContentFailure::MissingChoices))?; + if let Some(reason) = choice.finish_reason.as_deref() + && reason != "stop" + { + return Err(anyhow::Error::new(ModelContentFailure::NonTerminal { + reason: reason.to_string(), + })); + } let content = choice .message .content diff --git a/src/review.rs b/src/review.rs index 3d190f6..a22c158 100644 --- a/src/review.rs +++ b/src/review.rs @@ -39,7 +39,11 @@ pub(crate) const MAX_HOSTED_SELECTED_BATCHES: usize = 5; pub(crate) const MAX_LARGE_DIFF_SELECTED_BATCHES: usize = 24; pub(crate) const MAX_LARGE_DIFF_CONCURRENCY: usize = 4; const LARGE_SOURCE_REVIEW_MAX_TOKENS: u32 = 6_000; -const SYNTHESIS_REVIEW_MAX_TOKENS: u32 = 2_000; +// Synthesis requests join evidence from several source windows. Reasoning +// models need room for both analysis and the final structured review. The +// single exhausted-output retry may expand this to 8,000 tokens, and hosted +// admission prices that full exposure before any provider request begins. +const SYNTHESIS_REVIEW_MAX_TOKENS: u32 = 4_000; pub(crate) const MAX_HOSTED_PLANNER_CANDIDATES: usize = 96; pub(crate) const MAX_MODELS_PER_REQUEST: usize = 3; pub(crate) const MAX_SCORER_PROMPT_BYTES: usize = 56_000; @@ -2265,12 +2269,12 @@ mod tests { #[test] fn preflight_and_runtime_share_large_review_output_limits() { assert_eq!(review_output_token_limit(false, true), 6_000); - assert_eq!(review_output_token_limit(true, true), 2_000); + assert_eq!(review_output_token_limit(true, true), 4_000); assert_eq!( review_output_token_limit(false, false), crate::llm::REVIEW_MAX_TOKENS ); - assert_eq!(review_output_token_limit(true, false), 2_000); + assert_eq!(review_output_token_limit(true, false), 4_000); } #[test] diff --git a/tests/e2e.rs b/tests/e2e.rs index 9b5439c..0593126 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -1723,7 +1723,7 @@ fn qualification_candidate_admits_fixture_51_shape_at_fireworks_price_bounds() { // ceiling used by runtime for each generator request. assert_eq!( envelope["reviewAdmission"]["outputTokens"], - 12_000 + 4_000 + 3_136 + 12_000 + 8_000 + 3_136 ); assert!( envelope["reviewAdmission"]["serializedInputBytes"] @@ -3233,7 +3233,7 @@ async fn bounded_synthesis_repairs_source_exact_evidence_without_relaxing_valida let body: Value = request.body_json().unwrap(); let serialized = String::from_utf8_lossy(&request.body); let expected = if serialized.contains("bounded synthesis window") { - 2_000 + 4_000 } else { 8_000 }; @@ -4956,7 +4956,7 @@ async fn scorer_error_fails_open_and_preserves_generator_values() { } #[tokio::test] -async fn reasoning_only_scorer_response_is_invalid_output_not_a_provider_outage() { +async fn reasoning_only_scorer_length_response_is_nonterminal_invalid_output() { let server = MockServer::start().await; mock_review_model( &server, @@ -4997,11 +4997,10 @@ async fn reasoning_only_scorer_response_is_invalid_output_not_a_provider_outage( let envelope: Value = serde_json::from_slice(&out.get_output().stdout).unwrap(); assert_eq!(envelope["findings"][0]["path"], "src/auth.rs"); + let scorer_error = envelope["scorerError"].as_str().unwrap(); assert!( - envelope["scorerError"] - .as_str() - .unwrap() - .contains("no choices/content") + scorer_error.contains("model response was nonterminal (length)"), + "unexpected scorer error: {scorer_error}" ); assert!( envelope["modelIncidents"] @@ -5012,6 +5011,8 @@ async fn reasoning_only_scorer_response_is_invalid_output_not_a_provider_outage( |incident| incident["phase"] == "scorer" && incident["category"] == "invalidOutput" ) ); + let stderr = String::from_utf8_lossy(&out.get_output().stderr); + assert!(!stderr.contains("returned empty content")); let requests = server.received_requests().await.unwrap(); let scorer_max_tokens = requests .iter() @@ -5020,7 +5021,7 @@ async fn reasoning_only_scorer_response_is_invalid_output_not_a_provider_outage( (body["model"] == "scorer-model").then(|| body["max_tokens"].as_u64().unwrap()) }) .collect::>(); - assert_eq!(scorer_max_tokens, vec![400, 400]); + assert_eq!(scorer_max_tokens, vec![400]); } #[tokio::test] @@ -6574,6 +6575,60 @@ async fn exhausted_reasoning_budget_expands_the_same_model_retry() { assert_eq!(max_tokens, vec![8_000, 16_000]); } +#[tokio::test] +async fn repeated_exhausted_reasoning_budget_does_not_consume_an_empty_retry() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/chat/completions")) + .and(body_string_contains("primary-model")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "choices": [{ + "finish_reason": "length", + "message": {"content": null, "reasoning": "budget exhausted"} + }], + "usage": { + "prompt_tokens": 4_194, + "completion_tokens": 8_000, + "completion_tokens_details": {"reasoning_tokens": 8_000} + } + }))) + .expect(2) + .mount(&server) + .await; + + let dir = tempfile::tempdir().unwrap(); + let diff = write_diff(dir.path()); + let out = postil() + .current_dir(dir.path()) + .env("POSTIL_API_BASE", server.uri()) + .env("REVIEW_MODEL", "primary-model") + .env("POSTIL_DISABLE_SCORER", "1") + .args(["review", "--diff-file"]) + .arg(&diff) + .args(["--output", "json"]) + .assert() + .code(1); + + let envelope: Value = serde_json::from_slice(&out.get_output().stdout).unwrap(); + assert_eq!(envelope["findings"][0]["path"], ".postil/model-output"); + assert_eq!(envelope["modelUsage"].as_array().unwrap().len(), 2); + let stderr = String::from_utf8_lossy(&out.get_output().stderr); + assert!(stderr.contains("retrying the complete request with 16000 tokens")); + assert!(!stderr.contains("returned empty content")); + + let requests = server.received_requests().await.unwrap(); + assert_eq!(requests.len(), 2); + let max_tokens = requests + .iter() + .map(|request| { + request.body_json::().unwrap()["max_tokens"] + .as_u64() + .unwrap() + }) + .collect::>(); + assert_eq!(max_tokens, vec![8_000, 16_000]); +} + #[tokio::test] async fn openai_truncation_retries_the_complete_original_request() { let server = MockServer::start().await;