diff --git a/docs/design/openai-tool-schema-grammar-compatibility.md b/docs/design/openai-tool-schema-grammar-compatibility.md new file mode 100644 index 00000000000..45b8d81f6e5 --- /dev/null +++ b/docs/design/openai-tool-schema-grammar-compatibility.md @@ -0,0 +1,27 @@ +# OpenAI Tool Schema Grammar Compatibility + +## Problem + +Some OpenAI-compatible runtimes compile every function schema in a request into one grammar. Older llama.cpp builds reject empty `properties` maps and large string or array repetition limits, so one valid but unsupported schema prevents the entire request from starting. Qwen Code ships tools with both shapes and can also receive them from MCP servers. + +Disabling all tools avoids grammar construction but also removes the functionality the user asked the agent to use. It is therefore a diagnostic fallback, not the primary fix. + +## Design + +Keep the registered tool set unchanged. Before an OpenAI-compatible request is sent, recursively relax only the wire copy of each schema: + +- omit empty `properties` maps and `additionalProperties: false` on object-capable schemas with zero declared properties; +- omit `minLength`, `maxLength`, `minItems`, and `maxItems` values at or above 1999, the lowest failing boundary measured across the four keywords; +- preserve smaller limits and all other supported constraints. + +Apply these grammar-specific relaxations only when the source schema passes the existing isolated strict compilation for its selected dialect and has no top-level `$id`. Schemas with a top-level `$id` keep their constraints because runtime validation uses a shared schema registry where duplicate IDs can prevent enforcement. If local validation cannot enforce the complete schema, keep its grammar constraints on the wire rather than broadening both enforcement layers. + +The original schema remains attached to the tool and continues to drive client-side parameter validation. The provider receives a schema it can compile, while Qwen Code still rejects tool calls that violate the original limits. + +## Compatibility + +This applies to both built-in tools and MCP-provided schemas because they share the same OpenAI conversion boundary. Native Gemini requests are unchanged. Providers that accept the original constraints receive a slightly relaxed wire schema, but local validation preserves their behavior. + +## Verification + +Unit coverage exercises recursive empty objects, the 1998/1999 boundary, the actual OpenAI tool converter, and source-schema immutability. A live LM Studio smoke remains useful when that runtime is available, but the regression test pins the exact request shapes that caused grammar initialization to fail. diff --git a/packages/core/src/core/openaiContentGenerator/converter.test.ts b/packages/core/src/core/openaiContentGenerator/converter.test.ts index 4d601a748b5..1a59786b5b8 100644 --- a/packages/core/src/core/openaiContentGenerator/converter.test.ts +++ b/packages/core/src/core/openaiContentGenerator/converter.test.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; import { OpenAIContentConverter } from './converter.js'; import { StreamingToolCallParser } from './streamingToolCallParser.js'; import { TaggedThinkingParser } from './taggedThinkingParser.js'; @@ -24,6 +24,7 @@ import { convertToFunctionResponse } from '../coreToolScheduler.js'; import { getToolCallPreparations } from '../tool-call-preparation.js'; import { isOpenAIReasoningThoughtPart } from '../../utils/thoughtUtils.js'; import { getGenAiUsageProvenance } from '../../telemetry/gen-ai-usage.js'; +import { SchemaValidator } from '../../utils/schemaValidator.js'; describe('OpenAIContentConverter', () => { let converter: typeof OpenAIContentConverter; @@ -6035,6 +6036,31 @@ describe('OpenAIContentConverter', () => { }); describe('convertLlmToolsToOpenAI', () => { + it('compiles a stable tool schema only once', async () => { + const parametersJsonSchema = { + type: 'object', + properties: { + value: { type: 'string', maxLength: 1999 }, + }, + }; + const tools = [ + { + functionDeclarations: [ + { name: 'stable', parametersJsonSchema }, + ], + }, + ] as Tool[]; + const compileStrict = vi.spyOn(SchemaValidator, 'compileStrict'); + + try { + await converter.convertLlmToolsToOpenAI(tools); + await converter.convertLlmToolsToOpenAI(tools); + expect(compileStrict).toHaveBeenCalledTimes(1); + } finally { + compileStrict.mockRestore(); + } + }); + it('removes uniqueItems from function-calling wire schemas', async () => { const parametersJsonSchema = { type: 'object', @@ -6081,6 +6107,140 @@ describe('OpenAIContentConverter', () => { expect(parametersJsonSchema.properties.blockedBy.uniqueItems).toBe(true); }); + it('only relaxes grammar constraints backed by local validation', async () => { + const supportedSchema = { + type: 'object', + properties: {}, + additionalProperties: false, + }; + const unsupportedSchema = { + $schema: 'https://json-schema.org/draft/2019-09/schema', + type: 'object', + properties: {}, + additionalProperties: false, + }; + const unsupportedVocabularySchema = { + type: 'object', + properties: { + tuple: { + type: 'array', + prefixItems: [ + { + type: 'object', + properties: {}, + additionalProperties: false, + }, + ], + }, + }, + }; + const tools = [ + { + functionDeclarations: [ + { name: 'supported', parametersJsonSchema: supportedSchema }, + { name: 'unsupported', parametersJsonSchema: unsupportedSchema }, + { + name: 'unsupported_vocabulary', + parametersJsonSchema: unsupportedVocabularySchema, + }, + { + name: 'without_local_schema', + parameters: { + type: Type.OBJECT, + properties: {}, + additionalProperties: false, + }, + }, + ], + }, + ] as Tool[]; + + const result = await converter.convertLlmToolsToOpenAI(tools); + + expect(result.map(({ function: declaration }) => declaration)).toEqual([ + { name: 'supported', description: '', parameters: { type: 'object' } }, + { + name: 'unsupported', + description: '', + parameters: { + type: 'object', + properties: {}, + additionalProperties: false, + }, + }, + { + name: 'unsupported_vocabulary', + description: '', + parameters: unsupportedVocabularySchema, + }, + { + name: 'without_local_schema', + description: '', + parameters: { + type: 'object', + properties: {}, + additionalProperties: false, + }, + }, + ]); + expect(supportedSchema).toEqual({ + type: 'object', + properties: {}, + additionalProperties: false, + }); + expect(unsupportedSchema.$schema).toBe( + 'https://json-schema.org/draft/2019-09/schema', + ); + expect( + unsupportedVocabularySchema.properties.tuple.prefixItems[0] + .additionalProperties, + ).toBe(false); + }); + + it('keeps grammar constraints for schemas with a top-level $id', async () => { + const sharedId = 'https://qwen-code.test/shared-tool-schema'; + const makeSchema = () => ({ + $id: sharedId, + type: 'object', + properties: { + value: { type: 'string', maxLength: 1999 }, + }, + }); + const tools = [ + { + functionDeclarations: [ + { name: 'first', parametersJsonSchema: makeSchema() }, + { name: 'second', parametersJsonSchema: makeSchema() }, + ], + }, + ] as Tool[]; + + const result = await converter.convertLlmToolsToOpenAI(tools); + + expect(result.map(({ function: declaration }) => declaration)).toEqual([ + { + name: 'first', + description: '', + parameters: { + type: 'object', + properties: { + value: { type: 'string', maxLength: 1999 }, + }, + }, + }, + { + name: 'second', + description: '', + parameters: { + type: 'object', + properties: { + value: { type: 'string', maxLength: 1999 }, + }, + }, + }, + ]); + }); + it('should convert Gemini tools with parameters field', async () => { const llmTools = [ { diff --git a/packages/core/src/core/openaiContentGenerator/converter.ts b/packages/core/src/core/openaiContentGenerator/converter.ts index a9d2e70f7bb..3068fa05f73 100644 --- a/packages/core/src/core/openaiContentGenerator/converter.ts +++ b/packages/core/src/core/openaiContentGenerator/converter.ts @@ -44,6 +44,7 @@ import { import { InvalidStreamError } from '../invalid-stream-error.js'; import { normalizeMcpToolName } from '../../utils/tool-name-utils.js'; import { setGenAiUsageProvenance } from '../../telemetry/gen-ai-usage.js'; +import { SchemaValidator } from '../../utils/schemaValidator.js'; const debugLogger = createDebugLogger('CONVERTER'); const SPLIT_TOOL_MEDIA_TEXT = '(attached media from previous tool call)'; @@ -334,6 +335,18 @@ export function convertLlmToolParametersToOpenAI( * Handles both Gemini tools (using 'parameters' field) and MCP tools * (using 'parametersJsonSchema' field). */ +const grammarSchemaValidationCache = new WeakMap(); + +function isStrictlyValidSchema(schema: object): boolean { + const cached = grammarSchemaValidationCache.get(schema); + if (cached !== undefined) { + return cached; + } + const valid = SchemaValidator.compileStrict(schema) === null; + grammarSchemaValidationCache.set(schema, valid); + return valid; +} + export async function convertLlmToolsToOpenAI( llmTools: ToolListUnion, schemaCompliance: SchemaComplianceMode = 'auto', @@ -373,6 +386,13 @@ export async function convertLlmToolsToOpenAI( } if (parameters) { + const sourceSchema = func.parametersJsonSchema; + const canValidateLocally = + typeof sourceSchema === 'object' && + sourceSchema !== null && + !Array.isArray(sourceSchema) && + !('$id' in sourceSchema) && + isStrictlyValidSchema(sourceSchema); parameters = convertSchema(parameters, schemaCompliance); // #7315: gateways enforcing OpenAI's structured-output contract // promote every property to required when an object level has @@ -380,7 +400,10 @@ export async function convertLlmToolsToOpenAI( // mutually exclusive optional fields (Agent working_dir vs // isolation). Relax the wire schema; client-side // validateToolParams still enforces the source schema. - parameters = relaxSchemaForFunctionCalling(parameters); + parameters = relaxSchemaForFunctionCalling( + parameters, + canValidateLocally, + ); } openAITools.push({ diff --git a/packages/core/src/utils/schemaConverter.test.ts b/packages/core/src/utils/schemaConverter.test.ts index f3acc3010c0..f893dabd583 100644 --- a/packages/core/src/utils/schemaConverter.test.ts +++ b/packages/core/src/utils/schemaConverter.test.ts @@ -335,13 +335,6 @@ describe('relaxSchemaForFunctionCalling', () => { ).toBe(false); }); - it('keeps additionalProperties:false when there are no properties to promote', () => { - const empty = { type: 'object', additionalProperties: false }; - expect(relaxSchemaForFunctionCalling(empty)['additionalProperties']).toBe( - false, - ); - }); - it('relaxes nested object levels independently', () => { const nested = { type: 'object', @@ -503,6 +496,59 @@ describe('relaxSchemaForFunctionCalling', () => { }); }); + it('removes grammar-hostile empty objects and repetition limits', () => { + const schema = { + type: 'object', + properties: { + empty: { + type: 'object', + properties: {}, + additionalProperties: false, + }, + closed: { type: 'object', additionalProperties: false }, + typelessClosed: { additionalProperties: false }, + nullableClosed: { + type: ['object', 'null'], + additionalProperties: false, + }, + bounded: { type: 'string', maxLength: 1998 }, + long: { type: 'string', maxLength: 1999 }, + padded: { type: 'string', minLength: 1999 }, + many: { + type: 'array', + maxItems: 1999, + items: { type: 'string' }, + }, + largeBatch: { + type: 'array', + minItems: 1999, + items: { type: 'string' }, + }, + }, + }; + + expect(relaxSchemaForFunctionCalling(schema, true)).toEqual({ + type: 'object', + properties: { + empty: { type: 'object' }, + closed: { type: 'object' }, + typelessClosed: {}, + nullableClosed: { type: ['object', 'null'] }, + bounded: { type: 'string', maxLength: 1998 }, + long: { type: 'string' }, + padded: { type: 'string' }, + many: { + type: 'array', + items: { type: 'string' }, + }, + largeBatch: { + type: 'array', + items: { type: 'string' }, + }, + }, + }); + }); + it('never treats schema-map keys as schema keywords', () => { const namedUniqueItems = { uniqueItems: { type: 'string' } }; const schema = { diff --git a/packages/core/src/utils/schemaConverter.ts b/packages/core/src/utils/schemaConverter.ts index db351a89040..c9260432392 100644 --- a/packages/core/src/utils/schemaConverter.ts +++ b/packages/core/src/utils/schemaConverter.ts @@ -191,14 +191,17 @@ function toOpenAPI30(schema: Record): Record { * client-side validation error until loop detection kills the run. * * The relaxation is deliberately surgical: - * - `additionalProperties: false` is removed ONLY on object levels that - * declare optional properties (some `properties` key missing from - * `required`). Levels where every property is required keep the + * - `additionalProperties: false` is removed on object levels that declare + * optional properties (some `properties` key missing from `required`) or + * no declared properties. Levels where every property is required keep the * constraint — there is nothing for a gateway to promote. * - `$schema` / `$id` metadata is dropped at every schema level (some * gateways reject unknown keywords). * - `uniqueItems` is dropped at every schema level because some * OpenAI-compatible function-calling endpoints reject it. + * - When the source schema can be validated locally, empty object declarations + * and string / array length limits at or above 1999 are dropped because + * grammar-based endpoints can turn them into invalid or rejected rules. * - Other constraints pass through untouched; client-side * `validateToolParams` still enforces the full source schema, so the * constraint is relaxed on the wire only. @@ -207,6 +210,7 @@ function toOpenAPI30(schema: Record): Record { */ export function relaxSchemaForFunctionCalling( schema: Record, + relaxGrammarConstraints = false, ): Record { const relax = (obj: unknown): unknown => { if (typeof obj !== 'object' || obj === null) { @@ -229,15 +233,46 @@ export function relaxSchemaForFunctionCalling( properties !== null && !Array.isArray(properties) && Object.keys(properties).some((key) => !required.includes(key)); + const hasEmptyProperties = + typeof properties === 'object' && + properties !== null && + !Array.isArray(properties) && + Object.keys(properties).length === 0; + const type = source['type']; + const canBeObject = + type === undefined || + type === 'object' || + (Array.isArray(type) && type.includes('object')); + const hasNoDeclaredProperties = + hasEmptyProperties || (canBeObject && properties === undefined); for (const [key, value] of Object.entries(source)) { if (key === '$schema' || key === '$id' || key === 'uniqueItems') { continue; } + if ( + relaxGrammarConstraints && + key === 'properties' && + hasEmptyProperties + ) { + continue; + } + if ( + relaxGrammarConstraints && + (key === 'minLength' || + key === 'maxLength' || + key === 'minItems' || + key === 'maxItems') && + typeof value === 'number' && + value >= 1999 + ) { + continue; + } if ( key === 'additionalProperties' && value === false && - hasOptionalProperties + (hasOptionalProperties || + (relaxGrammarConstraints && hasNoDeclaredProperties)) ) { continue; }