Skip to content

Feed malformed client-side tool call args back to the model#47

Open
ScriptSmith wants to merge 2 commits into
mainfrom
fix/client-tool-args-feedback
Open

Feed malformed client-side tool call args back to the model#47
ScriptSmith wants to merge 2 commits into
mainfrom
fix/client-tool-args-feedback

Conversation

@ScriptSmith
Copy link
Copy Markdown
Owner

No description provided.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 4, 2026

Greptile Summary

This PR fixes two bugs in the client-side tool-call pipeline: the SSE tracker was keyed on the provider call_id (toolu_xxx) instead of the item id (fc_xxx), causing all streaming argument deltas to miss their tracker entry; and malformed argument JSON previously returned an error result that silently dropped the call, leaving the turn in a false "completed" state. Both are replaced with a graceful invalid marker that surfaces the call through the normal completion path so the tool loop can feed the parse error back to the model.

  • Tracker re-keyed on item id: output_item.added and all downstream delta/done handlers now use item.id (fc_xxx) as the map key; callId is kept as a separate field for building function_call_output continuations.
  • Invalid-argument recovery loop: arguments.done and output_item.done catch JSON parse errors and set state.invalid rather than returning an early error; output_item.done can also clear a stale invalid marker when its payload parses successfully. executeToolCalls feeds invalidArgumentsText(...) back as a function_call_output for any call flagged invalid.
  • Fallback path updated: The response.completed fallback in useChat.ts mirrors the same try/catch pattern to produce invalid-flagged entries without throwing and stranding sibling calls.

Confidence Score: 5/5

Safe to merge; the changes are narrowly scoped to error-handling paths that previously either silently dropped tool calls or crashed the streaming handler.

Both the tracker key fix and the invalid-argument recovery are well-tested by the new test suite. The only rough edge is that the functionCallItems continuation reconstructs an invalid call's arguments as "{}" rather than the original malformed string, which may slow the model's self-correction but does not break the loop.

The functionCallItems block in useChat.ts around line 2193 is the one spot where the new invalid-call flow interacts with pre-existing logic that was not updated to preserve the raw argument buffer.

Important Files Changed

Filename Overview
ui/src/pages/chat/utils/toolCallParser.ts Fixes tracker keying (item_id vs call_id), adds invalid state to surface unparseable tool calls rather than dropping them, adds recovery logic in output_item.done, and exports invalidArgumentsText helper.
ui/src/pages/chat/utils/toolExecutors.ts Adds an early-exit guard in executeToolCalls that feeds the parse error back as a function_call_output for invalid tool calls instead of attempting execution.
ui/src/pages/chat/useChat.ts Adds invalid-argument handling in the response.completed fallback path; however, the existing functionCallItems reconstruction loses the raw malformed argument string, sending "{}" to the model instead.
ui/src/pages/chat/utils/tests/toolCallParser.test.ts New test file covering item_id vs call_id keying, invalid-argument marking, output_item.done recovery, and invalidArgumentsText format — good coverage of all new paths.

Sequence Diagram

sequenceDiagram
    participant SSE as SSE Stream
    participant Parser as toolCallParser
    participant Tracker as ToolCallTracker
    participant Executor as executeToolCalls
    participant Model as Model (next round)

    SSE->>Parser: "output_item.added (id=fc_xxx, call_id=toolu_xxx)"
    Parser->>Tracker: set(fc_xxx, state)

    SSE->>Parser: "function_call_arguments.delta (item_id=fc_xxx)"
    Parser->>Tracker: get(fc_xxx) accumulate buffer

    SSE->>Parser: "function_call_arguments.done (item_id=fc_xxx, args=malformed)"
    Parser->>Tracker: get(fc_xxx), JSON.parse fails, state.invalid set

    SSE->>Parser: "output_item.done (id=fc_xxx, args=malformed)"
    Parser->>Tracker: get(fc_xxx), retry parse fails, state.invalid updated
    Parser-->>Executor: tool_call_complete (invalid set)

    Executor->>Executor: toolCall.invalid? skip tool, build error message
    Executor-->>Model: function_call_output with call_id toolu_xxx and parse error
    Model->>Model: self-correct and retry
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
ui/src/pages/chat/useChat.ts:2193-2199
**Invalid call reconstructed with `"{}"` args in continuation**

When `toolCall.invalid` is set, `tc.arguments` is `{}` (the fallback from `state.parsedArguments ?? {}`), so the `function_call` item sent back to the model has `arguments: "{}"` instead of the original malformed string. The model sees a call it apparently made with empty arguments, paired with a `function_call_output` error saying the JSON was invalid — a contradiction that may confuse the model and require extra rounds to self-correct. Preserving `argumentsBuffer` (the raw string) in `ParsedToolCall` and using it here for invalid calls would give the model accurate context about what it originally emitted.

Reviews (3): Last reviewed commit: "Review fixes" | Re-trigger Greptile

Comment thread ui/src/pages/chat/utils/toolCallParser.ts Outdated
@ScriptSmith
Copy link
Copy Markdown
Owner Author

@greptile-apps

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