fix(tool_calling): stop dropping a GLM tag beside a p-format tag

[senamakel]

Stacked on #102 — based on harness-tool-calling so the diff is just the fix (2 files). GitHub retargets this to main automatically when #102 merges.

Fixes the silent tool-call loss documented as an #[ignore]d test in #102, and un-ignores it.

The bug

parse_tool_calls_with_pformat walks <tool_call>-family tags, taking a p-format body where one parses and re-parsing the rest as JSON. Once any tag yields a p-format call, the walk never falls back to the canonical parse — so every remaining tag is on its own, and the JSON path is all it has.

A GLM body is not JSON:

<tool_call>echo[hello]</tool_call>
<tool_call>shell/command>ls -la</tool_call>

The shell call was dropped. Silently: the agent asked for a tool, didn't get it, and nothing reported it.

Inherited, not introduced by the relocation. The probe fails identically against the pre-port OpenHuman code and against 45557a2. #102 landed it #[ignore]d specifically so it stayed visible rather than being quietly carried forward.

The fix, and the two ways it could have gone wrong

A GLM fallback for a tag body the JSON path could not read. Both risks it introduces are pinned by tests, because both fail silently:

1. Ordering is load-bearing — the fallback runs only when the JSON path found nothing. GLM's name/key>value shape can appear inside a JSON string value:

{"name": "shell", "arguments": {"command": "cat a/b>c"}}

An unconditional fallback counts that body once as JSON and again as GLM, and the agent executes the same tool twice. → a_json_body_is_not_double_counted_by_the_glm_fallback

2. Routing through parse_tool_calls was the obvious alternative and is wrong. That function deliberately forbids the args/parameters/input argument-key aliases for a bare top-level object, so a plain JSON answer cannot be misread as a tool call. But a <tool_call> tag is an explicit marker, where the aliases do apply. Routing through it would have traded this silent drop for a different one, on aliased tagged calls. → a_tagged_body_still_honours_argument_key_aliases

So the permissive JSON path stays exactly as it was, and GLM is tried only after it comes back empty.

Scope note

Only the GLM grammar is added to that branch. A markdown-fenced JSON body already works — extract_json_values finds the JSON inside the fence — and that is pinned by a_pformat_tag_does_not_suppress_a_sibling_fenced_json_tag, which passed before this change and still does.

Verification

• cargo test --all-features --lib — 1823 passed, 0 failed, 0 ignored (the ignore from feat(harness): add tool-call parsing as harness::tool_calling #102 is gone)
• cargo clippy --all-features --all-targets — clean
• cargo fmt --check — clean

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3892aaf4-f155-4a79-8798-203dddd851d9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}