fix(cli, llm): Correct the token estimate, stop gating summaries

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<u32>,
-    overhead_chars: usize,
-) -> Option<String> {
-    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 {

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<Stri
         .collect()
 }
 
-/// A range comfortably inside the window is summarized as-is.
-#[test]
-fn a_range_that_fits_reports_no_overflow() {
-    let stream = build_stream_with_turns(4);
-    assert_eq!(window_overflow(&stream, Some(100_000), 0), None);
-}
-
-/// The reported failure's shape, on the summarizer path: a large range against
-/// a small-window model.
-/// Unlike title generation this is rejected rather than shortened, so the
-/// summary never covers less than the range it is stored for.
-#[test]
-fn a_range_past_the_window_overflows() {
-    let mut stream = ConversationStream::new_test();
-    for i in 0..200 {
-        stream.start_turn(format!("turn {i}: {}", "x".repeat(1000)));
-    }
-
-    let overflow = window_overflow(&stream, Some(1000), 0).expect("range must not fit");
-    assert_eq!(
-        overflow,
-        "are roughly 201890 characters, which exceeds the ~2700 that fit in the model's 1000 \
-         token context window"
-    );
-}
-
-/// Overhead is charged against the same window, so a range that fits on its own
-/// can still overflow once the instructions are counted.
-#[test]
-fn overhead_can_push_a_fitting_range_over() {
-    let mut stream = ConversationStream::new_test();
-    stream.start_turn("x".repeat(2000));
-
-    assert_eq!(window_overflow(&stream, Some(1000), 0), None);
-    assert!(window_overflow(&stream, Some(1000), 1000).is_some());
-}
-
-/// Providers that don't report a window (local llama.cpp, Ollama) have no
-/// budget to check against, so nothing is rejected.
-#[test]
-fn an_unknown_window_never_overflows() {
-    let mut stream = ConversationStream::new_test();
-    for i in 0..200 {
-        stream.start_turn(format!("turn {i}: {}", "x".repeat(1000)));
-    }
-
-    assert_eq!(window_overflow(&stream, None, 0), None);
-}
-
 /// A repair already recorded on the source applies to the summary request too.
 ///
 /// Overlays live beside the events rather than inside them, so a range stream

crates/jp_llm/src/provider/mock.rs

@@ -36,7 +36,7 @@ use serde_json::{Map, Value};
 
 use super::Provider;
 use crate::{
-    error::Result,
+    error::{Result, StreamError, StreamErrorKind},
     event::{Event, FinishReason},
     model::ModelDetails,
     query::ChatQuery,
@@ -62,6 +62,12 @@ pub struct MockProvider {
     /// Requests received so far, when capture is enabled.
     requests: Option<Arc<Mutex<Vec<ChatQuery>>>>,
 
+    /// 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<String>) -> 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

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,

crates/jp_llm/src/window_tests.rs

@@ -191,8 +191,8 @@ fn message_texts(events: &ConversationStream) -> Vec<String> {
 /// 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() {