refactor: Pull out Annotations structure rather than being an anonymous inner struct
#203
Conversation
WalkthroughThe changes introduce a new named struct, Changes
Assessment against linked issues
✨ 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:
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
mcp/resources.go(2 hunks)mcp/types.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
mcp/resources.go (1)
mcp/types.go (1)
Annotations(662-675)
mcp/types.go (1)
mcp/prompts.go (1)
Role(68-68)
🔇 Additional comments (3)
mcp/types.go (1)
662-675: Good refactoring of the annotation structure.Creating a named
Annotationsstruct improves code organization and provides a clear, reusable structure for annotation metadata. The JSON field names remain consistent, which helps maintain compatibility with existing JSON data.mcp/resources.go (2)
46-47: Update to use the new Annotations type is correct.The code properly instantiates a new
Annotationsstruct when the field is nil, consistent with the refactoring intypes.go.
94-95: Consistent use of the new Annotations type.This code follows the same pattern as in
WithAnnotations, properly instantiating a newAnnotationsstruct when needed.
| type Annotated struct { | ||
| Annotations *struct { | ||
| // Describes who the intended customer of this object or data is. | ||
| // | ||
| // It can include multiple entries to indicate content useful for multiple | ||
| // audiences (e.g., `["user", "assistant"]`). | ||
| Audience []Role `json:"audience,omitempty"` | ||
|
|
||
| // Describes how important this data is for operating the server. | ||
| // | ||
| // A value of 1 means "most important," and indicates that the data is | ||
| // effectively required, while 0 means "least important," and indicates that | ||
| // the data is entirely optional. | ||
| Priority float64 `json:"priority,omitempty"` | ||
| } `json:"annotations,omitempty"` | ||
| Annotations *Annotations `json:"annotations,omitempty"` | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider documenting this breaking change.
Changing Annotations from an inline anonymous struct to a pointer to a named struct is a breaking change that could affect existing code. Any code that directly accessed the fields of the anonymous struct now needs to handle potential nil pointers.
🏁 Script executed:
#!/bin/bash
# Search for code that might be affected by this change
rg -A2 "\.Annotations\." --type goLength of output: 329
🏁 Script executed:
#!/bin/bash
# Inspect context around usage of r.Annotations.Audience
rg -n -C3 "r\.Annotations\.Audience" mcp/resources.go
# Inspect context around usage of t.Annotations.Audience
rg -n -C3 "t\.Annotations\.Audience" mcp/resources.go
# Search for nil-checks or initialization of Annotations pointers
rg -n -C3 "Annotations == nil" --type go
rg -n -C3 "Annotations = &Annotations" --type goLength of output: 2195
Document Breaking Change to Annotations Field
Changing Annotations from an inline struct to a *Annotations pointer is a breaking change. Any code that previously did:
r := Resource{}
// r.Annotations.Priority == 0will now panic on r.Annotations.Priority because r.Annotations can be nil.
Please update your documentation and migration guide:
• Add a note under Breaking Changes in CHANGELOG.md (or RELEASE_NOTES) that the Annotations field is now a pointer.
• In README or docs, advise users to initialize with WithAnnotations(...) or check for nil before accessing fields.
• (Optional) Consider adding a custom UnmarshalJSON on Annotated to default the pointer.
Affected locations:
- mcp/types.go at
type Annotated struct { Annotations *Annotations ... } - mcp/resources.go builder funcs already guard against
nil, but any unmarshaled or literalAnnotatedmust be initialized.
ezynda3
changed the title
Annotations structure rather than being an anonymous inner structAnnotations structure rather than being an anonymous inner struct
2 participants
Fixes #177
Note: I guess this technically would be a breaking change
Summary by CodeRabbit