From e69aa03f5d51ab589f477b272c72b25d7ee47be4 Mon Sep 17 00:00:00 2001 From: Hare Date: Thu, 24 Sep 2026 18:21:29 +0900 Subject: [PATCH] feat: make image attachments request-local --- crates/agen/src/engine.rs | 13 + crates/agen/src/prune.rs | 73 ++++++ crates/agen/src/token_counter.rs | 40 +++- crates/worker/src/compact/token_counter.rs | 27 +++ crates/worker/src/compact/worker.rs | 79 +++++- crates/worker/src/worker.rs | 225 ++++++++++++++++-- docs/design/compaction.md | 16 ++ ...09-24-compaction-image-retention-budget.md | 76 ++++++ resources/prompts/internal/compact_system.md | 2 +- 9 files changed, 514 insertions(+), 37 deletions(-) create mode 100644 docs/report/2026-09-24-compaction-image-retention-budget.md diff --git a/crates/agen/src/engine.rs b/crates/agen/src/engine.rs index 2235614bc..1fa4fafd0 100644 --- a/crates/agen/src/engine.rs +++ b/crates/agen/src/engine.rs @@ -1790,6 +1790,19 @@ impl Engine { // this clone, so the caller-owned `history` stays intact. let mut request_context = history.items_cloned(); + // Binary tool attachments are single-consumption request details. Keep fresh + // attachments emitted after the latest model output for the immediate follow-up, + // but stop resending them once a model response has consumed them. Durable history + // remains untouched and retains the tool summary needed to fetch the file again. + let projected_attachments = + crate::prune::project_consumed_attachments(&mut request_context); + if projected_attachments > 0 { + debug!( + projected_attachments, + "Projected consumed tool-result attachments out of request context" + ); + } + // Prune projection: if both the config and the savings // estimator are configured, drop ToolResult.content from // prunable candidates whose estimated savings meet the diff --git a/crates/agen/src/prune.rs b/crates/agen/src/prune.rs index bdb35eb29..2ee018904 100644 --- a/crates/agen/src/prune.rs +++ b/crates/agen/src/prune.rs @@ -133,6 +133,33 @@ pub fn project(items: &mut [Item], indices: &[usize]) -> usize { count } +/// Remove binary attachments after the first model request that could consume them. +/// +/// Attachments on tool results following the latest model output are fresh outputs for the +/// immediate follow-up request and remain visible. Once a later assistant/reasoning/tool-call +/// item exists, the model has already had an opportunity to inspect the attachment. Subsequent +/// requests keep the durable tool summary and can call the originating tool again when the image +/// is still needed. Only the request-context clone is modified. +pub fn project_consumed_attachments(items: &mut [Item]) -> usize { + let latest_model_output = items.iter().rposition(|item| { + item.is_assistant_message() || item.is_reasoning() || item.is_tool_call() + }); + let mut count = 0; + for (index, item) in items.iter_mut().enumerate() { + let is_fresh = latest_model_output.is_some_and(|boundary| index > boundary); + if is_fresh { + continue; + } + if let Item::ToolResult { attachments, .. } = item + && !attachments.is_empty() + { + attachments.clear(); + count += 1; + } + } + count +} + /// Indices of detailed `Item::ToolResult` values that lie before /// the suffix protected by `protected_tokens`. Pure: does not mutate `items`. /// @@ -412,6 +439,52 @@ mod tests { )); } + fn image_result(call_id: &str, bytes: &[u8]) -> Item { + Item::tool_result_item_with_attachments( + call_id, + "Attached image.png", + None, + false, + vec![crate::tool::Attachment::Image( + crate::tool::ImageAttachment::new("image/png", bytes.to_vec()), + )], + ) + } + + #[test] + fn consumed_attachment_projection_keeps_only_fresh_tool_outputs() { + let mut immediate = vec![ + Item::tool_call("image-1", "ViewImage", r#"{"path":"first.png"}"#), + image_result("image-1", b"first"), + ]; + assert_eq!(project_consumed_attachments(&mut immediate), 0); + assert!(matches!( + &immediate[1], + Item::ToolResult { attachments, .. } if attachments.len() == 1 + )); + + let mut consumed = immediate.clone(); + consumed.push(Item::assistant_message("I inspected the image.")); + consumed.push(Item::user_message("continue")); + assert_eq!(project_consumed_attachments(&mut consumed), 1); + assert!(matches!( + &consumed[1], + Item::ToolResult { attachments, .. } if attachments.is_empty() + )); + + consumed.push(Item::tool_call( + "image-2", + "ViewImage", + r#"{"path":"second.png"}"#, + )); + consumed.push(image_result("image-2", b"second")); + assert_eq!(project_consumed_attachments(&mut consumed), 0); + assert!(matches!( + consumed.last(), + Some(Item::ToolResult { attachments, .. }) if attachments.len() == 1 + )); + } + #[test] fn project_skips_already_pruned_items() { // indices points at an item whose content is already None. diff --git a/crates/agen/src/token_counter.rs b/crates/agen/src/token_counter.rs index c79073be8..a91649197 100644 --- a/crates/agen/src/token_counter.rs +++ b/crates/agen/src/token_counter.rs @@ -47,11 +47,18 @@ pub fn prefix_bytes(items: &[Item]) -> Vec { prefix } -/// 1 Item の大きさ。JSON シリアライズ長を使う粗い近似。 -/// トークン数との絶対変換ではなく区間の按分にしか使わないので、 -/// プロバイダごとの overhead は比率でキャンセルされる。 +/// 1 Item の text-shaped size。JSON シリアライズ長を使う粗い近似。 +/// +/// Binary attachments are deliberately projected out. Their durable base64 encoding is a +/// storage representation, not provider text, so charging it at the text `bytes / 4` fallback +/// rate produces meaningless estimates. Exact provider usage measurements remain authoritative +/// when an attachment was actually sent. pub fn item_bytes(item: &Item) -> u64 { - serde_json::to_string(item) + let mut projected = item.clone(); + if let Item::ToolResult { attachments, .. } = &mut projected { + attachments.clear(); + } + serde_json::to_string(&projected) .map(|s| s.len() as u64) .unwrap_or(0) } @@ -265,6 +272,31 @@ mod tests { assert_eq!(est.tokens, 30_000 + delta_bytes / 4); } + #[test] + fn fallback_does_not_count_durable_image_bytes_as_text_tokens() { + let small = Item::tool_result_item_with_attachments( + "call", + "Attached image.png", + None, + false, + vec![crate::tool::Attachment::Image( + crate::tool::ImageAttachment::new("image/png", vec![1_u8; 8]), + )], + ); + let large = Item::tool_result_item_with_attachments( + "call", + "Attached image.png", + None, + false, + vec![crate::tool::Attachment::Image( + crate::tool::ImageAttachment::new("image/png", vec![1_u8; 1_000_000]), + )], + ); + + assert_eq!(item_bytes(&small), item_bytes(&large)); + assert_eq!(total_tokens(&[small], &[]), total_tokens(&[large], &[])); + } + #[test] fn total_zero_history_is_zero() { let est = total_tokens(&[], &[]); diff --git a/crates/worker/src/compact/token_counter.rs b/crates/worker/src/compact/token_counter.rs index dff101949..5dc7c0e46 100644 --- a/crates/worker/src/compact/token_counter.rs +++ b/crates/worker/src/compact/token_counter.rs @@ -338,6 +338,33 @@ mod tests { assert_eq!(cut.source, EstimateSource::Measured); } + #[test] + fn retained_split_does_not_treat_image_base64_as_text_tokens() { + fn history_with_image(bytes: usize) -> Vec { + vec![ + msg(&"older text ".repeat(2_000)), + Item::tool_call("image", "ViewImage", r#"{"path":"screen.png"}"#), + Item::tool_result_item_with_attachments( + "image", + "Attached image screen.png", + None, + false, + vec![agen::tool::Attachment::Image( + agen::tool::ImageAttachment::new("image/png", vec![7_u8; bytes]), + )], + ), + ] + } + + let small = history_with_image(8); + let large = history_with_image(1_000_000); + + assert_eq!( + split_for_retained_impl(&small, &[], 1_000), + split_for_retained_impl(&large, &[], 1_000) + ); + } + #[test] fn split_all_when_retained_zero() { let history = vec![msg("a"), msg("b")]; diff --git a/crates/worker/src/compact/worker.rs b/crates/worker/src/compact/worker.rs index 5aef61dff..2e450a2fd 100644 --- a/crates/worker/src/compact/worker.rs +++ b/crates/worker/src/compact/worker.rs @@ -93,10 +93,11 @@ struct SummaryParams { pub text: String, } -const MARK_DESCRIPTION: &str = "Inject a file's contents into the compacted context so the \ -next session starts with it already read. Use this for files the next task needs in full. \ -Optionally specify `offset` (0-based line) and `limit` (line count) to inject only a slice. \ -Counts against `auto_read_budget`; overflow returns an error and the mark is not recorded."; +const MARK_DESCRIPTION: &str = "Inject a UTF-8 text file's contents into the compacted context so the \ +next session starts with it already read. Images and binary files are rejected; use add_reference \ +for those so they can be fetched on demand. Optionally specify `offset` (0-based line) and `limit` \ +(line count) to inject only a slice. Counts against `auto_read_budget`; overflow returns an error \ +and the mark is not recorded."; const REFERENCE_DESCRIPTION: &str = "Record a Workdir-relative file path as a named reference in \ the compacted context without injecting its contents. Use for files that are contextually \ @@ -106,6 +107,14 @@ const SUMMARY_DESCRIPTION: &str = "Provide the final structured summary text. Su replace the previous content; only the last call is used. Must be called before the compact run \ ends or compaction fails."; +fn is_supported_image(bytes: &[u8]) -> bool { + bytes.starts_with(b"\x89PNG\r\n\x1a\n") + || bytes.starts_with(&[0xff, 0xd8, 0xff]) + || bytes.starts_with(b"GIF87a") + || bytes.starts_with(b"GIF89a") + || (bytes.len() >= 12 && bytes.starts_with(b"RIFF") && &bytes[8..12] == b"WEBP") +} + struct MarkReadRequiredTool { session: WorkdirSessionHandle, ctx: Arc>, @@ -136,9 +145,25 @@ impl Tool for MarkReadRequiredTool { }) .await .map_err(|e| ToolError::ExecutionFailed(format!("read failed: {e}")))?; - let text = String::from_utf8_lossy(&result.bytes); - let slice = text.as_ref(); - let estimated_tokens = estimate_tokens(slice.len()); + if result.truncated { + return Err(ToolError::ExecutionFailed( + "file is too large for auto-read; use add_reference so the next session can fetch it on demand" + .to_string(), + )); + } + if is_supported_image(&result.bytes) { + return Err(ToolError::ExecutionFailed( + "image files cannot be injected into compacted context; use add_reference so the next session can call ViewImage on demand" + .to_string(), + )); + } + let text = std::str::from_utf8(&result.bytes).map_err(|_| { + ToolError::ExecutionFailed( + "binary files cannot be injected into compacted context; use add_reference so the next session can fetch them on demand" + .to_string(), + ) + })?; + let estimated_tokens = estimate_tokens(text.len()); let mut guard = self.ctx.lock().expect("compact worker context poisoned"); let budget = guard.auto_read_budget; @@ -615,6 +640,46 @@ mod tests { assert!(guard.auto_read_consumed <= 1_000); } + #[tokio::test] + async fn mark_read_required_rejects_images_without_recording_auto_read() { + let tmp = tempfile::TempDir::new().unwrap(); + let path = tmp.path().join("screen.png"); + std::fs::write(&path, b"\x89PNG\r\n\x1a\nbody").unwrap(); + + let ctx = Arc::new(Mutex::new(CompactWorkerContext::with_budget(1_000))); + let tool: Arc = Arc::new(MarkReadRequiredTool { + session: make_fs(tmp.path()), + ctx: ctx.clone(), + }); + let input = serde_json::json!({ "file_path": "screen.png" }).to_string(); + + let error = tool.execute(&input, Default::default()).await.unwrap_err(); + + assert!(error.to_string().contains("use add_reference")); + let guard = ctx.lock().unwrap(); + assert!(guard.read_required.is_empty()); + assert_eq!(guard.auto_read_consumed, 0); + } + + #[tokio::test] + async fn mark_read_required_rejects_non_utf8_binary() { + let tmp = tempfile::TempDir::new().unwrap(); + let path = tmp.path().join("data.bin"); + std::fs::write(&path, [0xff, 0xfe, 0xfd]).unwrap(); + + let ctx = Arc::new(Mutex::new(CompactWorkerContext::with_budget(1_000))); + let tool: Arc = Arc::new(MarkReadRequiredTool { + session: make_fs(tmp.path()), + ctx: ctx.clone(), + }); + let input = serde_json::json!({ "file_path": "data.bin" }).to_string(); + + let error = tool.execute(&input, Default::default()).await.unwrap_err(); + + assert!(error.to_string().contains("use add_reference")); + assert!(ctx.lock().unwrap().read_required.is_empty()); + } + #[tokio::test] async fn mark_read_required_rejects_over_budget() { let tmp = tempfile::TempDir::new().unwrap(); diff --git a/crates/worker/src/worker.rs b/crates/worker/src/worker.rs index 31aae61b2..eea5fda12 100644 --- a/crates/worker/src/worker.rs +++ b/crates/worker/src/worker.rs @@ -1,4 +1,4 @@ -use std::collections::VecDeque; +use std::collections::{HashMap, VecDeque}; #[cfg(test)] use std::path::Path; use std::path::PathBuf; @@ -5571,11 +5571,20 @@ impl Worker { let history_entries = self.session.history().entries(); let retain_from = cut.index.min(history_entries.len()); let retained_history_entries = history_entries[retain_from..].to_vec(); - let retained_items = retained_history_entries + let mut retained_items = retained_history_entries .iter() .map(|entry| entry.item.clone()) .collect::>(); - let entries_to_summarise = history_entries[..retain_from].to_vec(); + let default_workdir_alias = (self.workdir_sessions.len() == 1) + .then(|| self.workdir_sessions.aliases().into_iter().next()) + .flatten() + .map(|alias| alias.as_str().to_string()); + let retained_image_references = + project_compaction_attachments(&mut retained_items, default_workdir_alias.as_deref()); + // The compactor explores summaries, text details, and ViewImage call arguments. It + // never needs durable image bodies; relevant images are nominated with add_reference. + let mut entries_to_summarise = history_entries[..retain_from].to_vec(); + project_compactor_capture_attachments(&mut entries_to_summarise); let items_to_summarise = entries_to_summarise .iter() .map(|entry| entry.item.clone()) @@ -5840,15 +5849,16 @@ impl Worker { "You have not called `write_summary` yet. Deliver the structured \ summary now (Completed Tasks / Active Task / Key Decisions / \ User Directives / Current Work) and nominate any files the next \ - session needs with `mark_read_required`." + session needs with `mark_read_required` for UTF-8 text or \ + `add_reference` for images, binary files, and on-demand reads." .to_string(), ) } else if snapshot.read_required.is_empty() && !default_refs.is_empty() { Some( - "Summary received. If any of the referenced files are required \ - for the next session to continue the task, call \ - `mark_read_required` on them now. Otherwise reply briefly to \ - close out." + "Summary received. If any of the referenced UTF-8 text files must \ + be present in the next session, call `mark_read_required` on them. \ + Use `add_reference` for images, binary files, or files that can be \ + fetched on demand. Otherwise reply briefly to close out." .to_string(), ) } else { @@ -5983,20 +5993,10 @@ impl Worker { } } - // Reference list as a single system message; omitted when empty. - let reference_message = (!final_ctx.references.is_empty()).then(|| { - let list = final_ctx - .references - .iter() - .map(|p| format!("- {}", p.display())) - .collect::>() - .join("\n"); - Item::system_message(format!( - "[Referenced files — read before compaction, contents not included]\n\ - {list}\n\ - Use read_file to access current contents if needed." - )) - }); + // Related files are carried by name only. Retained ViewImage payloads are + // projected out above and become explicit on-demand image references here. + let reference_message = + build_related_file_message(&final_ctx.references, &retained_image_references); // Count surviving user_messages before consuming `retained_items` // — needed to align `self.user_segments` after the swap below. @@ -7276,6 +7276,124 @@ impl From for WorkerRunResult { } } +#[derive(Debug, Clone, PartialEq, Eq)] +struct CompactionImageReference { + target_workdir: Option, + path: String, +} + +#[derive(serde::Deserialize)] +struct ViewImageCallArguments { + #[serde(default)] + target_workdir: Option, + path: String, +} + +/// Remove binary bodies from the immutable capture exposed to the compactor. +fn project_compactor_capture_attachments(entries: &mut [HistoryEntry]) -> usize { + let mut projected = 0; + for entry in entries { + if let Item::ToolResult { attachments, .. } = &mut entry.item + && !attachments.is_empty() + { + attachments.clear(); + projected += 1; + } + } + projected +} + +/// Remove every durable binary attachment from the compacted tail and preserve +/// re-readable ViewImage origins as related-file references. +fn project_compaction_attachments( + items: &mut [Item], + default_workdir_alias: Option<&str>, +) -> Vec { + let view_image_calls = items + .iter() + .filter_map(|item| match item { + Item::ToolCall { + call_id, + name, + arguments, + .. + } if name == "ViewImage" => serde_json::from_str::(arguments) + .ok() + .map(|arguments| { + ( + call_id.clone(), + CompactionImageReference { + target_workdir: arguments + .target_workdir + .or_else(|| default_workdir_alias.map(str::to_string)), + path: arguments.path, + }, + ) + }), + _ => None, + }) + .collect::>(); + + let mut references = Vec::new(); + for item in items { + let Item::ToolResult { + call_id, + attachments, + .. + } = item + else { + continue; + }; + if attachments.is_empty() { + continue; + } + attachments.clear(); + if let Some(reference) = view_image_calls.get(call_id) + && !references.contains(reference) + { + references.push(reference.clone()); + } + } + references +} + +fn build_related_file_message( + references: &[PathBuf], + image_references: &[CompactionImageReference], +) -> Option { + if references.is_empty() && image_references.is_empty() { + return None; + } + + let mut lines = Vec::with_capacity(references.len() + image_references.len()); + for reference in references { + let path = reference.display().to_string(); + if image_references + .iter() + .any(|image| image.target_workdir.is_none() && image.path == path) + { + continue; + } + lines.push(format!("- {path}")); + } + for reference in image_references { + match reference.target_workdir.as_deref() { + Some(target_workdir) => lines.push(format!( + "- {} (image; use ViewImage with target_workdir {:?})", + reference.path, target_workdir + )), + None => lines.push(format!("- {} (image; use ViewImage)", reference.path)), + } + } + + Some(Item::system_message(format!( + "[Related files — contents not included]\n{}\n\ + Use Read for text files and ViewImage for image references when their current contents \ + are needed.", + lines.join("\n") + ))) +} + #[derive(Debug, Clone, Copy)] struct SummaryInputOptions { overview_target_tokens: u64, @@ -7329,9 +7447,9 @@ fn build_summary_input( if !default_refs.is_empty() { out.push_str( "These files were touched recently in this session. Use `read_file` \ - on them as needed, then call `mark_read_required` for any whose \ - contents the next session must have, and `add_reference` for files \ - it should know about by name only.\n\n## Referenced files\n", + on text files as needed, then call `mark_read_required` for UTF-8 text \ + whose contents the next session must have. Call `add_reference` for images, \ + binary files, and files it should know about by name only.\n\n## Referenced files\n", ); for p in default_refs { out.push_str("- "); @@ -8456,6 +8574,63 @@ mod build_summary_prompt_tests { .text } + #[test] + fn retained_images_become_on_demand_related_files() { + let mut items = vec![ + Item::tool_call_json( + "image-1", + "ViewImage", + serde_json::json!({ + "path": "screenshots/result.png" + }), + ), + Item::tool_result_item_with_attachments( + "image-1", + "Attached image screenshots/result.png", + None, + false, + vec![agen::tool::Attachment::Image( + agen::tool::ImageAttachment::new("image/png", vec![7_u8; 100_000]), + )], + ), + ]; + + let mut capture_entries = items + .iter() + .cloned() + .map(|item| HistoryEntry::new(item, ())) + .collect::>(); + assert_eq!( + project_compactor_capture_attachments(&mut capture_entries), + 1 + ); + assert!(matches!( + &capture_entries[1].item, + Item::ToolResult { attachments, .. } if attachments.is_empty() + )); + + let references = project_compaction_attachments(&mut items, Some("main")); + + assert_eq!( + references, + vec![CompactionImageReference { + target_workdir: Some("main".to_string()), + path: "screenshots/result.png".to_string(), + }] + ); + assert!(matches!( + &items[1], + Item::ToolResult { attachments, .. } if attachments.is_empty() + )); + let message = build_related_file_message(&[], &references) + .and_then(|item| item.as_text().map(str::to_string)) + .unwrap(); + assert!(message.contains("[Related files — contents not included]")); + assert!(message.contains("screenshots/result.png")); + assert!(message.contains("target_workdir \"main\"")); + assert!(message.contains("ViewImage")); + } + #[test] fn strips_tool_call_arguments() { let items = vec![Item::tool_call_json( diff --git a/docs/design/compaction.md b/docs/design/compaction.md index 6d33041b5..fbe6e62d8 100644 --- a/docs/design/compaction.md +++ b/docs/design/compaction.md @@ -32,6 +32,22 @@ Compaction should preserve persisted reasoning history and avoid serializing unv The important property is explainability: after compaction, records should still show what summary replaced which older context and why future turns can rely on it. +## Binary tool details + +Durable binary attachments are not text and must not be charged by applying the text +`bytes / 4` fallback to their base64 persistence encoding. Provider usage measurements remain +authoritative for requests that actually include an attachment. + +An image returned by `ViewImage` is sent for the immediate follow-up model request. Later requests +project the consumed image body out while retaining the tool summary, so the Worker can call +`ViewImage` again when it needs the current file. Compaction likewise never carries image bodies +into the replacement Segment: retained images become related-file references with their Workdir +alias and path where available. + +The compactor may auto-read only UTF-8 text with `mark_read_required`. Images and other binary +files can be nominated only with `add_reference`; the next Worker fetches them on demand with +`ViewImage` or the appropriate read tool. + ## Activation boundary A compacted Segment is staged completely before it can become authoritative. The diff --git a/docs/report/2026-09-24-compaction-image-retention-budget.md b/docs/report/2026-09-24-compaction-image-retention-budget.md new file mode 100644 index 000000000..0fde99d1b --- /dev/null +++ b/docs/report/2026-09-24-compaction-image-retention-budget.md @@ -0,0 +1,76 @@ +# Compaction retained image payloads exceed the result budget after generation + +## Symptom + +A dogfood Worker repeatedly failed automatic compaction with: + +```text +pre-run compaction failed: compacted result context too large: 199726 tokens exceeds max 60000 +automatic compaction failed and the provider request remained unsafe: result_context_too_large +``` + +The triggering human message was `continue`. The failure appeared only after the Compactor had spent multiple minutes generating a summary. + +## Observed context shape + +The retained tail ended with one parallel group of five `ViewImage` calls and results. The PNG payloads totalled approximately 589 KB. Their durable tool-result attachments contained approximately 785 KB of base64 data: + +| Result | PNG bytes | base64 characters | +| ------------------------- | ----------: | ----------------: | +| representative mobile | 67,274 | 89,700 | +| partial failure narrow | 95,730 | 127,640 | +| conflict desktop | 144,897 | 193,196 | +| representative desktop | 145,057 | 193,412 | +| operation loading desktop | 136,012 | 181,352 | +| **Total** | **588,970** | **785,300** | + +The retained split requested an 8,000-token tail. Its byte-derived target was about 32 KB, but the last indivisible tool result alone serialized to about 182 KB. Pair-boundary repair then moved the cut backward to the first parallel `ViewImage` call, retaining all five calls and results. The repaired retained tail serialized to approximately 788 KB. + +The result check calls `agen::token_counter::total_tokens(&new_history, &[])`. With no result-segment usage records, this uses serialized JSON bytes divided by four. The retained image group alone therefore became an estimated 197k tokens. Summary, references, and TaskStore items raised the final estimate to 199,726 tokens. + +This is not a provider token measurement. It is a fallback estimate dominated by base64 image bytes. + +## Execution ordering + +`compact_impl` determines the retained items before starting the Internal Compactor, but checks `result_context_max_tokens` only after the Compactor has generated its summary, completed any nudge turn, re-read nominated files, and assembled `new_history`. + +The observed Worker made three deterministic failed attempts against the same source Segment: + +| Trigger | Duration | +| ----------------- | --------: | +| request threshold | 197.757 s | +| pre-run | 183.572 s | +| pre-run | 155.872 s | + +The final pre-run attempt finished 45 ms before the persisted `continue` input and run error. The ordinary Worker provider request was not sent. After pre-run compaction failed, the request-threshold interceptor saw the same unsafe occupancy and correctly failed closed with `result_context_too_large`. + +Thus the safety stop occurs before the ordinary provider request, but after an expensive Compactor provider run whose result can never pass the current result-budget calculation. + +## Design problems + +1. **Image attachments are counted as text.** Serialized base64 bytes divided by four are not an appropriate estimate for provider image-input occupancy. +2. **The retained budget is soft at item and pair boundaries.** One large result can exceed the budget, and pair repair can pull an entire parallel group back into the retained tail. +3. **An impossible result is detected too late.** Retained items are known before spawning the Compactor. Under the current estimator, their lower bound already exceeded the 60k result maximum. +4. **The same deterministic failure is retried on each logical run.** Failure suppression lasts only for the current run, so repeated `continue` submissions spend another 2–3 minutes producing a result that is discarded. +5. **The post-generation error obscures what consumed the budget.** The alert reports only the final total and maximum, not retained/summary/auto-read/task breakdown or the fallback estimate source. + +## Suggested direction + +- Give attachment/image inputs provider-shape-aware token accounting rather than charging their base64 serialization as text. +- Before starting the Compactor, calculate and report a retained-result lower bound. Do not run the model when no legal summary can fit the result maximum. +- Make retained-tail construction capable of projecting large tool-result details/attachments while preserving the durable summary and call/result pairing, or move the entire oversized completed tool group to the summarized side. +- Persist enough failure state to avoid retrying an unchanged, deterministically impossible source Segment. +- Include bounded breakdown fields in the failure diagnostic: estimate source, retained items/tokens, attachment bytes, summary tokens, auto-read tokens, and fixed synthetic context. + +The final post-compaction safety check should remain. The defect is that it uses the wrong accounting for images and is the first effective result-size check rather than the last guard after an earlier feasibility decision. + +## Implemented follow-up + +The follow-up change stops interpreting durable image bytes as text tokens and removes the need for compaction to carry image bodies: + +- fallback token accounting projects binary attachments out before JSON byte estimation; +- an image attachment is included in the immediate follow-up request, then projected out of later request contexts while its durable tool summary remains; +- retained compaction images are stripped from the replacement Segment and emitted as related-file references, preserving `target_workdir` and path where available; +- `mark_read_required` rejects images and non-UTF-8 files, directing the compactor to `add_reference` instead. + +Exact provider usage remains authoritative for the request that actually consumes an image. The post-compaction maximum remains as the final safety guard. diff --git a/resources/prompts/internal/compact_system.md b/resources/prompts/internal/compact_system.md index 2d30eb9f3..f0a4f53d6 100644 --- a/resources/prompts/internal/compact_system.md +++ b/resources/prompts/internal/compact_system.md @@ -7,7 +7,7 @@ The conversation input is a bounded overview/index, not the full transcript. Tre 1. Read the provided overview/index and current TaskStore snapshot. 2. If the provided index is not enough, call `ShowOverview` on the pinned capture. Use `SearchEntries` to locate relevant entries, then `ReadEntry` with the returned stable entry reference to inspect only what is needed. 3. Use `Read` to inspect referenced Workdir files before deciding what the next session needs. Prefer skimming over blind inclusion. -4. For files whose current contents are load-bearing for the active work, call `mark_read_required` to inject them into the next session. These count against the auto-read token budget — spend it deliberately. +4. For UTF-8 text files whose current contents are load-bearing for the active work, call `mark_read_required` to inject them into the next session. These count against the auto-read token budget — spend it deliberately. Images and binary files cannot be injected; record them with `add_reference` so the next session can fetch them with `ViewImage` or the appropriate read tool. 5. For files the next session should know about but can fetch on demand, call `add_reference` to record the path without embedding contents. 6. Finish with `write_summary` carrying the final text. You may call it multiple times; only the last call is kept.