From 7cad18b3433cfc8ff7b4cd19227d9c2a1e18db47 Mon Sep 17 00:00:00 2001 From: Cole Tebou Date: Mon, 18 May 2026 13:53:36 +0000 Subject: [PATCH 1/2] fix(provider): tolerate optional/null/zero fields in review output schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Run 20260517T190759-3c9e9e (1000 features, 78 errors) surfaced 4 zod-issue patterns inside reviewOutputSchema that dropped whole-feature output when the model omitted optional fields or returned 0 for "unknown line": - invalid_type @ findings[N].minimumFixScope (omitted) — 2 occurrences - invalid_type @ findings[N].reproduction (omitted) — 1 occurrence - too_small @ findings[N].evidence[N].startLine (0) — 1 occurrence - too_small @ findings[N].evidence[N].endLine (0) — 1 occurrence The storage-side findingRecordSchema already tolerated all of these via .optional()/.nullable() + a transform default, so the LLM-input boundary was unnecessarily stricter than the persistence boundary. Relax reviewOutputSchema and the shared evidenceRefSchema to mirror the record schema: - evidenceRefSchema.startLine/endLine: accept 0 and normalize to null (consumers in reporting.ts already handle null). - reviewOutputSchema.findings[].reproduction: nullish, default to null. - reviewOutputSchema.findings[].minimumFixScope: nullish, default to "". Because these schemas now contain transforms, providerJsonSchema must emit the input-side JSON Schema; switch z.toJSONSchema to { io: "input", unrepresentable: "any" }. The findingRecordSchema is unchanged. Composes cleanly with the safeParse partition change: those finding-level parse failures now pass with sensible defaults instead of dropping the finding at all. --- CHANGELOG.md | 1 + src/provider-schema.ts | 4 ++- src/provider.test.ts | 75 +++++++++++++++++++++++++++++++++++++++++- src/types.ts | 24 +++++++++++--- 4 files changed, 98 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 570182c..3678ee8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - Improved OpenCode malformed JSON diagnostics with output length, event kinds, and a bounded preview, thanks @rohitjavvadi. - Fixed Express route mapping for aliased Router imports that follow block comment banners, thanks @rohitjavvadi. - Fixed Bun package-manager detection to recognize the text `bun.lock` lockfile, thanks @austinm911. +- Fixed review-output schema to tolerate optional `reproduction` and `minimumFixScope` fields and zero-valued evidence line numbers (normalized to `null`), recovering 4 of 28 zod issue patterns observed in run `20260517T190759-3c9e9e` (78 errors over 1000 features) that previously dropped whole-feature output instead of the affected finding. ## 0.3.0 - 2026-05-18 diff --git a/src/provider-schema.ts b/src/provider-schema.ts index 78d25ba..e14a7cd 100644 --- a/src/provider-schema.ts +++ b/src/provider-schema.ts @@ -21,7 +21,9 @@ export const revalidateJsonSchema = providerJsonSchema(revalidateOutputSchema); export const fixPlanJsonSchema = providerJsonSchema(fixPlanOutputSchema); export function providerJsonSchema(schema: z.ZodType): object { - return stripProviderUnsupportedSchemaKeywords(z.toJSONSchema(schema)) as object; + return stripProviderUnsupportedSchemaKeywords( + z.toJSONSchema(schema, { io: "input", unrepresentable: "any" }), + ) as object; } function stripProviderUnsupportedSchemaKeywords(value: unknown): unknown { diff --git a/src/provider.test.ts b/src/provider.test.ts index 239185e..bafcd0d 100644 --- a/src/provider.test.ts +++ b/src/provider.test.ts @@ -2,7 +2,7 @@ import { afterEach, describe, expect, it } from "vitest"; import { ClawpatchError } from "./errors.js"; import { __testing, extractJson, providerByName } from "./provider.js"; import { safeProviderPreview } from "./provider-json.js"; -import { revalidateOutputSchema, reviewOutputSchema } from "./types.js"; +import { evidenceRefSchema, revalidateOutputSchema, reviewOutputSchema } from "./types.js"; // eslint-disable-next-line no-underscore-dangle const { @@ -569,3 +569,76 @@ describe("providerByName", () => { expect(providerByName("mock-fail").name).toBe("mock-fail"); }); }); + +function buildToleranceFinding(overrides: Record = {}): Record { + return { + title: "x", + category: "bug", + severity: "low", + confidence: "low", + evidence: [], + reasoning: "r", + reproduction: null, + recommendation: "rec", + whyTestsDoNotAlreadyCoverThis: "", + suggestedRegressionTest: null, + minimumFixScope: "", + ...overrides, + }; +} + +function buildToleranceOutput(finding: Record): Record { + return { + findings: [finding], + inspected: { files: [], symbols: [], notes: [] }, + }; +} + +describe("reviewOutputSchema tolerance", () => { + it("accepts findings with null reproduction", () => { + const parsed = reviewOutputSchema.parse( + buildToleranceOutput(buildToleranceFinding({ reproduction: null })), + ); + expect(parsed.findings[0]!.reproduction).toBeNull(); + }); + + it("accepts findings with omitted reproduction (becomes null)", () => { + const finding = buildToleranceFinding(); + delete finding["reproduction"]; + const parsed = reviewOutputSchema.parse(buildToleranceOutput(finding)); + expect(parsed.findings[0]!.reproduction).toBeNull(); + }); + + it("accepts findings with omitted minimumFixScope (becomes empty string)", () => { + const finding = buildToleranceFinding(); + delete finding["minimumFixScope"]; + const parsed = reviewOutputSchema.parse(buildToleranceOutput(finding)); + expect(parsed.findings[0]!.minimumFixScope).toBe(""); + }); +}); + +describe("evidenceRefSchema tolerance", () => { + it("accepts startLine 0 and normalizes to null", () => { + const parsed = evidenceRefSchema.parse({ + path: "src/index.ts", + startLine: 0, + endLine: 5, + symbol: null, + quote: null, + }); + expect(parsed.startLine).toBeNull(); + expect(parsed.endLine).toBe(5); + }); + + it("accepts endLine 0 and normalizes to null", () => { + const parsed = evidenceRefSchema.parse({ + path: "src/index.ts", + startLine: 5, + endLine: 0, + symbol: null, + quote: null, + }); + expect(parsed.startLine).toBe(5); + expect(parsed.endLine).toBeNull(); + }); +}); diff --git a/src/types.ts b/src/types.ts index 4c958c0..a78ff4c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -208,8 +208,18 @@ export type TrustBoundary = FeatureRecord["trustBoundaries"][number]; export const evidenceRefSchema = z.object({ path: z.string(), - startLine: z.number().int().positive().nullable(), - endLine: z.number().int().positive().nullable(), + startLine: z + .number() + .int() + .min(0) + .nullable() + .transform((v) => (v === 0 ? null : v)), + endLine: z + .number() + .int() + .min(0) + .nullable() + .transform((v) => (v === 0 ? null : v)), symbol: z.string().nullable(), quote: z.string().nullable(), }); @@ -358,11 +368,17 @@ export const reviewOutputSchema = z.object({ confidence: z.enum(["high", "medium", "low"]), evidence: z.array(evidenceRefSchema), reasoning: z.string(), - reproduction: z.string().nullable(), + reproduction: z + .string() + .nullish() + .transform((v) => v ?? null), recommendation: z.string(), whyTestsDoNotAlreadyCoverThis: z.string(), suggestedRegressionTest: z.string().nullable(), - minimumFixScope: z.string(), + minimumFixScope: z + .string() + .nullish() + .transform((v) => v ?? ""), }), ), inspected: z.object({ From 3482dacc12e69cbdf978ddb04cdf35b69de2b837 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 20 May 2026 03:47:49 +0100 Subject: [PATCH 2/2] fix(provider): keep tolerant review schemas strict --- src/provider-schema.ts | 8 ++++++++ src/provider.test.ts | 42 ++++++++++++++++++++++++++++++++++++++++-- src/types.ts | 32 +++++++++++++++----------------- 3 files changed, 63 insertions(+), 19 deletions(-) diff --git a/src/provider-schema.ts b/src/provider-schema.ts index e14a7cd..a43bba7 100644 --- a/src/provider-schema.ts +++ b/src/provider-schema.ts @@ -40,5 +40,13 @@ function stripProviderUnsupportedSchemaKeywords(value: unknown): unknown { } output[key] = stripProviderUnsupportedSchemaKeywords(item); } + if (output["type"] === "object" && isRecord(output["properties"])) { + output["additionalProperties"] = false; + output["required"] = Object.keys(output["properties"]); + } return output; } + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} diff --git a/src/provider.test.ts b/src/provider.test.ts index b095587..5be14fc 100644 --- a/src/provider.test.ts +++ b/src/provider.test.ts @@ -260,6 +260,20 @@ describe("providerJsonSchema", () => { expect(enumNodes.every((node) => node["type"] === "string")).toBe(true); } }); + + it("keeps object schemas strict even when parser input fields are optional", () => { + const schema = providerJsonSchema(reviewOutputSchema) as Record; + const findings = propertySchema(schema, "findings"); + const finding = itemSchema(findings); + const inspected = propertySchema(schema, "inspected"); + + for (const objectSchema of [schema, finding, inspected]) { + expect(objectSchema["additionalProperties"]).toBe(false); + expect(objectSchema["required"]).toEqual(Object.keys(propertiesOf(objectSchema))); + } + expect(finding["required"]).toContain("reproduction"); + expect(finding["required"]).toContain("minimumFixScope"); + }); }); describe("piThinkingLevel", () => { @@ -294,6 +308,30 @@ function enumSchemaNodes(value: unknown): Array> { return Array.isArray(node["enum"]) ? [node, ...nested] : nested; } +function propertySchema(schema: Record, name: string): Record { + const value = propertiesOf(schema)[name]; + if (typeof value !== "object" || value === null || Array.isArray(value)) { + throw new Error(`missing schema property: ${name}`); + } + return value as Record; +} + +function itemSchema(schema: Record): Record { + const value = schema["items"]; + if (typeof value !== "object" || value === null || Array.isArray(value)) { + throw new Error("missing item schema"); + } + return value as Record; +} + +function propertiesOf(schema: Record): Record { + const value = schema["properties"]; + if (typeof value !== "object" || value === null || Array.isArray(value)) { + throw new Error("missing schema properties"); + } + return value as Record; +} + describe("codexFailureMessage", () => { it("adds scope guidance for missing Responses API write permission", () => { const message = codexFailureMessage( @@ -805,7 +843,7 @@ describe("evidenceRefSchema tolerance", () => { quote: null, }); expect(parsed.startLine).toBeNull(); - expect(parsed.endLine).toBe(5); + expect(parsed.endLine).toBeNull(); }); it("accepts endLine 0 and normalizes to null", () => { @@ -816,7 +854,7 @@ describe("evidenceRefSchema tolerance", () => { symbol: null, quote: null, }); - expect(parsed.startLine).toBe(5); + expect(parsed.startLine).toBeNull(); expect(parsed.endLine).toBeNull(); }); }); diff --git a/src/types.ts b/src/types.ts index a78ff4c..70f7a78 100644 --- a/src/types.ts +++ b/src/types.ts @@ -206,23 +206,21 @@ export type FeatureRecord = z.infer; export type FeatureKind = FeatureRecord["kind"]; export type TrustBoundary = FeatureRecord["trustBoundaries"][number]; -export const evidenceRefSchema = z.object({ - path: z.string(), - startLine: z - .number() - .int() - .min(0) - .nullable() - .transform((v) => (v === 0 ? null : v)), - endLine: z - .number() - .int() - .min(0) - .nullable() - .transform((v) => (v === 0 ? null : v)), - symbol: z.string().nullable(), - quote: z.string().nullable(), -}); +const evidenceLineSchema = z.number().int().min(0).nullable(); + +export const evidenceRefSchema = z + .object({ + path: z.string(), + startLine: evidenceLineSchema, + endLine: evidenceLineSchema, + symbol: z.string().nullable(), + quote: z.string().nullable(), + }) + .transform((evidence) => + evidence.startLine === 0 || evidence.endLine === 0 + ? { ...evidence, startLine: null, endLine: null } + : evidence, + ); export const findingHistoryEntrySchema = z.object({ runId: z.string().nullable(),