change to strong content type #26
Conversation
WalkthroughThe pull request refactors the way JSON responses and content types are handled across the project. Inline JSON unmarshalling has been replaced with calls to centralized parsing functions, and generic interface types have been updated to a new, specific Changes
Sequence Diagram(s)sequenceDiagram
participant Client as SSEMCPClient
participant Parser as mcp.ParseGetPromptResult
participant Util as ParseContent
Client->>Parser: Invoke GetPrompt(...)
Parser->>Util: Parse raw JSON content
Util-->>Parser: Return structured Content
Parser-->>Client: Return GetPromptResult
sequenceDiagram
participant Client as SSEMCPClient
participant Parser as mcp.ParseCallToolResult
participant Util as ParseContent
Client->>Parser: Invoke CallTool(...)
Parser->>Util: Parse raw JSON content
Util-->>Parser: Return structured Content
Parser-->>Client: Return CallToolResult
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
mcp/utils.go (3)
310-310: Consider aligning resource content usage with the new Content interface.
Currently,NewReadResourceResultstill leverages[]interface{}{ TextResourceContents{...} }. IfTextResourceContentsqualifies as aContenttype or could be made to implementContent, you may want to make it consistent with the rest of the code by returning[]Content.
370-377: Verify silent fallback for missing or non-string data.
ExtractStringreturns""whenever the key does not exist or is not a string. This may mask errors if the caller expects a non-empty value.
379-386: Confirm behavior for missing or invalid map data.
ExtractMapreturnsnilif the specified key is absent or not a map, potentially hiding misconfigurations. Ensure this is intended and properly handled downstream.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
client/sse.go(2 hunks)client/sse_test.go(1 hunks)client/stdio.go(2 hunks)examples/everything/main.go(5 hunks)mcp/prompts.go(1 hunks)mcp/tools.go(1 hunks)mcp/types.go(3 hunks)mcp/utils.go(8 hunks)server/server_test.go(2 hunks)
🔇 Additional comments (25)
mcp/tools.go (1)
28-28: Improved type safety for Content fieldThe change from
[]interface{}to[]Contentenforces that only valid content types (TextContent, ImageContent, or EmbeddedResource) can be stored in this field, which enhances type safety and reduces potential runtime errors.client/sse_test.go (1)
28-33: Test updated to use stronger typingTest implementation properly updated to use the new
mcp.Contentinterface type instead of generic interfaces, maintaining consistency with the production code changes.mcp/types.go (4)
588-590: Good design for Content interfaceCreating a marker interface with a simple
isContent()method is a clean approach to establish type constraints while allowing for different concrete implementations.
601-601: TextContent implements Content interfaceProper implementation of the marker method to satisfy the Content interface.
614-614: ImageContent implements Content interfaceProper implementation of the marker method to satisfy the Content interface.
616-627: Well-designed EmbeddedResource typeThe EmbeddedResource type is a good addition that enhances the content type system, providing a way to embed resource contents directly in responses. The implementation properly satisfies the Content interface.
mcp/prompts.go (1)
80-81: Enhanced type safety for PromptMessageChanging from
interface{}to the specificContentinterface type ensures that only valid content types can be assigned to the Content field, improving type safety and maintainability.client/stdio.go (2)
391-391: Improved type safety through centralized response parsingThis change replaces inline JSON unmarshalling with a call to
mcp.ParseGetPromptResult(), centralizing the parsing logic and error handling. This enhances type safety by ensuring consistent parsing across client implementations.
420-420: Improved type safety through centralized response parsingSimilar to the GetPrompt method, this change replaces inline JSON unmarshalling with a call to
mcp.ParseCallToolResult(), ensuring consistent parsing behavior and error handling.server/server_test.go (1)
697-697: Strong type implementation for ContentChanging from
[]interface{}to[]mcp.Contentimproves type safety by ensuring that the Content field only contains values that implement the Content interface. This aligns with the PR's objective of strengthening content type safety.client/sse.go (2)
465-465: Improved type safety through centralized response parsingThis change replaces inline JSON unmarshalling with a call to
mcp.ParseGetPromptResult(), which centralizes the parsing logic and error handling. This promotes consistent behavior across client implementations.
494-494: Improved type safety through centralized response parsingSimilar to the GetPrompt method, this change replaces inline JSON unmarshalling with a call to
mcp.ParseCallToolResult(), promoting consistent behavior and error handling across client implementations.examples/everything/main.go (5)
283-283: Strong typing for Content in tool resultsUpdating the Content field from
[]interface{}to[]mcp.Contentimproves type safety and ensures that only valid content types are returned by the handler.
304-304: Strong typing for Content in tool resultsConsistent with the pattern, changing to
[]mcp.Contentensures type safety for the handleAddTool function's return value.
333-333: Strong typing for Content in tool resultsUpdated the Content field to use the safer
[]mcp.Contenttype for the notification tool handler.
376-376: Strong typing for Content in tool resultsChanged the Content field to use
[]mcp.Contentfor the long running operation tool handler, ensuring type safety and consistency.
415-415: Strong typing for Content in tool resultsUpdated the Content type for the image tool handler, maintaining consistency with the strongly typed Content interface pattern established throughout the codebase.
mcp/utils.go (8)
3-6: Imports look good.
No issues here. The addition ofencoding/jsonandfmtis appropriate for the new parsing and formatting logic.
185-185: Good move to stronger typing.
Switching frominterface{}toContentfor theNewPromptMessagefunction signature helps enforce type safety and consistency across the codebase.
220-227: No concerns with text content array initialization.
Using[]Content{...}here aligns well with the new strong-typed approach.
233-241: Consistent error flagging.
Creating a text content array and settingIsError = trueis a clear way to indicate errors in tool results.
246-257: Excellent usage of combined text + image content.
Enforcing[]Contentaligns with the strong type approach for tool results.
266-277: Resource-based content array looks consistent.
Embedding both text and resource content is handled cleanly with the new[]Contenttype.
434-494: Role validation may omit other valid roles.
Currently, roles other than"assistant"and"user"generate an error. Check if you need to handle additional roles such as"system"or custom roles.
496-545: Centralized tool result parsing looks clean.
This function properly defers toParseContentfor each item and handles errors gracefully, aligning well with the new strong type design.
Add strong type to Content.
It is easier to use, I think.
Summary by CodeRabbit
These updates enhance internal processing for improved reliability and consistency in handling responses, ensuring a smoother experience.
Refactor
Tests