fix: fall back for streamed text tool calls - #158
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The fallback is triggered only after streaming has already yielded text events, which can leak raw <tool_call> payloads to callers and also affects retry behavior by marking output as produced.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes streaming-mode tool execution for providers that emit tool-call payloads as plain text by detecting those payloads and falling back to the non-streaming turn path so tool calls aren’t silently dropped (Issue #140).
Changes:
- Added
_looks_like_tool_callheuristic and_StreamUnsupportedraise in_stream_turnto trigger non-streaming fallback when a plain-text tool call is detected. - Added test coverage for
<tool_call>…</tool_call>detection, bare-JSON detection, malformed payload handling, structured streaming tool calls (no fallback), and usage accounting around the failed streaming attempt.
File summaries
| File | Description |
|---|---|
src/opendot/agent/loop.py |
Adds plain-text tool-call detection in streaming and raises _StreamUnsupported to activate the existing non-streaming fallback path. |
tests/test_loop.py |
Adds tests validating detection/fallback behavior and ensuring usage isn’t double-counted when the streaming attempt is abandoned. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Address review: _stream_turn yielded each content delta as a text event while
streaming and only raised _StreamUnsupported at the end, so a plain-text
<tool_call> payload was briefly surfaced to the caller, and produced_output was
set True (which could suppress a retry if the non-stream fallback then hit a
transient error).
Now hold text back while the assembled content could still be a tool call
(starts with <tool_call>, a code fence, or a bare {), flushing the buffer as one
text event only once it is clearly prose. When it is a tool call we raise before
any flush, so nothing is yielded and produced_output stays False. Ordinary prose
still streams live after the first non-matching delta.
Test: a chunked plain-text tool call yields no text event before the raise
(regression-proven against the eager-yield behavior).
|
Pushed a follow-up for Copilot's finding: _stream_turn now buffers suspected tool-call text so no raw <tool_call> payload leaks before the fallback, and produced_output stays False so a fallback error can still retry. Added a regression test. Good to merge @Siteshcodes. |
What & why
Detect tool calls emitted as plain text during streaming and fall back to the non-streaming path so the tool call is executed correctly.
How I verified it
Added tests for plain-text
<tool_call>and bare JSON fallback, malformed payloads, normal structured streaming tool calls, and usage accounting.tests/test_loop.pyandtests/test_usage.pypass locally.Checklist
pytestpasses locallyreversibility/or a mutating tool: actions are snapshotted and undo still restores exactly (tests added)Related to #140