Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion agents-api/src/domains/run/agents/generation/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,17 @@ export async function runGenerate(

const shouldStream = ctx.isDelegatedAgent ? undefined : ctx.streamHelper;

const dataComponentsSchema = hasStructuredOutput ? buildDataComponentsSchema(ctx) : null;
let dataComponentsSchema: z.ZodType<any> | 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,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { z } from '@hono/zod-openapi';
import { normalizeDataComponentSchema } from '@inkeep/agents-core';
import { getLogger } from '../../../../logger';
import {
ArtifactCreateSchema,
ArtifactReferenceSchema,
} from '../../artifacts/artifact-component-schema';
import { SchemaProcessor } from '../../utils/SchemaProcessor';
import type { AgentRunContext } from '../agent-types';

const logger = getLogger('Agent');
Expand All @@ -13,7 +13,7 @@ export function buildDataComponentsSchema(ctx: AgentRunContext): z.ZodType<any>
const componentSchemas: z.ZodType<any>[] = [];

ctx.config.dataComponents?.forEach((dc) => {
const normalizedProps = SchemaProcessor.makeAllPropertiesRequired(dc.props);
const normalizedProps = normalizeDataComponentSchema(dc.props as Record<string, unknown>);
const propsSchema = z.fromJSONSchema(normalizedProps);
componentSchemas.push(
z.object({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type {
DataComponentInsert,
JsonSchemaForLlmSchemaType,
} from '@inkeep/agents-core';
import { normalizeDataComponentSchema } from '@inkeep/agents-core';
import type { JSONSchema } from 'zod/v4/core';
import { SchemaProcessor } from '../utils/SchemaProcessor';

Expand Down Expand Up @@ -102,7 +103,7 @@ export class ArtifactCreateSchema {
} satisfies JSONSchema.BaseSchema;

// Normalize schema for cross-provider compatibility
const normalizedPropsSchema = SchemaProcessor.makeAllPropertiesRequired(propsSchema);
const normalizedPropsSchema = normalizeDataComponentSchema(propsSchema);

return z.object({
id: z.string(),
Expand Down Expand Up @@ -156,7 +157,7 @@ export class ArtifactCreateSchema {
};

// Normalize schema for cross-provider compatibility
const normalizedPropsSchema = SchemaProcessor.makeAllPropertiesRequired(propsSchema);
const normalizedPropsSchema = normalizeDataComponentSchema(propsSchema);

return {
id: `artifact-create-${component.name.toLowerCase().replace(/\s+/g, '-')}`,
Expand Down
63 changes: 15 additions & 48 deletions agents-api/src/domains/run/utils/SchemaProcessor.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import {
makeAllPropertiesRequired as makeAllPropertiesRequiredCore,
stripUnsupportedConstraints,
} from '@inkeep/agents-core';
import jmespath from 'jmespath';
import type { JSONSchema } from 'zod/v4/core';
import { getLogger } from '../../../logger';
Expand Down Expand Up @@ -226,59 +230,22 @@ export class SchemaProcessor {

/**
* Makes all properties required recursively throughout the schema.
* This ensures compatibility across all LLM providers (OpenAI/Azure require it, Anthropic accepts it).
* Delegates to the shared `makeAllPropertiesRequired` utility from agents-core.
*/
static makeAllPropertiesRequired<
T extends JSONSchema.BaseSchema | 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 = SchemaProcessor.makeAllPropertiesRequired(prop);
const alreadyNullable =
(Array.isArray(processed.anyOf) &&
processed.anyOf.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 = SchemaProcessor.makeAllPropertiesRequired(normalized.items as any);
}
if (Array.isArray(normalized.anyOf)) {
normalized.anyOf = normalized.anyOf.map((s: any) =>
SchemaProcessor.makeAllPropertiesRequired(s)
);
}
if (Array.isArray(normalized.oneOf)) {
normalized.oneOf = normalized.oneOf.map((s: any) =>
SchemaProcessor.makeAllPropertiesRequired(s)
);
}
if (Array.isArray(normalized.allOf)) {
normalized.allOf = normalized.allOf.map((s: any) =>
SchemaProcessor.makeAllPropertiesRequired(s)
);
}
return makeAllPropertiesRequiredCore(schema as Record<string, unknown>) as T;
}

return normalized;
/**
* Strips JSON Schema constraints that are not supported by all LLM providers.
* Delegates to the shared `stripUnsupportedConstraints` utility from agents-core.
*/
static stripUnsupportedConstraints<
T extends JSONSchema.BaseSchema | Record<string, unknown> | null | undefined,
>(schema: T): T {
return stripUnsupportedConstraints(schema as Record<string, unknown>) as T;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -61,7 +61,9 @@ export async function POST(

// Define schema for generated output
const mockDataSchema = artifactComponent.props
? z.fromJSONSchema(artifactComponent.props)
? z.fromJSONSchema(
normalizeDataComponentSchema(artifactComponent.props as Record<string, unknown>)
)
: z.string();
const renderSchema = z.object({
component: z.string().describe('The React component code'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -55,10 +55,10 @@ export async function POST(
// Prepare model configuration
const modelConfig = ModelFactory.prepareGenerationConfig(project.models?.base as any);

// 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);
const normalizedProps = normalizeDataComponentSchema(
dataComponent.props as Record<string, unknown>
);
const mockDataSchema = z.fromJSONSchema(normalizedProps);
Comment on lines +58 to +61
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 MAJOR: Missing error handling for schema normalization

Issue: normalizeDataComponentSchema() and z.fromJSONSchema() are called without specific error handling. If normalization fails, the outer try/catch (line 124) catches it and returns a generic HTTP 500 with error.message. Users see "Internal server error" with no context about which data component failed or why.

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'),
Expand Down
175 changes: 175 additions & 0 deletions packages/agents-core/src/utils/__tests__/schema-normalization.test.ts
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');
});
});
Copy link
Contributor

Choose a reason for hiding this comment

The 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:

  1. oneOf/allOf recursive stripping — only anyOf is tested (line 76-82)
  2. nullable: true property handling — the code checks for this (schema-conversion.ts:167) but no test validates it
  3. Deeply nested schemas (3+ levels) — only one level of nesting is tested

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:

Loading
Loading