Skip to content

Unparseable tool arguments fail LOUD instead of being wrapped as _raw (#334) - #2159

Open
joelteply wants to merge 2 commits into
canaryfrom
fix/334-unparseable-tool-args
Open

Unparseable tool arguments fail LOUD instead of being wrapped as _raw (#334)#2159
joelteply wants to merge 2 commits into
canaryfrom
fix/334-unparseable-tool-args

Conversation

@joelteply

Copy link
Copy Markdown
Contributor

What

Glass-boxed from Asha's live capture. Devstral emitted write_file whose file_path ran away into a repeating token block, breaking the JSON. The adapter did:

serde_json::from_str(&t.arguments)
    .unwrap_or_else(|_| json!({ "_raw": t.arguments }))

That is a fallback, and a lossy one. Nothing in the tree reads _raw — verified, zero consumers — so the real cause ("the model's arguments are not valid JSON") was destroyed at the one seam that knew it, and a params object carrying a key nobody reads went downstream as if valid. The failure resurfaced elsewhere as a misleading typed-deser error about a field the model never successfully emitted.

Now

  • ai.tool_call.unparseable_args probe: tool name, the parser's own error, argument length, and a 200-char head (bounded deliberately — the corruption can be a runaway token block and must never flood the probe).
  • Marker becomes __malformed_tool_arguments { error, raw } — self-describing, so whatever rejects the call can say what actually went wrong.

Scope, stated honestly

This makes the failure visible and correctly named. It does not yet route a "your tool arguments were malformed, here is the parser error" result back into her working memory so she can retry — that belongs with #159's fail-loud work and is the next slice, not something to half-do here.

Not a fix for #181 (reasoning-channel repetition). That is a genuine sampling loop; this is a routing/validation defect that merely looked like one — the runaway text was in a field the model was forced to fill and had no value for.

Verification

unparseable_tool_arguments_are_marked_malformed_never_silently_wrapped — 28 adapter tests green, full cargo check clean.

[[fallbacks-are-illegal-fail-loud]]

🤖 Generated with Claude Code

https://claude.ai/code/session_01LoTjvf5j3Ez13g6k8mRkFo

joelteply and others added 2 commits August 6, 2026 16:56
…d as `_raw` (#334)

Glass-boxed from Asha's live capture. Devstral emitted `write_file` whose `file_path` ran
away into a repeating token block, which broke the JSON. The adapter did this:

    serde_json::from_str(&t.arguments)
        .unwrap_or_else(|_| json!({ "_raw": t.arguments }))

That is a fallback, and a lossy one. NOTHING in the tree reads `_raw` — verified, zero
consumers — so the real cause ("the model's arguments are not valid JSON") was destroyed
at the one seam that knew it, and a params object carrying a key nobody reads was handed
downstream as if it were valid. The failure then resurfaced somewhere else as a
misleading typed-deser error about a field the model never successfully emitted.

Now: a `ai.tool_call.unparseable_args` probe fires with the tool name, the parser's own
error, the argument length, and a 200-char head (bounded deliberately — the corruption
can BE a runaway token block and must never flood the probe), and the marker becomes
`__malformed_tool_arguments { error, raw }` — self-describing, so whatever rejects the
call can say what actually went wrong instead of inventing a missing-field story.

Scope note: this makes the failure VISIBLE and correctly named. It does not yet route a
"your tool arguments were malformed, here is the parser error" result back into her
working memory so she can retry — that belongs with #159's fail-loud-on-unknown-tool work
and is the next slice, not something to half-do here.

Not a fix for #181 (reasoning-channel repetition). That is a genuine sampling loop; this
is a routing/validation defect that merely looked like one — the runaway text was in a
field the model was FORCED to fill and had no value for.

[[fallbacks-are-illegal-fail-loud]]

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LoTjvf5j3Ez13g6k8mRkFo
…t has files (#336)

Glass-boxed from a live A/B. Same task, same persona, only the path differs:

  src/main.rs           → map describes the checkout, she read→edit→test, correct fix
  .gymtool/solution.rs  → map says: "It is EMPTY — there are no files or directories
                          here yet. There is nothing to read, list, or search …
                          To build something, CREATE files directly with code/write"

Every clause false, delivered through her highest-trust channel as a structural fact. It
does not merely omit the file — it asserts absence, denies that reading can work at all,
and steers her to create from scratch. That is a confabulation we authored and handed to
her as grounding. She recovered by trusting the task text over her own perception, and
still paid for it: after a successful code/edit she ALSO issued a full code/write of the
file, which reads as belt-and-braces against a workspace her map insisted was empty.

TWO bugs, both fixed at the source:

1. `list_dir(".", false)` — hidden entries excluded, so a dot-directory was invisible.
   Now `true` at all THREE reader sites (cwd, fixed-root, citizen-layer). The
   citizen-layer one is the reader personas actually use, so it was the one lying.

2. The empty gate was `top_level_dirs.is_empty()` — a claim about DIRECTORIES wearing
   the words "no files or directories". A workspace holding only `main.rs` at the root
   would ALSO have been declared empty. WorkspaceLayout now carries `top_level_entries`
   (files + dirs, hidden included) and `is_truly_empty()` is the only thing permitted to
   trigger the empty branch.

Why it matters beyond the one probe: tool-bugfix-rs puts EVERY task's file under
`.gymtool/`, so all three tasks of today's 0/3 ran with their workspace declared empty
and their persona steered toward create-not-read. Not proven to be the cause — she beat
it here — but no tool-gym number is worth anything while it stands.

The genuinely-empty grounding (#206, build-first so a from-scratch task doesn't loop on
the void) is preserved and pinned by the same test.

Doctrine this restores: [[empty-workspace-is-a-confabulation-not-infra]] — the memory was
already written; the defect was still shipping.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LoTjvf5j3Ez13g6k8mRkFo
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.

1 participant