Repository navigation
fix(reborn): repair oversized provider tool arguments - #4805
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughGateway detects provider tool-call outputs rejected for "arguments too large", appends omission-marked repair messages, retries the provider with repaired tool-call ChangesProvider Tool Argument Repair
Sequence DiagramNo sequence diagram generated. (Change is primarily a focused retry within the gateway; the added interactions map to existing Client→Gateway→Provider requests without introducing three or more new distinct actors requiring a new sequential visualization.) Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Review Notes (terse, evidence-first)
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
github-actions
Bot
added
size: M
![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 introduces a mechanism to automatically repair and retry LLM tool calls when the provider tool arguments exceed the host's size limit. It intercepts the oversized argument error, constructs a repair request that replaces the oversized arguments with an omission marker, and prompts the model to retry. Feedback suggests refining provider_tool_call_for_repair to selectively omit arguments only for the tool calls that actually exceed the limit, preserving valid ones, and to clear the reasoning field to prevent potential data leaks.
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.
![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_reborn/src/model_gateway.rs`:
- Around line 55-56: Replace the brittle byte-for-byte string match on
safe_summary (and the const PROVIDER_TOOL_ARGUMENTS_TOO_LARGE_SUMMARY) with a
shared, strongly-typed classifier from the safety crate: import the
authoritative constant, predicate function, or error enum (e.g., a new or
existing symbol in crates::ironclaw_safety such as arguments_too_large_constant,
is_provider_args_too_large(s: &str) or
ProviderValidationError::ArgumentsTooLarge) and use that to detect this retry
case in model_gateway.rs (instead of comparing safe_summary to the literal).
Update all occurrences (including the other match at lines referenced) to call
the shared predicate or match the exported enum/variant so the logic follows the
upstream limit changes automatically.
🪄 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: f797358a-1fa7-460b-ac13-93993698e6e2
📒 Files selected for processing (2)
crates/ironclaw_reborn/src/model_gateway.rscrates/ironclaw_reborn/tests/llm_gateway.rs
think-in-universe
left a comment
think-in-universe
left a comment
There was a problem hiding this comment.
Reviewed the PR and left one inline comment about accounting for the extra repair provider call.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_reborn/src/model_gateway.rs (1)
857-887:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve the original invalid-output error if the repair retry also fails.
This branch drops
erroronce it starts the retry. If the secondtool_response_to_host(...)still returnsHostManagedModelErrorKind::InvalidOutput, callers now get the retry's failure instead of the original oversize-validation path, so the fail-closed outcome depends on whatever the provider sent on the second round.Suggested fix
- Err(error) if is_repairable_provider_tool_output_error(&error) => { + Err(error) if is_repairable_provider_tool_output_error(&error) => { debug!( safe_summary = error.safe_summary.as_str(), "reborn model gateway retrying after repairable provider tool output" ); + let original_error = error; let mut repair_request = tool_request; repair_request .messages .extend(provider_tool_repair_messages( &response, - error.safe_summary.as_str(), + original_error.safe_summary.as_str(), )); let rejected_response = response; let response = provider .complete_with_tools(repair_request) .await @@ - return tool_response_to_host( + return match tool_response_to_host( response, capabilities, provider_turn_scope .as_deref() .unwrap_or("model_call=unknown"), &replay_identity, ) - .await; + .await + { + Err(retry_error) + if retry_error.kind == HostManagedModelErrorKind::InvalidOutput => + { + Err(original_error) + } + other => other, + }; }🤖 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_reborn/src/model_gateway.rs` around lines 857 - 887, The branch that handles Err(error) from is_repairable_provider_tool_output_error(&error) drops the original `error` when retrying and may return a different failure from the retry; capture the original error before doing the repair retry (e.g., store it in `original_error`) and after the retry, if the eventual failure returned by `tool_response_to_host(...)` maps to `HostManagedModelErrorKind::InvalidOutput`, return or propagate the preserved `original_error` instead; ensure this logic is applied around the `provider.complete_with_tools(...)`, `map_provider_error`, and `tool_response_to_host` call sites so callers receive the original oversize/invalid-output error when the repair attempt also fails.
🤖 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_reborn/src/model_gateway.rs`:
- Around line 1261-1275: The helper provider_tool_call_for_repair currently sets
reasoning: None which drops the original per-tool reasoning; change it to
preserve the original reasoning by assigning reasoning:
tool_call.reasoning.clone() (or otherwise copying the existing
ToolCall.reasoning) so repaired replays retain the model-produced reasoning for
calls whose arguments were not omitted; update the struct construction in
provider_tool_call_for_repair to use tool_call.reasoning.clone() instead of
None.
---
Outside diff comments:
In `@crates/ironclaw_reborn/src/model_gateway.rs`:
- Around line 857-887: The branch that handles Err(error) from
is_repairable_provider_tool_output_error(&error) drops the original `error` when
retrying and may return a different failure from the retry; capture the original
error before doing the repair retry (e.g., store it in `original_error`) and
after the retry, if the eventual failure returned by
`tool_response_to_host(...)` maps to `HostManagedModelErrorKind::InvalidOutput`,
return or propagate the preserved `original_error` instead; ensure this logic is
applied around the `provider.complete_with_tools(...)`, `map_provider_error`,
and `tool_response_to_host` call sites so callers receive the original
oversize/invalid-output error when the repair attempt also fails.
🪄 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: 510f935d-ea50-498b-9df2-406c426fd843
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!**/Cargo.lock
📒 Files selected for processing (5)
crates/ironclaw_reborn/Cargo.tomlcrates/ironclaw_reborn/src/model_gateway.rscrates/ironclaw_reborn/tests/llm_gateway.rscrates/ironclaw_safety/src/lib.rscrates/ironclaw_safety/src/provider_validation.rs
* fix(reborn): repair oversized provider tool arguments * fix(reborn): address provider tool repair review * fix(reborn): preserve repaired tool reasoning * ci: rerun checks --------- Co-authored-by: Robert Yan <mstr.raphael@gmail.com>
2 participants
Summary
Change Type
Linked Issue
Closes #4751
Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warningscargo buildcargo test -p ironclaw_reborn --features root-llm-provider --test llm_gateway gateway_repairs_oversized_provider_tool_arguments_before_registrationcargo test --features integrationif database-backed or integration behavior changedreview-prorpr-shepherd --fixwas run before requesting reviewAdditional validation:
git diff --checkSecurity Impact
Yes. This touches the provider-tool boundary before capability registration/execution. Oversized provider tool arguments are not registered or executed; they are replaced with a safe marker in a single repair retry. The retry remains fail-closed if provider output is still invalid.
Database Impact
None.
Blast Radius
Limited to
ironclaw_rebornmodel gateway handling of provider tool-call output and thellm_gatewayregression test fixture. Normal assistant replies and valid tool calls should continue through the existing path.Rollback Plan
Revert this PR to restore the previous behavior where oversized provider tool arguments fail immediately during host validation.
Review Follow-Through
Reviewer judgment requested on whether repair should remain scoped to the current host-validation safe summary only, or also cover provider-adapter
LlmError::InvalidResponseshapes in a later follow-up.Review track: C
Summary by CodeRabbit
Bug Fixes
Tests