Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 58 additions & 17 deletions src/openhuman/tinyagents/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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() {
Expand All @@ -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();
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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"
Expand All @@ -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)]
Expand Down
Loading