Mirror: fix: preserve original line_ranges format (#5370)#13
Mirror: fix: preserve original line_ranges format (#5370)#13jeremylongshore merged 2 commits intomainfrom
Conversation
…c-provider
When using anthropic-provider mode, the read_file tool's line_ranges parameter
was being converted from snake_case to camelCase and from tuple format to object
format before saving to conversation history. This caused inconsistency between
the API response format and the saved history format.
Changes:
- Add rawInput field to ToolUse interface to preserve original API parameters
- Save original args to rawInput in NativeToolCallParser.parseToolCall()
- Use rawInput (if available) when saving tool calls to history in Task.ts
- Add test case to verify rawInput preserves original line_ranges format
This ensures line_ranges stays as [[1, 50]] instead of being converted to
lineRanges: [{ start: 1, end: 50 }] in the conversation history.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @jeremylongshore, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where tool call arguments, specifically Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughA new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
No actionable comments were generated in the recent review. 🎉 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 |
Review Summary by QodoPreserve original line_ranges format in API history
WalkthroughsDescription• Preserve original API parameter formats in conversation history • Add rawInput field to ToolUse interface for storing unmodified arguments • Update NativeToolCallParser to capture original args before transformation • Use rawInput when saving tool calls to history in Task.ts • Add test case verifying line_ranges format preservation Diagramflowchart LR
A["API Response<br/>line_ranges: [[1, 50]]"] -->|parseToolCall| B["NativeToolCallParser<br/>stores rawInput"]
B -->|nativeArgs conversion| C["Converted Format<br/>lineRanges: [{start, end}]"]
B -->|rawInput preservation| D["Original Format<br/>line_ranges: [[1, 50]]"]
C --> E["nativeArgs<br/>for processing"]
D --> F["rawInput<br/>for history"]
E --> G["Task.ts<br/>uses rawInput for history"]
F --> G
File Changes1. src/shared/tools.ts
|
Code Review by Qodo
1. rawInput may be non-object
|
| // Use rawInput to preserve original API format for history consistency. | ||
| // This ensures parameters like line_ranges stay as [[1, 50]] instead of | ||
| // being converted to lineRanges with object format [{ start: 1, end: 50 }]. | ||
| // Fall back to nativeArgs for tools that don't have rawInput, then to params for legacy. | ||
| const input = toolUse.rawInput || toolUse.nativeArgs || toolUse.params | ||
|
|
||
| // Use originalName (alias) if present for API history consistency. | ||
| // When tool aliases are used (e.g., "edit_file" -> "search_and_replace"), |
There was a problem hiding this comment.
1. Rawinput may be non-object 🐞 Bug ⛯ Reliability
rawInput is assigned directly from JSON.parse() and then preferred when persisting tool_use.input into API conversation history. If a tool call’s arguments parse to a non-object JSON value (array/string/number/null), history may contain an invalid tool_use.input shape and later cause provider/API request failures when history is reused.
Agent Prompt
### Issue description
`rawInput` is set from `JSON.parse()` without ensuring the parsed value is a plain object. `Task` now prefers `rawInput` when constructing `tool_use` blocks for API conversation history. If parsed arguments are a non-object JSON value (e.g., `[]`, `"str"`, `null`), we can persist an invalid `tool_use.input` shape and potentially break subsequent API calls when replaying history.
### Issue Context
JSON allows non-object top-level values. The code currently assumes tool arguments are objects.
### Fix Focus Areas
- src/core/assistant-message/NativeToolCallParser.ts[613-616]
- src/core/assistant-message/NativeToolCallParser.ts[876-885]
- src/core/task/Task.ts[3559-3576]
### Notes
Implement a small `isPlainObject` check (or equivalent) and:
- Coerce `argsForProcessing` to `{}` when parsed value is not a plain object.
- Set `rawInput` only when the parsed value is a plain object.
- In `Task`, only prefer `rawInput` if it is a plain object; otherwise fall back to `nativeArgs`/`params`.
- Add a unit test covering a non-object parsed arguments payload to prevent regressions.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Code Review
This pull request correctly addresses the issue of preserving the original format of tool call arguments in the API conversation history. By introducing a rawInput field to the ToolUse object, the changes ensure that parameters like line_ranges maintain their original format (e.g., [[1, 50]]) instead of being converted to the internal representation. The implementation is clean, with appropriate fallback logic in Task.ts to maintain backward compatibility. The addition of a specific test case in NativeToolCallParser.spec.ts effectively validates this new behavior. Overall, this is a well-executed fix that improves data consistency.
Review Summary
Checklist
Findings1. Needs rebase against current main (blocking)The branch was created from an older commit. This error does NOT exist on current upstream main (22/22 packages pass). The PR needs a rebase to pick up the fix. Impact: No upstream CI ran ( 2. Missing changeset (non-blocking)No changeset included. This modifies 3. Fallback chain order matters (non-blocking, observation)The new fallback in const input = toolUse.rawInput || toolUse.nativeArgs || toolUse.params
Local VerificationWe merged this PR on our fork and ran the full test suite. Regression (existing tests)
Why check-types fails but we're not blocking on it: The type error is in Behavioral (author's test)
CI Status
Code AnalysisArchitectureThe fix adds a
The key insight: the Anthropic API sends Design Assessment
VerdictCOMMENT — The fix is well-designed and solves a real format inconsistency issue. The code passes all tests and follows existing patterns. However, the branch needs a rebase against current main to resolve the pre-existing type error and trigger upstream CI. A changeset is also needed. Once rebased, this should be a straightforward approval. |
Review Journal: kilocode Kilo-Org#5370
SummaryPreserves original API parameter format ( What ChangedThe Anthropic API returns tool parameters in snake_case with tuple arrays. Kilo Code's Four files, each with a clear responsibility:
VerificationRegression (did we break anything?)
Behavioral (does the fix work?)
Lessons Learned
Review #20 of 75 | Multi-AI analysis | Methodology |
Mirror of Kilo-Org#5370
This PR mirrors the upstream change for multi-AI review analysis.
Bot Review Tracker
Summary by CodeRabbit
Bug Fixes
Tests