fix(provider): tolerate optional/null/zero fields in review output schema#85
Open
coletebou wants to merge 1 commit into
Open
fix(provider): tolerate optional/null/zero fields in review output schema#85coletebou wants to merge 1 commit into
coletebou wants to merge 1 commit into
Conversation
…hema
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
reviewOutputSchema(the LLM-input boundary) is stricter thanfindingRecordSchema(the storage boundary). The asymmetry means LLM output that the store would happily accept gets rejected at parse time. This PR aligns the two by relaxing four fields:evidenceRefSchema.startLinenumber().int().positive().nullable()number().int().min(0).nullable().transform(0 → null)evidenceRefSchema.endLinefindings[].reproductionstring().nullable()string().nullish().transform(undefined → null)findings[].minimumFixScopestring()string().nullish().transform(undefined → "")findingRecordSchemais unchanged — it was already tolerant via the existing.transform(...)at L259.The
0 → nulltransform on line numbers handles the common LLM pattern of emitting0for "unknown line." The downstreamvalidateReviewOutput.assertLineRangealready early-returns when both lines are null, so the validator is compatible.Why
Run
20260517T190759-3c9e9ehad 4 zod failure patterns this PR addresses:invalid_type @ findings[N].minimumFixScope(got undefined)invalid_type @ findings[N].reproduction(got undefined)too_small @ findings[N].evidence[N].startLine(got 0)too_small @ findings[N].evidence[N].endLine(got 0)After this PR, all four pass.
Files
src/types.ts— 4 schema relaxationssrc/provider-schema.ts— switchedz.toJSONSchemato{ io: "input", unrepresentable: "any" }because zod v4 refuses to emit JSON Schema for transforms;io: "input"is the right surface for LLM constraint anywaysrc/provider.test.ts— 5 new casesValidation
pnpm format:check— cleanpnpm typecheck— cleanpnpm lint— cleanpnpm build— cleanpnpm test— 546 passed, 1 skipped (+5 new)Coordination
Composes with the safeparse-partition PR — without partition, even with this tolerance landed, ANY remaining bad finding would still drop its whole feature. With both landed, dropped findings are minimal AND the schema is permissive enough that the model rarely produces them.