From 3b60edd2764b97afe91b5950864eeeb8bad86a09 Mon Sep 17 00:00:00 2001 From: can1357 Date: Tue, 26 May 2026 07:28:42 +0200 Subject: [PATCH] refactor(coding-agent/tools): removed output schema evaluator and simplified validator handling - Deleted the `ValidationVerdict` type and `evaluateOutputAgainstSchema` API from the output schema validator. - Updated `yield.ts` to bind `buildOutputValidator`'s error directly to `schemaError` during validator setup. - Removed the obsolete evaluator tests and adjusted validation success fixture to match the raw summary input shape. --- .../src/tools/output-schema-validator.ts | 21 ++++----------- packages/coding-agent/src/tools/yield.ts | 3 +-- .../tools/output-schema-validator.test.ts | 26 +------------------ 3 files changed, 7 insertions(+), 43 deletions(-) diff --git a/packages/coding-agent/src/tools/output-schema-validator.ts b/packages/coding-agent/src/tools/output-schema-validator.ts index 2c833d8dd..4041793e1 100644 --- a/packages/coding-agent/src/tools/output-schema-validator.ts +++ b/packages/coding-agent/src/tools/output-schema-validator.ts @@ -15,9 +15,6 @@ import { } from "@oh-my-pi/pi-ai/utils/schema"; import { jtdToJsonSchema, normalizeSchema } from "./jtd-to-json-schema"; -/** Verdict for a successful or failed validation, summarized for callers. */ -export type ValidationVerdict = { ok: true } | { ok: false; message: string; missingRequired: string[] }; - /** A validator bound to a specific output schema. */ export interface OutputValidator { /** Run JSON Schema validation; returns the raw `success`/`issues` shape so callers may inspect every failure. */ @@ -45,9 +42,11 @@ export interface BuildOutputValidatorResult { * Build the canonical validator for a JTD-or-JSON-Schema output declaration. * * Returns: - * - `{ validator, jsonSchema }` for constraining schemas — both callers use this path. - * - `{}` for absent or fully-permissive schemas (e.g. `true`, `undefined`) — no validation. - * - `{ error }` when the schema cannot be honored (invalid syntax, `false`, malformed JTD). + * - `{ validator, jsonSchema, normalized }` for constraining schemas — both callers use this path. + * - `{ normalized: true }` for an intentionally unconstrained schema (the JSON Schema literal `true`). + * No validator, but distinguishable from "no schema provided". + * - `{}` for an absent schema (`undefined`). + * - `{ error, normalized? }` when the schema cannot be honored (invalid syntax, `false`, malformed JTD). */ export function buildOutputValidator(schema: unknown): BuildOutputValidatorResult { const { normalized, error: normalizeError } = normalizeSchema(schema); @@ -89,16 +88,6 @@ export function summarizeValidationFailure( return { message, missingRequired: missing }; } -/** Reduce a `BuildOutputValidatorResult` + value to the executor's high-level verdict. */ -export function evaluateOutputAgainstSchema(schema: unknown, value: unknown): ValidationVerdict | { ok: true } { - const { validator } = buildOutputValidator(schema); - if (!validator) return { ok: true }; - const result = validator.validate(value); - if (result.success) return { ok: true }; - const { message, missingRequired } = summarizeValidationFailure(result, value, validator.requiredFields); - return { ok: false, message, missingRequired }; -} - export function extractRequiredFields(jsonSchema: unknown): string[] { if (!jsonSchema || typeof jsonSchema !== "object") return []; const required = (jsonSchema as { required?: unknown }).required; diff --git a/packages/coding-agent/src/tools/yield.ts b/packages/coding-agent/src/tools/yield.ts index e4d24440b..8cf6930f5 100644 --- a/packages/coding-agent/src/tools/yield.ts +++ b/packages/coding-agent/src/tools/yield.ts @@ -121,9 +121,8 @@ export class YieldTool implements AgentTool { validator, jsonSchema: normalizedSchema, normalized, - error: validatorError, + error: schemaError, } = buildOutputValidator(session.outputSchema); - const schemaError = validatorError; if (validator) { validate = value => validator.validate(value); } diff --git a/packages/coding-agent/test/tools/output-schema-validator.test.ts b/packages/coding-agent/test/tools/output-schema-validator.test.ts index bfe8f2b35..0657e2d8e 100644 --- a/packages/coding-agent/test/tools/output-schema-validator.test.ts +++ b/packages/coding-agent/test/tools/output-schema-validator.test.ts @@ -2,7 +2,6 @@ import { describe, expect, it } from "bun:test"; import { buildOutputValidator, computeMissingRequired, - evaluateOutputAgainstSchema, extractRequiredFields, formatAllValidationIssues, formatValidationIssueHeadline, @@ -71,32 +70,9 @@ describe("buildOutputValidator", () => { ]); }); }); - -describe("evaluateOutputAgainstSchema", () => { - it("returns ok for unconstrained schemas", () => { - expect(evaluateOutputAgainstSchema(undefined, { anything: 1 })).toEqual({ ok: true }); - expect(evaluateOutputAgainstSchema(true, { anything: 1 })).toEqual({ ok: true }); - }); - - it("returns ok for conforming payloads", () => { - const schema = { properties: { x: { type: "string" } } }; - expect(evaluateOutputAgainstSchema(schema, { x: "hi" })).toEqual({ ok: true }); - }); - - it("returns the executor headline + missingRequired for failures", () => { - const schema = { properties: { x: { type: "string" }, y: { type: "number" } } }; - const verdict = evaluateOutputAgainstSchema(schema, { x: "ok" }); - expect(verdict).toMatchObject({ ok: false }); - if (verdict.ok === false) { - expect(verdict.missingRequired).toContain("y"); - expect(verdict.message).toMatch(/y/); - } - }); -}); - describe("summarizeValidationFailure", () => { it("returns an empty summary when the result is a success", () => { - const summary = summarizeValidationFailure({ success: true }, {}, []); + const summary = summarizeValidationFailure({ success: true, issues: [] }, {}, []); expect(summary).toEqual({ message: "", missingRequired: [] }); });