fix(ai): disabled lossy argument repairs on failed union branches
- Prevent schema validation from applying lossy repairs such as container stringification and key deletion to values diagnosed within failed `anyOf` and `oneOf` branches. - Remove the `coerceArguments` tool option and let repair provenance dictate whether lossy coercions are allowed.
This commit is contained in:
@@ -5,10 +5,10 @@
|
||||
### Added
|
||||
|
||||
- Added model metadata fields (`context_length`, `max_output_tokens`, `input_modalities`, etc.) to auth gateway model listing responses
|
||||
- Added `Tool.coerceArguments` (default `true`): setting it `false` opts a tool out of every LLM-quirk argument repair pass (JSON-string parsing, object→string stringification, unrecognized-key dropping, singleton array wrapping) so validation runs verbatim. Tools whose arguments are the deliverable payload — like the subagent `yield` tool — use it to keep lossy repairs from silently corrupting data their own validate-and-retry loop is designed to correct.
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed the tool-argument repair layer applying lossy repairs on union-branch diagnoses: when a value failed every `anyOf`/`oneOf` variant, the first failing branch's issues were treated as authoritative, so object payloads got JSON-stringified into string-typed fields and unrecognized keys were silently deleted — corrupting subagent `yield` payloads (validation "passed" and parents received `summary: "{\"purge\":13,…}"` instead of a retryable error). Issues surfaced from a failed union branch are now marked at every depth and only receive lossless repairs (JSON-string parsing, boolean spellings, scalar coercion); container stringification, key deletion, and singleton-array wrapping require an authoritative non-union diagnosis.
|
||||
- Fixed local OpenAI-compatible servers with strict `chat_template_kwargs` whitelists (e.g. NInfer) failing every Qwen 3.8+ turn with `400 chat_template_kwargs.reasoning_effort is not supported` after the effort routing fix: the reasoning-effort fallback now recognizes a rejection of the kwargs spelling itself, retries with the kwarg stripped while keeping the effort on the standard top-level `reasoning_effort` field (hoisting it there for the kwargs-only vLLM dialect), and remembers the shape for the rest of the session. Value-level rejections and drops now also update the `chat_template_kwargs.reasoning_effort` twin instead of leaving a stale effort for kwargs-reading renderers, and unknown-parameter 400s naming `reasoning_effort` are recognized as effort rejections.
|
||||
|
||||
## [17.3.8] - 2026-08-19
|
||||
|
||||
@@ -1212,16 +1212,6 @@ export interface Tool<TParameters extends TSchema = TSchema> {
|
||||
parameters: TParameters;
|
||||
/** If true, tool is strictly typed and validated against the parameters schema before execution */
|
||||
strict?: boolean;
|
||||
/**
|
||||
* If false, argument validation runs verbatim: none of the LLM-quirk
|
||||
* normalization/repair passes (JSON-string parsing, object→string
|
||||
* stringification, unrecognized-key dropping, singleton array wrapping)
|
||||
* are applied. For tools whose arguments ARE the deliverable payload —
|
||||
* e.g. a subagent's `yield` — lossy repairs silently corrupt data that
|
||||
* the tool's own validation/retry loop is designed to correct instead.
|
||||
* Defaults to true.
|
||||
*/
|
||||
coerceArguments?: boolean;
|
||||
/**
|
||||
* Optional grammar constraint for OpenAI custom-tool emission.
|
||||
* When set, providers that support grammar-constrained tools (currently only
|
||||
|
||||
@@ -21,11 +21,11 @@ export interface JsonSchemaValidationIssue {
|
||||
expectedTypes?: string[];
|
||||
keyword?: string;
|
||||
/**
|
||||
* Marks issues that originate inside a failed `anyOf` / `oneOf` branch.
|
||||
* Consumers such as the tool-argument coercion layer use this to avoid
|
||||
* applying type repairs (e.g. singleton-array wrapping) that would be
|
||||
* authoritative outside of a combinator but are only one candidate
|
||||
* branch's expectation here.
|
||||
* Marks issues surfaced from a failed `anyOf` / `oneOf` branch (at any
|
||||
* depth). Such a diagnosis is one candidate branch's guess, not
|
||||
* authoritative: the tool-argument coercion layer keeps lossy repairs
|
||||
* (container stringification, unrecognized-key deletion, singleton-array
|
||||
* wrapping) off for these while still applying lossless ones.
|
||||
*/
|
||||
fromUnionBranch?: boolean;
|
||||
}
|
||||
@@ -68,6 +68,35 @@ function getValueIdentity(ctx: ValidationContext, value: object): number {
|
||||
function isJsonObject(value: unknown): value is Record<string, unknown> {
|
||||
return typeof value === "object" && value !== null && !Array.isArray(value);
|
||||
}
|
||||
/**
|
||||
* Whether `value` matches every `const`/`enum` discriminator property the
|
||||
* branch declares (with at least one such property present and matching).
|
||||
* A uniquely tag-selected branch's validation issues are authoritative — the
|
||||
* model named its intended variant — so the coercion layer may apply lossy
|
||||
* repairs to them; without a unique tag, branch issues are guesses.
|
||||
*/
|
||||
function isTagSelectedBranch(branch: unknown, value: unknown): boolean {
|
||||
if (!isJsonObject(branch) || !isJsonObject(value)) return false;
|
||||
const props = branch.properties;
|
||||
if (!isJsonObject(props)) return false;
|
||||
let matched = false;
|
||||
for (const key in props) {
|
||||
const propSchema = props[key];
|
||||
if (!isJsonObject(propSchema)) continue;
|
||||
const hasConst = Object.hasOwn(propSchema, "const");
|
||||
const enumValues = Array.isArray(propSchema.enum) ? propSchema.enum : undefined;
|
||||
if (!hasConst && !enumValues) continue;
|
||||
if (!Object.hasOwn(value, key)) return false;
|
||||
const candidate = value[key];
|
||||
if (hasConst) {
|
||||
if (!areJsonValuesEqual(candidate, propSchema.const)) return false;
|
||||
} else if (enumValues && !enumValues.some(entry => areJsonValuesEqual(entry, candidate))) {
|
||||
return false;
|
||||
}
|
||||
matched = true;
|
||||
}
|
||||
return matched;
|
||||
}
|
||||
|
||||
function pushIssue(
|
||||
issues: JsonSchemaValidationIssue[],
|
||||
@@ -239,27 +268,35 @@ function validateSchemaNode(
|
||||
|
||||
let matches = 0;
|
||||
let firstIssues: JsonSchemaValidationIssue[] | undefined;
|
||||
let selectedIssues: JsonSchemaValidationIssue[] | undefined;
|
||||
let selectedCount = 0;
|
||||
for (const branch of branches) {
|
||||
const branchIssues: JsonSchemaValidationIssue[] = [];
|
||||
if (validateSchemaNode(branch, value, path, ctx, branchIssues)) {
|
||||
matches += 1;
|
||||
} else if (!firstIssues) {
|
||||
firstIssues = branchIssues;
|
||||
continue;
|
||||
}
|
||||
if (!firstIssues) firstIssues = branchIssues;
|
||||
if (isTagSelectedBranch(branch, value)) {
|
||||
selectedCount += 1;
|
||||
if (selectedCount === 1) selectedIssues = branchIssues;
|
||||
}
|
||||
}
|
||||
const branchValid = keyword === "anyOf" ? matches > 0 : matches === 1;
|
||||
if (!branchValid) {
|
||||
if (matches === 0 && firstIssues && firstIssues.length > 0) {
|
||||
// Only tag issues that sit at the combinator's own path as
|
||||
// union-branch; deeper issues describe a specific field within
|
||||
// the failed branch and should remain individually repairable.
|
||||
const unionDepth = path.length;
|
||||
if (matches === 0 && selectedCount === 1 && selectedIssues && selectedIssues.length > 0) {
|
||||
// A const/enum discriminator uniquely identifies the intended
|
||||
// variant, so its diagnosis is authoritative: surface untagged and
|
||||
// keep every repair (including lossy ones) available.
|
||||
issues.push(...selectedIssues);
|
||||
} else if (matches === 0 && firstIssues && firstIssues.length > 0) {
|
||||
// No variant matched and no tag picks one: everything reported is
|
||||
// the first failing branch's guess — another variant may accept the
|
||||
// value as-is. Surface all issues (deep ones remain individually
|
||||
// repairable by lossless coercions) but mark their provenance so
|
||||
// lossy repairs (stringify, key deletion, singleton wrap) stay off.
|
||||
for (const branchIssue of firstIssues) {
|
||||
if (branchIssue.path.length === unionDepth) {
|
||||
issues.push({ ...branchIssue, fromUnionBranch: true });
|
||||
} else {
|
||||
issues.push(branchIssue);
|
||||
}
|
||||
issues.push(branchIssue.fromUnionBranch ? branchIssue : { ...branchIssue, fromUnionBranch: true });
|
||||
}
|
||||
} else {
|
||||
pushIssue(
|
||||
|
||||
@@ -136,12 +136,20 @@ function tryCoerceBooleanToNumber(value: unknown, expectedTypes: string[]): { va
|
||||
return { value: value ? 1 : 0, changed: true };
|
||||
}
|
||||
|
||||
function tryCoerceString(value: unknown, expectedTypes: string[]): { value: unknown; changed: boolean } {
|
||||
function tryCoerceString(
|
||||
value: unknown,
|
||||
expectedTypes: string[],
|
||||
allowLossy: boolean,
|
||||
): { value: unknown; changed: boolean } {
|
||||
if (!expectedTypes.includes("string") || typeof value === "string" || value === null || value === undefined) {
|
||||
return { value, changed: false };
|
||||
}
|
||||
|
||||
if (Array.isArray(value) || typeof value === "object") {
|
||||
// JSON.stringify is irreversible (downstream consumers receive encoded
|
||||
// text where they expected structure), so it requires an authoritative
|
||||
// diagnosis — never a union-branch guess.
|
||||
if (!allowLossy) return { value, changed: false };
|
||||
try {
|
||||
const stringified = JSON.stringify(value);
|
||||
if (stringified === undefined) return { value, changed: false };
|
||||
@@ -158,7 +166,16 @@ function tryCoerceString(value: unknown, expectedTypes: string[]): { value: unkn
|
||||
return { value: String(value), changed: true };
|
||||
}
|
||||
|
||||
function tryCoerceForExpectedTypes(value: unknown, expectedTypes: string[]): { value: unknown; changed: boolean } {
|
||||
/**
|
||||
* Schema-directed value repair for a single type issue. `allowLossy` gates the
|
||||
* irreversible repairs (container→string stringification); lossless repairs
|
||||
* (JSON parsing, boolean spellings, scalar stringification) always apply.
|
||||
*/
|
||||
function tryCoerceForExpectedTypes(
|
||||
value: unknown,
|
||||
expectedTypes: string[],
|
||||
allowLossy: boolean,
|
||||
): { value: unknown; changed: boolean } {
|
||||
if (typeof value === "string") {
|
||||
const parsed = tryParseJsonForTypes(value, expectedTypes);
|
||||
if (parsed.changed) return parsed;
|
||||
@@ -171,7 +188,7 @@ function tryCoerceForExpectedTypes(value: unknown, expectedTypes: string[]): { v
|
||||
const numericCoercion = tryCoerceBooleanToNumber(value, expectedTypes);
|
||||
if (numericCoercion.changed) return numericCoercion;
|
||||
|
||||
return tryCoerceString(value, expectedTypes);
|
||||
return tryCoerceString(value, expectedTypes, allowLossy);
|
||||
}
|
||||
|
||||
function tryParseLeadingJsonContainer(value: string): unknown | undefined {
|
||||
@@ -1578,7 +1595,12 @@ function coerceArgsFromIssues(args: unknown, issues: FlatIssue[]): { value: unkn
|
||||
let nextArgs: unknown = args;
|
||||
|
||||
for (const issue of issues) {
|
||||
// Issues surfaced from a failed union branch are guesses from that
|
||||
// branch's diagnosis, not authoritative: another variant may accept the
|
||||
// value as-is. Lossy repairs (key deletion, container stringification,
|
||||
// singleton wrapping) stay off for them; lossless repairs still apply.
|
||||
if (issue.keyword === "unrecognized") {
|
||||
if (issue.unionBranch) continue;
|
||||
const previous = nextArgs;
|
||||
nextArgs = deleteValueAtPointer(nextArgs, issue.instancePath);
|
||||
if (nextArgs !== previous) changed = true;
|
||||
@@ -1588,7 +1610,7 @@ function coerceArgsFromIssues(args: unknown, issues: FlatIssue[]): { value: unkn
|
||||
if (issue.expectedTypes.length === 0) continue;
|
||||
|
||||
const currentValue = getValueAtPointer(nextArgs, issue.instancePath);
|
||||
const result = tryCoerceForExpectedTypes(currentValue, issue.expectedTypes);
|
||||
const result = tryCoerceForExpectedTypes(currentValue, issue.expectedTypes, !issue.unionBranch);
|
||||
let coercedValue = result.changed ? result.value : undefined;
|
||||
if (
|
||||
coercedValue === undefined &&
|
||||
@@ -1923,18 +1945,6 @@ export function validateToolArguments(tool: Tool, toolCall: ToolCall): ToolCall[
|
||||
);
|
||||
}
|
||||
const ctx = getValidationContext(tool);
|
||||
// Verbatim mode: the tool opted out of every repair pass because its
|
||||
// arguments are payload, not plumbing. Validate as-is; on failure the
|
||||
// caller's lenient path (if any) hands the raw args to the tool, whose
|
||||
// own error messaging drives the model's retry.
|
||||
if (tool.coerceArguments === false) {
|
||||
const verbatim = validateContext(ctx, originalArgs);
|
||||
if (verbatim.success) return verbatim.value as ToolCall["arguments"];
|
||||
const errors = verbatim.messages.join("\n") || "Unknown validation error";
|
||||
throw new AIError.ValidationError(
|
||||
`Validation failed for tool "${toolCall.name}":\n${errors}\n\nReceived arguments:\n${JSON.stringify(truncateArgsForError(originalArgs), null, 2)}`,
|
||||
);
|
||||
}
|
||||
const { json } = ctx;
|
||||
|
||||
// Always normalize first — strip null/string "null" from optional fields,
|
||||
|
||||
@@ -56,48 +56,91 @@ describe("Tool argument coercion", () => {
|
||||
|
||||
expect(result.payload).toBe('{"a":1,"nested":["x"]}');
|
||||
});
|
||||
it("coerceArguments: false rejects object values for string fields instead of stringifying", () => {
|
||||
it("does not stringify container values diagnosed inside a failed union branch", () => {
|
||||
// Regression: a subagent yield payload with an object in a string-typed
|
||||
// schema field sat under the tool's `anyOf` wrapper; the stringify repair
|
||||
// fired on that branch's guess, so validation "passed" and downstream
|
||||
// consumers received encoded text instead of a retryable error.
|
||||
const tool: Tool = {
|
||||
name: "verbatim",
|
||||
name: "union-string",
|
||||
description: "",
|
||||
coerceArguments: false,
|
||||
parameters: type({ payload: type("string") }),
|
||||
parameters: {
|
||||
type: "object",
|
||||
additionalProperties: false,
|
||||
properties: {
|
||||
payload: { anyOf: [{ type: "string" }, { type: "number" }] },
|
||||
},
|
||||
required: ["payload"],
|
||||
} as never,
|
||||
};
|
||||
|
||||
expect(() =>
|
||||
validateToolArguments(tool, {
|
||||
type: "toolCall",
|
||||
id: "call-verbatim-object",
|
||||
name: "verbatim",
|
||||
id: "call-union-object",
|
||||
name: "union-string",
|
||||
arguments: { payload: { a: 1 } },
|
||||
}),
|
||||
).toThrow(/payload/);
|
||||
});
|
||||
|
||||
it("coerceArguments: false still passes conforming args and rejects invalid JSON buffers", () => {
|
||||
it("does not delete unrecognized keys diagnosed inside a failed union branch", () => {
|
||||
const tool: Tool = {
|
||||
name: "verbatim-ok",
|
||||
name: "union-closed",
|
||||
description: "",
|
||||
coerceArguments: false,
|
||||
parameters: type({ payload: type("string") }),
|
||||
parameters: {
|
||||
type: "object",
|
||||
additionalProperties: false,
|
||||
properties: {
|
||||
op: {
|
||||
anyOf: [
|
||||
{
|
||||
type: "object",
|
||||
additionalProperties: false,
|
||||
properties: { kind: { type: "string" }, value: { type: "number" } },
|
||||
required: ["kind", "value"],
|
||||
},
|
||||
{ type: "string" },
|
||||
],
|
||||
},
|
||||
},
|
||||
required: ["op"],
|
||||
} as never,
|
||||
};
|
||||
|
||||
// `extra` fails the closed object variant; deleting it would silently
|
||||
// drop payload data on a branch guess. Must surface as a validation error.
|
||||
expect(() =>
|
||||
validateToolArguments(tool, {
|
||||
type: "toolCall",
|
||||
id: "call-union-extra-key",
|
||||
name: "union-closed",
|
||||
arguments: { op: { kind: "set", value: 1, extra: "keep me" } },
|
||||
}),
|
||||
).toThrow(/op/);
|
||||
});
|
||||
|
||||
it("still applies lossless repairs inside union branches", () => {
|
||||
const tool: Tool = {
|
||||
name: "union-lossless",
|
||||
description: "",
|
||||
parameters: {
|
||||
type: "object",
|
||||
additionalProperties: false,
|
||||
properties: {
|
||||
payload: { anyOf: [{ type: "number" }, { type: "boolean" }] },
|
||||
},
|
||||
required: ["payload"],
|
||||
} as never,
|
||||
};
|
||||
|
||||
const result = validateToolArguments(tool, {
|
||||
type: "toolCall",
|
||||
id: "call-verbatim-ok",
|
||||
name: "verbatim-ok",
|
||||
arguments: { payload: "fine" },
|
||||
}) as { payload: string };
|
||||
expect(result.payload).toBe("fine");
|
||||
|
||||
expect(() =>
|
||||
validateToolArguments(tool, {
|
||||
type: "toolCall",
|
||||
id: "call-verbatim-parse-error",
|
||||
name: "verbatim-ok",
|
||||
arguments: { __parseError: "Unexpected token", __rawJson: '{"payload": ' },
|
||||
}),
|
||||
).toThrow(/not valid JSON/);
|
||||
id: "call-union-numeric-string",
|
||||
name: "union-lossless",
|
||||
arguments: { payload: "300" },
|
||||
}) as { payload: number };
|
||||
expect(result.payload).toBe(300);
|
||||
});
|
||||
|
||||
it("stringifies array values when schema expects string", () => {
|
||||
|
||||
@@ -24,7 +24,7 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed subagent structured returns being silently corrupted by the generic tool-argument repair layer: the `yield` tool's parameters embed the caller's output schema, so the repair passes could JSON-stringify object payloads into string-typed fields (parents received `summary: "{\"purge\":13,…}"` instead of prose) and drop unrecognized keys — bypassing yield's own validate-and-retry loop. `yield` now opts out via `coerceArguments: false`, so mismatches surface as retryable schema errors and accepted payloads arrive verbatim.
|
||||
- Fixed subagent structured returns being silently corrupted by the tool-argument repair layer: the `yield` tool's parameters embed the caller's output schema under an `anyOf` wrapper, and lossy repairs fired on union-branch guesses — JSON-stringifying object payloads into string-typed fields (parents received `summary: "{\"purge\":13,…}"` instead of prose) and deleting unrecognized keys — bypassing yield's own validate-and-retry loop. The repair layer (pi-ai) now restricts union-branch diagnoses to lossless repairs, so mismatches surface as retryable schema errors and accepted payloads arrive verbatim.
|
||||
- Fixed the `yield` tool bouncing common weak-caller envelope shapes with `result must be an object containing either data or error` retries (the dominant structured-output failure in Gemini-flash subagent traces): `type: "result"` with the `result` wrapper omitted entirely now finalizes as the documented last-turn yield, top-level `data`/`error` payloads missing the wrapper are salvaged, and `result` or `data` sent as a JSON-encoded string is parsed losslessly (mirroring executor finalization) before consuming a schema retry.
|
||||
- Fixed the `yield` tool's instructions teaching weak callers the wrong call shape for structured tasks: the description led with `Pass type:"result" to finalize; when data is omitted, your last assistant turn becomes the raw final result` before ever stating the `result: { data }` wrapper, producing wrapper-less `{type:"result"}` punts and top-level `data:` payloads in Gemini-flash traces. The description (now `prompts/tools/yield.md`) and the subagent system prompt lead with the wrapper contract and only advertise last-turn extraction when no output schema is declared; a schema-bound last-turn finalize with no accumulated incremental sections is rejected in-band as a retryable error instead of terminating the child into an uncorrectable post-mortem `schema_violation`.
|
||||
- Fixed a prompt cancelled during turn setup (Esc while the pre-stream spinner is up, after dispatch had started) vanishing entirely: it was never persisted to the session — so the `/tree` and `/branch` selectors had nothing to rewind to — and was not returned to the editor either, while its optimistic transcript row kept lingering. A prompt dropped before reaching the agent (abort or usage-preflight denial racing setup) is now handed back: the stale transcript row is removed and the typed text and image attachments are restored to the editor for editing.
|
||||
|
||||
@@ -268,14 +268,6 @@ export class YieldTool implements AgentTool<TSchema, YieldDetails> {
|
||||
strict = true;
|
||||
readonly intent = "omit" as const;
|
||||
lenientArgValidation = true;
|
||||
/**
|
||||
* The args ARE the subagent's deliverable: the generic repair layer's lossy
|
||||
* coercions (object→string stringification, unrecognized-key deletion) would
|
||||
* silently corrupt the payload and bypass this tool's own validate-and-retry
|
||||
* loop. Verbatim validation + `lenientArgValidation` routes every mismatch
|
||||
* through `execute()`'s salvage/retry messaging instead.
|
||||
*/
|
||||
coerceArguments = false;
|
||||
|
||||
readonly #validate?: (value: unknown) => JsonSchemaValidationResult;
|
||||
readonly #validateSection?: ReadonlyMap<string, (value: unknown) => JsonSchemaValidationResult>;
|
||||
|
||||
@@ -168,11 +168,12 @@ describe("YieldTool", () => {
|
||||
expect(result.details).toEqual({ data: { n: 4 }, status: "success", error: undefined });
|
||||
});
|
||||
|
||||
it("verbatim arg validation rejects object payloads in string-typed fields instead of stringifying", () => {
|
||||
// Regression: the generic tool-arg repair layer used to JSON.stringify an
|
||||
// object submitted for a string-typed schema field, so validation
|
||||
// "passed" and the parent received `summary: "{\"purge\":13,…}"` instead
|
||||
// of a retry prompt. `coerceArguments = false` must surface the mismatch.
|
||||
it("arg validation rejects object payloads in string-typed fields instead of stringifying", () => {
|
||||
// Regression: the repair layer used to JSON.stringify an object submitted
|
||||
// for a string-typed schema field even though the diagnosis came from a
|
||||
// failed `anyOf` branch of the yield wrapper, so validation "passed" and
|
||||
// the parent received `summary: "{\"purge\":13,…}"` instead of a retry
|
||||
// prompt. Union-branch diagnoses must not trigger lossy repairs.
|
||||
const tool = new YieldTool(
|
||||
createSession({
|
||||
outputSchema: {
|
||||
@@ -182,7 +183,6 @@ describe("YieldTool", () => {
|
||||
},
|
||||
}),
|
||||
);
|
||||
expect(tool.coerceArguments).toBe(false);
|
||||
expect(() =>
|
||||
validateToolArguments(tool as never, {
|
||||
type: "toolCall",
|
||||
@@ -193,7 +193,7 @@ describe("YieldTool", () => {
|
||||
).toThrow(/summary/);
|
||||
});
|
||||
|
||||
it("verbatim arg validation passes conforming args through unmodified", () => {
|
||||
it("arg validation passes conforming args through unmodified", () => {
|
||||
const tool = new YieldTool(
|
||||
createSession({
|
||||
outputSchema: {
|
||||
|
||||
Reference in New Issue
Block a user