From a20a62302e13b28563992bf9bc756c70d03122f4 Mon Sep 17 00:00:00 2001 From: Noah Gift Date: Thu, 25 Jun 2026 21:18:42 +0200 Subject: [PATCH] =?UTF-8?q?fix(agent):=20preserve=20tool=5Fcall=20structur?= =?UTF-8?q?e=20across=20turns=20+=20Markdown->tool=5Fcall=20salvage=20pars?= =?UTF-8?q?er=20=E2=80=94=20stop=20the=20apr-code=20text-loop=20that=20def?= =?UTF-8?q?eats=20tool-calling=20(CCPA=20m296)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GROUNDED BUG (CCPA m296 distill feasibility spike): the apr-code agentic loop had a HARNESS bug independent of the model. Even a format-correct model (one that emits a valid ) reverted to 0/N tool_calls across a multi-turn run because a prior assistant TOOL_CALL turn could be retained / re-rendered as raw Markdown prose, re-priming prose mode — a self-reinforcing text loop. Until fixed, ANY fine-tune result is uninterpretable. WHAT WAS WRONG - runtime.rs (EndTurn branch) pushed `response.text` verbatim into multi-turn history as `Message::Assistant(...)`. When the driver's parser failed to recognize a model's tool-call shape (anything outside the exact / ```json envelope — a bare {"name","input"} object or a ```tool_call/```rust fence), that turn was scored as inert prose AND its raw tool-call Markdown was re-injected into the next turn's prompt, eroding tool-calling. THE FIX (two correctness surfaces) 1. SALVAGE PARSER (realizar.rs `salvage_tool_calls`): when the envelope parser finds nothing, conservatively recover a tool call from (a) a generically- fenced block (any language tag or none) whose body is tool-call JSON, or (b) a bare top-level {"name","input"} JSON object. Only objects with a string `name` AND an `input` field are salvaged — plain JSON / prose are never mistaken for tool calls. Salvage events are logged (id `salvage-N`). Applies to BOTH the embedded RealizarDriver and the apr-serve HTTP path (shared parser). This recovers the "model almost emitted a tool_call" near-misses. 2. STRUCTURED RETENTION (runtime.rs `retain_assistant_text`): an assistant turn's text is stripped of lingering / markup before it enters history, so a tool-using turn is NEVER re-rendered as capability- breaking raw Markdown that re-primes prose mode. Genuine prose passes through unchanged; the structured AssistantToolUse/ToolResult messages already carry that turn's tool semantics (chat_template.rs renders them as the canonical + envelope — no "### Continue:" prose nudge). FALSIFIERS (mutation-verified, oracle = the structured/expected render) - falsify_toolcall_retention_001: next-turn render of a prior tool-call turn preserves + and contains no "### Continue:" nudge. - falsify_toolcall_retention_002: history keeps structured tool messages, never raw markup as Assistant prose. - falsify_toolcall_retention_003: retain_assistant_text strips residue, keeps prose. - test_salvage_* (realizar): recover bare/fenced tool-call JSON; reject plain JSON / name-without-input; envelope still takes precedence. - Mutation verified: making retain_assistant_text identity turns 003 RED; reverting salvage to envelope-only turns the 3 salvage-recovery tests RED. CONTRACT: contracts/apr-code-toolcall-retention-v1.yaml (OBLIG-APR-CODE-TOOLCALL-RETENTION) — kind: kernel, 5 single-line cargo-test falsifier refs. `pv validate` + `pv lint contracts/` PASS (0 errors/0 warnings on this file). GREEN: cargo test -p aprender-orchestrate --lib (6514 pass; the lone failure is agent::tool::mcp_client::test_discover_tools_via_echo — a pre-existing subprocess/stdio flake unrelated to this change, 1 fail / 3 runs, different module). clippy -p aprender-orchestrate --all-targets clean (exit 0). fmt clean. Co-Authored-By: Claude Opus 4.8 --- contracts/apr-code-toolcall-retention-v1.yaml | 157 ++++++++++++++ .../src/agent/driver/realizar.rs | 202 ++++++++++++++++++ .../aprender-orchestrate/src/agent/runtime.rs | 61 +++++- .../src/agent/runtime_tests_multi_turn.rs | 166 ++++++++++++++ 4 files changed, 584 insertions(+), 2 deletions(-) create mode 100644 contracts/apr-code-toolcall-retention-v1.yaml diff --git a/contracts/apr-code-toolcall-retention-v1.yaml b/contracts/apr-code-toolcall-retention-v1.yaml new file mode 100644 index 000000000..a80d96110 --- /dev/null +++ b/contracts/apr-code-toolcall-retention-v1.yaml @@ -0,0 +1,157 @@ +metadata: + kind: kernel + version: 1.0.0 + created: '2026-06-25' + author: PAIML Engineering + description: > + apr-code tool-call retention (CCPA-m296). Pins the agentic-loop harness so a + format-correct model's tool-calling is RETAINED across multi-turn runs rather + than eroded to 0/N by a self-reinforcing text loop. Two correctness surfaces: + (1) a prior assistant TOOL_CALL turn is re-rendered STRUCTURALLY (canonical + + ), never as re-flattened raw Markdown prose with a + capability-breaking "### Continue:" nudge; (2) a post-decode SALVAGE PARSER + conservatively recovers a tool call emitted outside the exact + envelope (a generic fenced block or a bare {"name","input"} JSON object) so a + near-miss becomes a real tool call instead of inert prose text. + references: + - 'crates/aprender-orchestrate/src/agent/runtime.rs — retain_assistant_text(), EndTurn history retention' + - 'crates/aprender-orchestrate/src/agent/driver/realizar.rs — parse_tool_calls(), salvage_tool_calls()' + - 'crates/aprender-orchestrate/src/agent/driver/chat_template.rs — structured AssistantToolUse/ToolResult render' + - 'CCPA m296 distill feasibility spike — agentic-loop harness bug independent of the model' +equations: + toolcall_structural_retention: + formula: render(history + AssistantToolUse(c) + ToolResult(r)) preserves AND + domain: history with a format-correct assistant tool-call turn, any chat template + codomain: next-turn prompt string + invariants: + - The prior tool_call survives structurally as the canonical envelope + - The prior tool_result survives structurally as the envelope + - No "### Continue:" prose nudge is injected after a tool-using turn + - Raw tool-call markup never enters history as a Message::Assistant prose blob + - Genuine text turns are retained verbatim (behavior unchanged for prose) + preconditions: + - the prior assistant turn emitted a valid tool call + postconditions: + - prompt contains and + - prompt does not contain "### Continue:" + lean_theorem: Theorems.Apr_Code_Toolcall_Structural_Retention + toolcall_salvage_recovery: + formula: parse(text) recovers an unambiguous tool-call JSON outside the envelope + domain: model output text with no /```json envelope match + codomain: (remaining_text, [ToolCall]) + invariants: + - A generic fenced block whose body is {"name","input"} is salvaged + - A bare top-level {"name","input"} JSON object is salvaged + - JSON without both a string name AND an input field is NEVER salvaged (conservative) + - A proper envelope is owned by the envelope parser, not salvage + preconditions: + - the envelope parser found no tool call + postconditions: + - tool-call-shaped JSON yields a non-empty [ToolCall] + - non-tool-call JSON yields an empty [ToolCall] + lean_theorem: Theorems.Apr_Code_Toolcall_Salvage_Recovery +proof_obligations: +- type: invariant + property: Prior tool-call turn renders structurally with no prose Continue nudge + formal: render(history) ∋ ∧ render(history) ∋ ∧ render(history) ∌ "### Continue:" + applies_to: toolcall_structural_retention +- type: invariant + property: Tool-call markup is never retained as Assistant prose + formal: ∀ m ∈ history, m = Assistant(s) ⟹ s ∌ "" + applies_to: toolcall_structural_retention +- type: equivalence + property: Salvage recovers an unambiguous out-of-envelope tool call + formal: parse(bare_or_fenced {"name":n,"input":i}) = (_, [ToolCall{name:n, input:i}]) + applies_to: toolcall_salvage_recovery +- type: invariant + property: Salvage is conservative — no false positives + formal: ¬(has_name_string ∧ has_input) ⟹ salvage = (text, []) + applies_to: toolcall_salvage_recovery +kernel_structure: + phases: + - name: decode + description: Model emits text; parse_tool_calls extracts /```json envelopes + invariant: a proper envelope is parsed by the envelope path (id local-N) + - name: salvage + description: On envelope miss, salvage_tool_calls recovers a fenced/bare tool-call JSON + invariant: only {"name"(string),"input"}-shaped objects are salvaged (id salvage-N) + - name: retain + description: retain_assistant_text strips lingering tool-call markup before history + invariant: no capability-breaking raw Markdown re-enters the next-turn prompt +simd_dispatch: + toolcall_structural_retention: + scalar: batuta::agent::runtime::retain_assistant_text + toolcall_salvage_recovery: + scalar: batuta::agent::driver::realizar::salvage_tool_calls +enforcement: + structural_retention: + description: A prior tool-call turn must re-render structurally, never as prose + "### Continue:" + check: contract_tests::FALSIFY-TCR-001 + severity: ERROR + history_no_prose_markup: + description: Raw tool-call markup must not be retained as Assistant prose + check: contract_tests::FALSIFY-TCR-003 + severity: ERROR + salvage_recovery: + description: An out-of-envelope tool-call JSON must be salvaged, conservatively + check: contract_tests::FALSIFY-TCR-004 + severity: ERROR +falsification_tests: +- id: FALSIFY-TCR-001 + rule: Prior tool-call turn renders structurally with no prose Continue nudge + prediction: next-turn prompt preserves +, contains no "### Continue:" + test: 'cargo test -p aprender-orchestrate --lib falsify_toolcall_retention_001_structured_render_no_continue_nudge' + red_state: re-rendering the prior turn as raw Markdown or appending "### Continue:" FAILS + green_state: structured ChatML render preserves the envelope and injects no prose nudge (PASS) + if_fails: harness re-flattens the tool-call turn to prose and re-primes text mode across turns +- id: FALSIFY-TCR-002 + rule: Tool-call turn keeps structured history (not Assistant prose) + prediction: after a format-correct tool turn, history carries AssistantToolUse+ToolResult, no raw markup + test: 'cargo test -p aprender-orchestrate --lib falsify_toolcall_retention_002_history_keeps_structure_not_prose' + red_state: retaining raw tool-call text as Message::Assistant prose FAILS the no-markup assertion + green_state: structured tool messages retained; no Assistant prose carries (PASS) + if_fails: runtime pushes response.text verbatim into history, re-injecting Markdown next turn +- id: FALSIFY-TCR-003 + rule: retain_assistant_text strips residue, keeps prose + prediction: pure tool-call markup collapses to empty; genuine prose passes through unchanged + test: 'cargo test -p aprender-orchestrate --lib falsify_toolcall_retention_003_retain_strips_residue_keeps_prose' + red_state: identity retain_assistant_text leaves markup in prose, FAILS + green_state: tool-call spans stripped, prose preserved (PASS) + if_fails: lingering tool-call markup re-primes prose mode on the next turn +- id: FALSIFY-TCR-004 + rule: Salvage recovers an out-of-envelope tool call (bare JSON) + prediction: a bare {"name","input"} object in prose is recovered as a real tool call (salvage-N id) + test: 'cargo test -p aprender-orchestrate --lib test_salvage_bare_top_level_json_tool_call' + red_state: envelope-only parser scores the bare JSON as inert text, FAILS + green_state: salvage_tool_calls extracts the tool call from bare JSON (PASS) + if_fails: the "model almost emitted a tool_call" near-miss is lost to prose mode +- id: FALSIFY-TCR-005 + rule: Salvage recovers a generically-fenced tool call + prediction: a ```tool_call / ```rust fenced {"name","input"} body is recovered as a tool call + test: 'cargo test -p aprender-orchestrate --lib test_salvage_generic_fenced_block_non_json_tag' + red_state: envelope-only parser (knows only ```json) misses the fence, FAILS + green_state: salvage_tool_calls extracts the tool call from a generic fence (PASS) + if_fails: coder-finetuned models that fence with ```tool_call lose their tool calls to prose +kani_harnesses: +- id: KANI-TCR-001 + obligation: Prior tool-call turn renders structurally + property: render(history) ∋ ∧ ∌ "### Continue:" + bound: 4 + strategy: exhaustive + harness: verify_toolcall_structural_retention +- id: KANI-TCR-002 + obligation: Salvage is conservative + property: ¬(has_name ∧ has_input) ⟹ salvage = (text, []) + bound: 8 + strategy: exhaustive + harness: verify_toolcall_salvage_conservative +qa_gate: + id: F-TCR-001 + name: apr-code Tool-Call Retention Contract + description: Pins the agentic-loop harness so tool-calling is retained across turns (CCPA-m296) + checks: + - structural_retention + - history_no_prose_markup + - salvage_recovery + pass_criteria: All 5 falsification tests PASS (GREEN) on the fixed harness + falsification: Make retain_assistant_text identity OR drop salvage — FALSIFY-TCR-001/003/004/005 FAIL diff --git a/crates/aprender-orchestrate/src/agent/driver/realizar.rs b/crates/aprender-orchestrate/src/agent/driver/realizar.rs index a6bbea2f8..e18fb8eb3 100644 --- a/crates/aprender-orchestrate/src/agent/driver/realizar.rs +++ b/crates/aprender-orchestrate/src/agent/driver/realizar.rs @@ -10,6 +10,7 @@ use async_trait::async_trait; use std::path::PathBuf; +use tracing::info; use super::chat_template::{format_prompt_with_template, ChatTemplate}; use super::validate::validate_model_file; @@ -132,6 +133,14 @@ impl LlmDriver for RealizarDriver { /// 2. `{"name":...}` — unclosed XML (small model fallback) /// 3. `` ```json\n{"name":...}\n``` `` — markdown code block (Qwen native) /// +/// CCPA-m296 SALVAGE: if the envelope parser above finds NOTHING but the +/// text is recoverably tool-call-shaped — a generically-fenced block +/// (`` ```...{"name":..,"input":..}... ``` ``, any language tag or none) or a +/// bare top-level `{"name":..,"input":..}` JSON object — [`salvage_tool_calls`] +/// recovers it. This converts "the model almost emitted a tool_call" near-misses +/// into real tool calls instead of letting the raw Markdown re-prime prose mode +/// across turns (the self-reinforcing text loop that defeats tool-calling). +/// /// Returns the remaining text (with tool call blocks removed) /// and the extracted tool calls. /// Public wrapper for tool call parsing (used by AprServeDriver). @@ -140,6 +149,21 @@ pub fn parse_tool_calls_pub(text: &str) -> (String, Vec) { } fn parse_tool_calls(text: &str) -> (String, Vec) { + let (remaining, calls) = parse_tool_calls_envelope(text); + if !calls.is_empty() { + return (remaining, calls); + } + // CCPA-m296: envelope parser found no tool call — try the conservative + // salvage parser before scoring this turn as inert prose text. + let (salvaged_remaining, salvaged) = salvage_tool_calls(&remaining); + if !salvaged.is_empty() { + info!(count = salvaged.len(), "salvaged tool call(s) from non-envelope output (CCPA-m296)"); + return (salvaged_remaining, salvaged); + } + (remaining, calls) +} + +fn parse_tool_calls_envelope(text: &str) -> (String, Vec) { let mut tool_calls = Vec::new(); let mut remaining = String::new(); let mut call_counter = 0u32; @@ -204,6 +228,117 @@ fn parse_tool_calls(text: &str) -> (String, Vec) { (remaining.trim().to_string(), tool_calls) } +/// CCPA-m296 salvage parser: recover a tool call the model emitted OUTSIDE the +/// exact `` / ```json envelope, but in an unambiguous, recoverable +/// shape. Two recoverable shapes are accepted, in priority order: +/// +/// 1. A generically-fenced code block — `` ```\n{...}\n``` `` — whose +/// inner content parses as a tool-call-shaped JSON object. (The envelope +/// parser only recognises the exact `` ```json `` tag; coder-finetuned models +/// routinely emit `` ```tool_call ``, `` ```rust ``, or a bare `` ``` ``.) +/// 2. A bare top-level `{"name": "...", "input": {...}}` JSON object embedded in +/// prose (no fence, no tags). +/// +/// CONSERVATIVE BY DESIGN: only JSON objects that (a) parse cleanly and (b) have +/// a string `name` field AND an `input` field are salvaged. Plain JSON examples +/// (e.g. `{"key": "value"}`) and prose are never mistaken for tool calls. This +/// directly recovers the "model almost emitted a tool_call" near-misses that +/// would otherwise be scored as inert text and re-prime prose mode next turn. +/// +/// Returns the remaining text (with the salvaged span removed) and the +/// recovered calls (`salvage-{n}` ids so salvage events stay traceable). +fn salvage_tool_calls(text: &str) -> (String, Vec) { + // Shape 1: a generic fenced block ```\n ... \n``` + if let Some((before, inner, after)) = extract_first_fenced_block(text) { + if let Some(call) = tool_call_from_json_str(inner.trim(), 1) { + let remaining = format!("{before}{after}"); + return (remaining.trim().to_string(), vec![call]); + } + } + + // Shape 2: a bare top-level {"name":..,"input":..} object embedded in prose. + if let Some((start, end)) = find_balanced_json_object(text) { + if let Some(call) = tool_call_from_json_str(text[start..end].trim(), 1) { + let remaining = format!("{}{}", &text[..start], &text[end..]); + return (remaining.trim().to_string(), vec![call]); + } + } + + (text.trim().to_string(), Vec::new()) +} + +/// Parse a JSON string into a tool call iff it is unambiguously tool-call-shaped: +/// a JSON object with a string `name` field AND an `input` field. Returns `None` +/// otherwise (plain JSON, arrays, scalars, prose). +fn tool_call_from_json_str(json_str: &str, idx: u32) -> Option { + let parsed = serde_json::from_str::(json_str).ok()?; + let obj = parsed.as_object()?; + // Require BOTH name (string) and an explicit input field — stricter than the + // envelope parser (which defaults input to {}) so prose/JSON examples that + // merely contain a "name" key are never salvaged. + let name = obj.get("name")?.as_str()?.to_string(); + if name.is_empty() { + return None; + } + let input = obj.get("input")?.clone(); + Some(ToolCall { id: format!("salvage-{idx}"), name, input }) +} + +/// Extract the first ```...``` fenced block: returns (text-before, inner, text-after). +/// Accepts any language tag (or none); the inner content is everything between the +/// opening fence's newline and the closing fence. +fn extract_first_fenced_block(text: &str) -> Option<(&str, &str, &str)> { + let open = text.find("```")?; + let before = &text[..open]; + let rest = &text[open + 3..]; + // Skip the optional language tag up to (and including) the first newline. + let inner_start = rest.find('\n').map(|i| i + 1)?; + let body = &rest[inner_start..]; + let close = body.find("```")?; + let inner = &body[..close]; + let after = &body[close + 3..]; + Some((before, inner, after)) +} + +/// Find the first balanced top-level `{...}` JSON object span in `text`. +/// Returns `(start, end)` byte indices (end exclusive) of the object including +/// braces, tracking string literals + escapes so braces inside strings don't +/// unbalance the scan. Returns `None` if no balanced object is found. +fn find_balanced_json_object(text: &str) -> Option<(usize, usize)> { + let bytes = text.as_bytes(); + let start = text.find('{')?; + let mut depth = 0i32; + let mut in_str = false; + let mut escaped = false; + let mut i = start; + while i < bytes.len() { + let c = bytes[i]; + if in_str { + if escaped { + escaped = false; + } else if c == b'\\' { + escaped = true; + } else if c == b'"' { + in_str = false; + } + } else { + match c { + b'"' => in_str = true, + b'{' => depth += 1, + b'}' => { + depth -= 1; + if depth == 0 { + return Some((start, i + 1)); + } + } + _ => {} + } + } + i += 1; + } + None +} + /// Sanitize model output: strip echoed system prompt and chat template markers. /// /// Small models (<3B) often echo the system prompt or leak chat template @@ -393,4 +528,71 @@ not valid json let cleaned = sanitize_output(output, None); assert_eq!(cleaned, "Here is my response."); } + + // ── CCPA-m296 salvage parser tests ── + + #[test] + fn test_salvage_bare_top_level_json_tool_call() { + // Model emitted a bare {"name","input"} object with NO envelope/fence. + // Without salvage this scores as inert prose and re-primes prose mode. + let input = + "Sure, I'll read it.\n{\"name\": \"file_read\", \"input\": {\"path\": \"src/lib.rs\"}}"; + let (text, calls) = parse_tool_calls(input); + assert_eq!(calls.len(), 1, "salvage must recover a bare tool-call JSON object"); + assert_eq!(calls[0].name, "file_read"); + assert_eq!(calls[0].input["path"], "src/lib.rs"); + assert!(calls[0].id.starts_with("salvage-"), "salvaged calls carry a traceable id"); + assert!(text.contains("Sure, I'll read it"), "prose around the call is preserved"); + assert!(!text.contains("file_read"), "the salvaged JSON span is removed from text"); + } + + #[test] + fn test_salvage_generic_fenced_block_non_json_tag() { + // Coder models fence tool calls with ```tool_call / ```rust, not ```json. + // The envelope parser only knows ```json; salvage must catch the rest. + let input = + "```tool_call\n{\"name\": \"shell\", \"input\": {\"command\": \"cargo test\"}}\n```"; + let (_text, calls) = parse_tool_calls(input); + assert_eq!(calls.len(), 1, "salvage must recover a generically-fenced tool call"); + assert_eq!(calls[0].name, "shell"); + assert_eq!(calls[0].input["command"], "cargo test"); + } + + #[test] + fn test_salvage_conservative_rejects_plain_json() { + // A bare JSON object WITHOUT name+input is NOT a tool call — never salvage it. + let input = "Here is some config:\n{\"key\": \"value\", \"count\": 3}"; + let (text, calls) = parse_tool_calls(input); + assert!(calls.is_empty(), "plain JSON (no name+input) must not be salvaged"); + assert!(text.contains("config")); + } + + #[test] + fn test_salvage_conservative_rejects_name_without_input() { + // Stricter than the envelope parser: salvage requires an explicit `input`. + let input = "{\"name\": \"file_read\"}"; + let (_text, calls) = parse_tool_calls(input); + assert!(calls.is_empty(), "name without input is too ambiguous to salvage"); + } + + #[test] + fn test_salvage_handles_braces_inside_strings() { + // The balanced-object scanner must not unbalance on braces inside strings. + let input = "{\"name\": \"shell\", \"input\": {\"command\": \"echo ${HOME} and }{\"}}"; + let (_text, calls) = parse_tool_calls(input); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].name, "shell"); + assert_eq!(calls[0].input["command"], "echo ${HOME} and }{"); + } + + #[test] + fn test_envelope_takes_precedence_over_salvage() { + // A proper envelope must be parsed by the envelope path + // (id "local-1"), never falling through to salvage. + let input = + "\n{\"name\": \"glob\", \"input\": {\"pattern\": \"*.rs\"}}\n"; + let (_text, calls) = parse_tool_calls(input); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].id, "local-1", "envelope parser owns this, not salvage"); + } } diff --git a/crates/aprender-orchestrate/src/agent/runtime.rs b/crates/aprender-orchestrate/src/agent/runtime.rs index 90e4d8ca1..e4349c74a 100644 --- a/crates/aprender-orchestrate/src/agent/runtime.rs +++ b/crates/aprender-orchestrate/src/agent/runtime.rs @@ -143,8 +143,15 @@ pub async fn run_agent_turn( for msg in &messages[new_start..] { history.push(msg.clone()); } - if !response.text.is_empty() { - history.push(Message::Assistant(response.text.clone())); + // CCPA-m296: only retain GENUINE assistant prose in history. If the + // final text still carries un-parsed tool-call markup (a defense-in- + // depth residue the driver's parser+salvage couldn't recover), pushing + // it verbatim as Message::Assistant re-injects capability-breaking raw + // Markdown into the NEXT turn's prompt — the self-reinforcing text loop + // that erodes tool-calling. Strip the residue first; never re-prime prose. + let retained = retain_assistant_text(&response.text); + if !retained.is_empty() { + history.push(Message::Assistant(retained)); } return finish_loop(&response, &guard, manifest, query, memory, stream_tx.as_ref()) .await; @@ -191,6 +198,56 @@ pub async fn run_agent_turn( } } +/// CCPA-m296: sanitize an assistant turn's text before it enters multi-turn +/// history. Strips any lingering tool-call markup so a prior tool-using turn is +/// NEVER re-rendered as capability-breaking raw Markdown that re-primes prose +/// mode on the next turn. +/// +/// Genuine text turns pass through unchanged. Turns whose entire content was +/// tool-call markup collapse to empty (the structured `AssistantToolUse` / +/// `ToolResult` messages already carry that turn's tool semantics into history). +/// +/// Returns the trimmed prose with `...` / unclosed +/// `` / `...` spans removed. +fn retain_assistant_text(text: &str) -> String { + let mut out = text.to_string(); + + // Remove well-formed ... spans. + out = strip_spans(&out, "", ""); + // Remove ... spans (model sometimes echoes them). + out = strip_spans(&out, "", ""); + + // Remove an unclosed trailing (small models omit the close tag): + // everything from the marker to end-of-string is tool-call residue, not prose. + if let Some(pos) = out.find("") { + out.truncate(pos); + } + + out.trim().to_string() +} + +/// Remove every `open..close` span (inclusive) from `text`. Spans without a +/// matching close are left untouched (handled separately by the caller). +fn strip_spans(text: &str, open: &str, close: &str) -> String { + let mut out = String::with_capacity(text.len()); + let mut cursor = text; + loop { + let Some(start) = cursor.find(open) else { + out.push_str(cursor); + break; + }; + let after_open = &cursor[start + open.len()..]; + let Some(rel_end) = after_open.find(close) else { + // No closing tag — leave the remainder for the caller to handle. + out.push_str(cursor); + break; + }; + out.push_str(&cursor[..start]); + cursor = &after_open[rel_end + close.len()..]; + } + out +} + fn check_verdict(verdict: LoopVerdict) -> Result<(), AgentError> { match verdict { LoopVerdict::CircuitBreak(msg) | LoopVerdict::Block(msg) => { diff --git a/crates/aprender-orchestrate/src/agent/runtime_tests_multi_turn.rs b/crates/aprender-orchestrate/src/agent/runtime_tests_multi_turn.rs index 3ab0c12e6..70359b02d 100644 --- a/crates/aprender-orchestrate/src/agent/runtime_tests_multi_turn.rs +++ b/crates/aprender-orchestrate/src/agent/runtime_tests_multi_turn.rs @@ -148,3 +148,169 @@ async fn test_run_agent_loop_delegates_to_turn() { .expect("run_agent_loop failed"); assert_eq!(result.text, "compat"); } + +// ═══════════════════════════════════════════════════════════════════════════ +// CCPA-m296 FALSIFIERS — OBLIG-APR-CODE-TOOLCALL-RETENTION +// +// The apr-code agent loop had a HARNESS bug independent of the model: a prior +// assistant TOOL_CALL turn was retained / re-rendered as raw Markdown prose, +// re-priming prose mode and eroding a format-correct model's tool-calling to +// 0/N across a multi-turn run (a self-reinforcing text loop). +// +// Oracle = the STRUCTURED, tool-call-preserving render. These tests are +// mutation-verified: reverting the fix (pushing raw response.text into history, +// or dropping the structured AssistantToolUse/ToolResult render) turns them RED. +// ═══════════════════════════════════════════════════════════════════════════ + +use crate::agent::driver::chat_template::{format_prompt_with_template, ChatTemplate}; +use crate::agent::driver::{CompletionRequest, ToolResultMsg}; + +/// FALSIFY-TOOLCALL-RETENTION-001: a prior format-correct assistant TOOL_CALL +/// turn renders STRUCTURALLY on the next turn — the canonical assistant +/// `` + the `` — and NEVER as re-flattened raw Markdown +/// with a capability-breaking "### Continue:" prose nudge. +/// +/// Oracle: the structured ChatML render. Mutation: re-rendering the prior turn +/// as raw Markdown (or appending "### Continue:") makes the negative assertions +/// FAIL. +#[test] +fn falsify_toolcall_retention_001_structured_render_no_continue_nudge() { + // History as produced by handle_tool_calls() after a format-correct turn. + let history = vec![ + Message::User("fix the off-by-one in src/lib.rs".into()), + Message::AssistantToolUse(ToolCall { + id: "local-1".into(), + name: "file_read".into(), + input: serde_json::json!({"path": "src/lib.rs"}), + }), + Message::ToolResult(ToolResultMsg { + tool_use_id: "local-1".into(), + content: "fn f() { return (i, j); }".into(), + is_error: false, + }), + ]; + + let request = CompletionRequest { + model: "qwen3-1.7b".into(), + messages: history, + tools: vec![], + max_tokens: 256, + temperature: 0.2, + system: Some("You are apr code.".into()), + }; + + let prompt = format_prompt_with_template(&request, ChatTemplate::ChatMl); + + // (a) STRUCTURE PRESERVED: the tool_call + tool_result survive into the + // next-turn prompt as the canonical envelope, not as flattened prose. + assert!( + prompt.contains(""), + "tool_call must be preserved structurally across turns, got:\n{prompt}" + ); + assert!( + prompt.contains("\"name\":\"file_read\"") || prompt.contains("\"name\": \"file_read\""), + "the tool name must survive in the structured render, got:\n{prompt}" + ); + assert!( + prompt.contains(""), + "tool_result must be preserved structurally across turns, got:\n{prompt}" + ); + + // (b) NO PROSE RE-PRIMING: the harness must not re-inject a capability- + // breaking "### Continue:" prose nudge after a tool-using turn. + assert!( + !prompt.contains("### Continue"), + "must NOT re-inject a '### Continue:' prose nudge after a tool turn, got:\n{prompt}" + ); + assert!( + !prompt.contains("Continue:"), + "must NOT append any prose Continue nudge that re-primes text mode, got:\n{prompt}" + ); +} + +/// FALSIFY-TOOLCALL-RETENTION-002: when a format-correct turn yields a tool +/// call, the multi-turn history retains the STRUCTURED AssistantToolUse + +/// ToolResult — NOT a raw `Message::Assistant(markdown)` prose blob. +/// +/// Oracle: `Message::AssistantToolUse` / `Message::ToolResult` present in +/// history; no Assistant message carrying raw `` markup. Mutation: +/// retaining raw tool-call text as Assistant prose makes the final assertion +/// FAIL. +#[tokio::test] +async fn falsify_toolcall_retention_002_history_keeps_structure_not_prose() { + let manifest = default_manifest(); + let driver = MockDriver::new(vec![ + // Turn 1: a clean tool call (format-correct model). + CompletionResponse { + text: String::new(), + stop_reason: StopReason::ToolUse, + tool_calls: vec![ToolCall { + id: "1".into(), + name: "echo".into(), + input: serde_json::json!({"text": "hi"}), + }], + usage: TokenUsage { input_tokens: 10, output_tokens: 5 }, + }, + // Then a clean text conclusion. + CompletionResponse { + text: "done".into(), + stop_reason: StopReason::EndTurn, + tool_calls: vec![], + usage: TokenUsage { input_tokens: 12, output_tokens: 4 }, + }, + ]); + let mut tools = ToolRegistry::new(); + tools.register(Box::new(EchoTool)); + let memory = InMemorySubstrate::new(); + let mut history = Vec::new(); + + run_agent_turn(&manifest, &mut history, "use echo", &driver, &tools, &memory, None) + .await + .expect("turn failed"); + + // History must carry the STRUCTURED tool call + result. + assert!( + history.iter().any(|m| matches!(m, Message::AssistantToolUse(_))), + "structured AssistantToolUse must be retained, got: {history:?}" + ); + assert!( + history.iter().any(|m| matches!(m, Message::ToolResult(_))), + "structured ToolResult must be retained, got: {history:?}" + ); + // No Assistant prose message may carry raw tool-call markup back into history. + assert!( + !history.iter().any(|m| matches!(m, Message::Assistant(s) if s.contains(""))), + "raw markup must NOT be retained as Assistant prose, got: {history:?}" + ); +} + +/// FALSIFY-TOOLCALL-RETENTION-003: `retain_assistant_text` strips lingering +/// tool-call markup so a tool-using turn cannot be re-rendered as prose that +/// re-primes text mode. Genuine prose passes through unchanged. +/// +/// Oracle: residue stripped, prose preserved. Mutation: making +/// `retain_assistant_text` an identity function makes the strip assertions FAIL. +#[test] +fn falsify_toolcall_retention_003_retain_strips_residue_keeps_prose() { + // Pure tool-call residue collapses to empty (structure lives elsewhere). + let only_call = + "\n{\"name\": \"shell\", \"input\": {\"command\": \"ls\"}}\n"; + assert_eq!( + super::retain_assistant_text(only_call), + "", + "a turn that was entirely tool-call markup must not enter history as prose" + ); + + // Unclosed trailing residue is truncated. + let unclosed = "Let me check.\n {\"name\": \"glob\", \"input\": {}}"; + assert_eq!(super::retain_assistant_text(unclosed), "Let me check."); + + // Genuine prose passes through untouched. + let prose = "The bug is an off-by-one on line 42."; + assert_eq!(super::retain_assistant_text(prose), prose); + + // Mixed: prose is kept, the tool-call span is removed. + let mixed = + "Here is the fix.\n\n{\"name\": \"file_edit\", \"input\": {}}\n"; + assert_eq!(super::retain_assistant_text(mixed), "Here is the fix."); +}