feat: add WithInteger and WithIntegerItems options #458
Conversation
Signed-off-by: Calum Murray <[email protected]>
WalkthroughThis change generalizes number-related property option functions to support Changes
Possibly related PRs
Suggested reviewers
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 ✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
mcp/tools.go (1)
822-842: Fix typo in the function documentation.The function implementation is correct and follows established patterns, but there's a typo in the comment.
Apply this diff to fix the typo:
-// It accepts propety operations to configure the integer property's behavior and constraints. +// It accepts property options to configure the integer property's behavior and constraints.mcp/tools_test.go (2)
147-193: Fix comment error in the test.The test correctly verifies the new
WithIntegerfunctionality, but there's a copy-paste error in the comment.Apply this diff to fix the comment:
- // Verify itemName object + // Verify itemCount object
707-746: Fix inconsistent description in test case.The test cases correctly verify backward compatibility for integer array items, but there's an inconsistent description in the second test case.
Apply this diff to fix the description:
- WithDescription("Tool with constrained number array using new API"), + WithDescription("Tool with constrained integer array using new API"),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
mcp/tools.go(3 hunks)mcp/tools_test.go(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
mcp/tools_test.go (5)
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.
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 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: octo
PR: mark3labs/mcp-go#149
File: mcptest/mcptest.go:0-0
Timestamp: 2025-04-21T21:26:32.945Z
Learning: In the mcptest package, prefer returning errors from helper functions rather than calling t.Fatalf() directly, giving callers flexibility in how to handle errors.
Learnt from: floatingIce91
PR: mark3labs/mcp-go#401
File: server/server.go:1082-1092
Timestamp: 2025-06-23T11:10:42.948Z
Learning: In Go MCP server, ServerTool.Tool field is only used for tool listing and indexing, not for tool execution or middleware. During handleToolCall, only the Handler field is used, so dynamic tools don't need the Tool field populated.
mcp/tools.go (2)
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.
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.
🧬 Code Graph Analysis (1)
mcp/tools_test.go (1)
mcp/tools.go (11)
NewTool(569-591)WithDescription(612-616)WithString(868-886)Description(671-675)Required(679-683)WithInteger(824-842)WithArray(913-931)Items(983-987)WithIntegerItems(1087-1099)Min(759-763)Max(751-755)
🔇 Additional comments (5)
mcp/tools.go (5)
743-747: LGTM! Good use of generics for numeric types.The generalization to support
int,int64, andfloat64improves API flexibility while maintaining backward compatibility.
751-755: LGTM! Consistent generalization.The generic type constraints align well with the
DefaultNumberfunction changes and improve usability.
759-763: LGTM! Consistent with other numeric function generalizations.The generic type constraints maintain consistency across all numeric property option functions.
767-771: LGTM! Completes the numeric function generalization.All numeric property option functions now consistently support the same generic type constraints.
1076-1099: LGTM! Well-implemented integer array items function.The function follows established patterns, includes comprehensive documentation with examples, and correctly implements integer item schema configuration.
Description
Type of Change
Checklist
Additional Information
The
WithIntegertool option is already in the docs, but does not exist in the source code yet. This PR adds the option and some associated tests, as well as an option for specifying that array items are of type integer.Summary by CodeRabbit
New Features
Tests