Skip to content

fix(agent): decode element-form calls for offered tools - #31

Merged
senamakel merged 6 commits into
tinyhumansai:mainfrom
M3gA-Mind:fix/element-calls
Sep 28, 2026
Merged

senamakel merged 6 commits into
tinyhumansai:mainfrom
M3gA-Mind:fix/element-calls

Conversation

@M3gA-Mind

@M3gA-Mind M3gA-Mind commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #30 (which is stacked on #29). This branch carries #29's commits (fdb223f, 32d6eb4), #30's (396e15c, cc7107b), and this PR's commits 5f7161d and 10622c4 (review follow-up). Please review only those two. I'll rebase onto main as the lower PRs merge.

Summary

Under the Python code dialect, DeepSeek V4 Flash writes some calls as plain elements: <todo>\n<todos>\n[{…}, …]\n</todos>\n</todo>. That is the tool name as the tag and each parameter as a child. No grammar read this form, so the call was dropped and the turn ended on narration.

This PR adds a new grammar/element.rs, placed last in GRAMMARS, plus CallSource::Element (the enum is #[non_exhaustive]). Because any tag could be a tool name, it is gated hard. A block is claimed only when:

  1. <NAME> has no attributes, NAME is an offered tool with a P-Format registry entry, and NAME is not a tag another grammar owns;
  2. the matching </NAME> is present;
  3. the body is child elements and whitespace, nothing else.

A claimed block decodes to a call when every child is a parameter of NAME and every [/{ value is valid JSON. Other values are read the same way invoke_xml reads them. Otherwise the block is Decoded::Malformed, so the host gets a MalformedBlock { source: Element, .. } diagnostic instead of a silent drop. A block that fails the gate is left in the text untouched.

In a stream, a trailing partial <na… that could still become an eligible tool's opener is held back. Tool names are not static openers(), so without this the scrubber released <todo as visible text before its > arrived. An opener whose closer hasn't arrived is held only while its body is still a viable child-only prefix. A mis-closed <todo>…</todos></tool_call> therefore releases the rest of the stream live instead of stalling it until flush (10622c4). Reserved names (tool_call, invoke, function, …) are never parameter children, so a malformed element wrapping a real <tool_call> leaves that call to its own grammar.

Widened surface: as before, a call outside any language-tagged fence executes, and that includes inline-code examples. Element form widens this from the fixed call markers to any offered tool name used as a tag. Under the registry gate, a model writing <todo><todos>[…]</todos></todo> as an illustration will dispatch it.

Registry gate: parameter names reach the parser only through the registry, which the harness passes for the P-Format/code dialects. The Xml dialect never sees element calls; the module carries a ponytail: note naming that ceiling.

Known, intentional gap: in the user's record 27, the leading todo block closes with </todos>\n</tool_call> instead of </todo>; the model mismatched the closer. The matching-closer gate correctly declines it, so that todo is not recovered. The same record's GITHUB_SEARCH_REPOSITORIES call still is, via #29/#30.

Related issue

Refs tinyhumansai/openhuman#6722. Verified on the real user records with a registry built from their recorded schemas (counts only): five todo records that previously gave 0 calls now give 1 call each (todo, Element) with 0 diagnostics. Record 27 gives its invoke call and, as intended, not its mis-closed todo. Prose and <div> give 0. A code call plus a todo element gives both, in order.

API or behavior changes

  • Additive: CallSource::Element (the enum is #[non_exhaustive], so this is not breaking).
  • Behaviour: with a registry, a gated element block is now a call or a MalformedBlock. Before this change it was plain text.

Validation

Commands actually run, with their outcome:

  • cargo fmt --all -- --check: clean
  • cargo clippy --all-targets --all-features -- -D warnings: ran without --all-features (cargo clippy --all-targets -- -D warnings): clean
  • cargo build --all-targets --all-features: not run locally with --all-features; CI runs it
  • cargo test --all-features: ran cargo test (default features): all pass (113 + 346 + 20 + 1)

Also ran RUSTDOCFLAGS="-D warnings" cargo doc --no-deps: clean.

Tests

The new src/parse/test/element.rs has 10 tests.

Fixtures:

  • a mis-closed todo element streamed in 7-byte chunks → narration after it is released before flush, and the fenced invoke is still recovered;
  • a malformed <todo> wrapping a real <tool_call> → the inner call survives, with no MalformedBlock;
  • a todo element → 1 call whose args hold the JSON array;
  • a todo element followed by a ```<tool_call> wrapped invoke → [todo, tool_search] in order;
  • a streamed todo in 5-byte chunks → 1 call with no markup shown;
  • a child that is not a parameter → MalformedBlock;
  • undecodable JSON → 0 calls and 1 MalformedBlock from both parse_text and StreamScrubber, with the markup not shown.

Controls:

  • todo not offered → 0 calls, text kept;
  • no registry → 0 calls;
  • <div><p>x</p></div> → 0 calls, no diagnostic;
  • prose inside <todo> → 0 calls, no diagnostic, text kept;
  • a ```xml fence → 0 calls.

Revert check: with Element removed from GRAMMARS, the first 5 element fixtures fail and all 5 controls pass. With the stream hold made unconditional, only the stall test fails. With reserved names allowed as children, only the survive test fails.

Documentation

grammar/element.rs has module docs that cover the gate, the malformed rule and the registry ceiling.

Checklist

  • The change is focused on one logical change
  • No new #[allow(...)], #[ignore], or relaxed lints
  • No secrets, tokens, or .env contents in the diff or the description

Review status: tinysweeper/review timed out at both heads, 5f7161d and 10622c4 (900s, 'No code was reviewed'), so tinysweeper has never reviewed this PR. It is advisory, not a required check. The fleet reviewer reviewed 5f7161d and re-checked 10622c4 (346/346 on a clean target dir).

Told to call tools inside <tool_call> tags, DeepSeek V4 writes its
native invoke XML there. The tagged grammar claimed the block, found no
JSON body, and dropped the call as malformed, though the same invoke
parses bare. When a tag body opens with a named invoke, decode it with
invoke_xml. The anchor keeps an invoke quoted inside other body text
from executing.

Refs tinyhumansai/openhuman#6722
Adds a closed JSON tag body quoting an invoke (fails without the anchor)
and a <tool_call><function=…> body (dropped on main). Narrows the
decode_body doc to what the anchor guarantees: only the first invoke is
anchored, as on the bare path.
DeepSeek V4 Flash wrote the opener as the fence info string,
```<tool_call>, and never closed the fence. The info string read as a
language, so the unclosed fence protected the call to end of text as
an example and it was dropped. A fence whose info string opens with a
complete call tag (tag-family opener, named invoke, bare <invoke>) is
now a call fence. A language-tagged fence still protects its contents.

Refs tinyhumansai/openhuman#6722
opens_with_call_tag reused NAMED_INVOKE_OPEN_RE, which also accepts
<function …> and any XML namespace, so an XSLT fence such as
```<xsl:function name="f"> became a call. Only a tag-family opener or
an (optionally DSML-prefixed) <invoke> now marks a call fence.
DeepSeek V4 Flash writes some calls as plain elements under the Python
code dialect: <todo><todos>[…]</todos></todo>. No grammar read the form,
so the call was dropped and the turn ended on narration.

A new registry-gated grammar claims <NAME>…</NAME> when NAME is an
offered tool with a registry entry and the body is child elements only.
It decodes to a call when every child is a parameter and every JSON-
looking value parses, and reports a MalformedBlock otherwise. Anything
else stays in the text. In a stream, a partial opener of an eligible
tool is held back so the markup is not released as text.

Refs tinyhumansai/openhuman#6722
@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 44a39dc0-2a59-479e-b77f-6f90446483aa


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

@tinysweeper

tinysweeper Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

⚠️ Review failed for 10622c4fd911. the review of #31 did not finish within 900s

@M3gA-Mind

Copy link
Copy Markdown
Contributor Author

Review of 5f7161d only (comment only). 345/345 tests pass locally at this head. I ran probes locally (not committed) through parse_text and a StreamScrubber with known tools and a registry, as the harness wires them (tinyagents-harness/src/agent_loop/dialect.rs:399-401, :470-472).

1. Known-tool + registry gate: sound, and strict. ParseOptions::knows is an exact match, and an empty list means false (types.rs:151). So a block needs a registry entry and an offered name, and must not be a RESERVED tag. <div><p>, narration like The <todo> tag holds <todos> entries.</todo>, and a ```python fence all give 0 calls and 0 diagnostics. One consequence to be aware of: openhuman's own prompt-guided path parses with parse_tool_calls_with_pformat(text, registry) (openhuman-core/src/agent/tinyagents/model.rs:92), which sets a registry but no known tools. So the element grammar can never fire there. The fix reaches only turns parsed by the harness dialect. Worth confirming that is the path the affected users' turns take.

2. Stream hold-back: the partial opener is fine, but a complete opener stalls the rest of the stream.

  • partial_opener resolves itself: "x <t" + "han y" is released as soon as the prefix stops matching, and a dangling "<to" is released at flush.

  • But once a complete eligible <todo> arrives without its </todo>, probe_decided returns Probe::Pending (element.rs, the find(&closer) miss in ScanMode::Stream). Everything after it is held until flush, even when the body can no longer be a child-only block:

    • "I'll update the " | "<todo>" | " list now. " | …: only "I'll update the " is released live, and the rest arrives at flush.
    • The record-27 shape (a todo mis-closed with </todos>\n</tool_call>, then narration, then a ```<tool_call> invoke), fed in 7-byte chunks: 0 characters released live, and all 97 visible characters appear at flush. The tool_search call is still recovered (1 call), so this is a streaming UX stall, not data loss. But it is triggered by exactly the output this PR targets.

    Suggested fix: in stream mode, return Pending only while the buffered body is still a viable prefix of child-only content (whitespace, <child>…</child> runs, or a partial child), and continue as soon as it isn't. The record-27 body becomes ineligible at </tool_call>, and the prose case at " list".

3. Prose inside a known tool's tags is not claimed, which matches the controls. Two edges to note:

  • Inline code executes. To add items, write `<todo><todos>[…]</todos></todo>` in your reply. gives a todo call, and the text becomes "To add items, write `\n` in your reply.". This is existing policy, not new: the same sentence with a backticked <tool_call>{…}</tool_call> or <invoke …> also dispatches today. But this grammar widens it to any offered tool name used as a tag (<shell><command>…</command></shell> in an explanation). Consider protecting inline code spans at least for this grammar, or pin the current behaviour with a test so it's a deliberate choice.
  • A claimed-but-malformed block swallows a real call inside it. <todo>\n<todos>[]</todos>\n<tool_call>{"name":"tool_search",…}</tool_call>\n</todo> gives 0 calls and one MalformedBlock { source: Element }. children() accepts <tool_call> as a child name, decode rejects it, and the whole span is consumed. Since RESERVED already lists the protocol tags, rejecting a reserved child name in children() would leave the inner call to its own grammar.

Minor: a prose value inside a parameter child is taken as a string (<todo><todos>just some prose</todos></todo> gives todos: "just some prose"), even though the schema says array. The tool will reject it, so it's harmless, but a type check against the registry schema could make it Malformed instead.

An element opener with no closer held the whole rest of the stream
until flush. Hold only while the body is still a viable child-only
prefix, so a mis-closed <todo>…</tool_call> releases text live.

Reserved names (tool_call, invoke, …) are no longer parameter children,
so a malformed element wrapping a real <tool_call> leaves that call to
its own grammar instead of swallowing it.
@M3gA-Mind

Copy link
Copy Markdown
Contributor Author

Addressed in 10622c4:

  1. Stall: an element opener with no closer now returns Pending only while the body so far is a viable child-only prefix (viable_prefix: complete children, then at most one partial child or a partial outer closer). New test a_mis_closed_todo_element_does_not_stall_the_stream uses the sanitized mis-closed shape in 7-byte chunks and asserts the narration is released before flush. With the hold made unconditional again, it fails at element.rs:199.
  2. Swallow: reserved names (the RESERVED list, compared case-insensitively) are rejected as parameter children, so the element is left unclaimed and the inner <tool_call> decodes. New test a_call_inside_a_malformed_element_survives. With reserved children allowed, it fails at :210.
  3. Second parse path: the model.rs:92 path is not live in production. prompt_guided_text_response and native_model_response_for_request are called only from tests (agent_tests.rs, model_g1_usage_tests_tests.rs), and production uses native_model_response with parsing off. Code(Python) turns recover through the harness: run_loop.rs:1159 (forced_text_dialect) → recover_text_dialect_calls → dialect.rs recover_text_calls, with known tools and registry_for. No openhuman change is needed for element calls.
  4. I noted in the PR body that element form widens the executes-outside-a-language-fence surface to any offered tool name used as a tag.
    Gates: fmt, clippy -D warnings, doc and cargo test (346 lib tests) are clean.

@M3gA-Mind

Copy link
Copy Markdown
Contributor Author

Re-checked at 10622c4. Both points are fixed, and the new tests bite. 348/348 locally, in an isolated target dir.

  • Stall: the record-27 shape streamed in 7-byte chunks now releases all 97 visible chars live (was 0) and still yields the tool_search call. Prose after a bare <todo> streams as it arrives.
  • Reserved children: <todo><todos>[]</todos><tool_call>{…}</tool_call></todo> now yields the inner tool_search call with no MalformedBlock.
  • Revert-check: with element.rs restored to 5f7161d, exactly a_mis_closed_todo_element_does_not_stall_the_stream and a_call_inside_a_malformed_element_survives fail.

Two small notes, neither blocking:

  • viable_prefix still holds while an opened child is unclosed (<todo><x> followed by prose that never closes x). That's a much narrower stall than before, but the same shape. A length cap on the held body would bound it.
  • The mis-closed todo on record 27 is (as intended) left as text, so the user sees <todo>\n<todos>[…]</todos> plus a stray ``` from the call fence's opener line. That's cosmetic, and it's the "known gap" in the PR body.
  • Inline code (`<todo><todos>…</todos></todo>`) still dispatches. That's the existing policy shared with <tool_call>/<invoke>, so it's consistent, but it's still unpinned by a test.

@M3gA-Mind

Copy link
Copy Markdown
Contributor Author

Correction to my count above: the "348/348" included 2 local probe tests (w5_probe_element, w5_probe_element_2) that exist only in my review worktree. The PR's own tinytools-agent lib count at 10622c4 is 346, and all pass. The findings are unchanged.

@senamakel
senamakel merged commit 82c0d97 into tinyhumansai:main Sep 28, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants