Skip to content

test(harness): pin the actionable error text for unrepairable tool args - #586

Merged
M3gA-Mind merged 2 commits into
tinyhumansai:mainfrom
M3gA-Mind:test/arg-recovery-actionable-error
Oct 27, 2026
Merged

M3gA-Mind merged 2 commits into
tinyhumansai:mainfrom
M3gA-Mind:test/arg-recovery-actionable-error

Conversation

@M3gA-Mind

@M3gA-Mind M3gA-Mind commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

What

Adds arg_recovery_keeps_the_actionable_error_text_for_unrepairable_arguments to agent_loop/mod_tests.rs. It runs the agent loop with ArgRecoveryMiddleware installed, the way the host stack does, and sends a provider-invalid tool call that relaxed-JSON repair cannot fix ({"query": [1 2]}). It asserts that:

  • the tool is never executed;
  • the transcript has the corrective word for word: "Tool call lookup was not run: its arguments are not valid JSON", plus "Re-send it as well-formed JSON that matches the tool's argument schema".

The existing unrepairable_arguments_are_answered_with_an_actionable_error runs without the middleware, and only checks "invalid JSON arguments" and the schema phrase. It never checks the "not valid JSON" / "Re-send" text that the model acts on.

Revert check (cuttlefish, at 53464e9)

  • Dropping the words "Re-send it as well-formed JSON that matches the tool's argument schema. " from unparseable_tool_call_message (tools.rs) makes the test fail at the Re-send assert. Restoring the file makes it pass.
  • Bypassing ArgRecoveryMiddleware's provider-invalid branch (if false && call.invalid.is_some()) leaves the test passing: the loop's own unparseable-call path still produces the same text. So the test pins the wording, not which layer produces it, and its doc comment says so.

Checks (cuttlefish, at f8bdfcc)

  • cargo fmt --all --check: clean
  • the new test: pass; agent_loop:: at 53464e9: 541 passed, 0 failed
  • cargo clippy -p tinyagents-harness --tests -- -D warnings: exit 0

Test-only change: +43 lines in one file.

Summary by CodeRabbit

  • Tests
    • Added regression coverage for invalid tool arguments that cannot be repaired. The test verifies that the tool is not run and that the conversation displays an explanation of the invalid JSON along with guidance to resend the arguments in the expected format. This improves confidence that unrepairable input is handled clearly and does not trigger unintended tool execution.

With ArgRecoveryMiddleware installed, an unrepairable provider-invalid call
must still reach admission with its raw string and be answered with the
"not valid JSON ... Re-send it as well-formed JSON" corrective.
@tinysweeper

tinysweeper Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Reviewed head: f8bdfcc09f14e85a73c4322f3a6aca2552f26cc3

Verdict: Ready for maintainer review · Priority: none
Coverage: Reviewed 1 of 1 files.
Pull request purpose (title): test(harness): pin t…

Diagnostics and supporting details

Full summary

Pull request purpose (title): test(harness): pin the actionable error text for unrepairable tool args. Tiny Sweeper completed its review; deterministic results follow.

Review snapshot

Change surface Files Review signal Count
Production 0 Active findings 0
Tests 1 Noted findings 0
Documentation 0 Resolved findings 0
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

Features

  • Added — Test pinning the actionable error text for unrepairable tool arguments: Locks in the exact corrective wording surfaced when a provider-invalid, unrepairable tool call is not executed under `ArgRecoveryMiddleware`; the doc comment states it pins the wording of `unparseable_tool_call_message`, not which layer produces it. (crates/tinyagents-harness/src/agent_loop/mod_tests.rs async fn unrepairable_arguments_are_answered_with_an_actionable_error() {)

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Before merge

None.

Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The critique lane is answered and reports no concrete defect; it notes the names used by the test exist at the reviewed head and the assertions match the recoverable-default behavior pinned by the surrounding suite, and it explicitly declined to report a speculative finding about which layer emits the exact wording.
  • Lane summary: The added test is a new test in the harness agent-loop test file; all names it uses (`invalid_tool_call_response`, `text_response`, `crate::testkit::FakeTool::returning`, `FixedModelResolver::named`, `ArgRecoveryMiddleware::new`) exist at the reviewed head, and the assertions match the recoverable-default behaviour the surrounding suite pins. One thing I could not verify from the visible evidence is the exact wording produced by `unparseable_tool_call_message` (whether `ArgRecoveryMiddleware` or the loop layer emits it), so the two string assertions are reported at low confidence only if I had evidence to the contrary — none is available here. I have no concrete defect to report, so I am returning no findings rather than a speculative one. _The code index is behind this pull request (indexed at `af72f7f3bd9c`), so retrieved context may be out of date._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The security lane is answered and reports that the change introduces no production code, no new sinks, and no security-relevant surface, with nothing in the diff being a reportable defect.
  • Lane summary: This change adds a single async test to `crates/tinyagents-harness/src/agent_loop/mod_tests.rs` that pins the corrective wording produced for unrepairable tool arguments; it introduces no production code, no new sinks, and no security-relevant surface. Nothing in the diff is a reportable defect. _The code index is behind this pull request (indexed at `af72f7f3bd9c`), so retrieved context may be out of date._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The tests lane is answered and reports no defects, noting the change is test-only so no test was needed for it and none is missing, and that the test's assertions are concrete and would catch regressions in either the fail-closed non-execution or the wording.
  • Lane summary: The diff adds only a test (plus its doc comment) to an existing test module; it changes no production behaviour, so no test was needed for it and none is missing. The test's assertions are concrete (tool did not run; transcript contains the two exact corrective phrases), it exercises the real loop end to end via AgentHarness, and it would fail if either the fail-closed non-execution or the wording of `unparseable_tool_call_message` regressed. The one invariant the doc comment asserts — that this pins the wording rather than which layer produces it — is consistent with the test's observable checks. No defects found. _The code index is behind this pull request (indexed at `af72f7f3bd9c`), so retrieved context may be out of date._

commits

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The commits lane is answered and reports no sensitive content in committed files.
  • Lane summary: No sensitive content in committed files. _The code index is behind this pull request (indexed at `af72f7f3bd9c`), so retrieved context may be out of date._

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The description lane is answered and reports no defects, noting the test is accurate about what it exercises, the asserted strings match `unparseable_tool_call_message` in tools.rs verbatim, and nothing in the diff changes production behavior.
  • Lane summary: Adds a single test-only change to `agent_loop/mod_tests.rs` that pins the exact corrective wording produced for a provider-invalid, unrepairable tool call while `ArgRecoveryMiddleware` is installed. The test is accurate about what it exercises (the doc comment and PR body both note it pins the wording, not the producing layer), the strings it asserts match `unparseable_tool_call_message` in tools.rs verbatim, and nothing in the diff changes production behaviour. No defects found. _The code index is behind this pull request (indexed at `af72f7f3bd9c`), so retrieved context may be out of date._
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash
  • Spend: $0.004674
  • Tokens: 94541 input · 2665 output · 20096 cached · 179 embedding
  • Updated: 2026-10-27T18:15:55Z
Head State Pass summary
f8bdfcc09f14 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-27T18:15:55Z)

tinysweeper 0.1.2

@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 680b8b57-c74c-4f5c-b798-405f73589f63
Commits

Reviewing files that changed from the base of the PR and between 45c9d8c and f8bdfcc.

Files selected for processing: 1
  • crates/tinyagents-harness/src/agent_loop/mod_tests.rs

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

Walkthrough

Adds an end-to-end regression test for unrepairable tool arguments with ArgRecoveryMiddleware. The test checks that the run completes, the tool is not executed, and the transcript contains parse-error and corrective resend messages.

Changes: Tool argument recovery

Layer / File(s) Summary
Middleware recovery regression test
crates/tinyagents-harness/src/agent_loop/mod_tests.rs
Adds a test that checks the run result, confirms the tool is not called, and verifies both corrective transcript messages.

Priority: Low

Estimated code review effort: 1 (Trivial) | ~4 minutes

Change: Other

Possibly related issues

  • tinyhumansai/openhuman issue 6999: The test covers rejected tool calls and checks for corrective text when malformed arguments cannot be repaired.

Possibly related PRs

  • tinyhumansai/tinyagents#584: This PR adds middleware-installed regression coverage for the argument recovery behavior and corrective text changed in that PR.

Suggested reviewers: senamakel

Merge Risk: Minimal · up to f8bdf

No merge-blocking risk was identified in this test-only change.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the harness test and its purpose: preserving actionable error text for unrepairable tool arguments. It matches the main change.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within survi…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit checks the tool-call trail,
The broken arguments cannot prevail.
No tool runs on malformed fare,
A parse error and resend guide appear.
The transcript keeps the answer clear.

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

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tinysweeper found nothing blocking. Approving.

Diagnostics

Total spend: $0.0047

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Oct 27, 2026
@M3gA-Mind
M3gA-Mind merged commit 77aa282 into tinyhumansai:main Oct 27, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant