Preserve atomic tool history during compaction - #643
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| text.push('\n'); | ||
| } | ||
| match item { | ||
| ToolResultContent::Text(value) => text.push_str(&value.text), |
There was a problem hiding this comment.
This still copies the full malformed result into text before the later truncation runs, so a huge orphaned tool output can spike memory during recovery. Append only the remaining bounded characters here.
| ToolResultContent::Text(value) => text.push_str(&value.text), | |
| ToolResultContent::Text(value) => { | |
| let remaining = MAX_UNTRUSTED_RESULT_CHARS.saturating_sub(text.chars().count()); | |
| text.extend(value.text.chars().take(remaining)); | |
| } |
Ports the recovery half of PR spacedriveapp#643 onto main. Main already had the prevention half — aligned cuts at every trim site and the send-boundary pairing pass — so the cut selection and its telemetry are not carried over; spacedriveapp#643's atomic_history_cut and advance_past_stranded_tool_results solve the same problem and main's already holds the caller's floor. The send-boundary pass dropped an unpairable result outright. The output it carried is usually the most expensive thing in the history, so it is now rewritten as bounded, delimited plain text instead. The delimiters mark it as historical tool data so a shell transcript cannot read as an instruction once it stops being a protocol message. Results that are stale or duplicated are treated the same way, since providers reject those as firmly as an orphan. Because every message survives the pass, a history of nothing but orphans no longer repairs to zero messages, and the error path for that case is gone. A mismatch that survives the pre-send pass is the other half of the protocol: an assistant call nothing answers, which no result-side repair can reach and which Anthropic rejects. is_tool_history_mismatch_error recognises those 400s across providers, and completion drops unanswered non-trailing calls and retries exactly once, only when the history actually changed. A trailing call is a loop still in flight and is left alone. Routing and the fallback chain moved into dispatch_completion so both attempts share one path. validate_tool_history reports the first violation a provider would reject. spacedriveapp#643 wired an equivalent into the compaction paths with expect(), which panics on a history whose calls are simply still awaiting results — the normal mid-loop shape. It is available for observability instead. Covers the failure that took down a live worker: a fork's cut removed the assistant turn holding a read_skill call while its result stayed at the head of the retained history.
Summary
Root cause
Raw message-count truncation could cut between an assistant tool-call batch and one of its delayed result messages. That left a retained
function_call_output/ tool result whose call had been removed. Provider 400 handling then retried the unchanged deterministic request.The regression fixture reproduces the observed
039643a1orphan-result shape.Validation
cargo test --lib(1210 passed)cargo check --features metricscargo clippy --lib --all-features -- -D warningscargo fmt --all -- --checkgit diff --check