Address the schema architecture crisis

This commit is contained in:
olaservo 2025-06-01 09:03:02 -07:00
commit b555e41c0b
4 changed files with 114 additions and 40 deletions

View file

@ -113,6 +113,8 @@ const DynamicJsonForm = ({
currentValue: JsonValue, currentValue: JsonValue,
path: string[] = [], path: string[] = [],
depth: number = 0, depth: number = 0,
parentSchema?: JsonSchemaType,
propertyName?: string,
) => { ) => {
if ( if (
depth >= maxDepth && depth >= maxDepth &&
@ -122,7 +124,7 @@ const DynamicJsonForm = ({
return ( return (
<JsonEditor <JsonEditor
value={JSON.stringify( value={JSON.stringify(
currentValue ?? generateDefaultValue(propSchema), currentValue ?? generateDefaultValue(propSchema, propertyName, parentSchema),
null, null,
2, 2,
)} )}
@ -140,6 +142,9 @@ const DynamicJsonForm = ({
); );
} }
// Check if this property is required in the parent schema
const isRequired = parentSchema?.required?.includes(propertyName || "") ?? false;
switch (propSchema.type) { switch (propSchema.type) {
case "string": case "string":
return ( return (
@ -148,16 +153,11 @@ const DynamicJsonForm = ({
value={(currentValue as string) ?? ""} value={(currentValue as string) ?? ""}
onChange={(e) => { onChange={(e) => {
const val = e.target.value; const val = e.target.value;
// Allow clearing non-required fields by setting undefined // Always allow setting string values, including empty strings
// This preserves the distinction between empty string and unset handleFieldChange(path, val);
if (!val && !propSchema.required) {
handleFieldChange(path, undefined);
} else {
handleFieldChange(path, val);
}
}} }}
placeholder={propSchema.description} placeholder={propSchema.description}
required={propSchema.required} required={isRequired}
/> />
); );
case "number": case "number":
@ -167,9 +167,7 @@ const DynamicJsonForm = ({
value={(currentValue as number)?.toString() ?? ""} value={(currentValue as number)?.toString() ?? ""}
onChange={(e) => { onChange={(e) => {
const val = e.target.value; const val = e.target.value;
// Allow clearing non-required number fields if (!val && !isRequired) {
// This preserves the distinction between 0 and unset
if (!val && !propSchema.required) {
handleFieldChange(path, undefined); handleFieldChange(path, undefined);
} else { } else {
const num = Number(val); const num = Number(val);
@ -179,7 +177,7 @@ const DynamicJsonForm = ({
} }
}} }}
placeholder={propSchema.description} placeholder={propSchema.description}
required={propSchema.required} required={isRequired}
/> />
); );
case "integer": case "integer":
@ -190,9 +188,7 @@ const DynamicJsonForm = ({
value={(currentValue as number)?.toString() ?? ""} value={(currentValue as number)?.toString() ?? ""}
onChange={(e) => { onChange={(e) => {
const val = e.target.value; const val = e.target.value;
// Allow clearing non-required integer fields if (!val && !isRequired) {
// This preserves the distinction between 0 and unset
if (!val && !propSchema.required) {
handleFieldChange(path, undefined); handleFieldChange(path, undefined);
} else { } else {
const num = Number(val); const num = Number(val);
@ -203,7 +199,7 @@ const DynamicJsonForm = ({
} }
}} }}
placeholder={propSchema.description} placeholder={propSchema.description}
required={propSchema.required} required={isRequired}
/> />
); );
case "boolean": case "boolean":
@ -213,7 +209,64 @@ const DynamicJsonForm = ({
checked={(currentValue as boolean) ?? false} checked={(currentValue as boolean) ?? false}
onChange={(e) => handleFieldChange(path, e.target.checked)} onChange={(e) => handleFieldChange(path, e.target.checked)}
className="w-4 h-4" className="w-4 h-4"
required={propSchema.required} required={isRequired}
/>
);
case "object":
if (!propSchema.properties) {
return (
<JsonEditor
value={JSON.stringify(currentValue ?? {}, null, 2)}
onChange={(newValue) => {
try {
const parsed = JSON.parse(newValue);
handleFieldChange(path, parsed);
setJsonError(undefined);
} catch (err) {
setJsonError(err instanceof Error ? err.message : "Invalid JSON");
}
}}
error={jsonError}
/>
);
}
return (
<div className="space-y-2 border rounded p-3">
{Object.entries(propSchema.properties).map(([key, subSchema]) => (
<div key={key}>
<label className="block text-sm font-medium mb-1">
{key}
{propSchema.required?.includes(key) && (
<span className="text-red-500 ml-1">*</span>
)}
</label>
{renderFormFields(
subSchema as JsonSchemaType,
(currentValue as Record<string, JsonValue>)?.[key],
[...path, key],
depth + 1,
propSchema,
key,
)}
</div>
))}
</div>
);
case "array":
return (
<JsonEditor
value={JSON.stringify(currentValue ?? [], null, 2)}
onChange={(newValue) => {
try {
const parsed = JSON.parse(newValue);
handleFieldChange(path, parsed);
setJsonError(undefined);
} catch (err) {
setJsonError(err instanceof Error ? err.message : "Invalid JSON");
}
}}
error={jsonError}
/> />
); );
default: default:

View file

@ -7,7 +7,7 @@ import { TabsContent } from "@/components/ui/tabs";
import { Textarea } from "@/components/ui/textarea"; import { Textarea } from "@/components/ui/textarea";
import DynamicJsonForm from "./DynamicJsonForm"; import DynamicJsonForm from "./DynamicJsonForm";
import type { JsonValue, JsonSchemaType } from "@/utils/jsonUtils"; import type { JsonValue, JsonSchemaType } from "@/utils/jsonUtils";
import { generateDefaultValue } from "@/utils/schemaUtils"; import { generateDefaultValue, isPropertyRequired } from "@/utils/schemaUtils";
import { import {
CompatibilityCallToolResult, CompatibilityCallToolResult,
ListToolsResult, ListToolsResult,
@ -48,7 +48,7 @@ const ToolsTab = ({
selectedTool?.inputSchema.properties ?? [], selectedTool?.inputSchema.properties ?? [],
).map(([key, value]) => [ ).map(([key, value]) => [
key, key,
generateDefaultValue(value as JsonSchemaType), generateDefaultValue(value as JsonSchemaType, key, selectedTool?.inputSchema as JsonSchemaType),
]); ]);
setParams(Object.fromEntries(params)); setParams(Object.fromEntries(params));
}, [selectedTool]); }, [selectedTool]);
@ -92,13 +92,15 @@ const ToolsTab = ({
{Object.entries(selectedTool.inputSchema.properties ?? []).map( {Object.entries(selectedTool.inputSchema.properties ?? []).map(
([key, value]) => { ([key, value]) => {
const prop = value as JsonSchemaType; const prop = value as JsonSchemaType;
const inputSchema = selectedTool.inputSchema as JsonSchemaType;
const required = isPropertyRequired(key, inputSchema);
return ( return (
<div key={key}> <div key={key}>
<Label <Label
htmlFor={key} htmlFor={key}
className="block text-sm font-medium text-gray-700 dark:text-gray-300" className="block text-sm font-medium text-gray-700 dark:text-gray-300"
> >
{key} {key}{required && <span className="text-red-500 ml-1">*</span>}
</Label> </Label>
{prop.type === "boolean" ? ( {prop.type === "boolean" ? (
<div className="flex items-center space-x-2 mt-2"> <div className="flex items-center space-x-2 mt-2">

View file

@ -17,7 +17,7 @@ export type JsonSchemaType = {
| "object" | "object"
| "null"; | "null";
description?: string; description?: string;
required?: boolean; required?: string[];
default?: JsonValue; default?: JsonValue;
properties?: Record<string, JsonSchemaType>; properties?: Record<string, JsonSchemaType>;
items?: JsonSchemaType; items?: JsonSchemaType;

View file

@ -81,46 +81,65 @@ export function hasOutputSchema(toolName: string): boolean {
/** /**
* Generates a default value based on a JSON schema type * Generates a default value based on a JSON schema type
* @param schema The JSON schema definition * @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 { export function generateDefaultValue(
if ("default" in schema) { schema: JsonSchemaType,
propertyName?: string,
parentSchema?: JsonSchemaType
): JsonValue {
if ("default" in schema && schema.default !== undefined) {
return schema.default; return schema.default;
} }
if (!schema.required) { // Check if this property is required in the parent schema
if (schema.type === "array") return []; const isRequired = parentSchema?.required?.includes(propertyName || "") ?? false;
if (schema.type === "object") return {};
return undefined;
}
switch (schema.type) { switch (schema.type) {
case "string": case "string":
return ""; return isRequired ? "" : undefined;
case "number": case "number":
case "integer": case "integer":
return 0; return isRequired ? 0 : undefined;
case "boolean": case "boolean":
return false; return isRequired ? false : undefined;
case "array": case "array":
return []; return [];
case "object": { case "object": {
if (!schema.properties) return {}; if (!schema.properties) return {};
const obj: JsonObject = {}; const obj: JsonObject = {};
Object.entries(schema.properties) // Only include properties that are required according to the schema's required array
.filter(([, prop]) => prop.required) Object.entries(schema.properties).forEach(([key, prop]) => {
.forEach(([key, prop]) => { const isPropertyRequired = schema.required?.includes(key) ?? false;
const value = generateDefaultValue(prop); if (isPropertyRequired) {
obj[key] = value; const value = generateDefaultValue(prop, key, schema);
}); if (value !== undefined) {
obj[key] = value;
}
}
});
return obj; return obj;
} }
default: case "null":
return 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 * Formats a field key into a human-readable label
* @param key The field key to format * @param key The field key to format