mirror of
https://github.com/Mesh-LLM/mesh-llm.git
synced 2026-08-08 22:23:19 -04:00
fix(moa): recognise OpenAI-shape inline tool JSON in worker output
PR #566 review feedback (Apr 2026): > In the read-tool probe, the model wrote text that looked like a > tool call instead of actually invoking the read tool. Agent harnesses (Goose, OpenCode, pi) only act on real `tool_calls`. If a worker emits inline OpenAI-shape tool JSON \u2014 "I'll read the README. {\"function\": \"read_file\", \"arguments\": {\"path\": \"README.md\"}}" \u2014 today's normalizer's `try_json_parse` requires a `kind` field in the JSON, which the OpenAI tool-call shape never has. `try_json_parse` returns None, the heuristic classifier doesn't have an action verb that matches ("I'll read" isn't on its list), and the worker output falls through to `OutputKind::Answer`. Three workers all return the same text \u2192 arbiter agrees \u2192 `chat_response(text)` \u2014 the agent gets the JSON-bearing prose as `content`, no `tool_calls` field, and silently does nothing. ## Test (added first, observed failing) `tests/sim_tool_call_text_not_passed_as_content.rs` \u2014 two scenarios: * `workers_with_inline_tool_json_emit_real_tool_call` \u2014 workers return prose with embedded `{"function": "read_file", "arguments": {...}}`. The response body must carry a real `tool_calls` array with the proposed function name. **Failed pre-fix**: body had `content` with the prose, no `tool_calls`. * `workers_describing_tool_call_must_emit_structured_tool_call` \u2014 workers describe a tool call in pure prose with no JSON. Today the heuristic catches this and synthesises a `tool_calls` entry (with empty arguments). Pinned so the JSON-shape fix below doesn't regress the pure-prose path. ## Fix `normalize::try_json_parse` now also recognises the OpenAI tool-call shape when no `kind` field is present: {"function": "read_file", "arguments": {...}} {"name": "read_file", "arguments": {...}} {"tool": "read_file", "arguments": {...}} A structurally well-formed inline tool proposal scores confidence 0.75 (above the heuristic's 0.6) so the arbiter prefers it on ties. The rest of the original `kind`-driven envelope path is unchanged. ## Validation `cargo test -p mesh-mixture-of-agents` \u2014 70 unit + 7 integration tests pass (this PR\u2019s 2 new tests + earlier sim files). `cargo test -p mesh-llm-host-runtime --lib` \u2014 1434/1434 pass. `cargo clippy -p mesh-mixture-of-agents --all-targets -- -D warnings` \u2014 clean. `cargo fmt --all -- --check` \u2014 clean.
This commit is contained in:
Michael Neale
parent
000ae50cf6
commit
64c4e0ec83
2 changed files with 250 additions and 0 deletions
|
|
@ -136,6 +136,47 @@ fn try_json_parse(
|
|||
let json_str = extract_json_object(raw)?;
|
||||
let obj: Value = serde_json::from_str(&json_str).ok()?;
|
||||
|
||||
// First, recognise the OpenAI tool-call shape that models commonly
|
||||
// emit even without our `kind`/`confidence` envelope:
|
||||
//
|
||||
// {"function": "read_file", "arguments": {"path": "README.md"}}
|
||||
// {"name": "read_file", "arguments": {...}}
|
||||
// {"tool": "read_file", "arguments": {...}}
|
||||
//
|
||||
// Agent harnesses (Goose, OpenCode) only act on real `tool_calls`
|
||||
// — if the worker writes inline tool JSON and we miss it, MoA leaks
|
||||
// the JSON back as `content` and the agent does nothing. This is
|
||||
// the failure mode PR #566 review called out.
|
||||
if obj.get("kind").is_none() {
|
||||
let openai_tool_name = obj
|
||||
.get("function")
|
||||
.and_then(|v| v.as_str())
|
||||
.or_else(|| obj.get("name").and_then(|v| v.as_str()))
|
||||
.or_else(|| obj.get("tool").and_then(|v| v.as_str()));
|
||||
if let Some(tname) = openai_tool_name {
|
||||
let args = obj.get("arguments").cloned().or_else(|| {
|
||||
obj.get("arguments")
|
||||
.and_then(|a| a.as_str())
|
||||
.and_then(|s| serde_json::from_str(s).ok())
|
||||
});
|
||||
return Some(WorkerOutput {
|
||||
kind: OutputKind::ToolProposal,
|
||||
// OpenAI-shape tool calls have no native confidence
|
||||
// marker, but a structurally well-formed proposal is a
|
||||
// stronger signal than a heuristic catch — score it
|
||||
// higher than the heuristic's 0.6 so the arbiter
|
||||
// prefers it on tie.
|
||||
confidence: 0.75,
|
||||
tool_name: Some(tname.to_string()),
|
||||
tool_arguments: args,
|
||||
payload: raw.to_string(),
|
||||
model: model.to_string(),
|
||||
role,
|
||||
elapsed_ms,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
let kind = match obj.get("kind").and_then(|k| k.as_str()) {
|
||||
Some("tool_proposal") => OutputKind::ToolProposal,
|
||||
Some("critique") => OutputKind::Critique,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,209 @@
|
|||
//! Pin the contract: a tool-call-shaped worker reply must produce a
|
||||
//! real `tool_calls` field in the response, not get returned as
|
||||
//! free-form `content`.
|
||||
//!
|
||||
//! Background — PR #566 review feedback (Apr 2026):
|
||||
//!
|
||||
//! > In the read-tool probe, the model wrote text that looked like a
|
||||
//! > tool call instead of actually invoking the read tool.
|
||||
//!
|
||||
//! Agent harnesses (Goose, OpenCode, pi) only act on `tool_calls`.
|
||||
//! If a worker writes "I'll use read_file to inspect README.md" and
|
||||
//! that text leaks out as `choices[0].message.content` instead of
|
||||
//! `choices[0].message.tool_calls[*]`, the harness sees a text reply
|
||||
//! and takes no action. This is the core blocker for agent loops.
|
||||
//!
|
||||
//! This test drives `handle_turn` with mock workers that all return
|
||||
//! the same "I'll use read_file" prose, and asserts that the
|
||||
//! response carries a structured tool call. The contract:
|
||||
//!
|
||||
//! * `choices[0].message.tool_calls` is a non-empty array, OR
|
||||
//! * `choices[0].finish_reason == "tool_calls"`, OR
|
||||
//! * the response is an error / reducer-escalation (i.e. MoA
|
||||
//! refused to return prose when the request had `tools` and
|
||||
//! workers proposed using one).
|
||||
//!
|
||||
//! What is NOT acceptable is a response with `content: "I'll use
|
||||
//! read_file..."` and no `tool_calls` \u2014 the agent harness would
|
||||
//! silently do nothing.
|
||||
|
||||
use async_trait::async_trait;
|
||||
use mesh_mixture_of_agents as moa;
|
||||
use serde_json::{json, Value};
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
|
||||
/// Backend that returns a fixed text on every call.
|
||||
struct FixedTextBackend {
|
||||
text: String,
|
||||
}
|
||||
|
||||
impl FixedTextBackend {
|
||||
fn new(text: impl Into<String>) -> Arc<Self> {
|
||||
Arc::new(Self { text: text.into() })
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl moa::ModelBackend for FixedTextBackend {
|
||||
async fn chat_completion(
|
||||
&self,
|
||||
_model: &str,
|
||||
_messages: &[Value],
|
||||
_tools: Option<&Value>,
|
||||
_max_tokens: u32,
|
||||
_timeout: Duration,
|
||||
_sampling: moa::SamplingParams,
|
||||
) -> Result<Value, String> {
|
||||
// Modest delay so the runtime doesn't optimize the whole turn into
|
||||
// a single sync poll.
|
||||
tokio::time::sleep(Duration::from_millis(5)).await;
|
||||
Ok(json!({
|
||||
"choices": [{"message": {"content": self.text}}],
|
||||
}))
|
||||
}
|
||||
}
|
||||
|
||||
fn config_with_three_workers_returning(text: &str) -> moa::GatewayConfig {
|
||||
let a = FixedTextBackend::new(text);
|
||||
let b = FixedTextBackend::new(text);
|
||||
let c = FixedTextBackend::new(text);
|
||||
|
||||
let backends: Vec<Arc<dyn moa::ModelBackend>> = vec![a, b, c];
|
||||
let models = vec![
|
||||
moa::ModelEntry {
|
||||
name: "worker-a-3b".into(),
|
||||
backend_index: 0,
|
||||
},
|
||||
moa::ModelEntry {
|
||||
name: "worker-b-13b".into(),
|
||||
backend_index: 1,
|
||||
},
|
||||
moa::ModelEntry {
|
||||
name: "worker-c-32b".into(),
|
||||
backend_index: 2,
|
||||
},
|
||||
];
|
||||
|
||||
moa::GatewayConfig {
|
||||
backends,
|
||||
models,
|
||||
worker_timeout: Duration::from_secs(2),
|
||||
hedge_delay: Duration::from_millis(50),
|
||||
reducer_timeout: Duration::from_secs(2),
|
||||
}
|
||||
}
|
||||
|
||||
fn user_request_with_read_file_tool(content: &str) -> Value {
|
||||
json!({
|
||||
"model": "mesh",
|
||||
"tools": [{
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "read_file",
|
||||
"description": "Read a file",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {"path": {"type": "string"}},
|
||||
"required": ["path"],
|
||||
}
|
||||
}
|
||||
}],
|
||||
"messages": [{"role": "user", "content": content}],
|
||||
"max_tokens": 128,
|
||||
})
|
||||
}
|
||||
|
||||
/// Helper: does the response body have a real, non-empty `tool_calls`
|
||||
/// array?
|
||||
fn has_tool_calls(body: &Value) -> bool {
|
||||
body.pointer("/choices/0/message/tool_calls")
|
||||
.and_then(|v| v.as_array())
|
||||
.map(|a| !a.is_empty())
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
/// Helper: does the response signal an explicit failure (the all-workers-
|
||||
/// fail or reducer-failed path)?
|
||||
fn is_explicit_error(body: &Value) -> bool {
|
||||
if body.get("error").is_some() {
|
||||
return true;
|
||||
}
|
||||
body.pointer("/choices/0/finish_reason")
|
||||
.and_then(|v| v.as_str())
|
||||
.map(|s| s == "error" || s == "moa_failed")
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
/// Helper: did the response just smuggle the worker prose back to the
|
||||
/// agent as `content`? That's the failure shape from the PR review.
|
||||
fn returned_prose_to_agent(body: &Value) -> bool {
|
||||
body.pointer("/choices/0/message/content")
|
||||
.and_then(|v| v.as_str())
|
||||
.map(|s| !s.is_empty())
|
||||
.unwrap_or(false)
|
||||
&& !has_tool_calls(body)
|
||||
&& !is_explicit_error(body)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn workers_describing_tool_call_must_emit_structured_tool_call() {
|
||||
// Three workers ALL reply with the same agentic-prose. Today the
|
||||
// heuristic classifier marks this as ToolProposal (good), but
|
||||
// `extract_tool_proposal` has no JSON in the text to pull
|
||||
// arguments from, so the gateway returns chat_response(payload)
|
||||
// — i.e. the prose — instead of a tool_call_response.
|
||||
let config = config_with_three_workers_returning(
|
||||
"I'll use read_file to inspect README.md and report what it contains.",
|
||||
);
|
||||
let body = user_request_with_read_file_tool("Read README.md and tell me what it says.");
|
||||
|
||||
let result = moa::handle_turn(&config, &body).await;
|
||||
let body = &result.response_body;
|
||||
|
||||
assert!(
|
||||
has_tool_calls(body) || is_explicit_error(body),
|
||||
"tool-flavored worker prose must produce either a real tool_calls field \
|
||||
or a clear failure response. Returning the prose as plain content is the \
|
||||
agent-harness failure mode the PR review called out. \
|
||||
turn_kind={:?}, reducer_used={}, body={body}",
|
||||
result.turn_kind,
|
||||
result.reducer_used,
|
||||
);
|
||||
|
||||
assert!(
|
||||
!returned_prose_to_agent(body),
|
||||
"must not smuggle prose to agent as `content` without `tool_calls`; body={body}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn workers_with_inline_tool_json_emit_real_tool_call() {
|
||||
// Counterpart: when worker output IS structurally a tool proposal
|
||||
// (JSON with function+arguments), MoA should emit a real
|
||||
// `tool_calls`. This already works today; the test is here to
|
||||
// pin the success case so the fix to the previous test doesn't
|
||||
// regress the normal path.
|
||||
let config = config_with_three_workers_returning(
|
||||
r#"I'll read the README. {"function": "read_file", "arguments": {"path": "README.md"}}"#,
|
||||
);
|
||||
let body = user_request_with_read_file_tool("Read README.md.");
|
||||
|
||||
let result = moa::handle_turn(&config, &body).await;
|
||||
let body = &result.response_body;
|
||||
|
||||
assert!(
|
||||
has_tool_calls(body),
|
||||
"well-formed inline tool JSON must produce tool_calls; \
|
||||
turn_kind={:?}, body={body}",
|
||||
result.turn_kind,
|
||||
);
|
||||
let name = body
|
||||
.pointer("/choices/0/message/tool_calls/0/function/name")
|
||||
.and_then(|v| v.as_str());
|
||||
assert_eq!(
|
||||
name,
|
||||
Some("read_file"),
|
||||
"tool_call function name must be the proposed tool"
|
||||
);
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue