Repository navigation
fix(llm): recover near.ai streaming tool-call args with trailing content - #5951
Conversation
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
github-actions
Bot
added
size: M
📝 WalkthroughWalkthroughStreaming tool-call parsing now recovers a leading valid JSON value before trailing content, while the canonical parser remains strict. Unit and Tokio SSE tests verify the recovered arguments and cleared parse-error metadata. ChangesTool argument recovery
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant SseServer
participant NearAiStreaming
participant ToolArgumentParser
SseServer->>NearAiStreaming: stream tool arguments with trailing content
NearAiStreaming->>ToolArgumentParser: recover leading JSON value
ToolArgumentParser-->>NearAiStreaming: parsed arguments without error
NearAiStreaming-->>SseServer: emit one recovered tool call
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the tool-call argument parsing logic in crates/ironclaw_llm/src/tool_args.rs to tolerate trailing content after a valid JSON object, which can occur when streaming from certain reasoning models. It achieves this by using serde_json::Deserializer to extract the first valid JSON value and ignore any subsequent stray tokens. Additionally, corresponding unit and integration tests have been added to verify this recovery behavior. I have no feedback to provide as the changes are well-implemented and covered by tests.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Pull request overview
This PR fixes near.ai SSE streaming tool-call argument parsing in ironclaw_llm so that when a model emits a valid JSON arguments value followed by trailing junk, IronClaw recovers the first complete JSON value instead of collapsing tool arguments to {} (which could invoke tools with missing args and fail intermittently).
Changes:
- Updated
parse_tool_call_argsto parse the first completeserde_json::Valueand ignore any trailing content. - Added a
tool_argsunit test covering the trailing-content recovery behavior (including the lossy wrapper). - Added a near.ai streaming regression test that reproduces and asserts correct recovery for
arguments:"{...}trailing".
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/ironclaw_llm/src/tool_args.rs | Switches tool-args parsing to recover the first JSON value and adds a unit regression test for trailing-content streams. |
| crates/ironclaw_llm/src/nearai_chat.rs | Adds a caller-level near.ai SSE streaming regression test to ensure recovered args are preserved through the real streaming finalizer path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Some tool-capable reasoning models append a stray token after a complete | ||
| // JSON arguments object (`{...}` + junk). A strict `from_str` rejects that as | ||
| // "trailing characters", and on the streaming tool-call path | ||
| // (`NearAiStreamingToolCallState::into_tool_call`) that error fails the whole | ||
| // turn after the model already emitted its reasoning. Take the first complete | ||
| // JSON value and ignore any trailing content; only empty or genuinely | ||
| // malformed input errors. |
The near.ai SSE streaming tool-call finalizer parsed accumulated arguments
with a strict serde_json::from_str that rejects trailing characters. Reasoning
models (DeepSeek-V4-Flash) stream a valid arguments object followed by a stray
token; the strict parse errored and parse_tool_call_args_lossy collapsed the
arguments to empty {}, so the tool ran with no arguments (observed as
"the tool input could not be encoded" memory_read failures).
Parse the first complete JSON value and ignore trailing content; empty and
truncated inputs keep their existing repair behavior.
Tests: tool_args unit case for trailing content + a caller-level nearai_chat
streaming test asserting the recovered arguments reach the tool call.
2371ea7 to
7693ddd
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_llm/src/nearai_chat.rs`:
- Around line 2026-2057: Update the mock TCP servers in
complete_streaming_salvages_text_when_stream_ends_after_delta and the
additionally referenced tests to accept connections in a loop, parse each
request path, and respond harmlessly to background pricing requests until POST
/v1/chat/completions arrives. Keep the existing chat-request assertions and SSE
response for the matching path, preventing NearAiChatProvider::new’s
fetch_pricing request from causing panics or flaky tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fb0f826c-9986-4a3f-8201-619b1eb90cd7
📒 Files selected for processing (1)
crates/ironclaw_llm/src/nearai_chat.rs
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_llm/src/nearai_chat.rs`:
- Around line 2026-2057: Update the mock TCP servers in
complete_streaming_salvages_text_when_stream_ends_after_delta and the
additionally referenced tests to accept connections in a loop, parse each
request path, and respond harmlessly to background pricing requests until POST
/v1/chat/completions arrives. Keep the existing chat-request assertions and SSE
response for the matching path, preventing NearAiChatProvider::new’s
fetch_pricing request from causing panics or flaky tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fb0f826c-9986-4a3f-8201-619b1eb90cd7
📒 Files selected for processing (1)
crates/ironclaw_llm/src/nearai_chat.rs
🛑 Comments failed to post (1)
crates/ironclaw_llm/src/nearai_chat.rs (1)
2026-2057: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Make the mock servers tolerate the pricing bootstrap request.
NearAiChatProvider::newspawnsfetch_pricingwhen a Tokio runtime is active (Line 221 through Line 247). These added servers accept one connection and requirePOST /v1/chat/completions; the background pricing request can arrive first, causing a panic and flaky tests. Loop until the chat request, responding harmlessly to other paths.Also applies to: 2083-2110, 2129-2158, 2184-2213, 2342-2376
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_llm/src/nearai_chat.rs` around lines 2026 - 2057, Update the mock TCP servers in complete_streaming_salvages_text_when_stream_ends_after_delta and the additionally referenced tests to accept connections in a loop, parse each request path, and respond harmlessly to background pricing requests until POST /v1/chat/completions arrives. Keep the existing chat-request assertions and SSE response for the matching path, preventing NearAiChatProvider::new’s fetch_pricing request from causing panics or flaky tests.
c5d4bf7 to
95230e8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_llm/src/tool_args.rs`:
- Around line 43-61: parse_tool_call_args currently discards trailing model
output after the first JSON value. Update the successful recovery flow in
parse_tool_call_args and nearai_chat.rs handling around
NearAiStreamingToolCallState::into_tool_call to retain the original
raw_arguments or ignored suffix in the canonical ToolCall/audit record, while
still using the first parsed JSON value for execution; do not clear or overwrite
this preserved data when arguments_parse_error is cleared.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 81bba11d-fb0a-49c9-a9c0-7e0a525c0487
📒 Files selected for processing (2)
crates/ironclaw_llm/src/nearai_chat.rscrates/ironclaw_llm/src/tool_args.rs
serrrfirat
left a comment
serrrfirat
left a comment
There was a problem hiding this comment.
Code quality review finding:
[P2] crates/ironclaw_llm/src/tool_args.rs:38 changes the shared “fail-loud” parser into a permissive prefix parser. The bug is NearAI streaming-specific, but parse_tool_call_args is documented as the shared migration primitive for providers and as returning Err on malformed JSON. With this change, {"a":1} garbage, {}{} or any future provider’s malformed concatenated output becomes Ok(first_value). That makes the canonical boundary looser and hides malformed provider output.
Cleaner shape: keep parse_tool_call_args strict with serde_json::from_str, and add an explicitly named NearAI/streaming salvage helper or policy, e.g. parse_tool_call_args_allow_trailing / parse_first_tool_call_args_value, called from NearAiStreamingToolCallState::into_tool_call. That keeps the repair behavior where it belongs without turning every future provider migration into silent trailing-junk acceptance.
Verification I ran on PR head 95230e831:
cargo test -p ironclaw_llm complete_with_tools_streaming_recovers_args_with_trailing_contentcargo test -p ironclaw_llm parse_args_ignores_trailing_content_after_first_value
Per review feedback: the previous change loosened the shared, fail-loud
`parse_tool_call_args` primitive for every provider to fix a NearAI
streaming-specific bug, making `{"a":1} garbage` / `{}{}` silently parse to the
first value at the canonical tool-call boundary.
Keep `parse_tool_call_args` strict (`serde_json::from_str`) and move the
"recover first value, ignore trailing junk" behavior into an explicitly-named
`parse_tool_call_args_allow_trailing_lossy`, called only from
`NearAiStreamingToolCallState::into_tool_call`. It delegates to
`parse_tool_call_args_lossy` for the clean/malformed paths so the shared bridge
stays referenced (no dead code).
Add `parse_tool_call_args_stays_strict_on_trailing_content` locking that the
shared primitive still rejects trailing/concatenated JSON.
| /// Streaming-salvage variant of [`parse_tool_call_args_lossy`]. | ||
| /// | ||
| /// Some streaming providers (e.g. NearAI reasoning models like DeepSeek) append | ||
| /// a stray token after a complete arguments object (`{...}` + junk). This parses |
There was a problem hiding this comment.
Fixed — updated both PR descriptions to match the implementation after the refactor:
parse_tool_call_argsstays strict; recovery lives in the new, explicitly-namedparse_tool_call_args_allow_trailing_lossy(called only fromNearAiStreamingToolCallState::into_tool_call).- Referenced test names corrected to
allow_trailing_lossy_recovers_first_valueandparse_tool_call_args_stays_strict_on_trailing_content, plus the caller-levelcomplete_with_tools_streaming_recovers_args_with_trailing_content.
Thanks for catching the drift.
Summary
{}(viaparse_tool_call_args_lossy). A reasoning model (e.g. DeepSeek-V4-Flash) that appends a stray token after a complete arguments object had the tool invoked with no arguments — surfacing as intermittentmemory_read"the tool input could not be encoded" failures on hosted deployments.parse_tool_call_args_allow_trailing_lossy, which parses the first complete JSON value and ignores trailing content.NearAiStreamingToolCallState::into_tool_callnow calls it.parse_tool_call_argsprimitive stays strict — trailing/concatenated JSON still errors for other providers. The permissive behavior is scoped to the streaming path, not the canonical boundary.parse_tool_call_args_lossyfor clean/malformed inputs, preserving existing repair behavior.Change Type
Linked Issue
None (bug fix). Builds on the recent
into_tool_call→parse_tool_call_args_lossychange onmain: that switch stopped a trailing-character error from aborting the whole streaming turn, but the arguments were still silently collapsed to{}. This PR recovers them.Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warningscargo buildtool_args::allow_trailing_lossy_recovers_first_value,tool_args::parse_tool_call_args_stays_strict_on_trailing_content,nearai_chat::complete_with_tools_streaming_recovers_args_with_trailing_content; existing..._preserves_malformed_.../..._marks_empty_...streaming tests unchanged and passing.cargo test --features integration— N/A (no database-backed or integration behavior)memory_readfailures on a hosted-single-tenant + PostgreSQL deployment running DeepSeek-V4-Flash; unit + caller-level tests cover the recovery.review-prorpr-shepherd --fixwas run before requesting reviewSecurity Impact
None. Provider-layer JSON parsing only. The streaming salvage helper narrows accepted input to the first complete JSON value; trailing content is discarded, never dispatched to a tool. The shared
parse_tool_call_argsboundary is unchanged (still strict). No permissions, network, secrets, file access, tool execution, or sandbox policy change.Reborn Trust-Boundary Checklist
N/A — provider-layer tool-call argument parsing. No new trust-bearing types, prompt-envelope changes, hashes, status/exit/policy/runtime/error variants (
ArgsParseErrorunchanged),serde(default)fields, or unbounded buffers. The change adds a scoped salvage helper for the near.ai streaming path and leaves the shared fail-loud boundary intact.Database Impact
None.
Blast Radius
Adds
ironclaw_llm::tool_args::parse_tool_call_args_allow_trailing_lossyand routesNearAiStreamingToolCallState::into_tool_callto it. The sharedparse_tool_call_args(fail-loud primitive) is unchanged. Behavior change is confined to the near.ai streaming tool-call path: a valid JSON value followed by trailing content now recovers the leading value instead of collapsing to{}. Truncated ({"query":) and empty ("") inputs are unchanged; other providers are unaffected. Low risk; alltool_argsand streaming tests pass.Rollback Plan
Revert the new helper + call-site swap in
crates/ironclaw_llm/src/{tool_args.rs,nearai_chat.rs}and the added tests. Pure code, no data/schema/migration — safe and immediate to revert.Review Follow-Through
?aborting the whole turn) was already resolved onmain; this PR addresses only the remaining silent argument-drop on the streaming path.parse_tool_call_argsstrict and scoped the trailing-content tolerance to a named streaming helper (parse_tool_call_args_allow_trailing_lossy) rather than loosening the shared boundary. Addedparse_tool_call_args_stays_strict_on_trailing_contentto lock that.Review track: A (small bug fix, test-backed; no security/runtime/DB/CI surface)