From 34ce521368ad00c4fafdd6670cc22b7ab9988b16 Mon Sep 17 00:00:00 2001 From: Shizuku <2163018547@qq.com> Date: Sat, 26 Sep 2026 20:52:39 +0800 Subject: [PATCH 1/4] fix(compaction): survive a provider request-body 413 while making room MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A provider body cap is a byte limit the token-side budget cannot see: an image that costs a flat token estimate can still spend megabytes of base64, and a summary request carries the whole history. Compaction then failed exactly when it was needed most, and no later pass could clear the history stuck behind the cap. The summary call now descends a two-rung byte ladder before failing: - re-encode the inline images under a 2 MiB budget (per-image share with a 96 KiB floor; alpha stays PNG, everything else becomes JPEG); - if the request is still refused, replace the images with text notes for that one summary pass, so the handoff still says an image was there. Each rung announces itself on the status line and the failure text names the rung it reached. Session history keeps the real images; only the outbound copy of the request changes. The detector walks the whole error chain, so a gateway HTML page ("413 Request Entity Too Large ... openresty") counts the same as the API's own body reader, and it takes precedence over the context-window ladder — a byte rejection is not a token one. Files: - crates/tui/src/image_attach.rs: shrink and placeholder helpers - crates/tui/src/compaction.rs: notice sink, request-size ladder, detector - crates/tui/src/core/engine/compaction.rs: engine notice sink Tests: 9 new cases covering both rejection wordings, both image carriers, both retry rungs and the full-ladder failure. --- crates/tui/CHANGELOG.md | 7 + crates/tui/src/compaction.rs | 445 ++++++++++++++++++++++- crates/tui/src/core/engine/compaction.rs | 42 +++ crates/tui/src/image_attach.rs | 335 ++++++++++++++++- crates/tui/src/image_attach/tests.rs | 130 +++++++ 5 files changed, 955 insertions(+), 4 deletions(-) diff --git a/crates/tui/CHANGELOG.md b/crates/tui/CHANGELOG.md index 1469fe5e71..695eb62f42 100644 --- a/crates/tui/CHANGELOG.md +++ b/crates/tui/CHANGELOG.md @@ -12,6 +12,13 @@ with English/Chinese recovery links to home and docs ([#6419](https://github.com/Hmbown/Codewhale/issues/6419), [#6420](https://github.com/Hmbown/Codewhale/pull/6420)). +Making room now recovers from a provider's request-body limit (HTTP 413) +instead of failing the pass. A summary request that is refused for size +re-encodes the conversation's inline images smaller and retries, says so in the +status line while it does, and — if the request is still refused — replaces the +images with text notes for that one summary pass. Session history keeps the +real images either way. + Planned for Codewhale v0.10.1: a reliability and first-run release. Turns that stall now say so, approvals keep what you approved, plugin suggestions are quieter, and Fleet runs can be checked before they spend anything. diff --git a/crates/tui/src/compaction.rs b/crates/tui/src/compaction.rs index 282c95aca4..bc40a7938f 100644 --- a/crates/tui/src/compaction.rs +++ b/crates/tui/src/compaction.rs @@ -3,6 +3,7 @@ use anyhow::Result; use std::collections::HashMap; use std::fmt::Write; +use std::sync::Arc; use std::time::Duration; use crate::config::DEFAULT_TEXT_MODEL; @@ -65,9 +66,20 @@ pub struct CompactionConfig { pub retained_user_message_tokens: usize, } +/// Host callback for user-visible progress during a compaction pass. +/// +/// Compaction runs inside the engine's provider boundary, where the engine +/// owns the only channel that reaches the person. Injecting a sink lets a +/// downgrade (re-encoding images after an HTTP 413, say) say what it is doing +/// while it is doing it, instead of surfacing only when the pass ends. +pub trait CompactionNoticeSink: Send + Sync + std::fmt::Debug { + /// Deliver one already-rendered, user-visible sentence. + fn notice(&self, message: String); +} + /// Host-prepared configuration carried from compaction eligibility through /// the replacement-history commit. -#[derive(Debug, Clone, PartialEq)] +#[derive(Clone)] pub struct PreparedCompactionEnvelope { pub config: CompactionConfig, /// Durable handoff owner; set by the engine, never added to the stable prefix. @@ -80,6 +92,33 @@ pub struct PreparedCompactionEnvelope { /// request that omits it shares no cacheable prefix with the turn it /// summarizes and re-bills the whole history uncached. pub reasoning_effort: Option, + /// User-visible progress sink supplied by the host that owns the run + /// (the interactive engine, typically). `None` keeps every notice in the + /// log only — the pass itself never depends on it. + pub notice_sink: Option>, +} + +impl std::fmt::Debug for PreparedCompactionEnvelope { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("PreparedCompactionEnvelope") + .field("config", &self.config) + .field("session_id", &self.session_id) + .field("tools", &self.tools) + .field("reasoning_effort", &self.reasoning_effort) + .field("notice_sink", &self.notice_sink.as_ref().map(|_| "")) + .finish() + } +} + +impl PartialEq for PreparedCompactionEnvelope { + /// The notice sink is host plumbing, not envelope content: two passes over + /// the same config are equal whether or not their host delivers notices. + fn eq(&self, other: &Self) -> bool { + self.config == other.config + && self.session_id == other.session_id + && self.tools == other.tools + && self.reasoning_effort == other.reasoning_effort + } } impl PreparedCompactionEnvelope { @@ -90,6 +129,7 @@ impl PreparedCompactionEnvelope { session_id: None, tools: None, reasoning_effort: None, + notice_sink: None, } } } @@ -1228,6 +1268,7 @@ pub async fn compact_messages_safe( system_prompt, prepared.tools.as_deref(), prepared.reasoning_effort.as_deref(), + prepared.notice_sink.as_deref(), &mut quality_retries, invocation_usage, ) @@ -1391,6 +1432,7 @@ async fn compact_messages( None, None, None, + None, &mut quality_retries, &mut invocation_usage, ) @@ -1405,6 +1447,7 @@ async fn compact_messages_with_metadata( system_prompt: Option<&SystemPrompt>, tools: Option<&[Tool]>, reasoning_effort: Option<&str>, + notice_sink: Option<&dyn CompactionNoticeSink>, quality_retries: &mut u32, invocation_usage: &mut Usage, ) -> Result<(Vec, Option, CompactionCoverage)> { @@ -1419,6 +1462,7 @@ async fn compact_messages_with_metadata( system_prompt, tools, reasoning_effort, + notice_sink, quality_retries, invocation_usage, ) @@ -1629,6 +1673,7 @@ async fn create_summary( system_prompt: Option<&SystemPrompt>, tools: Option<&[Tool]>, reasoning_effort: Option<&str>, + notice_sink: Option<&dyn CompactionNoticeSink>, quality_retries: &mut u32, invocation_usage: &mut Usage, ) -> Result { @@ -1658,6 +1703,12 @@ async fn create_summary( }); let mut quality_retry_used = false; + // Request-size ladder: an HTTP 413 caps the request *body*, which the + // token-side budget cannot predict (a flat per-image token estimate can + // hide megabytes of base64). A refused summary call gets two byte-side + // downgrades before it fails — re-encode the inline images smaller, then + // replace them with text notes — each followed by exactly one retry. + let mut size_ladder = RequestSizeLadder::Start; loop { // Codex compaction is a normal model generation over the existing // cached prefix. Do the same here: the resolved route decides how @@ -1678,7 +1729,14 @@ async fn create_summary( let cost_scope = crate::cost_status::scope_token(); let response = match client.create_message(request).await { Ok(response) => response, - Err(err) if is_context_window_error(&err) && request_messages.len() > 2 => { + // A byte-side size rejection can also read like a length problem + // (gateway HTML pages carry their own wording); the size ladder + // owns it, not the drop-oldest ladder. + Err(err) + if !is_request_too_large_error(&err) + && is_context_window_error(&err) + && request_messages.len() > 2 => + { logging::warn(format!( "Compaction summary input over the context window ({err}); \ dropping the oldest history item and retrying" @@ -1686,6 +1744,50 @@ async fn create_summary( drop_oldest_history_messages(&mut request_messages); continue; } + Err(err) if is_request_too_large_error(&err) => match size_ladder { + RequestSizeLadder::Start => { + let shrunk = + crate::image_attach::shrink_images_for_request(&mut request_messages); + if shrunk.images == 0 { + return Err(err.context( + "The summary request exceeded the provider's request-body limit and the history carries no inline images to re-encode", + )); + } + size_ladder = RequestSizeLadder::ImagesShrunk; + let message = format!( + "Making room exceeded the provider's request-body limit (HTTP 413); re-encoded {} inline image(s) smaller ({} to {}) and is retrying the summary.", + shrunk.images, + crate::image_attach::human_bytes(shrunk.bytes_before), + crate::image_attach::human_bytes(shrunk.bytes_after), + ); + logging::warn(&message); + deliver_compaction_notice(notice_sink, message); + continue; + } + RequestSizeLadder::ImagesShrunk => { + let replaced = crate::image_attach::replace_images_with_placeholders( + &mut request_messages, + "the summary request exceeded the provider's request-body limit (HTTP 413)", + ); + if replaced == 0 { + return Err(err.context( + "The summary request still exceeded the provider's request-body limit and no inline images were left to replace", + )); + } + size_ladder = RequestSizeLadder::ImagesReplaced; + let message = format!( + "Making room still exceeded the provider's request-body limit; replaced {replaced} inline image(s) with text notes for this summary pass and is retrying.", + ); + logging::warn(&message); + deliver_compaction_notice(notice_sink, message); + continue; + } + RequestSizeLadder::ImagesReplaced => { + return Err(err.context( + "The summary request exceeded the provider's request-body limit even after re-encoding and then replacing every inline image", + )); + } + }, Err(err) => return Err(err), }; @@ -1771,6 +1873,47 @@ no replacement checkpoint was committed", } } +/// How far the request-size ladder for one summary call has descended. +/// +/// The ladder exists because HTTP 413 rejects the request *body* by bytes, +/// which the token-side context budget that governs compaction cannot see. +/// Each rung is one retry: re-encode the inline images under a byte budget, +/// then replace them with text notes. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum RequestSizeLadder { + /// No size downgrade applied yet. + Start, + /// Inline images were re-encoded smaller. + ImagesShrunk, + /// Inline images were replaced with text notes. + ImagesReplaced, +} + +/// Whether the provider refused the request body for size (HTTP 413 and its +/// common wordings). A smaller payload can succeed where this one did not, +/// which is exactly what the request-size ladder trades on. +/// +/// Walks the whole error chain: the rejection may be stated by a fronting +/// gateway (an HTML page from openresty reading "413 Request Entity Too +/// Large") and then wrapped again by the client's own error text, so the +/// top-level message alone is not reliable. +fn is_request_too_large_error(e: &anyhow::Error) -> bool { + e.chain().any(|cause| { + let lower = cause.to_string().to_lowercase(); + lower.contains("http 413") + || lower.contains("payload too large") + || lower.contains("request entity too large") + || lower.contains("length limit exceeded") + }) +} + +/// Forward one user-visible progress sentence, when the host supplied a sink. +fn deliver_compaction_notice(sink: Option<&dyn CompactionNoticeSink>, message: String) { + if let Some(sink) = sink { + sink.notice(message); + } +} + fn is_context_window_error(e: &anyhow::Error) -> bool { let text = e.to_string(); if crate::error_taxonomy::classify_error_message(&text) @@ -2150,6 +2293,304 @@ mod tests { 2. Key technical concepts — sqlite. 7. Pending tasks — finish the fixed clock. \ 8. Current work — rerunning the session tests."; + /// A real PNG whose bytes are worth shrinking. Noise defeats compression, + /// which is the point: the shrink ladder must actually re-encode. + fn noisy_png_bytes(width: u32, height: u32) -> Vec { + use image::ImageEncoder as _; + let mut pixels = image::RgbImage::new(width, height); + for (x, y, pixel) in pixels.enumerate_pixels_mut() { + *pixel = image::Rgb([ + (x.wrapping_mul(31) ^ y.wrapping_mul(17)) as u8, + (x.wrapping_mul(7) ^ y.wrapping_mul(29)) as u8, + (x.wrapping_add(y).wrapping_mul(13)) as u8, + ]); + } + let mut bytes = Vec::new(); + image::codecs::png::PngEncoder::new_with_quality( + &mut bytes, + image::codecs::png::CompressionType::Fast, + image::codecs::png::FilterType::NoFilter, + ) + .write_image( + pixels.as_raw(), + width, + height, + image::ExtendedColorType::Rgb8, + ) + .expect("encode fixture png"); + bytes + } + + fn png_data_url(width: u32, height: u32) -> String { + use base64::Engine as _; + format!( + "data:image/png;base64,{}", + base64::engine::general_purpose::STANDARD.encode(noisy_png_bytes(width, height)) + ) + } + + fn user_image_message(data_url: &str) -> Message { + Message { + role: Role::User, + content: vec![ + ContentBlock::Text { + text: "look at this screenshot".to_string(), + cache_control: None, + }, + ContentBlock::ImageUrl { + image_url: ImageUrlContent { + url: data_url.to_string(), + }, + }, + ], + } + } + + /// Total inline-image URL bytes a request carries, the byte side an HTTP + /// 413 boundary actually measures. + fn inline_image_bytes(request: &MessageRequest) -> usize { + request + .messages + .iter() + .flat_map(|message| message.content.iter()) + .map(|block| match block { + ContentBlock::ImageUrl { image_url } => image_url.url.len(), + ContentBlock::ToolResult { content_blocks, .. } => { + content_blocks.as_ref().map_or(0, |blocks| { + blocks.iter().fold(0, |sum, block| { + let nested = block + .get("data") + .and_then(serde_json::Value::as_str) + .map_or(0, str::len); + sum + nested + }) + }) + } + _ => 0, + }) + .sum() + } + + fn summary_content() -> Vec { + vec![ContentBlock::Text { + text: "1. Primary request: keep working on the session store. \ + 7. Pending: rerun the tests." + .to_string(), + cache_control: None, + }] + } + + fn request_body_413() -> anyhow::Error { + anyhow::anyhow!( + "LLM error: HTTP 413: Failed to buffer the request body: length limit exceeded" + ) + } + + /// The same boundary stated by a fronting gateway (openresty) instead of + /// the API's own body reader. Reported on 2026-09-26; the ladder must + /// treat it as the same byte-side rejection. + fn request_body_413_html_gateway() -> anyhow::Error { + anyhow::anyhow!( + "LLM error: HTTP 413: DeepSeek API returned an HTML error page (HTTP 413): \ + 413 Request Entity Too Large 413 Request Entity Too Large openresty" + ) + } + + #[derive(Debug, Default)] + struct RecordingNoticeSink { + messages: std::sync::Mutex>, + } + + impl RecordingNoticeSink { + fn messages(&self) -> Vec { + self.messages.lock().expect("notice sink").clone() + } + } + + impl CompactionNoticeSink for RecordingNoticeSink { + fn notice(&self, message: String) { + self.messages.lock().expect("notice sink").push(message); + } + } + + /// A 900x900 noise PNG exceeds the 2 MiB inline-image budget, so the + /// ladder must actually rewrite it. + fn body_413_retry_envelope( + sink: std::sync::Arc, + ) -> PreparedCompactionEnvelope { + let mut envelope = prepared(&CompactionConfig { + enabled: false, + ..CompactionConfig::default() + }); + envelope.notice_sink = Some(sink); + envelope + } + + #[tokio::test] + async fn request_body_413_reencodes_images_smaller_and_retries() { + let _environment = crate::test_support::lock_test_env(); + let messages = vec![ + msg("user", "context before the screenshots"), + user_image_message(&png_data_url(900, 900)), + ]; + let client = ScriptedSummaryClient::with_outcomes(vec![ + Err(request_body_413()), + Ok(summary_content()), + ]); + let sink = std::sync::Arc::new(RecordingNoticeSink::default()); + let envelope = body_413_retry_envelope(sink.clone()); + let mut usage = Usage::default(); + + let result = compact_messages_safe(&client, &messages, None, &envelope, &mut usage).await; + assert!( + result.is_ok(), + "the retry after shrinking must succeed: {result:?}" + ); + + let requests = client.requests.lock().expect("requests").clone(); + assert_eq!(requests.len(), 2, "one rejection, one retry"); + let first = inline_image_bytes(&requests[0]); + let second = inline_image_bytes(&requests[1]); + assert!(first > 0, "the fixture must carry the image"); + assert!( + second > 0 && second < first, + "the retry must carry smaller image bytes ({second} < {first})" + ); + let notices = sink.messages(); + assert_eq!(notices.len(), 1, "one notice per downgrade: {notices:?}"); + assert!( + notices[0].contains("re-encoded") && notices[0].contains("413"), + "the notice must name the downgrade: {notices:?}" + ); + } + + #[tokio::test] + async fn request_body_413_after_shrinking_replaces_images_with_notes() { + let _environment = crate::test_support::lock_test_env(); + let messages = vec![ + msg("user", "context before the screenshots"), + user_image_message(&png_data_url(900, 900)), + ]; + let client = ScriptedSummaryClient::with_outcomes(vec![ + Err(request_body_413()), + Err(request_body_413()), + Ok(summary_content()), + ]); + let sink = std::sync::Arc::new(RecordingNoticeSink::default()); + let envelope = body_413_retry_envelope(sink.clone()); + let mut usage = Usage::default(); + + let result = compact_messages_safe(&client, &messages, None, &envelope, &mut usage).await; + assert!( + result.is_ok(), + "the retry after replacing images must succeed: {result:?}" + ); + + let requests = client.requests.lock().expect("requests").clone(); + assert_eq!(requests.len(), 3, "reject, shrink retry, replace retry"); + let first = inline_image_bytes(&requests[0]); + let second = inline_image_bytes(&requests[1]); + let third = inline_image_bytes(&requests[2]); + assert!(second > 0 && second < first); + assert_eq!(third, 0, "the last rung carries no image bytes"); + let note_present = requests[2].messages.iter().any(|message| { + message.content.iter().any(|block| match block { + ContentBlock::Text { text, .. } => text.contains("omitted from this summary pass"), + _ => false, + }) + }); + assert!(note_present, "the summarizer is told what was there"); + let notices = sink.messages(); + assert_eq!(notices.len(), 2, "one notice per rung: {notices:?}"); + assert!(notices[1].contains("replaced"), "{notices:?}"); + } + + #[tokio::test] + async fn request_body_413_from_a_gateway_html_page_enters_the_same_ladder() { + let _environment = crate::test_support::lock_test_env(); + let messages = vec![ + msg("user", "context before the screenshots"), + user_image_message(&png_data_url(900, 900)), + ]; + let client = ScriptedSummaryClient::with_outcomes(vec![ + Err(request_body_413_html_gateway()), + Ok(summary_content()), + ]); + let sink = std::sync::Arc::new(RecordingNoticeSink::default()); + let envelope = body_413_retry_envelope(sink.clone()); + let mut usage = Usage::default(); + + let result = compact_messages_safe(&client, &messages, None, &envelope, &mut usage).await; + assert!( + result.is_ok(), + "the gateway-page rejection must enter the ladder: {result:?}" + ); + let requests = client.requests.lock().expect("requests").clone(); + assert_eq!(requests.len(), 2, "one rejection, one retry"); + assert!( + inline_image_bytes(&requests[1]) < inline_image_bytes(&requests[0]), + "the retry must carry smaller image bytes" + ); + let notices = sink.messages(); + assert_eq!( + notices.len(), + 1, + "the user hears about the downgrade: {notices:?}" + ); + assert!(notices[0].contains("413"), "{notices:?}"); + } + + #[tokio::test] + async fn request_body_413_without_inline_images_fails_with_context() { + let _environment = crate::test_support::lock_test_env(); + let messages = vec![msg("user", "no images anywhere in this history")]; + let client = ScriptedSummaryClient::with_outcomes(vec![Err(request_body_413())]); + let sink = std::sync::Arc::new(RecordingNoticeSink::default()); + let envelope = body_413_retry_envelope(sink.clone()); + let mut usage = Usage::default(); + + let error = compact_messages_safe(&client, &messages, None, &envelope, &mut usage) + .await + .expect_err("nothing left to shrink, the pass must fail"); + let text = format!("{error:#}"); + assert!( + text.contains("request-body limit"), + "the failure must name the boundary: {text}" + ); + let requests = client.requests.lock().expect("requests").clone(); + assert_eq!(requests.len(), 1, "no pointless identical retry"); + assert!(sink.messages().is_empty(), "nothing was downgraded"); + } + + #[tokio::test] + async fn request_body_413_after_replacements_fails_with_the_full_ladder() { + let _environment = crate::test_support::lock_test_env(); + let messages = vec![ + msg("user", "context before the screenshots"), + user_image_message(&png_data_url(900, 900)), + ]; + let client = ScriptedSummaryClient::with_outcomes(vec![ + Err(request_body_413()), + Err(request_body_413()), + Err(request_body_413()), + ]); + let sink = std::sync::Arc::new(RecordingNoticeSink::default()); + let envelope = body_413_retry_envelope(sink.clone()); + let mut usage = Usage::default(); + + let error = compact_messages_safe(&client, &messages, None, &envelope, &mut usage) + .await + .expect_err("every rung refused, the pass must fail"); + let text = format!("{error:#}"); + assert!( + text.contains("even after re-encoding and then replacing"), + "the failure must report the full ladder: {text}" + ); + let requests = client.requests.lock().expect("requests").clone(); + assert_eq!(requests.len(), 3, "two retries, then stop"); + assert_eq!(sink.messages().len(), 2, "both rungs announced"); + } + struct ScriptedSummaryClient { responses: std::sync::Mutex>>>, requests: std::sync::Mutex>, diff --git a/crates/tui/src/core/engine/compaction.rs b/crates/tui/src/core/engine/compaction.rs index 3e0f15f33b..276958959e 100644 --- a/crates/tui/src/core/engine/compaction.rs +++ b/crates/tui/src/core/engine/compaction.rs @@ -11,6 +11,23 @@ pub(super) struct CompactionPass { pub usage: Usage, } +/// Engine-side sink for compaction downgrade notices. +/// +/// Delivered as `Event::Status`, the same channel other engine status lines +/// use, so a long recovery says what it is doing while it runs. A full event +/// channel drops the notice instead of stalling the pass; the same sentence +/// is already in the log via `logging::warn`. +#[derive(Debug)] +struct EngineCompactionNoticeSink { + tx: mpsc::Sender, +} + +impl crate::compaction::CompactionNoticeSink for EngineCompactionNoticeSink { + fn notice(&self, message: String) { + let _ = self.tx.try_send(Event::Status { message }); + } +} + impl Engine { pub(super) async fn emit_compaction_started( &mut self, @@ -183,6 +200,9 @@ impl Engine { .get_or_insert_with(|| self.config.workspace.clone()); let mut prepared = PreparedCompactionEnvelope::new(config); prepared.session_id = Some(self.session.id.clone()); + prepared.notice_sink = Some(std::sync::Arc::new(EngineCompactionNoticeSink { + tx: self.tx_event.clone(), + })); // The summary request must carry the reasoning tier the turn sends: // reasoning routes render it at the head of the prompt, so omitting // it forfeited the whole cached history prefix (#6540). @@ -709,3 +729,25 @@ pub(super) fn is_provider_rejection(err: &anyhow::Error) -> bool { | ErrorCategory::Timeout ) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::compaction::CompactionNoticeSink as _; + + /// The engine sink is the one link between a compaction downgrade and the + /// person watching: the notice must land on the status line, not only in + /// the log. + #[tokio::test] + async fn compaction_notice_sink_delivers_a_status_event() { + let (tx, mut rx) = mpsc::channel(4); + let sink = EngineCompactionNoticeSink { tx }; + sink.notice("Making room re-encoded 2 inline image(s)".to_string()); + match rx.recv().await { + Some(Event::Status { message }) => { + assert!(message.contains("re-encoded"), "{message}"); + } + other => panic!("expected a Status event, got {other:?}"), + } + } +} diff --git a/crates/tui/src/image_attach.rs b/crates/tui/src/image_attach.rs index d7e1918690..b1b18e2f6a 100644 --- a/crates/tui/src/image_attach.rs +++ b/crates/tui/src/image_attach.rs @@ -46,7 +46,10 @@ use anyhow::{Result, bail}; use codewhale_protocol::runtime::{ MAX_RUNTIME_IMAGE_BYTES, MAX_RUNTIME_IMAGE_TOTAL_BYTES, MAX_RUNTIME_IMAGES, RuntimeImageInput, }; -use image::{DynamicImage, ImageReader, Limits}; +use image::codecs::jpeg::JpegEncoder; +use image::codecs::png::{CompressionType, FilterType as PngFilter, PngEncoder}; +use image::imageops::FilterType; +use image::{DynamicImage, ExtendedColorType, GenericImageView, ImageEncoder, ImageReader, Limits}; use std::io::Cursor; use std::path::Path; @@ -260,7 +263,7 @@ impl std::fmt::Display for ImageAttachError { impl std::error::Error for ImageAttachError {} -fn human_bytes(bytes: usize) -> String { +pub(crate) fn human_bytes(bytes: usize) -> String { if bytes >= 1024 * 1024 { format!("{:.1} MB", bytes as f64 / (1024.0 * 1024.0)) } else if bytes >= 1024 { @@ -768,6 +771,334 @@ pub fn strip_images_when_unsupported( stripped } +/// Total inline-image byte budget for one compaction retry that follows a +/// provider body-size rejection. +/// +/// An HTTP 413 boundary caps the request *body*, and the token-side context +/// budget that governs compaction cannot see it: an image that costs a flat +/// token estimate can still spend megabytes of base64. Nothing consults this +/// budget on the happy path — it exists only to recover a summary call that +/// has already been refused, where the cheapest useful move is to re-encode +/// the inline images under a cap at or below the smallest provider body limits +/// seen in practice and retry. +pub(crate) const COMPACTION_IMAGE_TOTAL_BUDGET_BYTES: usize = 2 * 1024 * 1024; + +/// Smallest per-image share of that budget, so an image-heavy session still +/// re-encodes each image instead of dividing the budget down to nothing. +const COMPACTION_IMAGE_MIN_BUDGET_BYTES: usize = 96 * 1024; + +/// Longest edge a re-encoded inline image keeps, in pixels. +const COMPACTION_IMAGE_MAX_EDGE: u32 = 1024; + +/// Longest edge the shrink ladder descends to before giving up on an image. +const COMPACTION_IMAGE_MIN_EDGE: u32 = 128; + +/// JPEG quality for re-encoded images that carry no meaningful alpha. +const COMPACTION_IMAGE_JPEG_QUALITY: u8 = 80; + +/// What one inline-image shrink pass changed. +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] +pub(crate) struct ShrunkInlineImages { + /// Images that were re-encoded smaller. + pub images: usize, + /// Decoded bytes of those images before the pass. + pub bytes_before: usize, + /// Decoded bytes of those images after the pass. + pub bytes_after: usize, +} + +/// Re-encode every inline image in `messages` under a total byte budget. +/// +/// Covers both carriers an outbound request can hold: an `image_url` block, +/// and the stored tool-result shape (`{"type":"image","mime_type","data"}`) +/// that the wire projection reads back out via +/// [`provider_tool_result_image_refs`]. +/// +/// `images == 0` means nothing was rewritten — there were no inline images, +/// or each was already under its share of the budget — so a caller that is +/// still looking at a body-size rejection must climb to the next rung rather +/// than resend the same bytes. +pub(crate) fn shrink_images_for_request( + messages: &mut [codewhale_models::Message], +) -> ShrunkInlineImages { + shrink_images_for_request_with_budget(messages, COMPACTION_IMAGE_TOTAL_BUDGET_BYTES) +} + +/// Budget-parameterized core of [`shrink_images_for_request`], kept separate +/// so tests can drive it with small budgets and small images. +pub(crate) fn shrink_images_for_request_with_budget( + messages: &mut [codewhale_models::Message], + budget: usize, +) -> ShrunkInlineImages { + let sizes = inline_image_sizes(messages); + let total: usize = sizes.iter().sum(); + if sizes.is_empty() || total <= budget { + return ShrunkInlineImages::default(); + } + let per_image = (budget / sizes.len()).max(COMPACTION_IMAGE_MIN_BUDGET_BYTES); + let mut outcome = ShrunkInlineImages::default(); + for message in messages.iter_mut() { + for block in &mut message.content { + match block { + ContentBlock::ImageUrl { image_url } => { + if let Some((url, shrunk)) = shrink_data_url(&image_url.url, per_image) { + image_url.url = url; + outcome.images += 1; + outcome.bytes_before += shrunk.before; + outcome.bytes_after += shrunk.after; + } + } + ContentBlock::ToolResult { content_blocks, .. } => { + let Some(blocks) = content_blocks.as_mut() else { + continue; + }; + for value in blocks.iter_mut() { + if let Some(shrunk) = shrink_stored_tool_image(value, per_image) { + outcome.images += 1; + outcome.bytes_before += shrunk.before; + outcome.bytes_after += shrunk.after; + } + } + } + _ => {} + } + } + } + outcome +} + +/// Replace every inline image with a text note that keeps the image's +/// existence visible without its bytes. +/// +/// This is the last rung of the request-size ladder. A summary pass that still +/// exceeds the provider's body cap after re-encoding has nothing left to give +/// but the pixels; the note keeps the fact that a tool or the user supplied an +/// image — and how large it was — so the handoff can still say so instead of +/// silently dropping the detail. Session history keeps the real image; this +/// only rewrites the outbound copy. +/// +/// Returns the number of images replaced. +pub(crate) fn replace_images_with_placeholders( + messages: &mut [codewhale_models::Message], + reason: &str, +) -> usize { + let mut replaced = 0; + for message in messages.iter_mut() { + for block in &mut message.content { + match block { + ContentBlock::ImageUrl { image_url } => { + let bytes = parse_data_url(&image_url.url) + .map(|(_, payload)| decoded_len_estimate(payload)); + *block = ContentBlock::Text { + text: image_placeholder_note(1, bytes, reason), + cache_control: None, + }; + replaced += 1; + } + ContentBlock::ToolResult { + content, + content_blocks, + .. + } => { + let Some(blocks) = content_blocks.as_ref() else { + continue; + }; + let mut count = 0usize; + let mut bytes = 0usize; + for value in blocks { + if let Some((_, payload)) = stored_tool_image_payload(value) { + count += 1; + bytes += decoded_len_estimate(payload); + } + } + if count == 0 { + continue; + } + // Same shape as `strip_images_when_unsupported`: the + // wire projection reads tool images back out of + // `content_blocks` and would count a text block placed + // there as an omitted image. + *content_blocks = None; + *content = format!( + "{content}\n{}", + image_placeholder_note(count, Some(bytes), reason) + ); + replaced += count; + } + _ => {} + } + } + } + replaced +} + +/// The in-band note that replaces an inline image byte payload for one +/// summary pass. It tells the summarizer what was there and what to do +/// about it (refer to it as an image; never invent its contents), rather than +/// leaving a silent gap. +fn image_placeholder_note(count: usize, bytes: Option, reason: &str) -> String { + let size = bytes.map_or(String::new(), |bytes| format!(" (~{})", human_bytes(bytes))); + format!( + "[{count} image(s){size} omitted from this summary pass: {reason}. \ + The image(s) remain in the session. Refer to them only as images the \ + conversation included; do not describe or guess what they showed.]" + ) +} + +/// Base64 length to decoded length: four characters carry three bytes. +fn decoded_len_estimate(payload: &str) -> usize { + payload.len() / 4 * 3 +} + +/// Decoded-size estimates for every inline image, in request order. +fn inline_image_sizes(messages: &[codewhale_models::Message]) -> Vec { + let mut sizes = Vec::new(); + for message in messages { + for block in &message.content { + match block { + ContentBlock::ImageUrl { image_url } => { + if let Some((_, payload)) = parse_data_url(&image_url.url) { + sizes.push(decoded_len_estimate(payload)); + } + } + ContentBlock::ToolResult { content_blocks, .. } => { + if let Some(blocks) = content_blocks.as_ref() { + for value in blocks { + if let Some((_, payload)) = stored_tool_image_payload(value) { + sizes.push(decoded_len_estimate(payload)); + } + } + } + } + _ => {} + } + } + } + sizes +} + +/// The `(mime_type, data)` pair of a stored tool-result image block, if the +/// block is one. +fn stored_tool_image_payload(value: &serde_json::Value) -> Option<(&str, &str)> { + if value.get("type").and_then(serde_json::Value::as_str) != Some("image") { + return None; + } + let mime_type = value.get("mime_type").and_then(serde_json::Value::as_str)?; + let data = value.get("data").and_then(serde_json::Value::as_str)?; + Some((mime_type, data)) +} + +/// One rewritten image, in decoded bytes. +#[derive(Debug, Clone, Copy)] +struct ShrunkPayload { + before: usize, + after: usize, +} + +/// Re-encode a `data:` URL image under `budget`, returning the replacement +/// URL plus the byte delta. +fn shrink_data_url(url: &str, budget: usize) -> Option<(String, ShrunkPayload)> { + let (_, payload) = parse_data_url(url)?; + let (mime, data, shrunk) = shrink_base64_image(payload, budget)?; + Some((format!("data:{mime};base64,{data}"), shrunk)) +} + +/// Re-encode one stored tool-result image block in place. +fn shrink_stored_tool_image(value: &mut serde_json::Value, budget: usize) -> Option { + let (_, payload) = stored_tool_image_payload(value)?; + let (mime, data, shrunk) = shrink_base64_image(payload, budget)?; + let object = value.as_object_mut()?; + object.insert("mime_type".to_string(), serde_json::Value::String(mime)); + object.insert("data".to_string(), serde_json::Value::String(data)); + Some(shrunk) +} + +/// Re-encode one base64 inline image to fit `budget` decoded bytes. +/// +/// `None` means the image already fits, cannot be decoded, or the re-encode +/// came back no smaller — so a caller's "did anything change" tally stays +/// honest about what a retry would actually send. +fn shrink_base64_image(data: &str, budget: usize) -> Option<(String, String, ShrunkPayload)> { + let bytes = STANDARD.decode(data).ok()?; + if bytes.len() <= budget { + return None; + } + let (decoded, _, _) = decode_and_guard_image(&bytes).ok()?; + let (mime, encoded) = reencode_within_budget(decoded, budget)?; + if encoded.len() >= bytes.len() { + return None; + } + Some(( + mime.to_string(), + STANDARD.encode(&encoded), + ShrunkPayload { + before: bytes.len(), + after: encoded.len(), + }, + )) +} + +/// Encode `image` down a short ladder until it fits `budget`. +/// +/// Rung order: longest edge capped at [`COMPACTION_IMAGE_MAX_EDGE`], then +/// halved until [`COMPACTION_IMAGE_MIN_EDGE`]. Alpha-bearing images stay PNG +/// (JPEG would flatten transparency); everything else becomes JPEG, which is +/// what actually makes screenshots and artwork small. The smallest rung wins +/// even when nothing fits `budget`, because a smaller-than-before payload is +/// still progress for the retry. +fn reencode_within_budget(image: DynamicImage, budget: usize) -> Option<(&'static str, Vec)> { + let mut current = image; + let (mut width, mut height) = current.dimensions(); + if width.max(height) > COMPACTION_IMAGE_MAX_EDGE { + let scale = f64::from(COMPACTION_IMAGE_MAX_EDGE) / f64::from(width.max(height)); + let scaled_width = ((f64::from(width) * scale).round() as u32).max(1); + let scaled_height = ((f64::from(height) * scale).round() as u32).max(1); + current = current.resize(scaled_width, scaled_height, FilterType::Lanczos3); + (width, height) = current.dimensions(); + } + let mut smallest: Option<(&'static str, Vec)> = None; + loop { + if let Some(candidate) = encode_inline_image(¤t) + && smallest + .as_ref() + .is_none_or(|(_, best)| candidate.1.len() < best.len()) + { + smallest = Some(candidate); + } + if smallest + .as_ref() + .is_some_and(|(_, bytes)| bytes.len() <= budget) + { + break; + } + if width.max(height) <= COMPACTION_IMAGE_MIN_EDGE { + break; + } + (width, height) = ((width / 2).max(1), (height / 2).max(1)); + current = current.resize(width, height, FilterType::Lanczos3); + } + smallest +} + +/// One encoder pass: PNG when alpha matters, JPEG otherwise. +fn encode_inline_image(image: &DynamicImage) -> Option<(&'static str, Vec)> { + let (width, height) = image.dimensions(); + let mut bytes = Vec::new(); + if image.color().has_alpha() { + let rgba = image.to_rgba8(); + PngEncoder::new_with_quality(&mut bytes, CompressionType::Best, PngFilter::Adaptive) + .write_image(rgba.as_raw(), width, height, ExtendedColorType::Rgba8) + .ok()?; + Some(("image/png", bytes)) + } else { + let rgb = image.to_rgb8(); + JpegEncoder::new_with_quality(&mut bytes, COMPACTION_IMAGE_JPEG_QUALITY) + .write_image(rgb.as_raw(), width, height, ExtendedColorType::Rgb8) + .ok()?; + Some(("image/jpeg", bytes)) + } +} + /// Render dropped-attachment notices as a block the model will read. /// /// Wrapped in a tag rather than appended as bare prose so the model can tell diff --git a/crates/tui/src/image_attach/tests.rs b/crates/tui/src/image_attach/tests.rs index 003875315a..94e451d38f 100644 --- a/crates/tui/src/image_attach/tests.rs +++ b/crates/tui/src/image_attach/tests.rs @@ -26,6 +26,136 @@ fn sniffs_every_accepted_format_from_magic_bytes() { ); } +/// Noise defeats compression, so the shrink ladder has to re-encode for real. +fn noise_payload(width: u32, height: u32) -> Vec { + use image::ImageEncoder as _; + let mut pixels = image::RgbImage::new(width, height); + for (x, y, pixel) in pixels.enumerate_pixels_mut() { + *pixel = image::Rgb([ + (x.wrapping_mul(31) ^ y.wrapping_mul(17)) as u8, + (x.wrapping_mul(7) ^ y.wrapping_mul(29)) as u8, + (x.wrapping_add(y).wrapping_mul(13)) as u8, + ]); + } + let mut bytes = Vec::new(); + image::codecs::png::PngEncoder::new_with_quality( + &mut bytes, + image::codecs::png::CompressionType::Fast, + image::codecs::png::FilterType::NoFilter, + ) + .write_image( + pixels.as_raw(), + width, + height, + image::ExtendedColorType::Rgb8, + ) + .expect("encode fixture png"); + bytes +} + +fn image_blocks_fixture() -> Vec { + let payload = STANDARD.encode(noise_payload(320, 320)); + vec![ + codewhale_models::Message { + role: Role::User, + content: vec![ + ContentBlock::Text { + text: "look at this".to_string(), + cache_control: None, + }, + ContentBlock::ImageUrl { + image_url: ImageUrlContent { + url: format!("data:image/png;base64,{payload}"), + }, + }, + ], + }, + codewhale_models::Message { + role: Role::User, + content: vec![ContentBlock::ToolResult { + tool_use_id: "call_1".to_string(), + content: "Read image file [image/png]".to_string(), + is_error: None, + content_blocks: Some(vec![serde_json::json!({ + "type": "image", + "mime_type": "image/png", + "data": payload, + })]), + }], + }, + ] +} + +#[test] +fn compaction_shrink_rewrites_both_image_carriers_under_budget() { + let budget = 64 * 1024; + let mut messages = image_blocks_fixture(); + let outcome = shrink_images_for_request_with_budget(&mut messages, budget); + assert_eq!(outcome.images, 2, "both carriers are rewritten"); + assert!(outcome.bytes_after < outcome.bytes_before); + + let ContentBlock::ImageUrl { image_url } = &messages[0].content[1] else { + panic!("image block survives the shrink"); + }; + let (mime, payload) = parse_data_url(&image_url.url).expect("data url"); + assert!(matches!(mime, "image/png" | "image/jpeg"), "{mime}"); + let bytes = STANDARD.decode(payload).expect("base64"); + assert!(bytes.len() <= budget, "{} <= {budget}", bytes.len()); + assert_eq!(sniff_media_type(&bytes), Some(mime)); + + let ContentBlock::ToolResult { content_blocks, .. } = &messages[1].content[0] else { + panic!("tool result survives the shrink"); + }; + let block = &content_blocks.as_ref().expect("blocks")[0]; + assert_eq!(block["type"], "image"); + let nested = STANDARD + .decode(block["data"].as_str().expect("data")) + .expect("nested base64"); + assert!(nested.len() <= budget); + assert_eq!( + sniff_media_type(&nested), + block["mime_type"].as_str(), + "the mime must match the rewritten bytes" + ); +} + +#[test] +fn compaction_shrink_leaves_an_under_budget_history_untouched() { + let mut messages = image_blocks_fixture(); + let before = messages.clone(); + let outcome = shrink_images_for_request_with_budget(&mut messages, 32 * 1024 * 1024); + assert_eq!(outcome, ShrunkInlineImages::default()); + assert_eq!(messages, before, "a fitting request is never rewritten"); +} + +#[test] +fn compaction_placeholder_keeps_the_image_visible_as_text() { + let mut messages = image_blocks_fixture(); + let replaced = replace_images_with_placeholders( + &mut messages, + "the summary request exceeded the provider's request-body limit (HTTP 413)", + ); + assert_eq!(replaced, 2); + + let ContentBlock::Text { text, .. } = &messages[0].content[1] else { + panic!("image_url becomes a note"); + }; + assert!(text.contains("omitted from this summary pass"), "{text}"); + assert!(text.contains("do not describe"), "{text}"); + + let ContentBlock::ToolResult { + content, + content_blocks, + .. + } = &messages[1].content[0] + else { + panic!("tool result survives"); + }; + assert!(content_blocks.is_none(), "no base64 may remain"); + assert!(content.contains("1 image(s)"), "{content}"); + assert!(content.contains("Read image file [image/png]"), "{content}"); +} + #[test] fn sniffing_ignores_the_extension_and_believes_the_bytes() { // A JPEG named .png must be declared image/jpeg, or the provider From ef5071605f8d7bf31336ab5821b2c7018d7107d4 Mon Sep 17 00:00:00 2001 From: Shizuku <2163018547@qq.com> Date: Sat, 26 Sep 2026 21:09:09 +0800 Subject: [PATCH 2/4] fix(compaction): treat in-budget images as present, not absent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up on the request-size ladder: - The shrink rung reported "0 images" both when there were none and when every image already fit its share of the budget, so a session whose images were small but whose endpoint cap was lower failed outright instead of reaching the replace rung. `ShrunkInlineImages` now carries `images_seen`, and an in-budget request goes straight to the replace rung — no identical retry, no dead end. - `is_request_too_large_error` also matches "request body too large"; the rendered status-code token remains the primary signal. - The per-image floor comment now states that many images can push the total past the budget, which the replace rung covers. Tests: a fixture with in-budget images that is refused anyway proves the direct-to-replace path; the under-budget shrink test asserts `images_seen`. --- crates/tui/src/compaction.rs | 124 ++++++++++++++++++++++----- crates/tui/src/image_attach.rs | 31 +++++-- crates/tui/src/image_attach/tests.rs | 8 +- 3 files changed, 134 insertions(+), 29 deletions(-) diff --git a/crates/tui/src/compaction.rs b/crates/tui/src/compaction.rs index bc40a7938f..7448047afd 100644 --- a/crates/tui/src/compaction.rs +++ b/crates/tui/src/compaction.rs @@ -1748,38 +1748,46 @@ async fn create_summary( RequestSizeLadder::Start => { let shrunk = crate::image_attach::shrink_images_for_request(&mut request_messages); - if shrunk.images == 0 { + if shrunk.images_seen == 0 { return Err(err.context( "The summary request exceeded the provider's request-body limit and the history carries no inline images to re-encode", )); } size_ladder = RequestSizeLadder::ImagesShrunk; - let message = format!( - "Making room exceeded the provider's request-body limit (HTTP 413); re-encoded {} inline image(s) smaller ({} to {}) and is retrying the summary.", - shrunk.images, - crate::image_attach::human_bytes(shrunk.bytes_before), - crate::image_attach::human_bytes(shrunk.bytes_after), - ); - logging::warn(&message); - deliver_compaction_notice(notice_sink, message); + if shrunk.images > 0 { + let message = format!( + "Making room exceeded the provider's request-body limit (HTTP 413); re-encoded {} inline image(s) smaller ({} to {}) and is retrying the summary.", + shrunk.images, + crate::image_attach::human_bytes(shrunk.bytes_before), + crate::image_attach::human_bytes(shrunk.bytes_after), + ); + logging::warn(&message); + deliver_compaction_notice(notice_sink, message); + continue; + } + // Images are present but every one already fits its share of + // the byte budget, and the body was still refused: the cap + // sits below the budget. A retry would send identical bytes, + // so replace the images now instead of failing the pass. + if let Err(replace_error) = + replace_inline_images_for_retry(&mut request_messages, notice_sink) + { + return Err(err.context(format!( + "The summary request exceeded the provider's request-body limit; {replace_error}" + ))); + } + size_ladder = RequestSizeLadder::ImagesReplaced; continue; } RequestSizeLadder::ImagesShrunk => { - let replaced = crate::image_attach::replace_images_with_placeholders( - &mut request_messages, - "the summary request exceeded the provider's request-body limit (HTTP 413)", - ); - if replaced == 0 { - return Err(err.context( - "The summary request still exceeded the provider's request-body limit and no inline images were left to replace", - )); + if let Err(replace_error) = + replace_inline_images_for_retry(&mut request_messages, notice_sink) + { + return Err(err.context(format!( + "The summary request still exceeded the provider's request-body limit and {replace_error}" + ))); } size_ladder = RequestSizeLadder::ImagesReplaced; - let message = format!( - "Making room still exceeded the provider's request-body limit; replaced {replaced} inline image(s) with text notes for this summary pass and is retrying.", - ); - logging::warn(&message); - deliver_compaction_notice(notice_sink, message); continue; } RequestSizeLadder::ImagesReplaced => { @@ -1900,9 +1908,13 @@ enum RequestSizeLadder { fn is_request_too_large_error(e: &anyhow::Error) -> bool { e.chain().any(|cause| { let lower = cause.to_string().to_lowercase(); + // The client always renders the status code into the message + // ("HTTP 413"), so that token is the primary signal; the phrase list + // below catches gateways that state the condition without it. lower.contains("http 413") || lower.contains("payload too large") || lower.contains("request entity too large") + || lower.contains("request body too large") || lower.contains("length limit exceeded") }) } @@ -1914,6 +1926,30 @@ fn deliver_compaction_notice(sink: Option<&dyn CompactionNoticeSink>, message: S } } +/// Replace every inline image for one retry and announce it. +/// +/// Shared by the two rungs that can reach the replace step: after a shrink +/// pass that changed something, and directly when the images already fit the +/// byte budget but the body was refused anyway. +fn replace_inline_images_for_retry( + messages: &mut Vec, + notice_sink: Option<&dyn CompactionNoticeSink>, +) -> Result { + let replaced = crate::image_attach::replace_images_with_placeholders( + messages, + "the summary request exceeded the provider's request-body limit (HTTP 413)", + ); + if replaced == 0 { + anyhow::bail!("no inline images were left to replace"); + } + let message = format!( + "Making room still exceeded the provider's request-body limit; replaced {replaced} inline image(s) with text notes for this summary pass and is retrying." + ); + logging::warn(&message); + deliver_compaction_notice(notice_sink, message); + Ok(replaced) +} + fn is_context_window_error(e: &anyhow::Error) -> bool { let text = e.to_string(); if crate::error_taxonomy::classify_error_message(&text) @@ -2540,6 +2576,50 @@ mod tests { assert!(notices[0].contains("413"), "{notices:?}"); } + #[tokio::test] + async fn request_body_413_with_in_budget_images_skips_the_noop_retry_and_replaces() { + // The images fit their share of the 2 MiB budget, yet the endpoint + // refused the body: the cap sits below the budget. The ladder must not + // report "no images" (which would fail the pass outright) and must not + // resend identical bytes — it goes straight to the replace rung. + let _environment = crate::test_support::lock_test_env(); + let messages = vec![ + msg("user", "context before the screenshot"), + user_image_message(&png_data_url(64, 64)), + ]; + let client = ScriptedSummaryClient::with_outcomes(vec![ + Err(request_body_413()), + Ok(summary_content()), + ]); + let sink = std::sync::Arc::new(RecordingNoticeSink::default()); + let envelope = body_413_retry_envelope(sink.clone()); + let mut usage = Usage::default(); + + let result = compact_messages_safe(&client, &messages, None, &envelope, &mut usage).await; + assert!( + result.is_ok(), + "in-budget images must still let the ladder finish: {result:?}" + ); + let requests = client.requests.lock().expect("requests").clone(); + assert_eq!(requests.len(), 2, "replace directly, no identical retry"); + assert!( + inline_image_bytes(&requests[0]) > 0, + "the fixture carried the image" + ); + assert_eq!( + inline_image_bytes(&requests[1]), + 0, + "the retry carries notes, not bytes" + ); + let notices = sink.messages(); + assert_eq!( + notices.len(), + 1, + "one notice for the replace rung: {notices:?}" + ); + assert!(notices[0].contains("replaced"), "{notices:?}"); + } + #[tokio::test] async fn request_body_413_without_inline_images_fails_with_context() { let _environment = crate::test_support::lock_test_env(); diff --git a/crates/tui/src/image_attach.rs b/crates/tui/src/image_attach.rs index b1b18e2f6a..efa88da0a9 100644 --- a/crates/tui/src/image_attach.rs +++ b/crates/tui/src/image_attach.rs @@ -801,6 +801,13 @@ const COMPACTION_IMAGE_JPEG_QUALITY: u8 = 80; pub(crate) struct ShrunkInlineImages { /// Images that were re-encoded smaller. pub images: usize, + /// Inline images the request carried, rewritten or not. + /// + /// `images == 0` alone cannot tell "nothing to do because every image + /// already fits" from "there were no images"; a request-size ladder must + /// not conflate the two, because only the second makes the next rung + /// pointless. + pub images_seen: usize, /// Decoded bytes of those images before the pass. pub bytes_before: usize, /// Decoded bytes of those images after the pass. @@ -814,10 +821,12 @@ pub(crate) struct ShrunkInlineImages { /// that the wire projection reads back out via /// [`provider_tool_result_image_refs`]. /// -/// `images == 0` means nothing was rewritten — there were no inline images, -/// or each was already under its share of the budget — so a caller that is +/// `images == 0` means nothing was rewritten — either there were no inline +/// images, or each was already under its share of the budget. Check +/// [`ShrunkInlineImages::images_seen`] to tell those apart: a caller that is /// still looking at a body-size rejection must climb to the next rung rather -/// than resend the same bytes. +/// than resend the same bytes, and when images are present but nothing was +/// rewritten, the next rung is the only one that can still change the payload. pub(crate) fn shrink_images_for_request( messages: &mut [codewhale_models::Message], ) -> ShrunkInlineImages { @@ -832,9 +841,20 @@ pub(crate) fn shrink_images_for_request_with_budget( ) -> ShrunkInlineImages { let sizes = inline_image_sizes(messages); let total: usize = sizes.iter().sum(); - if sizes.is_empty() || total <= budget { - return ShrunkInlineImages::default(); + let images_seen = sizes.len(); + if total <= budget { + // Images may be present and simply already fit: report them as seen + // without rewriting, so a caller can tell "nothing to shrink" from + // "nothing there". + return ShrunkInlineImages { + images_seen, + ..ShrunkInlineImages::default() + }; } + // A share of the budget per image, floored so a few large images still come + // back readable. With many images the floor can push the total past + // `budget`; the caller's next rung (replace with notes) covers that case, + // not a tighter share here. let per_image = (budget / sizes.len()).max(COMPACTION_IMAGE_MIN_BUDGET_BYTES); let mut outcome = ShrunkInlineImages::default(); for message in messages.iter_mut() { @@ -864,6 +884,7 @@ pub(crate) fn shrink_images_for_request_with_budget( } } } + outcome.images_seen = images_seen; outcome } diff --git a/crates/tui/src/image_attach/tests.rs b/crates/tui/src/image_attach/tests.rs index 94e451d38f..079f4e159c 100644 --- a/crates/tui/src/image_attach/tests.rs +++ b/crates/tui/src/image_attach/tests.rs @@ -124,8 +124,12 @@ fn compaction_shrink_leaves_an_under_budget_history_untouched() { let mut messages = image_blocks_fixture(); let before = messages.clone(); let outcome = shrink_images_for_request_with_budget(&mut messages, 32 * 1024 * 1024); - assert_eq!(outcome, ShrunkInlineImages::default()); - assert_eq!(messages, before, "a fitting request is never rewritten"); + assert_eq!(outcome.images, 0, "a fitting request is never rewritten"); + assert_eq!( + outcome.images_seen, 2, + "the request still carried images, and the ladder must be able to tell" + ); + assert_eq!(messages, before); } #[test] From e0c3c2fbe80a85b64de60870a5e9d36303310d52 Mon Sep 17 00:00:00 2001 From: Shizuku <2163018547@qq.com> Date: Sat, 26 Sep 2026 22:19:32 +0800 Subject: [PATCH 3/4] fix(compaction): keep the retry helper on a slice, sync the changelog slice CI follow-up on the fix commit: - clippy::ptr_arg on the newer stable toolchain: the replace helper took `&mut Vec` where `&mut [Message]` does. - The Unreleased entry belongs in the root CHANGELOG.md, which scripts/sync-changelog.sh slices into crates/tui/CHANGELOG.md; editing the slice directly tripped the version-drift check. --- CHANGELOG.md | 6 ++++++ crates/tui/CHANGELOG.md | 13 ++++++------- crates/tui/src/compaction.rs | 2 +- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7328bc3dbd..edf56d54d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,12 @@ quieter, and Fleet runs can be checked before they spend anything. ### Fixed +- Making room now recovers from a provider request-body limit (HTTP 413) + instead of failing the pass. A summary request that is refused for size + re-encodes the conversation's inline images smaller and retries, says so in + the status line while it does, and — if the request is still refused — + replaces the images with text notes for that one summary pass. Session + history keeps the real images either way. - `codewhale exec --auto` no longer exits 141 with no output when a child it writes to, such as a stdio MCP server, closes its pipe early. Headless exec now ignores SIGPIPE while it runs, as the interactive TUI already did, diff --git a/crates/tui/CHANGELOG.md b/crates/tui/CHANGELOG.md index 695eb62f42..e33a85f560 100644 --- a/crates/tui/CHANGELOG.md +++ b/crates/tui/CHANGELOG.md @@ -12,13 +12,6 @@ with English/Chinese recovery links to home and docs ([#6419](https://github.com/Hmbown/Codewhale/issues/6419), [#6420](https://github.com/Hmbown/Codewhale/pull/6420)). -Making room now recovers from a provider's request-body limit (HTTP 413) -instead of failing the pass. A summary request that is refused for size -re-encodes the conversation's inline images smaller and retries, says so in the -status line while it does, and — if the request is still refused — replaces the -images with text notes for that one summary pass. Session history keeps the -real images either way. - Planned for Codewhale v0.10.1: a reliability and first-run release. Turns that stall now say so, approvals keep what you approved, plugin suggestions are quieter, and Fleet runs can be checked before they spend anything. @@ -51,6 +44,12 @@ quieter, and Fleet runs can be checked before they spend anything. ### Fixed +- Making room now recovers from a provider request-body limit (HTTP 413) + instead of failing the pass. A summary request that is refused for size + re-encodes the conversation's inline images smaller and retries, says so in + the status line while it does, and — if the request is still refused — + replaces the images with text notes for that one summary pass. Session + history keeps the real images either way. - `codewhale exec --auto` no longer exits 141 with no output when a child it writes to, such as a stdio MCP server, closes its pipe early. Headless exec now ignores SIGPIPE while it runs, as the interactive TUI already did, diff --git a/crates/tui/src/compaction.rs b/crates/tui/src/compaction.rs index 7448047afd..f16d9a7248 100644 --- a/crates/tui/src/compaction.rs +++ b/crates/tui/src/compaction.rs @@ -1932,7 +1932,7 @@ fn deliver_compaction_notice(sink: Option<&dyn CompactionNoticeSink>, message: S /// pass that changed something, and directly when the images already fit the /// byte budget but the body was refused anyway. fn replace_inline_images_for_retry( - messages: &mut Vec, + messages: &mut [Message], notice_sink: Option<&dyn CompactionNoticeSink>, ) -> Result { let replaced = crate::image_attach::replace_images_with_placeholders( From 2a3f51b4138dfbd311f18c6365479de9f5406aee Mon Sep 17 00:00:00 2001 From: Shizuku <2163018547@qq.com> Date: Sat, 26 Sep 2026 22:49:39 +0800 Subject: [PATCH 4/4] chore: resync the changelog slice and the web changelog after the merge The upstream merge moved the root CHANGELOG.md, so both derived copies needed regeneration: - scripts/sync-changelog.sh for the packed slice the binary embeds; - web/scripts/derive-changelog.mjs for the website's generated changelog. Local scripts/release/check-versions.sh is green with the v0.9.13 tag present (feature release-note receipts and version state both OK). --- web/lib/changelog.generated.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/web/lib/changelog.generated.ts b/web/lib/changelog.generated.ts index f86e0d1076..50de8dc873 100644 --- a/web/lib/changelog.generated.ts +++ b/web/lib/changelog.generated.ts @@ -50,6 +50,7 @@ export const CHANGELOG: ChangelogRelease[] = [ { "heading": "Fixed", "items": [ + "Making room now recovers from a provider request-body limit (HTTP 413) instead of failing the pass. A summary request that is refused for size re-encodes the conversation's inline images smaller and retries, says so in the status line while it does, and — if the request is still refused — replaces the images with text notes for that one summary pass. Session history keeps the real images either way.", "codewhale exec --auto no longer exits 141 with no output when a child it writes to, such as a stdio MCP server, closes its pipe early. Headless exec now ignores SIGPIPE while it runs, as the interactive TUI already did, and exec ... | head still ends quietly. One-shot codewhale exec no longer prints DeepSeek's raw <｜｜DSML｜｜ calls> tool-call markup as its answer: the markup is removed, and an answer that was only a tool call fails at once with the reason and a pointer to…", "The installation page is generated from docs/INSTALL.md, so the website and the guide can no longer disagree; broken anchors and unsafe links fail the build (#6450).", "codewhale config set refuses a value of the wrong type for a known setting (a word for an on/off switch, text for a number, a choice outside the list) instead of saving it (#6568, thanks @dajiaohuang).", @@ -60,10 +61,9 @@ export const CHANGELOG: ChangelogRelease[] = [ "Continuing a conversation that is already open no longer adds a second thread, and a fork keeps its own session file, so autosave on one side no longer leaves the other unloadable (#6406, thanks @gaord).", "Upgrading Codewhale no longer turns off the built-in Computer Use. Each build writes the built-in bundle to its own directory, so an upgrade used to present it as never reviewed and disabled. Now the review and enablement carry to the new build when its capabilities are unchanged. Changed capabilities show capabilities-changed and wait for review, and a revoked trust never carries (#6303).", "\"Allow for this conversation\" records a grant for that tool and argument class instead of switching the whole thread to Full Access, so the call you just approved is no longer failed by a Permissions change. An approval also survives a Permissions change that only widens what is allowed, grants end when a thread is archived or deleted, and web.run open grants are scoped by host. Full Access covers MCP tools that declare themselves destructive in every host, including…", - "web.run retries a refused page once with a browser user agent, and one site's failure no longer fails the whole call or drops its search results.", - "Hooks treat bash, Bash and exec_shell as one tool in tool_name conditions, so the documented example fires." + "web.run retries a refused page once with a browser user agent, and one site's failure no longer fails the whole call or drops its search results." ], - "itemCount": 20 + "itemCount": 21 }, { "heading": "Removed",