Skip to content

fix(conversation): distinguish invalid tool-call arguments - #6237

Draft
yuhengcai1 wants to merge 1 commit into
OpenHands:mainfrom
yuhengcai1:codex/stuck-detector-invalid-arguments
Draft

yuhengcai1 wants to merge 1 commit into
OpenHands:mainfrom
yuhengcai1:codex/stuck-detector-invalid-arguments

Conversation

@yuhengcai1

Copy link
Copy Markdown

HUMAN:


AGENT:

Why

When tool validation fails, Agent._emit_tool_error() emits an ActionEvent with action=None and retains the original tool call. The stuck detector compares only the absent action, tool name, and narration, so distinct invalid argument values are treated as repeated identical calls. It can tell the agent that it used the same arguments three times and stop it after the fourth attempt even though each attempt changed the arguments.

Summary

  • When no executable action exists, require the retained argument strings to match before considering two calls identical. Validated actions continue using their existing comparison.
  • Apply the same rule in the TypeScript detector; absent raw call data cannot establish equality between null actions.
  • Exercise native validation failures through Conversation.run() and retain repeated-invalid-call positive controls.

Documentation: OpenHands/docs#1009

Issue Number

No existing issue identified for this defect. This branch starts directly from main and is independent of the narrative-thought change in #6235 and threshold fix in #6236.

How to Test

OH_PERSISTENCE_DIR=/tmp/openhands-stuck-check LITELLM_LOCAL_MODEL_COST_MAP=True \
  uv run --frozen pytest tests/cross/test_stuck_detector.py \
  tests/cross/test_stuck_detector_config.py \
  tests/sdk/conversation/local/test_stuck_detector_nudge.py -q

cd clients/typescript
npm ci
npm test -- --run src/__tests__/stuck-detector.test.ts
npm run test:coverage
npm run lint
npm run build
npm run format:check

On main ce5716a3c0a955a85bb0cc4dcfaa94d5fefcda50, native calls with valid JSON but invalid command types (0, 1, 2, 3 instead of a string) incorrectly end STUCK: 1 failed, 4 passed in the nudge suite. With the fix, the four distinct attempts consume the scripted final message and finish with no corrective nudge. Repeating the same invalid value still nudges and stops. All 17 related Python tests pass.

The TypeScript changing-arguments case also fails on base (1 failed, 18 passed before adding the missing-raw-data control). With the fix, all 20 detector tests and 399 tests across the full 27-file suite pass. Build, formatting, and pre-commit checks pass; ESLint has 0 errors and 5 existing warnings outside changed files.

Additional native controls pass: pause after two failures and resume without a new user message retains the streak (one nudge, stop after four total failures); inserting a new user message resets it (no nudge, normal finish); four distinct failing calls in a single LLM response do not trigger a repetition stop.

Runtime: macOS arm64, Python 3.13.17, Node 24.19.0, committed lockfiles. Python tests use TestLLM to supply messages and the real Agent, validation, events, and run loop. No paid inference or real-model benchmark was run. TypeScript's full suite needs localhost ports for temporary test fixtures.

Video/Screenshots

Non-visual fix; reproducible test commands and before/after outcomes are above.

Design Doc

Small comparison correction; no separate design document.

Type

  • Bug fix
  • Feature
  • Refactor
  • Breaking change
  • Docs / chore

Notes

The fallback compares retained argument strings exactly. It does not parse or repair invalid arguments, change validation, or redefine equality for executable actions. Thresholds, observation matching, and serialized events are unchanged.

The HUMAN section remains for the contributor. Please apply integration-test for agent-behavior evaluation; this contributor account lacks permission to add labels.

Co-authored-by: openhands <openhands@all-hands.dev>
@github-actions

Copy link
Copy Markdown
Contributor

📁 PR Artifacts Notice

This PR contains a .pr/ directory with temporary PR-specific documents. The directory will be automatically removed after this PR merges to main through a cleanup PR.

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