fix(agent): use canonical assistant history format in subagent_runner#606
Conversation
build_native_assistant_payload in subagent_runner.rs serialised the
assistant tool-call message using {"text": …, "tool_calls": [{
"type":"function","function":{…}}]} — two mismatches vs the format
parse::build_native_assistant_history produces and the backend
jinja template expects:
1. "text" instead of "content" — the downstream convert_messages
parser looks for "content" to detect assistant-with-tool-calls
messages; "text" is not recognised, so the message is treated
as a plain assistant message.
2. Nested {"type":"function","function":{"name":…,"arguments":…}}
wrappers instead of flat {"id":…,"name":…,"arguments":…}.
Result: every sub-agent that made a tool call and entered iteration
1+ hit a 400 from the backend: "jinja template rendering failed.
Message has tool role, but there was no previous assistant message
with a tool call!" — the backend saw a role=tool message without a
recognised assistant-with-tool-calls predecessor.
The fix replaces the broken inline copy with a direct call to
parse::build_native_assistant_history (the same serialiser
tool_loop.rs uses successfully for the orchestrator's tool calls).
The dead build_native_assistant_payload function is removed.
Introduced in PR tinyhumansai#474 (6465f3d, 2026-04-09). Latent since then
because the orchestrator self-heals by retrying via other agents.
E2E verified: skills_agent with gmail toolkit now progresses through
iterations 0→1→2 successfully (previously died at iteration 1 with
the 400 error every time).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe assistant message serialization in the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/agent/harness/subagent_runner.rs (1)
739-740: Add a regression assertion for assistant payload shape in multi-iteration tool-call tests.Current tests confirm
role=toolpresence, but not that the preceding assistant message is serialized withcontent+ flattool_calls[{id,name,arguments}](the actual bug vector).Proposed test hardening
@@ let second_call_messages = &captured[1].messages; let has_tool_msg = second_call_messages.iter().any(|m| m.role == "tool"); assert!( has_tool_msg, "second provider call should include role=tool message" ); + + let assistant_msg = second_call_messages + .iter() + .find(|m| m.role == "assistant") + .expect("assistant tool-call history message should be present"); + let payload: serde_json::Value = serde_json::from_str(&assistant_msg.content) + .expect("assistant tool-call history should be valid JSON"); + assert!(payload.get("content").is_some(), "missing `content` field"); + assert!(payload.get("tool_calls").is_some(), "missing `tool_calls` field"); + assert_eq!(payload["tool_calls"][0]["id"], "call-1"); + assert_eq!(payload["tool_calls"][0]["name"], "file_read");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/agent/harness/subagent_runner.rs` around lines 739 - 740, Add a regression assertion in the multi-iteration tool-call tests to validate the assistant message payload shape produced by super::parse::build_native_assistant_history: after building history (history variable) and pushing ChatMessage::assistant(assistant_history_content), assert that the assistant message serializes to an object with a top-level "content" field plus a flat "tool_calls" array where each entry has {id, name, arguments} (matching native_calls and response_text). Update the test that exercises build_native_assistant_history / ChatMessage::assistant to deserialize/inspect the assistant_history_content and explicitly assert presence and shape of "content" and that tool_calls is a flat list with each element containing id, name, and arguments to prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/openhuman/agent/harness/subagent_runner.rs`:
- Around line 739-740: Add a regression assertion in the multi-iteration
tool-call tests to validate the assistant message payload shape produced by
super::parse::build_native_assistant_history: after building history (history
variable) and pushing ChatMessage::assistant(assistant_history_content), assert
that the assistant message serializes to an object with a top-level "content"
field plus a flat "tool_calls" array where each entry has {id, name, arguments}
(matching native_calls and response_text). Update the test that exercises
build_native_assistant_history / ChatMessage::assistant to deserialize/inspect
the assistant_history_content and explicitly assert presence and shape of
"content" and that tool_calls is a flat list with each element containing id,
name, and arguments to prevent regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ed9eb8ae-2354-42bb-bb8a-b55f7ce81833
📒 Files selected for processing (1)
src/openhuman/agent/harness/subagent_runner.rs
Summary
"jinja template rendering failed. Message has tool role, but there was no previous assistant message with a tool call!"build_native_assistant_payloadused"text"instead of"content"and nested{"type":"function","function":{...}}wrappers instead of flat{id, name, arguments}— two mismatches vs the formatparse::build_native_assistant_historyproduces and the backend expects.parse::build_native_assistant_history(the same serialisertool_loop.rsuses successfully for the orchestrator's tool calls). Dead function removed.Problem
Every sub-agent that made a tool call and entered iteration 1+ hit a 400 from the backend. The backend's jinja template saw a
role=toolmessage without a recognisedassistant-with-tool-callspredecessor because the assistant message was formatted with the wrong keys and structure.Introduced in PR #474 (
6465f3d3, 2026-04-09). Latent since then because the orchestrator self-heals by retrying via other agents.Solution
One-line fix in
subagent_runner.rs: replacebuild_native_assistant_payload(&response_text, &native_calls)withsuper::parse::build_native_assistant_history(&response_text, &native_calls). Delete the dead function.E2E verification
skills_agent with gmail toolkit progressed through iterations 0->1->2 successfully (previously died at iteration 1 with the 400 error every time). code_executor also confirmed working through iteration 0->1 on staging API.
Submission Checklist
Impact
parse::build_native_assistant_historyviatool_loop.rs)Related
Summary by CodeRabbit