-
Notifications
You must be signed in to change notification settings - Fork 111
fix: normalize data component schemas for cross-provider LLM compatibility #2764
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 1 commit
58e5272
af5a952
4d46e4e
fcb5952
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 |
|---|---|---|
|
|
@@ -225,7 +225,17 @@ export async function runGenerate( | |
|
|
||
| const shouldStream = ctx.isDelegatedAgent ? undefined : ctx.streamHelper; | ||
|
|
||
| const dataComponentsSchema = hasStructuredOutput ? buildDataComponentsSchema(ctx) : null; | ||
| let dataComponentsSchema: ReturnType<typeof buildDataComponentsSchema> | null = null; | ||
| if (hasStructuredOutput) { | ||
| try { | ||
| dataComponentsSchema = buildDataComponentsSchema(ctx); | ||
| } catch (err) { | ||
| logger.error( | ||
| { agentId: ctx.config.id, err }, | ||
| 'Failed to build data components schema — continuing without structured output' | ||
| ); | ||
| } | ||
| } | ||
|
||
|
|
||
| const baseConfig = buildBaseGenerationConfig( | ||
| ctx, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ | |
| * 4. Streams NDJSON response back to client | ||
| */ | ||
|
|
||
| import { ModelFactory } from '@inkeep/agents-core'; | ||
| import { ModelFactory, normalizeDataComponentSchema } from '@inkeep/agents-core'; | ||
| import { Output, streamText } from 'ai'; | ||
| import type { NextRequest } from 'next/server'; | ||
| import { z } from 'zod'; | ||
|
|
@@ -57,8 +57,13 @@ export async function POST( | |
|
|
||
| // Define schema for generated output | ||
| // Dynamically create mockData schema from component's props JSON Schema. | ||
| // This ensures Anthropic gets proper types instead of z.any() which it rejects. | ||
| const mockDataSchema = z.fromJSONSchema(dataComponent.props); | ||
| // Normalize for cross-provider compatibility: | ||
| // - strips Anthropic-unsupported constraints (minimum/maximum on numbers) | ||
| // - ensures all properties are in required (OpenAI strict-mode requirement) | ||
| const normalizedProps = normalizeDataComponentSchema( | ||
anubra266 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| dataComponent.props as Record<string, unknown> | ||
| ); | ||
| const mockDataSchema = z.fromJSONSchema(normalizedProps); | ||
|
Comment on lines
+58
to
+61
Contributor
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. 🟠 MAJOR: Missing error handling for schema normalization Issue: Why: When clicking "Generate Component" in the Agent Builder UI fails, users cannot self-diagnose. They see a generic error toast with no actionable information. Every schema issue becomes a support ticket. Fix: Add specific error handling with actionable feedback: let normalizedProps: Record<string, unknown>;
let mockDataSchema: z.ZodType<any>;
try {
normalizedProps = normalizeDataComponentSchema(
dataComponent.props as Record<string, unknown>
);
mockDataSchema = z.fromJSONSchema(normalizedProps);
} catch (error) {
console.error('Schema normalization failed:', {
dataComponentId,
dataComponentName: dataComponent.name,
error: error instanceof Error ? error.message : String(error),
});
return new Response(
JSON.stringify({
error: 'Invalid data component schema',
message: 'The data component schema could not be processed. Please check that all field types are valid.',
details: error instanceof Error ? error.message : undefined,
}),
{
status: 400,
headers: { 'Content-Type': 'application/json' },
}
);
}Refs:
|
||
| const renderSchema = z.object({ | ||
| component: z.string().describe('The React component code'), | ||
| mockData: mockDataSchema.describe('Sample data matching the props schema'), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,175 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { | ||
| makeAllPropertiesRequired, | ||
| normalizeDataComponentSchema, | ||
| stripUnsupportedConstraints, | ||
| } from '../schema-conversion'; | ||
|
|
||
| describe('stripUnsupportedConstraints', () => { | ||
| it('strips minimum and maximum from number types', () => { | ||
| const schema = { | ||
| type: 'object', | ||
| properties: { | ||
| score: { type: 'number', minimum: 0, maximum: 1, description: 'A score' }, | ||
| }, | ||
| }; | ||
| const result = stripUnsupportedConstraints(schema) as any; | ||
| expect(result.properties.score).not.toHaveProperty('minimum'); | ||
| expect(result.properties.score).not.toHaveProperty('maximum'); | ||
| expect(result.properties.score.type).toBe('number'); | ||
| }); | ||
|
|
||
| it('strips exclusiveMinimum, exclusiveMaximum, and multipleOf from number types', () => { | ||
| const schema = { | ||
| type: 'number', | ||
| exclusiveMinimum: 0, | ||
| exclusiveMaximum: 100, | ||
| multipleOf: 5, | ||
| }; | ||
| const result = stripUnsupportedConstraints(schema) as any; | ||
| expect(result).not.toHaveProperty('exclusiveMinimum'); | ||
| expect(result).not.toHaveProperty('exclusiveMaximum'); | ||
| expect(result).not.toHaveProperty('multipleOf'); | ||
| }); | ||
|
|
||
| it('strips constraints from integer types', () => { | ||
| const schema = { type: 'integer', minimum: 1, maximum: 10 }; | ||
| const result = stripUnsupportedConstraints(schema) as any; | ||
| expect(result).not.toHaveProperty('minimum'); | ||
| expect(result).not.toHaveProperty('maximum'); | ||
| }); | ||
|
|
||
| it('does not strip minimum/maximum from non-number types', () => { | ||
| const schema = { type: 'string', minLength: 1, maxLength: 100 }; | ||
| const result = stripUnsupportedConstraints(schema) as any; | ||
| expect(result.minLength).toBe(1); | ||
| expect(result.maxLength).toBe(100); | ||
| }); | ||
|
|
||
| it('strips recursively in nested object properties', () => { | ||
| const schema = { | ||
| type: 'object', | ||
| properties: { | ||
| nested: { | ||
| type: 'object', | ||
| properties: { | ||
| value: { type: 'number', minimum: 0, maximum: 1 }, | ||
| }, | ||
| }, | ||
| }, | ||
| }; | ||
| const result = stripUnsupportedConstraints(schema) as any; | ||
| expect(result.properties.nested.properties.value).not.toHaveProperty('minimum'); | ||
| expect(result.properties.nested.properties.value).not.toHaveProperty('maximum'); | ||
| }); | ||
|
|
||
| it('strips recursively in array items', () => { | ||
| const schema = { | ||
| type: 'array', | ||
| items: { type: 'number', minimum: 0, maximum: 100 }, | ||
| }; | ||
| const result = stripUnsupportedConstraints(schema) as any; | ||
| expect(result.items).not.toHaveProperty('minimum'); | ||
| expect(result.items).not.toHaveProperty('maximum'); | ||
| }); | ||
|
|
||
| it('strips recursively in anyOf', () => { | ||
| const schema = { | ||
| anyOf: [{ type: 'number', minimum: 0 }, { type: 'null' }], | ||
| }; | ||
| const result = stripUnsupportedConstraints(schema) as any; | ||
| expect(result.anyOf[0]).not.toHaveProperty('minimum'); | ||
| }); | ||
|
|
||
| it('handles null and undefined gracefully', () => { | ||
| expect(stripUnsupportedConstraints(null)).toBeNull(); | ||
| expect(stripUnsupportedConstraints(undefined)).toBeUndefined(); | ||
| }); | ||
| }); | ||
|
|
||
| describe('makeAllPropertiesRequired', () => { | ||
| it('adds all property keys to required', () => { | ||
| const schema = { | ||
| type: 'object', | ||
| properties: { | ||
| name: { type: 'string' }, | ||
| score: { type: 'number' }, | ||
| }, | ||
| required: ['name'], | ||
| }; | ||
| const result = makeAllPropertiesRequired(schema) as any; | ||
| expect(result.required).toEqual(['name', 'score']); | ||
| }); | ||
|
|
||
| it('wraps originally-optional fields as nullable', () => { | ||
| const schema = { | ||
| type: 'object', | ||
| properties: { | ||
| required_field: { type: 'string' }, | ||
| optional_field: { type: 'string' }, | ||
| }, | ||
| required: ['required_field'], | ||
| }; | ||
| const result = makeAllPropertiesRequired(schema) as any; | ||
| expect(result.properties.required_field).toEqual({ type: 'string' }); | ||
| expect(result.properties.optional_field).toEqual({ | ||
| anyOf: [{ type: 'string' }, { type: 'null' }], | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| describe('normalizeDataComponentSchema', () => { | ||
| it('fixes the exact schema from the bug report', () => { | ||
| const schema = { | ||
| type: 'object', | ||
| required: [ | ||
| 'question', | ||
| 'category', | ||
| 'draft_answer', | ||
| 'confidence_score', | ||
| 'confidence_level', | ||
| 'sources_used', | ||
| 'needs_sme_review', | ||
| ], | ||
| properties: { | ||
| question: { type: 'string', description: 'The original RFP question' }, | ||
| confidence_score: { | ||
| type: 'number', | ||
| maximum: 1, | ||
| minimum: 0, | ||
| description: 'Confidence score from 0.0 to 1.0', | ||
| }, | ||
| limitations: { | ||
| type: 'array', | ||
| items: { type: 'string', description: 'A specific limitation' }, | ||
| description: 'Any stated limitations', | ||
| }, | ||
| }, | ||
| additionalProperties: false, | ||
| }; | ||
|
|
||
| const result = normalizeDataComponentSchema(schema) as any; | ||
|
|
||
| // Anthropic fix: minimum/maximum stripped from number | ||
| expect(result.properties.confidence_score).not.toHaveProperty('minimum'); | ||
| expect(result.properties.confidence_score).not.toHaveProperty('maximum'); | ||
|
|
||
| // OpenAI fix: limitations now in required | ||
| expect(result.required).toContain('limitations'); | ||
| }); | ||
|
|
||
| it('produces a schema where all properties are in required', () => { | ||
| const schema = { | ||
| type: 'object', | ||
| properties: { | ||
| a: { type: 'string' }, | ||
| b: { type: 'number', minimum: 0, maximum: 10 }, | ||
| }, | ||
| required: ['a'], | ||
| }; | ||
| const result = normalizeDataComponentSchema(schema) as any; | ||
| expect(result.required).toEqual(['a', 'b']); | ||
| expect(result.properties.b).not.toHaveProperty('minimum'); | ||
| expect(result.properties.b).not.toHaveProperty('maximum'); | ||
| }); | ||
| }); | ||
|
Contributor
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. 🟡 Minor: Consider adding test coverage for edge cases Issue: The test suite covers the core functionality well, but could benefit from additional edge cases:
Why: These edge cases are handled in the implementation but not validated by tests. Future refactoring could silently break them. Fix: Consider adding these test cases: it('strips recursively in oneOf', () => {
const schema = {
oneOf: [{ type: 'number', minimum: 0 }, { type: 'string' }]
};
const result = stripUnsupportedConstraints(schema) as any;
expect(result.oneOf[0]).not.toHaveProperty('minimum');
});
it('does not double-wrap properties already marked as nullable', () => {
const schema = {
type: 'object',
properties: { field: { type: 'string', nullable: true } },
required: []
};
const result = makeAllPropertiesRequired(schema) as any;
expect(result.properties.field).not.toHaveProperty('anyOf');
});Refs: |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,6 +81,135 @@ export function isZodSchema(value: any): value is z.ZodObject<any> { | |
| return value?._def?.type === 'object'; | ||
| } | ||
|
|
||
| /** | ||
| * Strips JSON Schema numeric constraints that are not supported by all LLM providers. | ||
| * | ||
| * Anthropic structured output rejects `minimum`, `maximum`, `exclusiveMinimum`, | ||
| * `exclusiveMaximum`, and `multipleOf` on `number`/`integer` types. | ||
| * Applied recursively to handle nested objects and arrays. | ||
| */ | ||
| export function stripUnsupportedConstraints<T extends Record<string, unknown> | null | undefined>( | ||
| schema: T | ||
| ): T { | ||
| if (!schema || typeof schema !== 'object') { | ||
| return schema; | ||
| } | ||
|
|
||
| const stripped: any = { ...schema }; | ||
|
|
||
| if (stripped.type === 'number' || stripped.type === 'integer') { | ||
| delete stripped.minimum; | ||
| delete stripped.maximum; | ||
| delete stripped.exclusiveMinimum; | ||
| delete stripped.exclusiveMaximum; | ||
| delete stripped.multipleOf; | ||
| } | ||
|
|
||
| if (stripped.properties && typeof stripped.properties === 'object') { | ||
| const strippedProperties: any = {}; | ||
| for (const [key, value] of Object.entries(stripped.properties)) { | ||
| strippedProperties[key] = stripUnsupportedConstraints(value as Record<string, unknown>); | ||
| } | ||
| stripped.properties = strippedProperties; | ||
| } | ||
|
|
||
| if (stripped.items) { | ||
| stripped.items = stripUnsupportedConstraints(stripped.items as Record<string, unknown>); | ||
| } | ||
| if (Array.isArray(stripped.anyOf)) { | ||
| stripped.anyOf = stripped.anyOf.map((s: any) => | ||
| stripUnsupportedConstraints(s as Record<string, unknown>) | ||
| ); | ||
| } | ||
| if (Array.isArray(stripped.oneOf)) { | ||
| stripped.oneOf = stripped.oneOf.map((s: any) => | ||
| stripUnsupportedConstraints(s as Record<string, unknown>) | ||
| ); | ||
| } | ||
| if (Array.isArray(stripped.allOf)) { | ||
| stripped.allOf = stripped.allOf.map((s: any) => | ||
| stripUnsupportedConstraints(s as Record<string, unknown>) | ||
| ); | ||
| } | ||
|
|
||
| return stripped; | ||
|
Comment on lines
+91
to
+135
Contributor
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. Neither This is likely fine for current data component schemas (they tend to be flat), but worth noting as a known limitation — or adding a |
||
| } | ||
|
|
||
| /** | ||
| * Makes all properties required in an object schema, wrapping originally-optional | ||
| * fields as `{ anyOf: [<schema>, { type: 'null' }] }`. | ||
| * | ||
| * OpenAI strict-mode structured output requires every key in `properties` to also | ||
| * appear in `required`. Applied recursively to handle nested objects and arrays. | ||
| */ | ||
| export function makeAllPropertiesRequired<T extends Record<string, unknown> | null | undefined>( | ||
| schema: T | ||
| ): T { | ||
| if (!schema || typeof schema !== 'object') { | ||
| return schema; | ||
| } | ||
|
|
||
| const normalized: any = { ...schema }; | ||
|
|
||
| if (normalized.properties && typeof normalized.properties === 'object') { | ||
| const originalRequired: string[] = Array.isArray(normalized.required) | ||
| ? normalized.required | ||
| : []; | ||
| normalized.required = Object.keys(normalized.properties); | ||
|
|
||
| const normalizedProperties: any = {}; | ||
| for (const [key, value] of Object.entries(normalized.properties)) { | ||
| const prop = value as Record<string, unknown>; | ||
| const processed = makeAllPropertiesRequired(prop); | ||
| const alreadyNullable = | ||
| (Array.isArray(processed.anyOf) && | ||
| (processed.anyOf as any[]).some((s: any) => s?.type === 'null')) || | ||
| processed.nullable === true; | ||
| normalizedProperties[key] = | ||
| originalRequired.includes(key) || alreadyNullable | ||
| ? processed | ||
| : { anyOf: [processed, { type: 'null' }] }; | ||
| } | ||
| normalized.properties = normalizedProperties; | ||
| } | ||
|
|
||
| if (normalized.items) { | ||
| normalized.items = makeAllPropertiesRequired(normalized.items as Record<string, unknown>); | ||
| } | ||
| if (Array.isArray(normalized.anyOf)) { | ||
| normalized.anyOf = normalized.anyOf.map((s: any) => | ||
| makeAllPropertiesRequired(s as Record<string, unknown>) | ||
| ); | ||
| } | ||
| if (Array.isArray(normalized.oneOf)) { | ||
| normalized.oneOf = normalized.oneOf.map((s: any) => | ||
| makeAllPropertiesRequired(s as Record<string, unknown>) | ||
| ); | ||
| } | ||
| if (Array.isArray(normalized.allOf)) { | ||
| normalized.allOf = normalized.allOf.map((s: any) => | ||
| makeAllPropertiesRequired(s as Record<string, unknown>) | ||
| ); | ||
| } | ||
|
|
||
| return normalized; | ||
| } | ||
|
|
||
| /** | ||
| * Normalizes a data component JSON Schema for cross-provider LLM compatibility. | ||
| * | ||
| * Applies two transformations in order: | ||
| * 1. `stripUnsupportedConstraints` — removes `minimum`/`maximum`/etc. from numbers | ||
| * (Anthropic structured output rejects these) | ||
| * 2. `makeAllPropertiesRequired` — ensures every property appears in `required`, | ||
| * wrapping optional fields as nullable (OpenAI strict-mode requires this) | ||
| */ | ||
| export function normalizeDataComponentSchema<T extends Record<string, unknown> | null | undefined>( | ||
| schema: T | ||
| ): T { | ||
| return makeAllPropertiesRequired(stripUnsupportedConstraints(schema)); | ||
| } | ||
|
|
||
| /** | ||
| * Extract preview fields from either JSON Schema or Zod schema | ||
| */ | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.