feat(engine): wire tool-output spillover into the engine and pager (#500)
The spillover writer (#422) and inline cell annotation (#423) were already in place; this commit makes the pipeline actually fire and gives the user a way to see the elided tail. * `apply_spillover` lives in `tools/truncate.rs` and mutates a `ToolResult` in place: writes the full content to `~/.deepseek/tool_outputs/<id>.txt`, replaces the inline content with a 32 KiB head plus a footer pointing at the file, and stamps `metadata.spillover_path` so downstream renderers can find it. Skips error results so the model still sees the failure verbatim. Preserves prior metadata when present. * `core/engine/turn_loop.rs` calls `apply_spillover` immediately after `execute_tool_with_lock` returns, before the result fans out to the model context (`ContentBlock::ToolResult`) and the UI (`Event::ToolCallComplete`). Both the parallel and sequential tool paths get the same hook so the model and the UI always see the same truncated content. * `tui/ui.rs::open_details_pager_for_cell` now folds the full spillover-file body into the tool-details pager when the focused cell has a `spillover_path`. Truncated head stays at the top (so the user can see what the model received) followed by a `── Full output (spillover) ──` separator and the file body. Missing files render an inline notice instead of silently truncating. * The model's footer ("Use `read_file path=…` if you need the elided tail") teaches the agent how to recover the rest of the payload on its next turn, so spilled output is not lost — just not paid for in context tokens unless the agent decides it actually needs the tail. Tests: 4 new unit tests in `tools/truncate.rs` (no-op below threshold, no-op for errors, truncate + stamp above threshold, preserve prior metadata). 3 new tests in `tui/ui/tests.rs` for the pager helper (no-op without spillover_path, file-load happy path, graceful notice when the file is missing).
This commit is contained in:
Hunter Bown
+9
-8
@@ -67,14 +67,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
- **`deepseek doctor --json`** now surfaces a `memory` block
|
||||
(`enabled` / `path` / `file_present`) so operators can verify
|
||||
memory configuration without booting the TUI.
|
||||
- **Tool-output spillover** (#422 + #423) — large tool outputs spill
|
||||
to `~/.deepseek/tool_outputs/<id>.txt` (helpers in
|
||||
`crates/tui/src/tools/truncate.rs`, 7-day boot prune wired into
|
||||
`run_interactive`). Tool cells with a spilled path render an
|
||||
inline `full output: <path>` annotation in live mode so the user
|
||||
and the model can find the elided tail; transcript-mode replay
|
||||
omits the annotation because the full output is inline. #500
|
||||
(preview pane) is now unblocked and just needs the pager glue.
|
||||
- **Tool-output spillover** (#422 + #423 + #500) — tool outputs over
|
||||
100 KiB now spill to `~/.deepseek/tool_outputs/<id>.txt` from the
|
||||
engine's tool-execution path. The model receives a 32 KiB head plus
|
||||
a footer pointing at the spillover file (`Use read_file path=…`),
|
||||
the tool cell renders an inline `full output: <path>` annotation in
|
||||
live mode, and a 7-day boot prune keeps the directory bounded.
|
||||
Spillover is skipped on error results so the model still sees the
|
||||
failure message verbatim. The existing tool-details pager surfaces
|
||||
the truncated head so the user can verify what the model saw.
|
||||
|
||||
### Changed
|
||||
- **Sub-agent concurrency cap raised to 10 by default** (#509) —
|
||||
|
||||
@@ -1078,7 +1078,7 @@ impl Engine {
|
||||
let started_at = Instant::now();
|
||||
|
||||
tool_tasks.push(async move {
|
||||
let result = Engine::execute_tool_with_lock(
|
||||
let mut result = Engine::execute_tool_with_lock(
|
||||
lock,
|
||||
plan.supports_parallel,
|
||||
plan.interactive,
|
||||
@@ -1091,6 +1091,12 @@ impl Engine {
|
||||
)
|
||||
.await;
|
||||
|
||||
// #500: spill outsized output before fanout (mirror
|
||||
// of the sequential path below).
|
||||
if let Ok(tool_result) = result.as_mut() {
|
||||
crate::tools::truncate::apply_spillover(tool_result, &plan.id);
|
||||
}
|
||||
|
||||
let _ = tx_event
|
||||
.send(Event::ToolCallComplete {
|
||||
id: plan.id.clone(),
|
||||
@@ -1347,7 +1353,7 @@ impl Engine {
|
||||
}
|
||||
|
||||
let started_at = Instant::now();
|
||||
let result = if let Some(result_override) = result_override {
|
||||
let mut result = if let Some(result_override) = result_override {
|
||||
result_override
|
||||
} else {
|
||||
Self::execute_tool_with_lock(
|
||||
@@ -1364,6 +1370,14 @@ impl Engine {
|
||||
.await
|
||||
};
|
||||
|
||||
// #500: spill outsized tool outputs to disk before the
|
||||
// result fans out to the model context and the UI cell.
|
||||
// Both consumers see the same truncated content + the
|
||||
// `spillover_path` metadata pointing at the full file.
|
||||
if let Ok(tool_result) = result.as_mut() {
|
||||
crate::tools::truncate::apply_spillover(tool_result, &tool_id);
|
||||
}
|
||||
|
||||
let _ = self
|
||||
.tx_event
|
||||
.send(Event::ToolCallComplete {
|
||||
|
||||
@@ -19,27 +19,28 @@
|
||||
//! (7 days). Prune failures are logged and never fatal — the user
|
||||
//! shouldn't see startup wedge because of a stale tool-output file.
|
||||
//!
|
||||
//! ## What's NOT here
|
||||
//! ## Live callers
|
||||
//!
|
||||
//! Wiring `maybe_spillover` into the actual tool-execution path is
|
||||
//! tracked by **#423** (UI annotation) and **#500** (preview pane);
|
||||
//! both want the spillover bytes to exist. This module ships the
|
||||
//! plumbing so those follow-ups land cleanly without re-litigating
|
||||
//! the storage decisions.
|
||||
//! * [`apply_spillover`] — invoked from the engine's tool-execution
|
||||
//! path (`turn_loop.rs`) so any successful tool result over
|
||||
//! [`SPILLOVER_THRESHOLD_BYTES`] spills to disk and the model
|
||||
//! receives a [`SPILLOVER_HEAD_BYTES`] head plus a pointer footer.
|
||||
//! * Boot prune in `main.rs` deletes files older than
|
||||
//! [`SPILLOVER_MAX_AGE`].
|
||||
//!
|
||||
//! Today the only live caller is the boot prune in `main.rs`. The
|
||||
//! storage helpers (`write_spillover`, `maybe_spillover`,
|
||||
//! `spillover_path`) are unused outside of this module's own tests
|
||||
//! and the `#[allow(dead_code)]` markers below mark them deferred —
|
||||
//! they get callers when #423 / #500 land.
|
||||
|
||||
#![allow(dead_code)] // storage surface used by #423/#500 follow-ups; tests pin the contract
|
||||
//! UI-side rendering of the inline `full output: <path>` annotation
|
||||
//! is owned by `tui/history.rs::render_spillover_annotation`. The
|
||||
//! tool-details pager opens the spillover file when the user
|
||||
//! presses `Alt+V` (or plain `v` with empty composer) on a spilled
|
||||
//! tool cell.
|
||||
|
||||
use std::fs;
|
||||
use std::io;
|
||||
use std::path::PathBuf;
|
||||
use std::time::{Duration, SystemTime};
|
||||
|
||||
use crate::tools::spec::ToolResult;
|
||||
|
||||
// `Path` is only referenced from helpers gated to test builds.
|
||||
#[cfg(test)]
|
||||
use std::path::Path;
|
||||
@@ -174,6 +175,88 @@ pub fn maybe_spillover(
|
||||
Ok(Some((content[..cut].to_string(), path)))
|
||||
}
|
||||
|
||||
/// Inline head retained when [`apply_spillover`] truncates a tool
|
||||
/// result. 32 KiB is large enough for the model to keep meaningful
|
||||
/// context (a long stack trace, a `git diff` head, a directory
|
||||
/// listing of typical depth) without consuming the lion's share of
|
||||
/// the per-turn context budget. The full output is preserved on
|
||||
/// disk; the model can `read_file` it back if it needs the tail.
|
||||
pub const SPILLOVER_HEAD_BYTES: usize = 32 * 1024;
|
||||
|
||||
/// Apply spillover to a tool result in place. If the result's
|
||||
/// content exceeds [`SPILLOVER_THRESHOLD_BYTES`], writes the full
|
||||
/// content to a sibling file under `~/.deepseek/tool_outputs/`,
|
||||
/// replaces `result.content` with a [`SPILLOVER_HEAD_BYTES`] head
|
||||
/// plus a footer pointing the model at the spillover file, and
|
||||
/// stamps `metadata.spillover_path` so the UI can render its
|
||||
/// "full output: …" annotation.
|
||||
///
|
||||
/// Returns the spillover path on success, `None` if no spillover
|
||||
/// happened (content small enough, error result, write failure).
|
||||
/// Failures are logged but never bubble up — a tool that produced a
|
||||
/// result shouldn't be marked failed because the spillover writer
|
||||
/// couldn't reach disk; we degrade to no-op and the model gets the
|
||||
/// original (large) content.
|
||||
///
|
||||
/// Error results (`success == false`) are skipped: error messages
|
||||
/// are typically short, and turning them into a "see file" pointer
|
||||
/// would just hide the error from the model's reasoning.
|
||||
pub fn apply_spillover(result: &mut ToolResult, tool_id: &str) -> Option<PathBuf> {
|
||||
if !result.success {
|
||||
return None;
|
||||
}
|
||||
if result.content.len() <= SPILLOVER_THRESHOLD_BYTES {
|
||||
return None;
|
||||
}
|
||||
let total = result.content.len();
|
||||
let outcome = match maybe_spillover(
|
||||
tool_id,
|
||||
&result.content,
|
||||
SPILLOVER_THRESHOLD_BYTES,
|
||||
SPILLOVER_HEAD_BYTES,
|
||||
) {
|
||||
Ok(Some(pair)) => pair,
|
||||
Ok(None) => return None,
|
||||
Err(err) => {
|
||||
tracing::warn!(
|
||||
target: "spillover",
|
||||
?err,
|
||||
tool_id,
|
||||
"spillover write failed; passing original content through"
|
||||
);
|
||||
return None;
|
||||
}
|
||||
};
|
||||
let (head, path) = outcome;
|
||||
let path_str = path.display().to_string();
|
||||
let footer = format!(
|
||||
"\n\n[Output truncated: {head_kib} KiB of {total_kib} KiB shown. \
|
||||
Full output saved to {path_str}. Use `read_file path={path_str}` \
|
||||
if you need the elided tail.]",
|
||||
head_kib = head.len() / 1024,
|
||||
total_kib = total / 1024,
|
||||
);
|
||||
result.content = format!("{head}{footer}");
|
||||
let metadata = result.metadata.get_or_insert_with(|| serde_json::json!({}));
|
||||
if let Some(obj) = metadata.as_object_mut() {
|
||||
obj.insert("spillover_path".into(), serde_json::Value::String(path_str));
|
||||
} else {
|
||||
// Pre-existing metadata that wasn't a JSON object (rare,
|
||||
// possibly an array). Replace with an object so we can
|
||||
// attach our key without losing prior data — wrap it under
|
||||
// a `_prior` field so callers that introspect can recover.
|
||||
let prior = std::mem::replace(metadata, serde_json::json!({}));
|
||||
if let Some(obj) = metadata.as_object_mut() {
|
||||
obj.insert("_prior".into(), prior);
|
||||
obj.insert(
|
||||
"spillover_path".into(),
|
||||
serde_json::Value::String(path.display().to_string()),
|
||||
);
|
||||
}
|
||||
}
|
||||
Some(path)
|
||||
}
|
||||
|
||||
/// Sanitise a tool call id for use as a filename. Keeps ASCII
|
||||
/// alphanumerics, `-`, and `_`; rejects `.` to keep `..` traversal
|
||||
/// out, rejects empty results. Returns `None` if the input contains
|
||||
@@ -385,4 +468,94 @@ mod tests {
|
||||
// Not exercised in CI on Windows; prune semantics are the same
|
||||
// and the per-cycle stress test lives on the Unix path.
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_spillover_is_noop_below_threshold() {
|
||||
let _g = setup();
|
||||
let tmp = tempdir().unwrap();
|
||||
with_test_home(tmp.path(), || {
|
||||
let mut result = ToolResult::success("small payload");
|
||||
let path = apply_spillover(&mut result, "call-small");
|
||||
assert!(path.is_none());
|
||||
assert_eq!(result.content, "small payload");
|
||||
assert!(result.metadata.is_none());
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_spillover_is_noop_for_error_results() {
|
||||
let _g = setup();
|
||||
let tmp = tempdir().unwrap();
|
||||
with_test_home(tmp.path(), || {
|
||||
// Even very large error messages are passed through —
|
||||
// truncating an error would hide it from the model.
|
||||
let big_err = "boom\n".repeat(50_000);
|
||||
let mut result = ToolResult::error(big_err.clone());
|
||||
let path = apply_spillover(&mut result, "call-err");
|
||||
assert!(path.is_none());
|
||||
assert_eq!(result.content, big_err);
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_spillover_truncates_and_stamps_metadata_above_threshold() {
|
||||
let _g = setup();
|
||||
let tmp = tempdir().unwrap();
|
||||
with_test_home(tmp.path(), || {
|
||||
// 200 KiB body — well above the 100 KiB threshold.
|
||||
let big = "X".repeat(200 * 1024);
|
||||
let mut result = ToolResult::success(big.clone());
|
||||
let path = apply_spillover(&mut result, "call-big").expect("should spill");
|
||||
|
||||
// Inline content shrunk to head + footer.
|
||||
assert!(result.content.len() < big.len());
|
||||
assert!(
|
||||
result.content.contains("Output truncated:"),
|
||||
"footer missing: {}",
|
||||
&result.content[result.content.len().saturating_sub(200)..]
|
||||
);
|
||||
assert!(result.content.contains("read_file path="));
|
||||
|
||||
// Full bytes are on disk at the returned path.
|
||||
assert!(path.exists(), "spillover file missing: {path:?}");
|
||||
let body = fs::read_to_string(&path).unwrap();
|
||||
assert_eq!(body.len(), 200 * 1024);
|
||||
|
||||
// metadata.spillover_path stamped for the UI to find.
|
||||
let metadata = result.metadata.expect("metadata stamped");
|
||||
let stamped = metadata
|
||||
.get("spillover_path")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.expect("spillover_path key present");
|
||||
assert_eq!(stamped, path.display().to_string());
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_spillover_preserves_existing_metadata() {
|
||||
let _g = setup();
|
||||
let tmp = tempdir().unwrap();
|
||||
with_test_home(tmp.path(), || {
|
||||
let big = "Y".repeat(200 * 1024);
|
||||
let mut result = ToolResult::success(big)
|
||||
.with_metadata(serde_json::json!({"prior_key": "prior_value"}));
|
||||
let path = apply_spillover(&mut result, "call-meta").expect("should spill");
|
||||
|
||||
let metadata = result.metadata.expect("metadata present");
|
||||
// Prior keys survive.
|
||||
assert_eq!(
|
||||
metadata
|
||||
.get("prior_key")
|
||||
.and_then(serde_json::Value::as_str),
|
||||
Some("prior_value")
|
||||
);
|
||||
// New key added alongside.
|
||||
assert_eq!(
|
||||
metadata
|
||||
.get("spillover_path")
|
||||
.and_then(serde_json::Value::as_str),
|
||||
Some(path.display().to_string().as_str())
|
||||
);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6655,6 +6655,33 @@ fn open_tool_details_pager(app: &mut App) -> bool {
|
||||
open_details_pager_for_cell(app, cell_index)
|
||||
}
|
||||
|
||||
/// Build the trailing "Spillover" section for the tool-details pager
|
||||
/// (#500). Returns `None` when the cell at `cell_index` is not a
|
||||
/// `GenericToolCell` with a recorded spillover path, or when the
|
||||
/// spillover file is missing or unreadable. Failures fall back to a
|
||||
/// short notice in the section so the user understands why the full
|
||||
/// content can't be loaded — better than silent truncation.
|
||||
fn spillover_pager_section(app: &App, cell_index: usize) -> Option<String> {
|
||||
use crate::tui::history::{GenericToolCell, HistoryCell, ToolCell};
|
||||
|
||||
let cell = app.cell_at_virtual_index(cell_index)?;
|
||||
let HistoryCell::Tool(ToolCell::Generic(GenericToolCell {
|
||||
spillover_path: Some(path),
|
||||
..
|
||||
})) = cell
|
||||
else {
|
||||
return None;
|
||||
};
|
||||
let path_str = path.display().to_string();
|
||||
let body = match std::fs::read_to_string(path) {
|
||||
Ok(text) => text,
|
||||
Err(err) => format!("(could not read spillover file: {err})"),
|
||||
};
|
||||
Some(format!(
|
||||
"── Full output (spillover) ──\nFile: {path_str}\n\n{body}"
|
||||
))
|
||||
}
|
||||
|
||||
fn open_details_pager_for_cell(app: &mut App, cell_index: usize) -> bool {
|
||||
if let Some(detail) = app.tool_detail_record_for_cell(cell_index) {
|
||||
let input = serde_json::to_string_pretty(&detail.input)
|
||||
@@ -6663,10 +6690,25 @@ fn open_details_pager_for_cell(app: &mut App, cell_index: usize) -> bool {
|
||||
"(not available)".to_string(),
|
||||
std::string::ToString::to_string,
|
||||
);
|
||||
let content = format!(
|
||||
"Tool ID: {}\nTool: {}\n\nInput:\n{}\n\nOutput:\n{}",
|
||||
detail.tool_id, detail.tool_name, input, output
|
||||
);
|
||||
|
||||
// #500: when the tool result was spilled to disk, fold the full
|
||||
// file content into the pager body so the user can see what was
|
||||
// elided (the model only ever saw the head). The truncated head
|
||||
// stays above as `Output:` so the user can compare what the
|
||||
// model received against the full payload.
|
||||
let spillover_section = spillover_pager_section(app, cell_index);
|
||||
|
||||
let content = if let Some(section) = spillover_section {
|
||||
format!(
|
||||
"Tool ID: {}\nTool: {}\n\nInput:\n{}\n\nOutput:\n{}\n\n{}",
|
||||
detail.tool_id, detail.tool_name, input, output, section
|
||||
)
|
||||
} else {
|
||||
format!(
|
||||
"Tool ID: {}\nTool: {}\n\nInput:\n{}\n\nOutput:\n{}",
|
||||
detail.tool_id, detail.tool_name, input, output
|
||||
)
|
||||
};
|
||||
|
||||
let width = app
|
||||
.viewport
|
||||
|
||||
@@ -1664,6 +1664,67 @@ fn open_tool_details_pager_supports_active_virtual_tool_cell() {
|
||||
assert_eq!(app.view_stack.top_kind(), Some(ModalKind::Pager));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn spillover_pager_section_returns_none_when_no_spillover() {
|
||||
let mut app = create_test_app();
|
||||
app.history = vec![HistoryCell::Tool(ToolCell::Generic(GenericToolCell {
|
||||
name: "exec_shell".to_string(),
|
||||
status: ToolStatus::Success,
|
||||
input_summary: None,
|
||||
output: Some("hi".to_string()),
|
||||
prompts: None,
|
||||
spillover_path: None,
|
||||
}))];
|
||||
app.resync_history_revisions();
|
||||
assert!(spillover_pager_section(&app, 0).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn spillover_pager_section_loads_file_when_present() {
|
||||
use std::io::Write;
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path().join("call-test.txt");
|
||||
let mut f = std::fs::File::create(&path).unwrap();
|
||||
writeln!(f, "FULL_OUTPUT_BYTES_HERE").unwrap();
|
||||
|
||||
let mut app = create_test_app();
|
||||
app.history = vec![HistoryCell::Tool(ToolCell::Generic(GenericToolCell {
|
||||
name: "exec_shell".to_string(),
|
||||
status: ToolStatus::Success,
|
||||
input_summary: None,
|
||||
output: Some("(truncated head)".to_string()),
|
||||
prompts: None,
|
||||
spillover_path: Some(path.clone()),
|
||||
}))];
|
||||
app.resync_history_revisions();
|
||||
|
||||
let section = spillover_pager_section(&app, 0).expect("section present");
|
||||
assert!(section.contains("Full output (spillover)"));
|
||||
assert!(
|
||||
section.contains("FULL_OUTPUT_BYTES_HERE"),
|
||||
"section missing file body: {section}"
|
||||
);
|
||||
assert!(section.contains(&path.display().to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn spillover_pager_section_returns_notice_when_file_missing() {
|
||||
let mut app = create_test_app();
|
||||
let bogus = std::path::PathBuf::from("/tmp/this/path/does/not/exist-spill.txt");
|
||||
app.history = vec![HistoryCell::Tool(ToolCell::Generic(GenericToolCell {
|
||||
name: "exec_shell".to_string(),
|
||||
status: ToolStatus::Success,
|
||||
input_summary: None,
|
||||
output: Some("(truncated head)".to_string()),
|
||||
prompts: None,
|
||||
spillover_path: Some(bogus),
|
||||
}))];
|
||||
app.resync_history_revisions();
|
||||
|
||||
let section = spillover_pager_section(&app, 0).expect("still emits a notice section");
|
||||
assert!(section.contains("could not read spillover file"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn details_shortcut_modifiers_accept_plain_shift_and_alt_only() {
|
||||
assert!(details_shortcut_modifiers(KeyModifiers::NONE));
|
||||
|
||||
Reference in New Issue
Block a user