-
Notifications
You must be signed in to change notification settings - Fork 229
Add several annotation fields #201
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
base: main
Are you sure you want to change the base?
Add several annotation fields #201
Conversation
648d8e5
to
9a0a4ac
Compare
* indicate aggregated or compiled content. Used for attribution, filtering, and | ||
* helping users identify trusted content origins. | ||
*/ | ||
contentSource?: string[]; |
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'm not sure if it makes sense to make this and contentType available to just resources or all objects containing Annotations - open to guidance
/** | ||
* Optional annotations for the client. | ||
*/ | ||
annotations?: ServerAnnotations; |
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 believe this is the best place to expose server annotations, but let me know if I'm missing anything.
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.
Could be added to Implementation
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.
infact these could both just be added to the implementation piece. potentially humanReadableName
but also, could we also just use Implementation.name
for this purpose? also - I'm thinking we could just use favicon for the server icon also?
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.
implementation is also used for ClientInfo, where it wouldn't make sense to send these fields. We can remove serverName in favor for implementation.name (wanted more emphasis on human readability but this does seems confusing).
I'm thinking we could just use favicon for the server icon also
Is this referring to the result favicons or is there another favicon that I'm missing?
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.
had a quick look, haven't thought very deeply on it though.
Also - the diff should be on the schema/draft/schema.* files instead of the released version
/** | ||
* Optional annotations for the client. | ||
*/ | ||
annotations?: ServerAnnotations; |
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.
Could be added to Implementation
/** | ||
* Optional annotations for the client. | ||
*/ | ||
annotations?: ServerAnnotations; |
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.
infact these could both just be added to the implementation piece. potentially humanReadableName
but also, could we also just use Implementation.name
for this purpose? also - I'm thinking we could just use favicon for the server icon also?
* Semantic category of the object. | ||
* | ||
* Represents the nature of the content in a user-facing format. Should use simple, recognizable terms | ||
* that categorize the data for users (e.g., "Webpage", "Song", "Article", "Image", "Document"). |
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 the type of the content i.e. ImageContent / TextContent could probably represent this information (especially for for the image piece).
Also - this seems specific to a search tool, how are you imagining this being used? on each content block from a tool you might have different UI treatment, independent of it being and image vs text?
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.
imo it makes sense to have a descriptive/semantic field as opposed to the format of the data itself. for example some tools might return the weather as text but it still makes sense to annotate it as "Weather". For image, an example might be "Map".
I did think about only having this on EmbeddedResource, which might force MCP servers to be more specific about the types they return, but I think the use cases above are still valid - clients can choose to extrapolate info off the type of content or use this field
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.
very simply, this can just be a text label in the UI denoting to the user what the content is. doesn't need to be an entirely different UI treatment
* Allows users to navigate to the source material in its native context. | ||
* May require authentication if pointing to protected resources. | ||
*/ | ||
sourceUrl?: string; |
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.
how does this differ from content source?
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.
content source is a human-readable (list of) name(s) describing where the information is from. sourceUrl is a single URL from which the content can be accessed in its original context
@jerome3o-anthropic my bad on changing the wrong file - I'll move them over once we resolve these threads |
@@ -934,6 +955,47 @@ export interface Annotations { | |||
* @maximum 1 | |||
*/ | |||
priority?: number; | |||
|
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.
@justin-yi-wang what happens if the tool result returns a list of items? Don't these fields need to map to each item within a tool result?
* This helps clients render appropriate UI treatments and helps users understand what kind of | ||
* content they're interacting with. | ||
*/ | ||
contentType?: string; |
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.
media type? For example: https://www.iana.org/assignments/media-types/media-types.xhtml
This PR adds several optional fields for clients to power UI elements with. Fields are added to Annotations (for ToolResults) and a new ServerAnnotations type (analogous to ToolAnnotations).
Motivation and Context
Having said fields allows for richer client experiences.
How Has This Been Tested?
Breaking Changes
Types of changes
Checklist
Additional context