From 93c7b3d6a3ea1c7eb24d54ab7730fabbdf8bddc9 Mon Sep 17 00:00:00 2001 From: YellowSnnowmann <167776381+YellowSnnowmann@users.noreply.github.com> Date: Tue, 16 Jun 2026 05:50:08 +0530 Subject: [PATCH] fix(agent): parse Claude-native attribute tool calls (#3622) Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: Steven Enamakel --- .../agent/harness/harness_gap_tests.rs | 108 +++++++++++++++ src/openhuman/agent/harness/parse.rs | 129 +++++++++++++++++- 2 files changed, 236 insertions(+), 1 deletion(-) diff --git a/src/openhuman/agent/harness/harness_gap_tests.rs b/src/openhuman/agent/harness/harness_gap_tests.rs index c6007e364..afb069b60 100644 --- a/src/openhuman/agent/harness/harness_gap_tests.rs +++ b/src/openhuman/agent/harness/harness_gap_tests.rs @@ -475,6 +475,114 @@ fn parse_tool_calls_invoke_tag_with_json_body() { ); } +// ───────────────────────────────────────────────────────────────────────────── +// Item 3b — parse_tool_calls: Claude-native attribute form +// with nested children (issue #3493). +// Claude-family models ignore the injected {json} template +// and emit their trained syntax; the parser must recover it instead of +// leaking the raw markup as assistant text. +// ───────────────────────────────────────────────────────────────────────────── + +#[test] +fn parse_tool_calls_invoke_attribute_form_single_param() { + use crate::openhuman::agent::harness::parse::parse_tool_calls; + + let input = + "Sure.\n\nhi\n\ndone"; + let (text, calls) = parse_tool_calls(input); + + assert_eq!( + calls.len(), + 1, + "attribute-form should parse one call" + ); + assert_eq!(calls[0].name, "echo"); + assert_eq!(calls[0].arguments, serde_json::json!({"value": "hi"})); + // Surrounding prose preserved; raw markup must not leak. + assert!(text.contains("Sure."), "text before tag preserved"); + assert!(text.contains("done"), "text after tag preserved"); + assert!( + !text.contains(" markup must not surface in assistant text" + ); + assert!( + !text.contains(" markup must not surface in assistant text" + ); +} + +#[test] +fn parse_tool_calls_invoke_attribute_form_multiple_params_scalar_policy() { + use crate::openhuman::agent::harness::parse::parse_tool_calls; + + // Multiple children. Scalar policy: a value that parses as JSON + // (number, bool) becomes that JSON type; anything else stays a string. A + // parameter with an empty name is skipped (it cannot key an argument). + let input = concat!( + "\n", + "rust parsers\n", + "5\n", + "true\n", + "ignored\n", + "" + ); + let (_text, calls) = parse_tool_calls(input); + + assert_eq!(calls.len(), 1, "should parse one call"); + assert_eq!(calls[0].name, "search"); + assert_eq!( + calls[0].arguments, + serde_json::json!({"query": "rust parsers", "limit": 5, "fuzzy": true}) + ); +} + +#[test] +fn parse_tool_calls_invoke_attribute_form_missing_close_tag_is_text() { + use crate::openhuman::agent::harness::parse::parse_tool_calls; + + // No closing : nothing to dispatch. The block is left as text + // rather than silently dropped. + let input = "before\n\nhi"; + let (text, calls) = parse_tool_calls(input); + + assert_eq!(calls.len(), 0, "unterminated yields no calls"); + assert!(text.contains("before"), "preceding text preserved"); + assert!( + text.contains("\nhi\n"; + let (_text, calls) = parse_tool_calls(input); + + assert_eq!(calls.len(), 0, "missing name attribute yields no calls"); +} + +#[test] +fn parse_tool_calls_mixed_tool_call_json_and_invoke_attribute() { + use crate::openhuman::agent::harness::parse::parse_tool_calls; + + // A canonical {json} block and a Claude-native attribute-form + // block in the same response are both recovered, earliest first. + let input = concat!( + "{\"name\":\"first\",\"arguments\":{\"a\":1}}\n", + "\ntwo\n" + ); + let (_text, calls) = parse_tool_calls(input); + + assert_eq!(calls.len(), 2, "both tag forms parsed"); + assert_eq!(calls[0].name, "first"); + assert_eq!(calls[0].arguments, serde_json::json!({"a": 1})); + assert_eq!(calls[1].name, "second"); + assert_eq!(calls[1].arguments, serde_json::json!({"b": "two"})); +} + #[test] fn parse_tool_calls_markdown_fence_yaml_like_json_body() { use crate::openhuman::agent::harness::parse::parse_tool_calls; diff --git a/src/openhuman/agent/harness/parse.rs b/src/openhuman/agent/harness/parse.rs index b1265c267..f8863b011 100644 --- a/src/openhuman/agent/harness/parse.rs +++ b/src/openhuman/agent/harness/parse.rs @@ -183,6 +183,92 @@ pub(crate) fn matching_tool_call_close_tag(open_tag: &str) -> Option<&'static st } } +/// ``) and Claude-native +/// attribute (``) forms. +const INVOKE_PREFIX: &str = "` open tag +/// (issue #3493). Matches `` form (next char `>`) is +/// intentionally skipped here; it is recognised as a literal tag with a JSON +/// body via [`TOOL_CALL_OPEN_TAGS`], preserving back-compat. +fn find_invoke_attr_tag(haystack: &str) -> Option { + let mut from = 0; + while let Some(rel) = haystack[from..].find(INVOKE_PREFIX) { + let idx = from + rel; + let after = &haystack[idx + INVOKE_PREFIX.len()..]; + match after.chars().next() { + Some(c) if c.is_whitespace() => return Some(idx), + _ => from = idx + INVOKE_PREFIX.len(), + } + } + None +} + +/// Scalar policy for `` values: a value that parses as JSON +/// (number, bool, null, array, object) is kept as that JSON type; anything +/// else — the common case of bare text — stays a string. Mirrors the tolerant +/// arg handling in [`parse_arguments_value`]. +fn parameter_scalar_value(raw: &str) -> serde_json::Value { + let trimmed = raw.trim(); + match serde_json::from_str::(trimmed) { + Ok( + value @ (serde_json::Value::Number(_) + | serde_json::Value::Bool(_) + | serde_json::Value::Null + | serde_json::Value::Array(_) + | serde_json::Value::Object(_)), + ) => value, + _ => serde_json::Value::String(trimmed.to_string()), + } +} + +/// Parse a Claude-native attribute-form invoke block whose text begins +/// immediately after the ``. `None` when the `name` attribute or the closing tag is +/// missing — the caller then leaves the markup as text rather than dropping it. +fn parse_invoke_attribute_block(after_prefix: &str) -> Option<(ParsedToolCall, usize)> { + static INVOKE_NAME_RE: LazyLock = + LazyLock::new(|| Regex::new(r#"name\s*=\s*"([^"]*)""#).unwrap()); + static PARAMETER_RE: LazyLock = LazyLock::new(|| { + Regex::new(r#"(?s)(.*?)"#).unwrap() + }); + + let open_end = after_prefix.find('>')?; + let attrs = &after_prefix[..open_end]; + let name = INVOKE_NAME_RE + .captures(attrs) + .and_then(|c| c.get(1)) + .map(|m| m.as_str().trim().to_string()) + .filter(|n| !n.is_empty())?; + + let body = &after_prefix[open_end + 1..]; + let close_rel = body.find("")?; + let inner = &body[..close_rel]; + + let mut arguments = serde_json::Map::new(); + for cap in PARAMETER_RE.captures_iter(inner) { + // Groups 1 (name) and 2 (value) are mandatory in the pattern, so a + // captured match always has both — index access is safe. + let key = cap[1].trim(); + if key.is_empty() { + continue; + } + arguments.insert(key.to_string(), parameter_scalar_value(&cap[2])); + } + + let consumed = open_end + 1 + close_rel + "".len(); + Some(( + ParsedToolCall { + name, + arguments: serde_json::Value::Object(arguments), + id: None, + }, + consumed, + )) +} + pub(crate) fn extract_first_json_value_with_end(input: &str) -> Option<(serde_json::Value, usize)> { let trimmed = input.trim_start(); let trim_offset = input.len().saturating_sub(trimmed.len()); @@ -439,7 +525,48 @@ pub(crate) fn parse_tool_calls(response: &str) -> (String, Vec) } // Fall back to XML-style tool-call tag parsing. - while let Some((start, open_tag)) = find_first_tag(remaining, &TOOL_CALL_OPEN_TAGS) { + loop { + let literal = find_first_tag(remaining, &TOOL_CALL_OPEN_TAGS); + let invoke_attr = find_invoke_attr_tag(remaining); + + // Choose the earliest-positioned recognised open tag. The bare + // `` literal and the attribute form `` never collide + // at one offset (one is followed by `>`, the other by whitespace), so a + // simple index comparison disambiguates them (issue #3493). + let use_invoke_attr = match (invoke_attr, literal.as_ref()) { + (Some(i), Some((l, _))) => i < *l, + (Some(_), None) => true, + _ => false, + }; + + if use_invoke_attr { + let start = invoke_attr.expect("use_invoke_attr implies Some"); + let before = &remaining[..start]; + if !before.trim().is_empty() { + text_parts.push(before.trim().to_string()); + } + + let after_prefix = &remaining[start + INVOKE_PREFIX.len()..]; + if let Some((parsed, consumed)) = parse_invoke_attribute_block(after_prefix) { + calls.push(parsed); + remaining = &after_prefix[consumed..]; + continue; + } + + // Unparseable attribute-form block (no `name`/no close tag): leave + // it and the rest as text instead of silently dropping content. + tracing::warn!( + body_chars = after_prefix.chars().count(), + "[agent_parse] malformed attribute block: missing name or close tag" + ); + remaining = &remaining[start..]; + break; + } + + let Some((start, open_tag)) = literal else { + break; + }; + // Everything before the tag is text. let before = &remaining[..start]; if !before.trim().is_empty() {