diff --git a/CHANGELOG.md b/CHANGELOG.md index 73c0878d..86d0d4ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ loosely while pre-1.0 (breaking changes can land on minor bumps). ## [Unreleased] +- **Stale tool results stay out of the next prompt.** The model view + keeps the newest tool batch in full and replaces older bulky envelopes + with the writ line, an `ok` / `ERR:` status, and any presence note or + loop warning. Short results (under 4 KB) stay. Stored history and + replay are unchanged. Compaction also recognizes live `[END TOOL RESULTS]` + envelopes when choosing the kept tail, and the summarize call uses the + same digest so it does not re-send every file body. Disable with + `ARBITER_TOOL_ELIDE_DISABLED=1`. See [Sessions](docs/tui/sessions.md). + ## [0.13.11] — 2026-09-21 - **Fleet dashboard pane (Phase C1).** The TUI consumes fleet SSE (`stream_id` + diff --git a/docs/cli/environment.md b/docs/cli/environment.md index 9a07b760..b70f0bf5 100644 --- a/docs/cli/environment.md +++ b/docs/cli/environment.md @@ -45,6 +45,7 @@ See [`docs/cli/connect.md`](connect.md). Local provider keys (`OPENROUTER_API_KE | `ARBITER_AUTOSAVE_INTERVAL_SEC` | Periodic dirty flush for conversation files. `0` disables the timer (post-turn and mid-turn `save_async` still run). | `30` | | `ARBITER_COMPACT_THRESHOLD` | Integer percent (1–100) of the model context window that triggers auto-compaction. | `75` | | `ARBITER_COMPACT_DISABLED` | When set to a non-empty, non-`0` value, disables automatic compaction. `/compact` still works. | unset | +| `ARBITER_TOOL_ELIDE_DISABLED` | When set to a non-empty, non-`0` value, the model view keeps every earlier tool-result body. By default only the newest batch is sent in full; older bulky bodies become a writ/status digest. | unset | See [`docs/tui/sessions.md`](../tui/sessions.md). diff --git a/docs/tui/sessions.md b/docs/tui/sessions.md index 5b4bff35..d9f01db1 100644 --- a/docs/tui/sessions.md +++ b/docs/tui/sessions.md @@ -93,7 +93,19 @@ to force it. `/reset` clears both history and compaction state for that agent. The summary call uses `constitution.advisor.model` when set, otherwise the executor model. Failures are fail-open: the turn proceeds with the uncompacted -view and a warning is logged. +view and a warning is logged. The summarize prompt itself drops bulky tool +bodies (same digest as below) so that call is not a second copy of every file +the agent already read. + +Before each model request, tool-result envelopes older than the newest batch +are replaced in that view with a short digest: the writ line (`[/read path]`, +`/exec`, …), `ok, N bytes omitted` or the first `ERR:` line, plus any +presence note or loop warning that was attached. The newest tool batch is +sent in full, so the model still has the evidence for the turn it is about +to take. File bodies under 4 KB stay as well — a short search hit is the +signal, not the cost. Image bytes on an older tool turn are dropped with a +one-line stand-in. Replay and session JSON keep the original envelopes. +Set `ARBITER_TOOL_ELIDE_DISABLED=1` to send every body. ## Sessions vs the structured memory graph diff --git a/include/context_compaction.h b/include/context_compaction.h index 443b9028..fa6e768a 100644 --- a/include/context_compaction.h +++ b/include/context_compaction.h @@ -48,6 +48,24 @@ CompactionConfig compaction_config_from_env(); [[nodiscard]] bool is_tool_results_message(const Message& m); +// Newest tool-result messages left verbatim in the model view. Older bulky +// envelopes become a writ/status digest. The live view keeps one so the +// model still sees the batch it is about to act on. +inline constexpr int kKeepRecentToolResults = 1; + +// Short search hits and errors stay whole. File reads, maps, and long +// command output are what blow the prompt up on the next turns. +inline constexpr size_t kToolElideMinBytes = 4096; + +// ARBITER_TOOL_ELIDE_DISABLED set to anything other than empty or `0` +// keeps every tool body in the model view and in the compaction prompt. +[[nodiscard]] bool tool_result_elision_enabled(); + +// Digest tool-result messages except the last `keep_recent` ones. +// Non-tool messages are left alone. Image parts on a digested message +// are dropped. Does not read the env flag — callers decide. +void elide_stale_tool_results(std::vector& messages, int keep_recent); + // Cut index so histories_[cut …] is the recent window (keep last N), snapped // so the kept tail starts on a real user turn when possible. [[nodiscard]] size_t compute_cut_index(const std::vector& history, diff --git a/src/context_compaction.cpp b/src/context_compaction.cpp index 3ad0a7d5..3ae390b5 100644 --- a/src/context_compaction.cpp +++ b/src/context_compaction.cpp @@ -29,9 +29,284 @@ CompactionConfig compaction_config_from_env() { bool is_tool_results_message(const Message& m) { if (m.role != "user") return false; + // Legacy fixtures and a few tests use this prefix. Live envelopes do + // not: execute_agent_commands starts at the writ block and only closes + // with the end marker, often after a presence or loop-warning prefix. static constexpr std::string_view kPrefix = "[TOOL RESULTS]"; - return m.content.size() >= kPrefix.size() && - m.content.compare(0, kPrefix.size(), kPrefix) == 0; + if (m.content.size() >= kPrefix.size() && + m.content.compare(0, kPrefix.size(), kPrefix) == 0) + return true; + // Closing marker alone is not enough: a person can mention it. Live + // envelopes also contain a writ header (`[/read …]`, `/exec`, …). + if (m.content.find("[END TOOL RESULTS]") == std::string::npos) return false; + if (m.content.find("\n[/") != std::string::npos) return true; + return m.content.size() >= 2 && m.content[0] == '[' && m.content[1] == '/'; +} + +namespace { + +constexpr std::string_view kToolPrefix = "[TOOL RESULTS]"; +constexpr std::string_view kEndTool = "[END TOOL RESULTS]"; + +bool line_starts_with(std::string_view s, size_t i, std::string_view pfx) { + return i + pfx.size() <= s.size() && + s.compare(i, pfx.size(), pfx) == 0; +} + +size_t next_line(std::string_view s, size_t i) { + const auto n = s.find('\n', i); + return n == std::string_view::npos ? s.size() : n + 1; +} + +bool is_ws_only(std::string_view s) { + for (char c : s) { + if (c != ' ' && c != '\t' && c != '\n' && c != '\r') return false; + } + return true; +} + +bool at_line_start(std::string_view s, size_t i) { + return i == 0 || s[i - 1] == '\n'; +} + +void append_marked_block(std::string& out, std::string_view src, + std::string_view begin, std::string_view end) { + size_t i = 0; + while (i < src.size()) { + const auto at = src.find(begin, i); + if (at == std::string_view::npos) return; + auto e = src.find(end, at + begin.size()); + if (e == std::string_view::npos) return; + e += end.size(); + while (e < src.size() && (src[e] == '\n' || src[e] == '\r')) ++e; + if (!out.empty() && out.back() != '\n') out.push_back('\n'); + out.append(src.data() + at, e - at); + i = e; + } +} + +void append_banner_lines(std::string& out, std::string_view src) { + for (size_t i = 0; i < src.size(); i = next_line(src, i)) { + if (!at_line_start(src, i)) continue; + if (!line_starts_with(src, i, "[TOOL RESULTS TRUNCATED") && + !line_starts_with(src, i, "[TOOL RESULTS CANCELLED")) + continue; + const auto eol = src.find('\n', i); + const size_t end = eol == std::string_view::npos ? src.size() : eol; + if (!out.empty() && out.back() != '\n') out.push_back('\n'); + out.append(src.data() + i, end - i); + out.push_back('\n'); + } +} + +size_t tool_payload_bytes(const Message& m) { + size_t n = m.content.size(); + for (const auto& p : m.parts) { + if (p.kind == ContentPart::IMAGE) + n += p.image_data.size() + p.image_url.size(); + } + return n; +} + +bool tool_message_has_image(const Message& m) { + for (const auto& p : m.parts) { + if (p.kind == ContentPart::IMAGE && + (!p.image_data.empty() || !p.image_url.empty())) + return true; + } + return false; +} + +std::string clip_chars(std::string_view s, size_t max) { + while (!s.empty() && + (s.back() == '\r' || s.back() == ' ' || s.back() == '\t')) + s.remove_suffix(1); + if (s.size() <= max) return std::string(s); + std::string out(s.substr(0, max)); + out += "…"; + return out; +} + +std::string first_err_line(std::string_view body) { + // Runtime failures are their own line (`ERR: …`). A match inside a + // source line (`return ERR:`) is file text, not a failed writ. + size_t i = 0; + while (i < body.size()) { + const auto eol = body.find('\n', i); + const size_t end = eol == std::string_view::npos ? body.size() : eol; + std::string_view line = body.substr(i, end - i); + while (!line.empty() && + (line.front() == ' ' || line.front() == '\t' || line.front() == '\r')) + line.remove_prefix(1); + if (line.size() >= 4 && line.compare(0, 4, "ERR:") == 0) + return clip_chars(line, 200); + i = end < body.size() ? end + 1 : body.size(); + } + return {}; +} + +size_t trimmed_body_bytes(std::string_view body) { + while (!body.empty() && (body.back() == '\n' || body.back() == '\r')) + body.remove_suffix(1); + return body.size(); +} + +bool is_writ_boundary(std::string_view s, size_t i) { + return line_starts_with(s, i, "[/") || + line_starts_with(s, i, "[END ") || + line_starts_with(s, i, "[TOOL RESULTS") || + line_starts_with(s, i, kEndTool); +} + +// Decision-bearing prefix (presence note, loop warning). File bodies that +// happen to sit before the first writ are not copied. +std::string keep_tool_prefix(std::string_view prefix) { + std::string out; + if (prefix.size() <= 4096) { + size_t i = 0; + while (i < prefix.size()) { + const auto eol = prefix.find('\n', i); + const size_t end = + eol == std::string_view::npos ? prefix.size() : eol; + std::string_view line = prefix.substr(i, end - i); + if (!line.empty() && line.back() == '\r') line.remove_suffix(1); + const bool plain_header = + line == kToolPrefix || + (line.size() > kToolPrefix.size() && + line.compare(0, kToolPrefix.size(), kToolPrefix) == 0 && + line.find("TRUNCATED") == std::string_view::npos && + line.find("CANCELLED") == std::string_view::npos && + line.find("omitted") == std::string_view::npos); + if (!plain_header) { + out.append(line); + out.push_back('\n'); + } + i = end < prefix.size() ? end + 1 : prefix.size(); + } + if (is_ws_only(out)) return {}; + return out; + } + append_marked_block(out, prefix, "[PRESENCE:", "[END PRESENCE]"); + append_marked_block(out, prefix, "[LOOP DETECTED]", "[END LOOP DETECTED]"); + append_banner_lines(out, prefix); + return out; +} + +std::string digest_tool_content(const Message& m) { + const std::string_view content = m.content; + size_t first_cmd = content.size(); + for (size_t i = 0; i < content.size(); i = next_line(content, i)) { + if (at_line_start(content, i) && line_starts_with(content, i, "[/")) { + first_cmd = i; + break; + } + } + + std::string out = keep_tool_prefix(content.substr(0, first_cmd)); + if (!out.empty() && out.back() != '\n') out.push_back('\n'); + out += "[TOOL RESULTS — earlier batch, bodies omitted]\n"; + + for (size_t i = first_cmd; i < content.size();) { + if (content[i] == '\n' || content[i] == '\r') { + ++i; + continue; + } + if (line_starts_with(content, i, kEndTool)) break; + if (line_starts_with(content, i, "[TOOL RESULTS TRUNCATED") || + line_starts_with(content, i, "[TOOL RESULTS CANCELLED")) { + const auto eol = content.find('\n', i); + const size_t end = + eol == std::string_view::npos ? content.size() : eol; + out.append(content.data() + i, end - i); + out.push_back('\n'); + i = end < content.size() ? end + 1 : content.size(); + continue; + } + if (line_starts_with(content, i, kToolPrefix)) { + i = next_line(content, i); + continue; + } + if (!line_starts_with(content, i, "[/")) { + i = next_line(content, i); + continue; + } + + const auto eol = content.find('\n', i); + const size_t header_end = + eol == std::string_view::npos ? content.size() : eol; + const std::string header = + clip_chars(content.substr(i, header_end - i), 240); + const size_t body_begin = + header_end < content.size() ? header_end + 1 : content.size(); + size_t body_end = content.size(); + for (size_t scan = body_begin; scan < content.size(); + scan = next_line(content, scan)) { + if (at_line_start(content, scan) && is_writ_boundary(content, scan)) { + body_end = scan; + break; + } + } + const std::string err = + first_err_line(content.substr(body_begin, body_end - body_begin)); + out += header; + if (!err.empty()) { + out.push_back(' '); + out += err; + } else { + out += " ok, "; + out += std::to_string(trimmed_body_bytes( + content.substr(body_begin, body_end - body_begin))); + out += " bytes omitted"; + } + out.push_back('\n'); + i = body_end; + if (i < content.size() && line_starts_with(content, i, "[END ") && + !line_starts_with(content, i, kEndTool)) { + i = next_line(content, i); + } + } + + for (const auto& p : m.parts) { + if (p.kind != ContentPart::IMAGE) continue; + if (p.image_data.empty() && p.image_url.empty()) continue; + out += "[image omitted — "; + out += p.media_type.empty() ? "image" : p.media_type; + out += ", "; + if (!p.image_url.empty() && p.image_data.empty()) + out += "url"; + else { + out += std::to_string(p.image_data.size()); + out += " bytes"; + } + out += "]\n"; + } + out += "[END TOOL RESULTS]"; + return out; +} + +} // namespace + +bool tool_result_elision_enabled() { + const char* env = std::getenv("ARBITER_TOOL_ELIDE_DISABLED"); + if (!env || !*env || *env == '0') return true; + return false; +} + +void elide_stale_tool_results(std::vector& messages, int keep_recent) { + if (keep_recent < 0) keep_recent = 0; + int trailing = 0; + for (size_t idx = messages.size(); idx-- > 0;) { + Message& m = messages[idx]; + if (!is_tool_results_message(m)) continue; + if (trailing < keep_recent) { + ++trailing; + continue; + } + const bool image = tool_message_has_image(m); + if (!image && tool_payload_bytes(m) < kToolElideMinBytes) continue; + m.content = digest_tool_content(m); + m.parts.clear(); + } } size_t compute_cut_index(const std::vector& history, @@ -79,12 +354,22 @@ build_model_messages(const std::vector& history, const size_t start = std::min(s.covered_until, history.size()); for (size_t i = start; i < history.size(); ++i) out.push_back(history[i]); + // Stored history stays intact for replay. Only this view drops bodies + // the model has already acted on. + if (tool_result_elision_enabled()) + elide_stale_tool_results(out, kKeepRecentToolResults); return out; } size_t model_view_char_count(const std::vector& msgs) { size_t n = 0; - for (const auto& m : msgs) n += m.content.size(); + for (const auto& m : msgs) { + n += m.content.size(); + for (const auto& p : m.parts) { + if (p.kind == ContentPart::IMAGE) + n += p.image_data.size() + p.image_url.size(); + } + } return n; } @@ -294,7 +579,11 @@ std::string summarize_history_slice( " - constraints, requirements, and user preferences\n" " - unresolved questions and next steps\n" "Do not invent facts. Prefer concrete names over vague " - "restatement. Output plain prose (no markdown fences).\n\n"; + "restatement. Tool-result bodies from earlier batches may " + "already be omitted down to the writ line and an ok/ERR " + "status — do not reconstruct file contents that are not " + "written out in the assistant turns or those status lines. " + "Output plain prose (no markdown fences).\n\n"; if (!prior_summary.empty()) { body << "[PRIOR SUMMARY]\n" << prior_summary @@ -305,8 +594,15 @@ std::string summarize_history_slice( << "\n[END PINNED FACTS]\n\n"; } + std::vector slice = older_slice; + // The slice is already outside the live window, so every bulky tool + // body can be digested. Assistant turns stay verbatim — that is where + // the decisions are. + if (tool_result_elision_enabled()) + elide_stale_tool_results(slice, 0); + body << "[MESSAGES TO SUMMARIZE]\n"; - for (const auto& m : older_slice) { + for (const auto& m : slice) { body << m.role << ": " << m.content << "\n---\n"; } body << "[END MESSAGES]\n"; diff --git a/tests/test_context_compaction.cpp b/tests/test_context_compaction.cpp index d10bcb4c..d352411a 100644 --- a/tests/test_context_compaction.cpp +++ b/tests/test_context_compaction.cpp @@ -5,6 +5,7 @@ #include "json.h" #include "model_context.h" +#include #include #include @@ -420,10 +421,145 @@ TEST_CASE("sanitize_compaction_state clears covered_until without summary") { CHECK(st.generation == 0); } -TEST_CASE("is_tool_results_message detects prefix") { +TEST_CASE("is_tool_results_message detects prefix and live envelopes") { CHECK(is_tool_results_message(Message{"user", "[TOOL RESULTS]\nok"})); CHECK_FALSE(is_tool_results_message(Message{"user", "hello"})); CHECK_FALSE(is_tool_results_message(Message{"assistant", "[TOOL RESULTS]"})); + CHECK(is_tool_results_message(Message{ + "user", "\n[/read src/a.cpp]\nbody\n[END READ]\n[END TOOL RESULTS]"})); + CHECK_FALSE(is_tool_results_message(Message{ + "user", "the closing marker is [END TOOL RESULTS], nothing else"})); + CHECK_FALSE(is_tool_results_message(Message{ + "assistant", "\n[/read x]\n[END TOOL RESULTS]"})); +} + +TEST_CASE("compute_cut_index skips a live tool envelope") { + const std::string envelope = + "\n[/read src/a.cpp]\nbody\n[END READ]\n[END TOOL RESULTS]"; + std::vector hist = { + {"user", "start"}, + {"assistant", "ok"}, + {"user", envelope}, + {"assistant", "done"}, + {"user", "next"}, + {"assistant", "y"}, + }; + CHECK(compute_cut_index(hist, 3) == 0); +} + +static std::string live_tool_envelope(const std::string& writs_and_tail) { + return "\n[PRESENCE: Jules]\nport is 8080\n[END PRESENCE]\n\n" + + writs_and_tail + "[END TOOL RESULTS]"; +} + +TEST_CASE("stale tool bodies are digested; the newest batch stays") { + const std::string bulk(5000, 'Q'); + const std::string old_tool = live_tool_envelope( + "[/read src/a.cpp]\nreturn ERR: not a failure\n" + bulk + + "\n[END READ]\n\n" + "[/exec npm test]\n" + bulk + "\nERR: exit status 1\n[END EXEC]\n\n" + "[TOOL RESULTS TRUNCATED: budget exhausted]\n\n"); + const std::string fresh = + "\n[/read src/b.cpp]\n" + bulk + "\n[END READ]\n[END TOOL RESULTS]"; + const std::string small = + "\n[/search arbiter]\nthree hits\n[END SEARCH]\n[END TOOL RESULTS]"; + + std::vector hist = { + {"user", "please read a"}, + {"assistant", "reading"}, + {"user", small}, + {"assistant", "search noted"}, + {"user", old_tool}, + {"assistant", "I changed src/a.cpp"}, + {"user", fresh}, + }; + const std::string old_copy = hist[4].content; + + auto view = build_model_messages(hist, {}); + REQUIRE(view.size() == hist.size()); + CHECK(hist[4].content == old_copy); + + CHECK(view[2].content == small); + CHECK(view[2].content.find("three hits") != std::string::npos); + + const std::string& digested = view[4].content; + CHECK(digested.find(bulk) == std::string::npos); + CHECK(digested.find("[PRESENCE: Jules]") != std::string::npos); + CHECK(digested.find("port is 8080") != std::string::npos); + CHECK(digested.find("bodies omitted") != std::string::npos); + CHECK(digested.find("not a failure") == std::string::npos); + CHECK(digested.find("[/read src/a.cpp] ok, 5026 bytes omitted") != + std::string::npos); + CHECK(digested.find("[/exec npm test] ERR: exit status 1") != + std::string::npos); + CHECK(digested.find("[TOOL RESULTS TRUNCATED: budget exhausted]") != + std::string::npos); + CHECK(digested.find("[END TOOL RESULTS]") != std::string::npos); + CHECK(digested.size() < 2000); + + CHECK(view[6].content == fresh); + CHECK(view[6].content.find(bulk) != std::string::npos); +} + +TEST_CASE("stale image tool results drop the bytes") { + Message oldm; + oldm.role = "user"; + oldm.content = "\n[/read shot.png]\n[read as image]\n[END READ]\n" + "[END TOOL RESULTS]"; + ContentPart img; + img.kind = ContentPart::IMAGE; + img.media_type = "image/png"; + img.image_data.assign(5000, 'A'); + oldm.parts.push_back(img); + + Message fresh = oldm; + fresh.content = "\n[/read other.png]\n[read as image]\n[END READ]\n" + "[END TOOL RESULTS]"; + + std::vector hist = { + {"user", "look"}, + {"assistant", "looking"}, + oldm, + {"assistant", "red button"}, + fresh, + }; + auto view = build_model_messages(hist, {}); + CHECK(view[2].parts.empty()); + CHECK(view[2].content.find("image/png") != std::string::npos); + CHECK(view[2].content.find("5000 bytes") != std::string::npos); + CHECK(view[2].content.find(std::string(5000, 'A')) == std::string::npos); + REQUIRE(view[4].parts.size() == 1); + CHECK(view[4].parts[0].image_data.size() == 5000); + CHECK(hist[2].parts.size() == 1); + CHECK(hist[2].parts[0].image_data.size() == 5000); +} + +TEST_CASE("tool elision can be disabled") { + const std::string bulk(5000, 'Q'); + const std::string old_tool = + "\n[/read src/a.cpp]\n" + bulk + "\n[END READ]\n[END TOOL RESULTS]"; + const std::string fresh = + "\n[/read src/b.cpp]\nok\n[END READ]\n[END TOOL RESULTS]"; + std::vector hist = { + {"user", old_tool}, + {"assistant", "done"}, + {"user", fresh}, + }; + setenv("ARBITER_TOOL_ELIDE_DISABLED", "1", 1); + auto view = build_model_messages(hist, {}); + unsetenv("ARBITER_TOOL_ELIDE_DISABLED"); + CHECK(view[0].content.find(bulk) != std::string::npos); +} + +TEST_CASE("model view counts image bytes") { + Message m; + m.role = "user"; + m.content = "hi"; + ContentPart img; + img.kind = ContentPart::IMAGE; + img.image_data.assign(100, 'B'); + m.parts.push_back(std::move(img)); + CHECK(model_view_char_count({m}) == 2 + 100); } TEST_CASE("context_window helpers match prior sidebar behavior") {