From b555e41c0b3c87cbc08cade9a92e587ee4912155 Mon Sep 17 00:00:00 2001 From: olaservo Date: Sun, 1 Jun 2025 09:03:02 -0700 Subject: [PATCH] Address the schema architecture crisis --- client/src/components/DynamicJsonForm.tsx | 89 ++++++++++++++++++----- client/src/components/ToolsTab.tsx | 8 +- client/src/utils/jsonUtils.ts | 2 +- client/src/utils/schemaUtils.ts | 55 +++++++++----- 4 files changed, 114 insertions(+), 40 deletions(-) diff --git a/client/src/components/DynamicJsonForm.tsx b/client/src/components/DynamicJsonForm.tsx index 6a5993c3..bd8c6cfd 100644 --- a/client/src/components/DynamicJsonForm.tsx +++ b/client/src/components/DynamicJsonForm.tsx @@ -113,6 +113,8 @@ const DynamicJsonForm = ({ currentValue: JsonValue, path: string[] = [], depth: number = 0, + parentSchema?: JsonSchemaType, + propertyName?: string, ) => { if ( depth >= maxDepth && @@ -122,7 +124,7 @@ const DynamicJsonForm = ({ return ( { const val = e.target.value; - // Allow clearing non-required fields by setting undefined - // This preserves the distinction between empty string and unset - if (!val && !propSchema.required) { - handleFieldChange(path, undefined); - } else { - handleFieldChange(path, val); - } + // Always allow setting string values, including empty strings + handleFieldChange(path, val); }} placeholder={propSchema.description} - required={propSchema.required} + required={isRequired} /> ); case "number": @@ -167,9 +167,7 @@ const DynamicJsonForm = ({ value={(currentValue as number)?.toString() ?? ""} onChange={(e) => { const val = e.target.value; - // Allow clearing non-required number fields - // This preserves the distinction between 0 and unset - if (!val && !propSchema.required) { + if (!val && !isRequired) { handleFieldChange(path, undefined); } else { const num = Number(val); @@ -179,7 +177,7 @@ const DynamicJsonForm = ({ } }} placeholder={propSchema.description} - required={propSchema.required} + required={isRequired} /> ); case "integer": @@ -190,9 +188,7 @@ const DynamicJsonForm = ({ value={(currentValue as number)?.toString() ?? ""} onChange={(e) => { const val = e.target.value; - // Allow clearing non-required integer fields - // This preserves the distinction between 0 and unset - if (!val && !propSchema.required) { + if (!val && !isRequired) { handleFieldChange(path, undefined); } else { const num = Number(val); @@ -203,7 +199,7 @@ const DynamicJsonForm = ({ } }} placeholder={propSchema.description} - required={propSchema.required} + required={isRequired} /> ); case "boolean": @@ -213,7 +209,64 @@ const DynamicJsonForm = ({ checked={(currentValue as boolean) ?? false} onChange={(e) => handleFieldChange(path, e.target.checked)} className="w-4 h-4" - required={propSchema.required} + required={isRequired} + /> + ); + case "object": + if (!propSchema.properties) { + return ( + { + try { + const parsed = JSON.parse(newValue); + handleFieldChange(path, parsed); + setJsonError(undefined); + } catch (err) { + setJsonError(err instanceof Error ? err.message : "Invalid JSON"); + } + }} + error={jsonError} + /> + ); + } + + return ( +
+ {Object.entries(propSchema.properties).map(([key, subSchema]) => ( +
+ + {renderFormFields( + subSchema as JsonSchemaType, + (currentValue as Record)?.[key], + [...path, key], + depth + 1, + propSchema, + key, + )} +
+ ))} +
+ ); + case "array": + return ( + { + try { + const parsed = JSON.parse(newValue); + handleFieldChange(path, parsed); + setJsonError(undefined); + } catch (err) { + setJsonError(err instanceof Error ? err.message : "Invalid JSON"); + } + }} + error={jsonError} /> ); default: diff --git a/client/src/components/ToolsTab.tsx b/client/src/components/ToolsTab.tsx index 8a7f6578..93073da3 100644 --- a/client/src/components/ToolsTab.tsx +++ b/client/src/components/ToolsTab.tsx @@ -7,7 +7,7 @@ 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, isPropertyRequired } from "@/utils/schemaUtils"; import { CompatibilityCallToolResult, ListToolsResult, @@ -48,7 +48,7 @@ const ToolsTab = ({ selectedTool?.inputSchema.properties ?? [], ).map(([key, value]) => [ key, - generateDefaultValue(value as JsonSchemaType), + generateDefaultValue(value as JsonSchemaType, key, selectedTool?.inputSchema as JsonSchemaType), ]); setParams(Object.fromEntries(params)); }, [selectedTool]); @@ -92,13 +92,15 @@ const ToolsTab = ({ {Object.entries(selectedTool.inputSchema.properties ?? []).map( ([key, value]) => { const prop = value as JsonSchemaType; + const inputSchema = selectedTool.inputSchema as JsonSchemaType; + const required = isPropertyRequired(key, inputSchema); return (
{prop.type === "boolean" ? (
diff --git a/client/src/utils/jsonUtils.ts b/client/src/utils/jsonUtils.ts index 28cbf303..83317938 100644 --- a/client/src/utils/jsonUtils.ts +++ b/client/src/utils/jsonUtils.ts @@ -17,7 +17,7 @@ export type JsonSchemaType = { | "object" | "null"; description?: string; - required?: boolean; + required?: string[]; default?: JsonValue; properties?: Record; items?: JsonSchemaType; diff --git a/client/src/utils/schemaUtils.ts b/client/src/utils/schemaUtils.ts index 30210532..fe643b95 100644 --- a/client/src/utils/schemaUtils.ts +++ b/client/src/utils/schemaUtils.ts @@ -81,46 +81,65 @@ export function hasOutputSchema(toolName: string): boolean { /** * Generates a default value based on a JSON schema type * @param schema The JSON schema definition - * @returns A default value matching the schema type, or null for non-required fields + * @param propertyName Optional property name for checking if it's required in parent schema + * @param parentSchema Optional parent schema to check required array + * @returns A default value matching the schema type */ -export function generateDefaultValue(schema: JsonSchemaType): JsonValue { - if ("default" in schema) { +export function generateDefaultValue( + schema: JsonSchemaType, + propertyName?: string, + parentSchema?: JsonSchemaType +): JsonValue { + if ("default" in schema && schema.default !== undefined) { return schema.default; } - if (!schema.required) { - if (schema.type === "array") return []; - if (schema.type === "object") return {}; - return undefined; - } + // Check if this property is required in the parent schema + const isRequired = parentSchema?.required?.includes(propertyName || "") ?? false; switch (schema.type) { case "string": - return ""; + return isRequired ? "" : undefined; case "number": case "integer": - return 0; + return isRequired ? 0 : undefined; case "boolean": - return false; + return isRequired ? false : undefined; case "array": return []; case "object": { if (!schema.properties) return {}; const obj: JsonObject = {}; - Object.entries(schema.properties) - .filter(([, prop]) => prop.required) - .forEach(([key, prop]) => { - const value = generateDefaultValue(prop); - obj[key] = value; - }); + // Only include properties that are required according to the schema's required array + Object.entries(schema.properties).forEach(([key, prop]) => { + const isPropertyRequired = schema.required?.includes(key) ?? false; + if (isPropertyRequired) { + const value = generateDefaultValue(prop, key, schema); + if (value !== undefined) { + obj[key] = value; + } + } + }); return obj; } - default: + case "null": return null; + default: + return undefined; } } +/** + * Helper function to check if a property is required in a schema + * @param propertyName The name of the property to check + * @param schema The parent schema containing the required array + * @returns true if the property is required, false otherwise + */ +export function isPropertyRequired(propertyName: string, schema: JsonSchemaType): boolean { + return schema.required?.includes(propertyName) ?? false; +} + /** * Formats a field key into a human-readable label * @param key The field key to format