-
Notifications
You must be signed in to change notification settings - Fork 464
refactor: make CallToolRequest.Arguments more flexible (Breaking Change) #287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This change allows for more flexible argument types in CallToolRequest. Arguments field is now of type 'any' instead of 'map[string]any'. Added GetArguments() and GetRawArguments() methods for backward compatibility. Fixes #104
""" WalkthroughThe changes refactor how tool handler functions and tests access tool call arguments, replacing direct field access with the new Changes
Assessment against linked issues
Possibly related PRs
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
a06d4ac
to
27e3a49
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
mcp/tools_test.go (2)
378-400
: Thorough JSON serialization test with helpful noteThis test properly verifies JSON marshaling and unmarshaling behavior. The comment about JSON numbers being unmarshaled as float64 is especially valuable to prevent confusion when working with numeric values.
Consider adding similar tests for JSON marshaling/unmarshaling with non-map argument types (string, struct) to ensure consistent behavior across all supported types.
312-400
: Consider additional edge cases for robustnessThe tests provide good coverage for the main use cases, but could be strengthened by adding tests for:
nil
arguments- Empty map arguments
- Nested complex structures (maps containing maps, arrays, etc.)
These additional tests would ensure comprehensive handling of all possible input types.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
mcp/tools_test.go
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
mcp/tools_test.go (2)
mcp/tools.go (1)
CallToolRequest
(44-59)mcp/types.go (1)
Params
(118-118)
🔇 Additional comments (3)
mcp/tools_test.go (3)
312-332
: Clean implementation of map argument testsThis test effectively verifies that both
GetArguments()
andGetRawArguments()
correctly handle the common case where Arguments is a map. Good use of type assertion with theok
variable to ensure proper error handling.
334-349
: Appropriate handling of non-map type casesThis test correctly verifies the expected behavior when Arguments is a string (non-map type):
GetArguments()
returns an empty map whileGetRawArguments()
preserves the original string value. The clear comment about returning an empty map helps explain the intended behavior.
351-376
: Well-structured struct argument test caseThe test thoroughly checks the behavior with custom struct arguments and properly verifies both accessor methods. Good use of JSON tags in the test struct to simulate real-world usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good first step but it does not necessarily address all the concerns in the issue like a function to unmarshal the arguments into a struct or helper functions to get the arguments to avoid manually type checking each argument.
Maybe we can address it in this PR itself if others can also jump in and share inputs on how this must be implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
mcp/tools.go (2)
48-50
: Breaking-change is fine, but please document accepted JSON shapesChanging
Arguments
frommap[string]any
toany
is what enables the new flexibility, but it also introduces ambiguity for consumers:
- Callers that always expected a map now compile, but will silently receive an empty map when the producer sends a non-map payload – this may hide bugs.
- Nothing in the comment explains which concrete JSON shapes are actually accepted/expected (map, array, scalar, struct‐like object, raw JSON, …).
Consider extending the doc-comment (or the project’s CHANGELOG) to specify the supported shapes and how each helper behaves when the shape does not match.
88-274
: Large block of almost-identical getters can be consolidatedThe five
Get*/Require*
pairs share ~90 % identical control flow. This bloats the file and makes future changes error-prone.You can remove duplication by extracting a small internal helper, e.g.:
func (r CallToolRequest) coerce(key string) (any, bool) { args := r.GetArguments() v, ok := args[key] return v, ok }…and then factor the conversion logic into generic functions or switches that return the desired type. A sketch:
func getNumber[T constraints.Integer | constraints.Float](v any, def T) T { … }Not only does this cut LOC, it centralises edge-case handling (e.g.
json.Number
,float32
) and keeps the public API unchanged.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
mcp/tools.go
(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
mcp/tools.go (3)
Learnt from: xinwo
PR: mark3labs/mcp-go#35
File: mcp/tools.go:0-0
Timestamp: 2025-03-04T07:00:57.111Z
Learning: The Tool struct in mark3labs/mcp-go handles both InputSchema and RawInputSchema formats. When unmarshaling JSON, it first tries to parse into a structured ToolInputSchema format, and if that fails, it falls back to using the raw schema format, providing symmetry with the MarshalJSON method.
Learnt from: xinwo
PR: mark3labs/mcp-go#35
File: mcp/tools.go:107-137
Timestamp: 2025-03-04T06:59:43.882Z
Learning: Tool responses from the MCP server shouldn't contain RawInputSchema, which is why the UnmarshalJSON method for the Tool struct is implemented to handle only the structured InputSchema format.
Learnt from: xinwo
PR: mark3labs/mcp-go#35
File: mcp/tools.go:0-0
Timestamp: 2025-03-04T07:00:57.111Z
Learning: The Tool struct in the mark3labs/mcp-go project should handle both InputSchema and RawInputSchema consistently between MarshalJSON and UnmarshalJSON methods, even though the tools response from MCP server typically doesn't contain rawInputSchema.
🧬 Code Graph Analysis (1)
mcp/tools.go (1)
mcp/types.go (1)
Params
(118-118)
🔇 Additional comments (1)
mcp/tools.go (1)
112-148
:json.Number
and overflow cases are not handled
encoding/json
unmarshals numbers intofloat64
when the destination isany
, unlessUseNumber
is enabled on the decoder, in which case you receivejson.Number
.If upstream ever enables
UseNumber
,GetInt/RequireInt
and friends will start returning the default/error even though the value is convertible.Recommend adding a branch:
case json.Number: if i, err := v.Int64(); err == nil { return int(i) // or overflow-check first }Similarly guard against
float64
values that exceedmath.MaxInt
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
We should add tests that use the new Bind and helper functions. Other than that I think we are good to go.
We can maybe wait until one of the issue openers review this and see if it is as the expected.
IMO, this is exactly how we should implement this but maybe others have a different idea and maybe more opinionated.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
// BindArguments unmarshals the Arguments into the provided struct | ||
// This is useful for working with strongly-typed arguments | ||
func (r CallToolRequest) BindArguments(target any) error { | ||
if target == nil || reflect.ValueOf(target).Kind() != reflect.Ptr { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a great start, but I think we should probably extend further so that when you register a tool you can 100% control the args parsing. I'm not sure what the best API for taht might be, but I could imagine AddTool
(or a AddToolWithCustomArgs
or some other better name) that takes two functions (one parses args into your custom struct, and the other would be like todays tool handler func but directly receive the return value of the parsed args).
To be clear, I think this API is a good improvement. I just think we can make it even better.
@ezynda3 / @pottekkat - What do y'all think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah sounds good. Will try to add some stuff a bit later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TypedToolHandlerFunc
looks good 👍🏽
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
mcp/tools.go (2)
63-70
: Using nil instead of empty map improves error detection.The method now returns
nil
instead of an empty map when Arguments is not a map type, which allows callers to distinguish between "no arguments" and "wrong argument type" scenarios.
78-96
: LGTM: Implemented BindArguments with suggested optimizations.The implementation properly includes:
- Validation that the target is a non-nil pointer
- Fast-path for raw JSON to avoid double marshaling
- Clear error messages
This follows the feedback from the previous review comment.
🧹 Nitpick comments (1)
mcp/tools.go (1)
63-468
: Consider adding helper functions for custom argument parsing.The current implementation is a great improvement, but as discussed in previous comments, extending the API to better support custom argument parsing at tool registration time would be valuable.
// Example addition to consider (pseudocode): +// RegisterToolWithArgsParser registers a tool with a custom argument parser and handler +func (s *Server) RegisterToolWithArgsParser( + tool Tool, + parser func(CallToolRequest) (any, error), + handler func(context.Context, any) (CallToolResult, error), +) { + // Wrap the handler in a function that parses arguments first + wrapperHandler := func(ctx context.Context, req CallToolRequest) (CallToolResult, error) { + parsedArgs, err := parser(req) + if err != nil { + return CallToolResult{IsError: true}, err + } + return handler(ctx, parsedArgs) + } + s.RegisterTool(tool, wrapperHandler) +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (6)
README.md
(3 hunks)examples/typed_tools/main.go
(1 hunks)mcp/tools.go
(3 hunks)mcp/tools_test.go
(1 hunks)mcp/typed_tools.go
(1 hunks)mcp/typed_tools_test.go
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🧰 Additional context used
🧬 Code Graph Analysis (2)
mcp/typed_tools.go (2)
mcp/tools.go (2)
CallToolRequest
(46-61)CallToolResult
(36-43)mcp/utils.go (1)
NewToolResultError
(300-310)
mcp/tools.go (1)
mcp/types.go (1)
Params
(118-118)
🔇 Additional comments (23)
mcp/typed_tools.go (1)
1-20
: Well-designed type-safe handler implementationThe implementation of
TypedToolHandlerFunc
andNewTypedToolHandler
introduces a clean, type-safe pattern for handling tool calls. The use of generics provides compile-time type checking while maintaining flexibility.This approach eliminates the need for manual type assertions and conversions in each handler, making code more robust and maintainable.
examples/typed_tools/main.go (4)
11-21
: Good type structure for argumentsThe
GreetingArgs
struct provides a clear, strongly-typed representation of tool arguments with proper JSON field tags and a well-organized nested structure.
31-64
: Comprehensive tool schema definitionThe tool schema definition is detailed and properly aligned with the
GreetingArgs
struct. It includes appropriate descriptions, validation constraints, and nested object definitions.
66-67
: Excellent example of typed handler registrationThis line demonstrates the clean integration of the new typed handler pattern with the existing MCP server framework. The use of
NewTypedToolHandler
creates a seamless bridge between type-safe handlers and the MCP infrastructure.
75-105
: Well-implemented typed handler functionThe handler effectively demonstrates proper validation, conditional processing based on input fields, and handling of nested data. The incremental building of the response shows a clear pattern for working with typed arguments.
mcp/tools_test.go (4)
312-336
: Good test coverage for argument bindingThis test validates that the
BindArguments
method correctly binds map arguments to typed structs, which is a fundamental capability for the typed tool handlers.
338-440
: Comprehensive test suite for helper functionsThe test covers all getter methods (with defaults) and "require" methods for different types, including handling of missing keys and error conditions. This thorough validation ensures the robustness of the argument access API.
442-462
: Tests verify flexibility with different argument typesThese tests validate that the new flexible argument handling works correctly with various input types (maps, strings, and structs), confirming that
GetArguments
andGetRawArguments
behave as expected for each type.Also applies to: 464-479, 481-506
508-530
: JSON serialization test handles numeric type conversionThis test correctly validates JSON marshaling/unmarshaling of arguments and acknowledges the standard Go behavior where JSON numbers are unmarshaled as float64. This is an important consideration for consumers of the API.
mcp/typed_tools_test.go (3)
12-76
: Robust testing of basic typed handler functionalityThis test thoroughly validates the core functionality of typed handlers, including proper argument binding and error handling scenarios such as type mismatches and missing fields.
78-145
: Good validation logic test casesThe test effectively verifies that typed handlers can implement validation logic and return appropriate error results, including domain-specific validation like preventing division by zero.
147-304
: Excellent coverage for complex nested structuresThis comprehensive test verifies the handler's ability to process complex nested objects with multiple levels, different field types, and various edge cases including partial data and raw JSON input. The assertions thoroughly check that all aspects of the nested structure are correctly processed.
mcp/tools.go (11)
7-8
: Added imports for new argument handling functionality.The addition of the
reflect
andstrconv
packages appropriately supports the new type conversion and validation functionality in the helper methods.
49-50
: LGTM: Type change supports flexible argument formats.Changing
Arguments
frommap[string]any
toany
enables more flexible argument types, aligning with the PR objective to makeCallToolRequest.Arguments
more flexible.
72-76
: LGTM: Added raw argument access.The
GetRawArguments()
method provides a clean way to access the arguments without type conversion, supporting the need for flexible argument types.
98-119
: LGTM: String argument helpers are well-implemented.The String helpers provide a convenient way to extract string arguments with appropriate default values or errors, handling type assertions correctly.
121-158
: LGTM: Integer helpers with robust type conversion.The Int helpers correctly handle various types that can be converted to integers, including native ints, float64 values, and strings that can be parsed as numbers.
160-197
: LGTM: Float helpers with proper type conversion and error handling.The Float helpers follow the same pattern as the Int helpers, with comprehensive type conversions and clear error messages.
199-240
: LGTM: Bool helpers with extensive type conversion.The Bool helpers handle a wide range of valid boolean representations, including:
- Native booleans
- Strings that can be parsed as booleans
- Numeric values (treating non-zero as true)
This provides flexible, user-friendly boolean argument handling.
242-284
: LGTM: String slice helpers with proper type handling.The String slice helpers correctly handle both native string slices and slices of interface values, with appropriate error messages for type mismatches.
286-344
: LGTM: Int slice helpers with type conversion.The Int slice implementations properly convert compatible types and provide clear error messages when conversion fails.
346-404
: LGTM: Float slice helpers with robust type handling.The Float slice helpers implement the same pattern as the Int slice helpers, with proper type conversion and error handling.
406-468
: LGTM: Bool slice helpers with extensive type support.The Bool slice implementations support various boolean representations, consistent with the single boolean helpers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
mcp/tools.go (1)
50-50
:⚠️ Potential issueCritical issue: Type change needed for flexible arguments
According to the PR objectives and failures in static analysis, the
Arguments
field should be changed frommap[string]any
toany
type to support the more flexible argument structure.The type assertions throughout the new methods won't work unless this change is made.
- Arguments map[string]any `json:"arguments,omitempty"` + Arguments any `json:"arguments,omitempty"`
🧹 Nitpick comments (1)
mcp/tools.go (1)
55-62
: Consider adding error feedback for invalid argument typesWhile returning
nil
when Arguments isn't a map is better than returning an empty map, consider adding additional methods that would help diagnose type issues, such as:
- A method that returns both the map and a boolean indicating success
- A method that returns an error when Arguments isn't a map
// Additional helper method to provide more feedback +func (r CallToolRequest) GetArgumentsWithOk() (map[string]any, bool) { + args, ok := r.Params.Arguments.(map[string]any) + return args, ok +}🧰 Tools
🪛 GitHub Check: test
[failure] 58-58:
invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface🪛 GitHub Check: lint
[failure] 58-58:
invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface
[failure] 58-58:
invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface
[failure] 58-58:
invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface
[failure] 58-58:
invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface🪛 GitHub Actions: go
[error] 58-58: invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
mcp/tools.go
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
mcp/tools.go (1)
mcp/types.go (1)
Params
(154-154)
🪛 GitHub Check: test
mcp/tools.go
[failure] 58-58:
invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface
[failure] 78-78:
invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface
🪛 GitHub Check: lint
mcp/tools.go
[failure] 58-58:
invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface
[failure] 78-78:
invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface) (typecheck)
[failure] 58-58:
invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface
[failure] 78-78:
invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface) (typecheck)
[failure] 58-58:
invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface
[failure] 78-78:
invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface) (typecheck)
[failure] 58-58:
invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface
🪛 GitHub Actions: go
mcp/tools.go
[error] 58-58: invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface
🔇 Additional comments (11)
mcp/tools.go (11)
64-68
: Looks good:GetRawArguments()
provides access to raw argument dataThis method is a good addition that allows access to the raw arguments in any format, supporting the flexible argument handling described in the PR objective.
70-88
: Good implementation:BindArguments()
includes validation and optimizationThe implementation includes:
- Validation that the target is a non-nil pointer
- Fast-path optimization for when Arguments is already a json.RawMessage
- Error wrapping with descriptive messages
These improvements align with previous review feedback.
🧰 Tools
🪛 GitHub Check: test
[failure] 78-78:
invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface🪛 GitHub Check: lint
[failure] 78-78:
invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface) (typecheck)
[failure] 78-78:
invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface) (typecheck)
[failure] 78-78:
invalid operation: r.Params.Arguments (variable of type map[string]any) is not an interface) (typecheck)
90-111
: Looks good: String accessor methods are well-implementedThe implementation for string type access is thorough with both default and required versions.
113-150
: Looks good: Integer accessor methods handle type conversionsThe implementation correctly handles different source types (int, float64, string) with proper conversion logic.
152-189
: Looks good: Float accessor methods handle type conversionsSimilar to the integer methods, these correctly handle conversion from various source types.
191-232
: Looks good: Boolean accessor methods handle multiple typesThe boolean conversion is comprehensive, handling various input types (bool, string, int, float64) with proper conversion logic.
234-276
: Looks good: String slice accessor methods properly handle type conversionsThe implementation correctly distinguishes between native string slices and slices of interfaces that need conversion.
278-336
: Looks good: Integer slice accessor methods handle multiple source typesThe implementation correctly handles conversion from various numeric and string types.
338-396
: Looks good: Float slice accessor methods handle multiple source typesSimilar to the integer slice methods, these correctly handle conversion from various types.
398-460
: Looks good: Boolean slice accessor methods handle multiple typesThe implementation correctly handles conversion from bool, string, int, and float64 values.
7-8
: Looks good: New imports support added functionalityThe addition of
reflect
andstrconv
packages supports the new type conversions and validations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
mcp/tools.go (2)
73-88
: Consider adding a JSON format validation.The
BindArguments
method assumes that the target struct matches the structure of the JSON data, but doesn't validate this before attempting to unmarshal. You might want to add schema validation for more complex use cases.func (r CallToolRequest) BindArguments(target any) error { if target == nil || reflect.ValueOf(target).Kind() != reflect.Ptr { return fmt.Errorf("target must be a non-nil pointer") } // Fast-path: already raw JSON if raw, ok := r.Params.Arguments.(json.RawMessage); ok { return json.Unmarshal(raw, target) } data, err := json.Marshal(r.Params.Arguments) if err != nil { return fmt.Errorf("failed to marshal arguments: %w", err) } + + // Optional validation could go here: + // if v, ok := target.(json.Validator); ok { + // // Pre-validate the JSON data before fully unmarshaling + // if err := json.Unmarshal(data, json.RawMessage{}); err != nil { + // return fmt.Errorf("invalid JSON format for target type: %w", err) + // } + // } return json.Unmarshal(data, target) }
90-460
: Consider adding more examples to the documentation.The implementation of all these accessor methods is comprehensive, but it would be helpful to include some usage examples in the documentation to guide users on how to effectively use these methods.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
mcp/tools.go
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
mcp/tools.go (1)
mcp/types.go (2)
Meta
(112-124)Params
(154-154)
🔇 Additional comments (12)
mcp/tools.go (12)
49-51
: LGTM! Field type change achieves the PR objective.Changing the
Arguments
field frommap[string]any
toany
enables more flexible argument types as intended, which will allow for both structured and unstructured data.
57-62
: Returnnil
for non-map arguments is the correct approach.Good implementation that now correctly returns
nil
when the arguments aren't a map instead of an empty map. This allows callers to distinguish between "no args" and "wrong type" scenarios, facilitating better error handling.
66-68
: LGTM! Simple, effective raw data accessor.This method provides direct access to the raw arguments without type conversion, completing the access pattern options for consumers.
72-88
: BindArguments implementation follows best practices.Excellent implementation that:
- Properly validates the target is a non-nil pointer
- Includes a fast-path for already marshaled JSON
- Uses clear error messages with proper wrapping
This matches the suggestions in previous review comments and provides a clean API for type-safe argument binding.
90-111
: LGTM! String accessor methods are well-implemented.The string accessor methods provide both default and required variants with appropriate error handling and type checking.
113-150
: Integer accessor methods handle type conversions properly.The implementation correctly handles various numeric types and string-to-int conversions, with proper error messaging for required accessors.
152-189
: Float accessor methods handle type conversions properly.The implementation correctly handles various numeric types and string-to-float conversions, with proper error messaging for required accessors.
191-232
: Boolean accessor methods handle multiple representations.Comprehensive implementation that converts from various types (string, int, float) to boolean values with appropriate error handling.
234-276
: String slice accessor methods handle different array representations.The implementation properly handles both native string slices and arrays of interfaces, with appropriate type checking in the required variant.
278-336
: Int slice accessor methods thoroughly handle conversions.Comprehensive handling of various source types, including proper error messages for conversion failures in the required variant.
338-396
: Float slice accessor methods are well-implemented.Consistent with the pattern established for other slice types, with proper error handling and type conversions.
398-460
: Boolean slice accessor methods provide robust type handling.The implementation properly converts from various source types to boolean values, maintaining consistency with other slice accessors.
This change allows for more flexible argument types in CallToolRequest.
Arguments field is now of type 'any' instead of 'map[string]any'.
Added GetArguments() and GetRawArguments() methods for backward compatibility.
Fixes #104
Summary by CodeRabbit
Refactor
New Features
Tests