Skip to content

Conversation

davidleitw
Copy link
Contributor

@davidleitw davidleitw commented Jun 15, 2025

feat: add type-safe array items helper functions

  • Add ItemsString(), ItemsNumber(), ItemsBoolean() for type-safe array schema construction
  • Add ItemsStringEnum() for string enum arrays with cleaner API
  • Improve Items() documentation with examples and usage guidance
  • Add comprehensive compatibility tests ensuring new APIs generate identical schemas

Description

This PR introduces type-safe helper functions for configuring array items in MCP tool schemas. The original Items() function accepts any type, which can lead to runtime errors when developers accidentally pass incompatible types. These new helper functions provide compile-time type safety for common array element types while maintaining full backward compatibility.

The new functions include:

  • ItemsString(opts ...PropertyOption) - for string arrays with optional constraints
  • ItemsNumber(opts ...PropertyOption) - for number arrays with optional constraints
  • ItemsBoolean(opts ...PropertyOption) - for boolean arrays with optional constraints
  • ItemsStringEnum(values []string) - for string enum arrays with predefined values

All functions generate identical JSON schemas to their manual Items() equivalents, ensuring seamless migration and compatibility.

Type of Change

  • New feature (non-breaking change that adds functionality)
  • Documentation update
  • Tests only (no functional changes)

Checklist

  • My code follows the code style of this project
  • I have performed a self-review of my own code
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the documentation accordingly

Additional Information

Implementation Scope and Design Decisions

This implementation focuses on simple array types (string, number, boolean) which cover the majority of common use cases. The design maintains the existing functional options pattern for consistency with the broader API.

API Consistency Analysis

During implementation, we identified an interesting API consistency gap: while top-level object properties enjoy full type-safe API support (WithString, WithNumber, etc.), complex object arrays still require manual map[string]any construction.

For example:

// Top-level: Clean, type-safe API ✅
WithString("name", Required(), Description("User name"))

// Array items: Still requires manual construction
Items(map[string]any{
    "type": "object", 
    "properties": map[string]any{...}  // Manual approach
})

Current Solution Sufficiency

Important: The existing manual construction approach using Items(map[string]any{...}) remains fully functional and sufficient for all complex scenarios. It provides complete flexibility for any JSON Schema construct without additional learning overhead.

Future Enhancement Proposal

Based on the API consistency analysis, I would like to respectfully propose considering a future enhancement to introduce an ItemsObject API that could provide type-safe construction for complex object arrays.

The approach I have in mind would use a Property constructor pattern:

// Potential future API
WithArray("products",
    ItemsObject(
        Property("id", WithString(Required(), Description("Product ID"))),
        Property("name", WithString(Required(), Description("Product name"))),
        Property("category", WithString(Enum("electronics", "books"))),
    ))

This would help maintain API consistency between top-level and array item definitions while leveraging existing WithString, WithNumber functions for reusability.

However, I fully understand this would introduce additional API complexity, and I want to be mindful of the project's goals for simplicity and maintainability.

Question for maintainers:

Would this direction be worth exploring in a future proposal, or would you prefer to keep the current manual construction approach as the primary solution for complex object arrays?

I'm very open to feedback and happy to discuss the trade-offs between API consistency and simplicity. If this isn't the right direction for the project, I completely understand and respect that decision.

Backward Compatibility

  • All existing code continues to work without changes
  • New functions are fully tested with compatibility tests verifying identical schema generation
  • The manual Items() approach remains the recommended solution for complex object arrays
  • Zero breaking changes to existing APIs

This implementation strikes a balance between improving developer experience for common cases while maintaining simplicity and full backward compatibility.

Summary by CodeRabbit

  • New Features
    • Introduced specialized options for configuring array item types in schemas, including helpers for string, string enum, number, and boolean types.
  • Documentation
    • Enhanced documentation with usage examples for new and existing array item configuration options.
  • Tests
    • Added tests to ensure the new array item helpers produce schemas consistent with previous methods.

Copy link
Contributor

coderabbitai bot commented Jun 15, 2025

"""

Walkthrough

This change introduces specialized helper functions for defining array item schemas in a more type-specific manner, including functions for string, string enum, number, and boolean types. The generic Items function is enhanced with improved documentation, and comprehensive tests are added to ensure compatibility and correctness of the new helpers.

Changes

Files Change Summary
mcp/tools.go Added WithStringItems, WithStringEnumItems, WithNumberItems, WithBooleanItems helper functions; improved Items() documentation and usage examples.
mcp/tools_test.go Added TestNewItemsAPICompatibility to verify new helpers produce schemas identical to original Items() usage.

Possibly related 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
Failed executing command with error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 974493e and 3021697.

📒 Files selected for processing (2)
  • mcp/tools.go (2 hunks)
  • mcp/tools_test.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • mcp/tools_test.go
  • mcp/tools.go
✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (4)
mcp/tools.go (3)

948-966: Docstring omits the newly-added ItemsStringEnum helper

The comment advertises the three “simple type” helpers but forgets to mention ItemsStringEnum. Being the most discoverable place for users, the omission will make the enum variant harder to find.

-// For simple types, use ItemsString(), ItemsNumber(), ItemsBoolean() instead.
+// For simple types, use ItemsString(), ItemsStringEnum(), ItemsNumber(),
+// and ItemsBoolean() instead.

1010-1023: Duplicate implementation — consider re-using ItemsString(Enum(...))

ItemsStringEnum is a thin wrapper that manually reproduces the map literal already produced by ItemsString(Enum(values...)). Re-using the existing helper avoids one code path diverging from the other in the future:

-func ItemsStringEnum(values []string) PropertyOption {
-	return func(schema map[string]any) {
-		schema["items"] = map[string]any{
-			"type": "string",
-			"enum": values,
-		}
-	}
-}
+func ItemsStringEnum(values []string) PropertyOption {
+	return ItemsString(Enum(values...))
+}

This keeps behaviour identical while removing duplication.


1032-1065: Four nearly-identical helpers – factor out the boilerplate

ItemsString, ItemsNumber, and ItemsBoolean share ~90 % of their code.
A tiny unexported helper would make future maintenance (and linting) easier:

func itemsOf(kind string, opts ...PropertyOption) PropertyOption {
	return func(schema map[string]any) {
		item := map[string]any{"type": kind}
		for _, o := range opts { o(item) }
		schema["items"] = item
	}
}

func ItemsString(opts ...PropertyOption)  PropertyOption { return itemsOf("string",  opts...) }
func ItemsNumber(opts ...PropertyOption)  PropertyOption { return itemsOf("number",  opts...) }
func ItemsBoolean(opts ...PropertyOption) PropertyOption { return itemsOf("boolean", opts...) }

Smaller surface, one place to apply option filtering (see previous comment), and no behavioural risk.

mcp/tools_test.go (1)

532-714: Great compatibility coverage – a few additions could harden the test

The test thoroughly checks equivalence of the produced items schemas across helpers – nice!
Two quick wins to make it future-proof:

  1. Also assert oldArrayProp["uniqueItems"] etc. if the array helper ever gains such options – avoids silent regressions.
  2. Add a negative case (e.g. pass an unsupported option like Required() to ItemsString and ensure the resulting schema excludes it). This will lock in the intended filtering logic once implemented.

Not critical for merge, but easy improvements.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 607df92 and 4b87866.

📒 Files selected for processing (2)
  • mcp/tools.go (2 hunks)
  • mcp/tools_test.go (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
mcp/tools_test.go (1)
mcp/tools.go (14)
  • Tool (472-483)
  • NewTool (569-591)
  • WithDescription (612-616)
  • WithArray (892-910)
  • Description (671-675)
  • Items (962-966)
  • ItemsString (996-1008)
  • ItemsStringEnum (1016-1023)
  • MinLength (723-727)
  • MaxLength (715-719)
  • ItemsNumber (1032-1044)
  • Min (759-763)
  • Max (751-755)
  • ItemsBoolean (1053-1065)

Copy link
Collaborator

@pottekkat pottekkat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these additions @davidleitw

I'm wondering if ItemsX is the best names for these functions. Wouldn't WithXItems be better?

Functionally, since this PR does not break any functionality and improves user experience, we can merge it.

The future proposal to improve the API sounds good to me. Users can still use the old way if they need to do complex stuff, but most common use cases can be captured with the abstractions like you mentioned.

I think that the API needs to be simple and intuitive and if it helps developers build schemas accurately faster, then we can definitely implement this.

Tagging @ezynda3 @rwjblue-glean and @dugenkui03 as well for their inputs.

@davidleitw
Copy link
Contributor Author

Hi, @pottekkat
Thanks for the feedback!

Yeah, I can definitely try refactoring the API to use the WithXItems naming pattern instead — that does sound more intuitive. I’ll experiment with it and see how it plays out across the different use cases.

Let me know if you have any specific preferences or edge cases you’d like me to consider while adjusting the naming!

@davidleitw
Copy link
Contributor Author

Hi @pottekkat

Thank you for the excellent naming suggestion! I've implemented the changes you recommended.

🔄 Changes Made

I've renamed all the Items functions to follow the WithXItems pattern for better API consistency:

  • ItemsString()WithStringItems()
  • ItemsStringEnum()WithStringEnumItems()
  • ItemsNumber()WithNumberItems()
  • ItemsBoolean()WithBooleanItems()

📝 What Was Updated

  1. Function names and signatures - All four functions renamed
  2. Documentation and examples - Updated all comments to use new function names
  3. Test cases - Updated TestNewItemsAPICompatibility with new function names
  4. API consistency - Now aligns perfectly with existing WithString, WithNumber, WithBoolean naming pattern

🎯 Current Behavior

The functions now work exactly the same as before, but with more intuitive naming:

// Before
WithArray("tags", ItemsString())
WithArray("scores", ItemsNumber(Min(0), Max(100)))

// After  
WithArray("tags", WithStringItems())
WithArray("scores", WithNumberItems(Min(0), Max(100)))

✅ Testing

All tests pass, confirming that:

  • Functionality remains identical
  • Schema generation is unchanged
  • API behavior is preserved

The new naming is much more intuitive and consistent with the rest of the API. Thank you for the great suggestion!

As for the future ItemsObject proposal — if this direction sounds good to everyone, I’d be happy to explore contributing it. I can work on a detailed design document and implementation plan in a separate PR. Open to discussion and feedback from the team!

Copy link
Collaborator

@pottekkat pottekkat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your quick response. Yes, if you are interested, you can go ahead with the proposal. I will wait for others to comment as well regarding this change and your planned itemsObject proposal.

- Add WithStringItems(), WithNumberItems(), WithBooleanItems() for type-safe array schema construction
- Add WithStringEnumItems() for string enum arrays with cleaner API
- Improve Items() documentation with examples and usage guidance
- Add comprehensive compatibility tests ensuring new APIs generate identical schemas
- Use WithXItems naming pattern for consistency with existing API design
@dugenkui03 dugenkui03 merged commit a077d27 into mark3labs:main Jun 17, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants