fix(flows): preserve literal tool-call markup in tool-less completions (#4750)

This commit is contained in:
Steven Enamakel
2026-07-14 07:11:10 +04:00
committed by GitHub
parent 81a666f34a
commit bc6e4947fc
+58 -17
View File
@@ -61,11 +61,11 @@ fn build_chat_inputs(
///
/// The text-mode fallback parse needs each tool's positional parameter layout
/// to reconstruct named JSON arguments from a P-Format `name[a|b]` body. The
/// harness always populates `request.tools` (schemas are rendered into the
/// prompt for prompt-guided providers, or advertised natively otherwise), so
/// the registry is available in both modes. An empty registry (no tools
/// advertised) makes the P-Format-aware parser short-circuit to the canonical
/// grammar, so this is behaviour-neutral when there are no tools.
/// harness populates `request.tools` when tools are available (schemas are
/// rendered into the prompt for prompt-guided providers, or advertised natively
/// otherwise), so the registry is available in both modes. Tool-less requests
/// skip fallback parsing entirely; this empty registry is therefore consulted
/// only alongside a non-empty advertised tool list.
fn pformat_registry_from_request(
request: &ModelRequest,
) -> crate::openhuman::agent::pformat::PFormatRegistry {
@@ -84,22 +84,25 @@ fn pformat_registry_from_request(
/// Translate an openhuman [`ChatResponse`] into a harness [`ModelResponse`]
/// (visible text + tool calls + token usage).
///
/// Native `tool_calls` take precedence; when absent, the response text is parsed
/// for prompt-guided (`<tool_call>…` / p-format) calls — matching the legacy
/// dispatcher — so text-mode models drive the tinyagents loop too. The visible
/// text is the prose with any tool-call markup stripped.
/// Native `tool_calls` take precedence; when absent and the request advertised
/// tools, the response text is parsed for prompt-guided (`<tool_call>…` /
/// p-format) calls — matching the legacy dispatcher — so text-mode models drive
/// the tinyagents loop too. Tool-less requests preserve response text verbatim,
/// including literal tool-call examples. When parsing is enabled, visible text
/// is the prose with any parsed tool-call markup stripped.
///
/// `pformat_registry` carries the advertised tools' positional layouts so the
/// text-mode fallback can recover P-Format (`name[a|b]`) calls that ~10 builtin
/// prompts still teach — the migrated parse path had dropped that grammar and
/// silently lost those calls (issue #4465). It is empty for the native-tool
/// path (where `response.tool_calls` is used directly) and for tool-less turns.
/// path, where `response.tool_calls` is used directly.
///
/// Unknown-tool recovery is handled by `RunPolicy::unknown_tool`, so the model
/// adapter preserves the provider-requested tool name.
fn response_to_model_response(
response: &ChatResponse,
pformat_registry: &crate::openhuman::agent::pformat::PFormatRegistry,
parse_text_tool_calls: bool,
) -> ModelResponse {
let (visible_text, tool_calls): (String, Vec<TaToolCall>) = if !response.tool_calls.is_empty() {
let calls = response
@@ -113,7 +116,8 @@ fn response_to_model_response(
})
.collect();
(response.text.clone().unwrap_or_default(), calls)
} else if let Some(text) = response.text.as_deref() {
} else if parse_text_tool_calls {
let text = response.text.as_deref().unwrap_or_default();
let (prose, parsed) =
crate::openhuman::agent::harness::parse_tool_calls_with_pformat(text, pformat_registry);
if parsed.is_empty() {
@@ -134,7 +138,7 @@ fn response_to_model_response(
(prose, calls)
}
} else {
(String::new(), Vec::new())
(response.text.clone().unwrap_or_default(), Vec::new())
};
let mut content = Vec::new();
@@ -632,7 +636,11 @@ impl ChatModel<()> for ProviderModel {
// Provider usage (charged USD / context window / cache-creation-reasoning)
// now reaches the event bridge via `UsageCarryMiddleware`, which reads it
// off the returned `ModelResponse` (G1) — the adapter no longer carries it.
Ok(response_to_model_response(&response, &pformat_registry))
Ok(response_to_model_response(
&response,
&pformat_registry,
!request.tools.is_empty(),
))
}
/// Stream the model response, forwarding openhuman's `ProviderDelta` events
@@ -650,6 +658,7 @@ impl ChatModel<()> for ProviderModel {
// Positional layouts for the text-mode P-Format fallback (issue #4465);
// built here so it can move into the `'static` producer task below.
let pformat_registry = pformat_registry_from_request(&request);
let parse_text_tool_calls = !request.tools.is_empty();
let provider = self.provider.clone();
let model = self.model.clone();
// Per-request temperature when set (see `invoke`), else the pinned value;
@@ -749,7 +758,11 @@ impl ChatModel<()> for ProviderModel {
// Provider usage rides the `Completed` response's crate `Usage`
// + raw (G1); `UsageCarryMiddleware` reads it off the folded
// response for the bridge, so the adapter no longer pushes here.
ModelStreamItem::Completed(response_to_model_response(&resp, &pformat_registry))
ModelStreamItem::Completed(response_to_model_response(
&resp,
&pformat_registry,
parse_text_tool_calls,
))
}
Err(e) => {
// Streaming failures ride `ModelStreamItem::Failed(String)`, which
@@ -827,7 +840,7 @@ mod g1_usage_tests {
}),
reasoning_content: None,
};
let model_response = response_to_model_response(&chat, &empty_registry());
let model_response = response_to_model_response(&chat, &empty_registry(), false);
// Crate Usage carries every token breakdown natively.
let usage = model_response.usage.expect("usage present");
@@ -860,7 +873,7 @@ mod g1_usage_tests {
}),
reasoning_content: None,
};
let model_response = response_to_model_response(&chat, &empty_registry());
let model_response = response_to_model_response(&chat, &empty_registry(), false);
assert!(
model_response.raw.is_none(),
"no charged USD / window ⇒ raw stays None"
@@ -879,9 +892,37 @@ mod g1_usage_tests {
usage: None,
reasoning_content: None,
};
let model_response = response_to_model_response(&chat, &empty_registry());
let model_response = response_to_model_response(&chat, &empty_registry(), false);
assert!(usage_info_from_response(&model_response).is_none());
}
#[test]
fn tool_less_response_preserves_literal_tool_call_markup() {
let text = r#"Example: <tool_call>{"name":"lookup","arguments":{}}</tool_call>"#;
let chat = ChatResponse {
text: Some(text.to_string()),
..Default::default()
};
let response = response_to_model_response(&chat, &empty_registry(), false);
assert_eq!(response.text(), text);
assert!(response.message.tool_calls.is_empty());
}
#[test]
fn tool_enabled_response_still_extracts_tool_call_markup() {
let chat = ChatResponse {
text: Some(r#"<tool_call>{"name":"lookup","arguments":{}}</tool_call>"#.to_string()),
..Default::default()
};
let response = response_to_model_response(&chat, &empty_registry(), true);
assert_eq!(response.text(), "");
assert_eq!(response.message.tool_calls.len(), 1);
assert_eq!(response.message.tool_calls[0].name, "lookup");
}
}
#[cfg(test)]