From d8a2821344d1d06225c01a12bf13d91863fe20b3 Mon Sep 17 00:00:00 2001 From: Basil Hosmer Date: Wed, 28 May 2025 22:18:19 -0400 Subject: [PATCH] refactor: Address PR comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract tool results rendering into separate ToolResults component - Hoist checkContentCompatibility function to module level following codebase conventions - Add ajv@6.12.6 as direct dependency to client package.json - All tests pass and linting is clean 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- client/package.json | 1 + client/src/components/ToolResults.tsx | 221 ++++++++++++++++++++++++ client/src/components/ToolsTab.tsx | 231 +------------------------- client/src/utils/schemaUtils.ts | 1 - package-lock.json | 6 +- 5 files changed, 229 insertions(+), 231 deletions(-) create mode 100644 client/src/components/ToolResults.tsx diff --git a/client/package.json b/client/package.json index 5b5ec19a..ee54df3d 100644 --- a/client/package.json +++ b/client/package.json @@ -25,6 +25,7 @@ "dependencies": { "@modelcontextprotocol/sdk": "^1.11.5", "@radix-ui/react-checkbox": "^1.1.4", + "ajv": "^6.12.6", "@radix-ui/react-dialog": "^1.1.3", "@radix-ui/react-icons": "^1.3.0", "@radix-ui/react-label": "^2.1.0", diff --git a/client/src/components/ToolResults.tsx b/client/src/components/ToolResults.tsx new file mode 100644 index 00000000..c6d90700 --- /dev/null +++ b/client/src/components/ToolResults.tsx @@ -0,0 +1,221 @@ +import JsonView from "./JsonView"; +import { + CallToolResultSchema, + CompatibilityCallToolResult, + Tool, +} from "@modelcontextprotocol/sdk/types.js"; +import { validateToolOutput, hasOutputSchema } from "@/utils/schemaUtils"; + +interface ToolResultsProps { + toolResult: CompatibilityCallToolResult | null; + selectedTool: Tool | null; +} + +const checkContentCompatibility = ( + structuredContent: unknown, + unstructuredContent: Array<{ + type: string; + text?: string; + [key: string]: unknown; + }>, +): { isCompatible: boolean; message: string } => { + if ( + unstructuredContent.length !== 1 || + unstructuredContent[0].type !== "text" + ) { + return { + isCompatible: false, + message: "Unstructured content is not a single text block", + }; + } + + const textContent = unstructuredContent[0].text; + if (!textContent) { + return { + isCompatible: false, + message: "Text content is empty", + }; + } + + try { + const parsedContent = JSON.parse(textContent); + const isEqual = + JSON.stringify(parsedContent) === JSON.stringify(structuredContent); + + if (isEqual) { + return { + isCompatible: true, + message: "Unstructured content matches structured content", + }; + } else { + return { + isCompatible: false, + message: "Parsed JSON does not match structured content", + }; + } + } catch { + return { + isCompatible: false, + message: "Unstructured content is not valid JSON", + }; + } +}; + +const ToolResults = ({ toolResult, selectedTool }: ToolResultsProps) => { + if (!toolResult) return null; + + if ("content" in toolResult) { + const parsedResult = CallToolResultSchema.safeParse(toolResult); + if (!parsedResult.success) { + return ( + <> +

Invalid Tool Result:

+ +

Errors:

+ {parsedResult.error.errors.map((error, idx) => ( + + ))} + + ); + } + const structuredResult = parsedResult.data; + const isError = structuredResult.isError ?? false; + + let validationResult = null; + const toolHasOutputSchema = + selectedTool && hasOutputSchema(selectedTool.name); + + if (toolHasOutputSchema) { + if (!structuredResult.structuredContent && !isError) { + validationResult = { + isValid: false, + error: + "Tool has an output schema but did not return structured content", + }; + } else if (structuredResult.structuredContent) { + validationResult = validateToolOutput( + selectedTool.name, + structuredResult.structuredContent, + ); + } + } + + let compatibilityResult = null; + if ( + structuredResult.structuredContent && + structuredResult.content.length > 0 && + selectedTool && + hasOutputSchema(selectedTool.name) + ) { + compatibilityResult = checkContentCompatibility( + structuredResult.structuredContent, + structuredResult.content, + ); + } + + return ( + <> +

+ Tool Result:{" "} + {isError ? ( + Error + ) : ( + Success + )} +

+ {structuredResult.structuredContent && ( +
+
Structured Content:
+
+ + {validationResult && ( +
+ {validationResult.isValid ? ( + "✓ Valid according to output schema" + ) : ( + <>✗ Validation Error: {validationResult.error} + )} +
+ )} +
+
+ )} + {!structuredResult.structuredContent && + validationResult && + !validationResult.isValid && ( +
+
+ ✗ Validation Error: {validationResult.error} +
+
+ )} + {structuredResult.content.length > 0 && ( +
+ {structuredResult.structuredContent && ( + <> +
+ Unstructured Content: +
+ {compatibilityResult && ( +
+ {compatibilityResult.isCompatible ? "✓" : "⚠"}{" "} + {compatibilityResult.message} +
+ )} + + )} + {structuredResult.content.map((item, index) => ( +
+ {item.type === "text" && ( + + )} + {item.type === "image" && ( + Tool result image + )} + {item.type === "resource" && + (item.resource?.mimeType?.startsWith("audio/") ? ( + + ) : ( + + ))} +
+ ))} +
+ )} + + ); + } else if ("toolResult" in toolResult) { + return ( + <> +

Tool Result (Legacy):

+ + + ); + } + + return null; +}; + +export default ToolResults; diff --git a/client/src/components/ToolsTab.tsx b/client/src/components/ToolsTab.tsx index 01ea25f7..8c72bd2a 100644 --- a/client/src/components/ToolsTab.tsx +++ b/client/src/components/ToolsTab.tsx @@ -7,13 +7,8 @@ import { TabsContent } from "@/components/ui/tabs"; import { Textarea } from "@/components/ui/textarea"; import DynamicJsonForm from "./DynamicJsonForm"; import type { JsonValue, JsonSchemaType } from "@/utils/jsonUtils"; +import { generateDefaultValue } from "@/utils/schemaUtils"; import { - generateDefaultValue, - validateToolOutput, - hasOutputSchema, -} from "@/utils/schemaUtils"; -import { - CallToolResultSchema, CompatibilityCallToolResult, ListToolsResult, Tool, @@ -22,6 +17,7 @@ import { Loader2, Send, ChevronDown, ChevronUp } from "lucide-react"; import { useEffect, useState } from "react"; import ListPane from "./ListPane"; import JsonView from "./JsonView"; +import ToolResults from "./ToolResults"; const ToolsTab = ({ tools, @@ -57,224 +53,6 @@ const ToolsTab = ({ setParams(Object.fromEntries(params)); }, [selectedTool]); - // Check compatibility between structured and unstructured content - const checkContentCompatibility = ( - structuredContent: unknown, - unstructuredContent: Array<{ - type: string; - text?: string; - [key: string]: unknown; - }>, - ): { isCompatible: boolean; message: string } => { - // Check if unstructured content is a single text block - if ( - unstructuredContent.length !== 1 || - unstructuredContent[0].type !== "text" - ) { - return { - isCompatible: false, - message: "Unstructured content is not a single text block", - }; - } - - const textContent = unstructuredContent[0].text; - if (!textContent) { - return { - isCompatible: false, - message: "Text content is empty", - }; - } - - try { - // Try to parse the text as JSON - const parsedContent = JSON.parse(textContent); - - // Deep equality check - const isEqual = - JSON.stringify(parsedContent) === JSON.stringify(structuredContent); - - if (isEqual) { - return { - isCompatible: true, - message: "Unstructured content matches structured content", - }; - } else { - return { - isCompatible: false, - message: "Parsed JSON does not match structured content", - }; - } - } catch { - return { - isCompatible: false, - message: "Unstructured content is not valid JSON", - }; - } - }; - - const renderToolResult = () => { - if (!toolResult) return null; - - if ("content" in toolResult) { - const parsedResult = CallToolResultSchema.safeParse(toolResult); - if (!parsedResult.success) { - return ( - <> -

Invalid Tool Result:

- -

Errors:

- {parsedResult.error.errors.map((error, idx) => ( - - ))} - - ); - } - const structuredResult = parsedResult.data; - const isError = structuredResult.isError ?? false; - - // Validate structured content if present and tool has output schema - let validationResult = null; - const toolHasOutputSchema = - selectedTool && hasOutputSchema(selectedTool.name); - - if (toolHasOutputSchema) { - if (!structuredResult.structuredContent && !isError) { - // Tool has output schema but didn't return structured content (and it's not an error) - validationResult = { - isValid: false, - error: - "Tool has an output schema but did not return structured content", - }; - } else if (structuredResult.structuredContent) { - // Validate the structured content - validationResult = validateToolOutput( - selectedTool.name, - structuredResult.structuredContent, - ); - } - } - - // Check compatibility if both structured and unstructured content exist - // AND the tool has an output schema - let compatibilityResult = null; - if ( - structuredResult.structuredContent && - structuredResult.content.length > 0 && - selectedTool && - hasOutputSchema(selectedTool.name) - ) { - compatibilityResult = checkContentCompatibility( - structuredResult.structuredContent, - structuredResult.content, - ); - } - - return ( - <> -

- Tool Result:{" "} - {isError ? ( - Error - ) : ( - Success - )} -

- {structuredResult.structuredContent && ( -
-
- Structured Content: -
-
- - {validationResult && ( -
- {validationResult.isValid ? ( - "✓ Valid according to output schema" - ) : ( - <>✗ Validation Error: {validationResult.error} - )} -
- )} -
-
- )} - {!structuredResult.structuredContent && - validationResult && - !validationResult.isValid && ( -
-
- ✗ Validation Error: {validationResult.error} -
-
- )} - {structuredResult.content.length > 0 && ( -
- {structuredResult.structuredContent && ( - <> -
- Unstructured Content: -
- {compatibilityResult && ( -
- {compatibilityResult.isCompatible ? "✓" : "⚠"}{" "} - {compatibilityResult.message} -
- )} - - )} - {structuredResult.content.map((item, index) => ( -
- {item.type === "text" && ( - - )} - {item.type === "image" && ( - Tool result image - )} - {item.type === "resource" && - (item.resource?.mimeType?.startsWith("audio/") ? ( - - ) : ( - - ))} -
- ))} -
- )} - - ); - } else if ("toolResult" in toolResult) { - return ( - <> -

Tool Result (Legacy):

- - - - ); - } - }; - return (
@@ -475,7 +253,10 @@ const ToolsTab = ({ )} - {toolResult && renderToolResult()} +
) : ( diff --git a/client/src/utils/schemaUtils.ts b/client/src/utils/schemaUtils.ts index d2440875..30210532 100644 --- a/client/src/utils/schemaUtils.ts +++ b/client/src/utils/schemaUtils.ts @@ -3,7 +3,6 @@ import Ajv from "ajv"; import type { ValidateFunction } from "ajv"; import type { Tool } from "@modelcontextprotocol/sdk/types.js"; -// Create a single Ajv instance following the SDK pattern const ajv = new Ajv(); // Cache for compiled validators diff --git a/package-lock.json b/package-lock.json index 0274b038..2920ca59 100644 --- a/package-lock.json +++ b/package-lock.json @@ -77,6 +77,7 @@ "@radix-ui/react-tabs": "^1.1.1", "@radix-ui/react-toast": "^1.2.6", "@radix-ui/react-tooltip": "^1.1.8", + "ajv": "^6.12.6", "class-variance-authority": "^0.7.0", "clsx": "^2.1.1", "cmdk": "^1.0.4", @@ -4002,7 +4003,6 @@ "version": "6.12.6", "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.12.6.tgz", "integrity": "sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g==", - "dev": true, "license": "MIT", "dependencies": { "fast-deep-equal": "^3.1.1", @@ -5710,7 +5710,6 @@ "version": "2.1.0", "resolved": "https://registry.npmjs.org/fast-json-stable-stringify/-/fast-json-stable-stringify-2.1.0.tgz", "integrity": "sha512-lhd/wF+Lk98HZoTCtlVraHtfh5XYijIjalXck7saUtuanSDyLMxnHhSXEDJqHxD7msR8D0uCmqlkwjCV8xvwHw==", - "dev": true, "license": "MIT" }, "node_modules/fast-levenshtein": { @@ -7740,7 +7739,6 @@ "version": "0.4.1", "resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-0.4.1.tgz", "integrity": "sha512-xbbCH5dCYU5T8LcEhhuh7HJ88HXuW3qsI3Y0zOZFKfZEHcpWiHU/Jxzk629Brsab/mMiHQti9wMP+845RPe3Vg==", - "dev": true, "license": "MIT" }, "node_modules/json-stable-stringify-without-jsonify": { @@ -8812,7 +8810,6 @@ "version": "2.3.1", "resolved": "https://registry.npmjs.org/punycode/-/punycode-2.3.1.tgz", "integrity": "sha512-vYt7UD1U9Wg6138shLtLOvdAu+8DsC/ilFtEVHcH+wydcSpNE20AfSOduf6MkRFahL5FY7X1oU7nKVZFtfq8Fg==", - "dev": true, "license": "MIT", "engines": { "node": ">=6" @@ -10488,7 +10485,6 @@ "version": "4.4.1", "resolved": "https://registry.npmjs.org/uri-js/-/uri-js-4.4.1.tgz", "integrity": "sha512-7rKUyy33Q1yc98pQ1DAmLtwX109F7TIfWlW1Ydo8Wl1ii1SeHieeh0HHfPeL2fMXK6z0s8ecKs9frCuLJvndBg==", - "dev": true, "license": "BSD-2-Clause", "dependencies": { "punycode": "^2.1.0"