-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Allow Prompt/Sampling Messages to contain multiple content blocks. #198
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
Changes from all commits
8baf498
eb4ad9a
34c215a
41bfd83
32fa465
c13b1d1
9661bf6
36634e5
0e7dbe1
4d34819
d1b2387
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -646,7 +646,7 @@ export type Role = "user" | "assistant"; | |||
| */ | ||||
| export interface PromptMessage { | ||||
| role: Role; | ||||
| content: TextContent | ImageContent | AudioContent | EmbeddedResource; | ||||
| content: (TextContent | ImageContent | AudioContent | EmbeddedResource)[]; | ||||
| } | ||||
|
|
||||
| /** | ||||
|
|
@@ -705,7 +705,7 @@ export interface CallToolResult extends Result { | |||
| * Whether the tool call ended in an error. | ||||
| * | ||||
| * If not set, this is assumed to be false (the call was successful). | ||||
| * | ||||
| * | ||||
| * Any errors that originate from the tool SHOULD be reported inside the result | ||||
| * object, with `isError` set to true, _not_ as an MCP protocol-level error | ||||
| * response. Otherwise, the LLM would not be able to see that an error occurred | ||||
|
|
@@ -816,7 +816,7 @@ export interface Tool { | |||
| }; | ||||
|
|
||||
| /** | ||||
| * An optional JSON Schema object defining the structure of the tool's output returned in | ||||
| * An optional JSON Schema object defining the structure of the tool's output returned in | ||||
| * the structuredContent field of a CallToolResult. | ||||
| */ | ||||
| outputSchema?: { | ||||
|
|
@@ -937,7 +937,7 @@ export interface CreateMessageResult extends Result, SamplingMessage { | |||
| */ | ||||
| export interface SamplingMessage { | ||||
| role: Role; | ||||
| content: TextContent | ImageContent | AudioContent; | ||||
| content: (TextContent | ImageContent | AudioContent)[]; | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One last-ditch ask for a backward compatible way to handle this. Defining an
Suggested change
Not certain if there's a reason why it wouldn't work, but thought I'd put it out there.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand the concern, however the original proposal is still my preference. My reasoning is:
So the trade-off is between a new type introduced for backwards compatibility, or expressing the Message content semantically. I think because it can be mitigated at the SDK with low effort I fall towards the latter. |
||||
| } | ||||
|
|
||||
| /** | ||||
|
|
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.
This should be a major change with a note that it's breaking