refactor: use direct struct pointer for Anthropic tool InputSchema#1927
refactor: use direct struct pointer for Anthropic tool InputSchema#1927davidrudduck wants to merge 1 commit intomaximhq:mainfrom
Conversation
Replace 30-line field-by-field copy of ToolFunctionParameters with direct pointer assignment. Both sides use the same struct type, so the copy is unnecessary and will silently drop any new fields added to ToolFunctionParameters in the future.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbitRelease NotesNo user-visible changes in this release. This update includes internal code optimizations and maintenance improvements to improve code maintainability. WalkthroughThis change refactors the Anthropic chat provider by simplifying InputSchema assignment for tool function parameters. The verbose field-by-field construction is replaced with a direct assignment from the existing tool.Function.Parameters structure, reducing code redundancy while maintaining identical behavior. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
|
||
| // Convert function parameters to input_schema | ||
| if tool.Function.Parameters != nil && (tool.Function.Parameters.Type != "" || tool.Function.Parameters.Properties != nil) { | ||
| anthropicTool.InputSchema = &schemas.ToolFunctionParameters{ |
There was a problem hiding this comment.
@Pratham-Mishra04 I think this was to avoid altering original request right?
There was a problem hiding this comment.
Good point — the pointer is only read (serialized to JSON) after assignment so it's safe today, but a field-by-field copy is more defensive against future mutations. Happy to revert this if you'd prefer to keep the defensive copy. The main concern was that new fields added to ToolFunctionParameters would silently be dropped here without a corresponding update, but I understand the tradeoff.
There was a problem hiding this comment.
Yes I think we directly serialize it after this so shouldnt be an issue but i'll confirm once
There was a problem hiding this comment.
If you'd prefer to keep mutation safety, we could do a shallow struct copy instead:
params := *tool.Function.Parameters
anthropicTool.InputSchema = ¶msThis gives you the defensive copy (no shared pointer) while automatically forwarding any new fields added to ToolFunctionParameters — so no more silent field drops when the schema struct grows. Best of both worlds?
3 participants
Summary
Replaces a 30-line field-by-field copy of
ToolFunctionParameterswith direct pointer assignment when building Anthropic tool definitions. Both the source (tool.Function.Parameters) and destination (anthropicTool.InputSchema) use the same*schemas.ToolFunctionParameterstype, making the manual copy unnecessary.This also prevents future bugs where new fields added to
ToolFunctionParameterswould be silently dropped by the Anthropic provider.Changes
anthropicTool.InputSchema = tool.Function.Parametersincore/providers/anthropic/chat.goType of change
Affected areas
How to test
Existing Anthropic provider tests validate tool parameter handling.
Breaking changes
Security considerations
No security impact — same data, simplified assignment.
Checklist