-
Notifications
You must be signed in to change notification settings - Fork 326
feat: use structuredContent with outputSchema in example servers #220
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 14 commits
36b18a9
d154771
8ad706e
075e14d
c999ada
9911544
22441e7
a8ab501
a6c34d7
671c6af
4ef967a
68eb5d9
51dfd6b
3401002
cf1ad22
af88f60
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 |
|---|---|---|
|
|
@@ -15,8 +15,8 @@ const log = { | |
|
|
||
|
|
||
| function extractTime(result: CallToolResult): string { | ||
| const { text } = result.content?.find((c) => c.type === "text")!; | ||
| return text; | ||
| const data = result.structuredContent as { time: string }; | ||
| return data.time; | ||
|
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. Originally, I wrote this as: const { time } = (result.structuredContent as { time?: string }) ?? {};
return time ?? "[ERROR]";to handle potential type desync, though I don't feel too strongly about it.
Contributor
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. Done! Updated to use the defensive pattern: const { time } = (result.structuredContent as { time?: string }) ?? {};
return time ?? "[ERROR]";This also matches what the quickstart guide shows (lines 223-224). |
||
| } | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -248,6 +248,7 @@ export function createServer(): McpServer { | |
| description: | ||
| "Returns budget configuration with 24 months of historical allocations and industry benchmarks by company stage", | ||
| inputSchema: {}, | ||
| outputSchema: BudgetDataResponseSchema, | ||
| _meta: { [RESOURCE_URI_META_KEY]: resourceUri }, | ||
| }, | ||
| async (): Promise<CallToolResult> => { | ||
|
|
@@ -279,6 +280,7 @@ export function createServer(): McpServer { | |
| text: JSON.stringify(response), | ||
|
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. Originally, this was richly formatted text — see 61cd4ed#diff-4360a472ed891e48c204f1cf177aec8fb637d96bff5047cba6a1e4a899971b1aL224-R270. Not sure whether we care about restoring that for these examples.
Contributor
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. Restored! Added The |
||
| }, | ||
| ], | ||
| structuredContent: response, | ||
| }; | ||
| }, | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -169,6 +169,7 @@ export function createServer(): McpServer { | |
| description: | ||
| "Returns cohort retention heatmap data showing customer retention over time by signup month", | ||
| inputSchema: GetCohortDataInputSchema.shape, | ||
| outputSchema: CohortDataSchema.shape, | ||
| _meta: { [RESOURCE_URI_META_KEY]: resourceUri }, | ||
| }, | ||
| async ({ metric, periodType, cohortCount, maxPeriods }) => { | ||
|
|
@@ -181,6 +182,7 @@ export function createServer(): McpServer { | |
|
|
||
| return { | ||
| content: [{ type: "text", text: JSON.stringify(data) }], | ||
|
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. Similarly, originally this was richly formatted text: 61cd4ed#diff-681e498d6715bbf8d9931228b1e274db793e8898940120cdd812cd320fabf692L152-R179.
Contributor
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. Restored! Added |
||
| structuredContent: data, | ||
| }; | ||
| }, | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,6 +64,13 @@ const GetScenarioDataInputSchema = z.object({ | |
| ), | ||
| }); | ||
|
|
||
| const GetScenarioDataOutputSchema = z.object({ | ||
| templates: z.array(ScenarioTemplateSchema), | ||
| defaultInputs: ScenarioInputsSchema, | ||
| customProjections: z.array(MonthlyProjectionSchema).optional(), | ||
| customSummary: ScenarioSummarySchema.optional(), | ||
| }); | ||
|
|
||
| // Types derived from schemas | ||
| type ScenarioInputs = z.infer<typeof ScenarioInputsSchema>; | ||
| type MonthlyProjection = z.infer<typeof MonthlyProjectionSchema>; | ||
|
|
@@ -269,6 +276,7 @@ export function createServer(): McpServer { | |
| description: | ||
| "Returns SaaS scenario templates and optionally computes custom projections for given inputs", | ||
| inputSchema: GetScenarioDataInputSchema.shape, | ||
| outputSchema: GetScenarioDataOutputSchema.shape, | ||
| _meta: { [RESOURCE_URI_META_KEY]: resourceUri }, | ||
| }, | ||
| async (args: { | ||
|
|
@@ -278,18 +286,21 @@ export function createServer(): McpServer { | |
| ? calculateScenario(args.customInputs) | ||
| : undefined; | ||
|
|
||
| const data = { | ||
| templates: SCENARIO_TEMPLATES, | ||
| defaultInputs: DEFAULT_INPUTS, | ||
| customProjections: customScenario?.projections, | ||
| customSummary: customScenario?.summary, | ||
| }; | ||
|
|
||
| return { | ||
| content: [ | ||
| { | ||
| type: "text", | ||
| text: JSON.stringify({ | ||
| templates: SCENARIO_TEMPLATES, | ||
| defaultInputs: DEFAULT_INPUTS, | ||
| customProjections: customScenario?.projections, | ||
| customSummary: customScenario?.summary, | ||
| }), | ||
| text: JSON.stringify(data), | ||
|
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.
Contributor
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. Restored! Added |
||
| }, | ||
| ], | ||
| structuredContent: data, | ||
| }; | ||
| }, | ||
| ); | ||
|
|
||
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 inclined to write this as either:
Or as:
depending on how strict we want to be about
contentvsstructuredContentequivalence.I also think we should update the Quickstart guide to match whatever
basic-server-vanillajsdoes, because it links to it as the full example.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.
Done! Updated to match the quickstart pattern:
The quickstart guide already uses this pattern (lines 127-132), so no changes needed there.