diff --git a/crates/jp_cli/src/cmd/conversation/summarize.rs b/crates/jp_cli/src/cmd/conversation/summarize.rs index 9ddbd5941..2c7a3b506 100644 --- a/crates/jp_cli/src/cmd/conversation/summarize.rs +++ b/crates/jp_cli/src/cmd/conversation/summarize.rs @@ -10,13 +10,12 @@ use jp_conversation::{ thread::ThreadBuilder, }; use jp_llm::{ - Provider, + Provider, StreamErrorKind, event::{Event, EventPatch, FinishReason, record_patches}, event_builder::EventBuilder, model::ModelDetails, provider, retry::{RetryConfig, collect_with_retry}, - window, }; use tracing::debug; @@ -86,25 +85,6 @@ pub async fn generate_summary( let provider = provider::get_provider(model_id.provider, &app_cfg.providers.llm)?; let model_details = provider.model_details(&model_id.name).await?; - // The instructions ride in the system prompt and the request in its own - // turn; both share the window with the range. - let overhead = - window::estimate_overhead_chars(Some(instructions), &[], &[], &[]) + user_message.len(); - - if let Some(overflow) = window_overflow(&stream, model_details.context_window, overhead) { - return Err(Error::Summarize { - model: model_id.to_string(), - // Stored indices are 0-based; turn numbers shown to the user are - // 1-based. - reason: format!( - "turns {}..{} {overflow}; compact a smaller range (`--from`/`--to`) or summarize \ - with a larger-window model", - range_from + 1, - range_to + 1, - ), - }); - } - summarize_stream( provider.as_ref(), &model_details, @@ -117,33 +97,6 @@ pub async fn generate_summary( .await } -/// Describe why `stream` does not fit `context_window`, or `None` when it does. -/// -/// A summary stands in for every turn it covers, so a range that doesn't fit is -/// rejected rather than shortened: summarizing only the tail would leave a -/// compaction that claims a range it never read, and the projected conversation -/// would quietly lose the rest. -/// -/// `overhead_chars` is the size of everything else sharing the window (see -/// [`window::estimate_overhead_chars`]). -/// An unknown window always fits — there is no budget to measure against. -fn window_overflow( - stream: &ConversationStream, - context_window: Option, - overhead_chars: usize, -) -> Option { - let context_window = context_window?; - let budget = window::budget_chars(context_window, overhead_chars); - let needed = window::estimate_chars(stream); - - (needed > budget).then(|| { - format!( - "are roughly {needed} characters, which exceeds the ~{budget} that fit in the model's \ - {context_window} token context window" - ) - }) -} - /// Request a summary of `stream`, honouring provider rebuild requests. /// /// A provider that answers with [`FinishReason::Retry`] supplies patches that @@ -186,7 +139,9 @@ async fn summarize_stream( tool_choice: jp_config::assistant::tool_choice::ToolChoice::default(), }; - let llm_events = collect_with_retry(provider, model_details, query, &retry_config).await?; + let llm_events = collect_with_retry(provider, model_details, query, &retry_config) + .await + .map_err(|error| summarize_error(model_id, error))?; let patches = match summarize_events(llm_events) { StreamOutcome::Summary(summary) => return Ok(summary), @@ -219,6 +174,28 @@ async fn summarize_stream( } } +/// Map a provider error into a summarization failure. +/// +/// A request the provider rejected as too large becomes [`Error::Summarize`], +/// keeping the provider's message — which reports the request's real token +/// count against the model's window — and adding the two ways to get under it. +/// Every other error takes its standard conversion. +fn summarize_error(model_id: &ModelIdConfig, error: jp_llm::Error) -> Error { + match error { + jp_llm::Error::Stream(stream) if stream.kind == StreamErrorKind::ContextWindowExceeded => { + Error::Summarize { + model: model_id.to_string(), + reason: format!( + "{}; compact a smaller range (`--from`/`--to`) or summarize with a \ + larger-window model", + stream.message() + ), + } + } + error => error.into(), + } +} + /// What one completed summarizer stream yielded. #[derive(Debug, PartialEq)] enum StreamOutcome { diff --git a/crates/jp_cli/src/cmd/conversation/summarize_tests.rs b/crates/jp_cli/src/cmd/conversation/summarize_tests.rs index c19dcf00f..db26a3f30 100644 --- a/crates/jp_cli/src/cmd/conversation/summarize_tests.rs +++ b/crates/jp_cli/src/cmd/conversation/summarize_tests.rs @@ -11,7 +11,7 @@ use jp_llm::{ use super::{ Error, StreamOutcome, build_range_stream, collect_range_events, failure_reason, - summarize_events, summarize_stream, window_overflow, + summarize_events, summarize_stream, }; /// A stream that produced `text` and then stopped for `reason`. @@ -107,6 +107,44 @@ async fn summarize_applies_the_configured_output_ceiling() { ); } +/// A request the provider rejects for size is reported as a summarization +/// failure carrying the provider's own numbers. +/// +/// The generic stream error would drop the summarizer framing (which model, +/// what to do next) and leave the reader with a bare API complaint. +#[tokio::test] +async fn a_range_the_provider_rejects_for_size_reports_a_summarize_failure() { + let provider = MockProvider::with_stream_error( + jp_llm::StreamErrorKind::ContextWindowExceeded, + "api error: invalid_request_error: prompt is too long: 1318026 tokens > 1000000 maximum", + ); + let model_id = test_model_id(); + let model_details = ModelDetails::empty(model_id.clone()); + + let error = summarize_stream( + &provider, + &model_details, + &model_id, + range_stream(&["sig"]), + "instructions", + "summarize", + Some(1_048_576), + ) + .await + .expect_err("an oversized request must fail"); + + let Error::Summarize { model, reason } = error else { + panic!("expected a summarize failure, got: {error:?}"); + }; + + assert_eq!(model, "test/mock-model"); + assert_eq!( + reason, + "api error: invalid_request_error: prompt is too long: 1318026 tokens > 1000000 maximum; \ + compact a smaller range (`--from`/`--to`) or summarize with a larger-window model" + ); +} + fn build_stream_with_turns(count: usize) -> ConversationStream { let mut stream = ConversationStream::new_test(); for i in 0..count { @@ -123,55 +161,6 @@ fn chat_request_texts(events: &[jp_conversation::ConversationEvent]) -> Vec>>>, + /// The error every request fails with, instead of returning events. + /// + /// Held as its parts rather than a [`StreamError`] so the provider stays + /// `Clone` and each request gets its own error value. + stream_error: Option<(StreamErrorKind, String)>, + /// Model details to return. model: ModelDetails, } @@ -78,6 +84,7 @@ impl MockProvider { events, batches: None, requests: None, + stream_error: None, model: Self::default_model(), } } @@ -100,10 +107,27 @@ impl MockProvider { events: vec![], batches: Some(Arc::new(Mutex::new(batches.into()))), requests: None, + stream_error: None, model: Self::default_model(), } } + /// Create a mock provider whose stream yields `kind` instead of events. + /// + /// The error arrives mid-stream rather than from [`chat_completion_stream`] + /// itself, which is where providers surface a rejected request: the + /// connection opens and the API's complaint comes back as the first thing + /// on it. + /// + /// [`chat_completion_stream`]: Provider::chat_completion_stream + #[must_use] + pub fn with_stream_error(kind: StreamErrorKind, message: impl Into) -> Self { + Self { + stream_error: Some((kind, message.into())), + ..Self::new(vec![]) + } + } + /// Create a mock provider that streams a simple message response. /// /// Useful for basic tests that just need some content to be streamed. @@ -224,6 +248,11 @@ impl Provider for MockProvider { requests.lock().expect("mock requests lock").push(query); } + if let Some((kind, message)) = &self.stream_error { + let error = StreamError::new(*kind, message.clone()); + return Ok(Box::pin(stream::iter([Err(error)]))); + } + let events = match &self.batches { None => self.events.clone(), Some(batches) => batches diff --git a/crates/jp_llm/src/window.rs b/crates/jp_llm/src/window.rs index f81745fb9..014fcf80c 100644 --- a/crates/jp_llm/src/window.rs +++ b/crates/jp_llm/src/window.rs @@ -19,7 +19,14 @@ use tracing::info; use crate::tool::ToolDefinition; /// Estimated chars-per-token ratio used for estimation. -pub const CHARS_PER_TOKEN: usize = 3; +/// +/// Measured against a real Anthropic request: a 4,220,150-byte serialized body +/// counted 1,317,976 input tokens, which works out to roughly 1.9-2.0 chars per +/// token once JSON framing and escaping are backed out of the byte count. +/// Code and structured payloads tokenize denser than prose, so a conversation +/// of mostly English text sits above this and is over-estimated — the safe +/// direction. +pub const CHARS_PER_TOKEN: usize = 2; /// Safety margin for tokenization imprecision (the chars-per-token ratio varies /// by content type) and provider framing overhead (JSON wrapping, role tags, diff --git a/crates/jp_llm/src/window_tests.rs b/crates/jp_llm/src/window_tests.rs index 42fc3dfa1..82fccb528 100644 --- a/crates/jp_llm/src/window_tests.rs +++ b/crates/jp_llm/src/window_tests.rs @@ -191,8 +191,8 @@ fn message_texts(events: &ConversationStream) -> Vec { /// event is dropped. /// /// The sizes are picked so the drop loop stops right after the request: a -/// 3000-char request against a 1000-token window needs 720 chars dropped, which -/// the request alone satisfies, leaving the 100-char response as the only +/// 3000-char request against a 1000-token window needs 1600 chars dropped, +/// which the request alone satisfies, leaving the 100-char response as the only /// survivor. #[test] fn truncate_empties_stream_when_no_chat_request_survives() {