feat(eval): enforce caller-supplied output schemas#5284
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a3ebda422
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| stderr = ""; | ||
| } | ||
| } else if (!hasOutputSchema && allowFallback && rawOutput.trim().length > 0) { | ||
| } else if (prepared.normalized === undefined && !prepared.error && allowFallback && rawOutput.trim().length > 0) { |
There was a problem hiding this comment.
Allow permissive malformed schemas to fall back
When a permissive subagent has a malformed output schema (for example the same empty-string schema that the yielded path treats as loose with only an invalid output schema warning), a clean run that writes raw output but misses yield now falls through to the hard missing-yield failure because this fallback requires prepared.error to be false, and resolveFallbackCompletion also returns null on that flag. This regresses the previous behavior where malformed schemas degraded to no validation for raw-output fallback; keep blocking this only for strict mode or treat schema-preparation errors as unconstrained in permissive mode.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ffa5929bd7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } else if (strict) { | ||
| reject("strict schema mode requires a valid yield submission", undefined); |
There was a problem hiding this comment.
Preserve non-yield execution failures in strict mode
When a strict structured subagent exits nonzero before it can call yield (for example an auth/rate-limit/provider error that leaves exitCode === 1 and stderr populated, but is not doneAborted/signalAborted), this branch unconditionally replaces the original failure with schema_violation: strict schema mode requires a valid yield submission. The permissive path preserves pre-existing nonzero failures by only adding the missing-yield error when exitCode === 0; strict mode should similarly avoid clobbering the actionable stderr/raw output for failures that happened before schema enforcement could run.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b72b3be19
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (this.#schemaMode === "strict") { | ||
| return this.#strictFailure( | ||
| "schema validation failed", | ||
| data, | ||
| sectionFailure, | ||
| sectionValidation?.requiredFields ?? this.#requiredFields, | ||
| ); |
There was a problem hiding this comment.
Reset schema retry budget after successful yields
In strict mode this fail-closed branch uses the cumulative #schemaValidationFailures counter, but the counter is never reset after a later valid yield. A subagent that makes two schema mistakes on one section, corrects it, then makes two mistakes on an independent section will hit schema_violation on the second section after only one retry, despite the prompt saying the budget is for consecutive correction attempts. Reset the counter after a successful validation so earlier corrected mistakes do not consume the strict budget for later yields.
Useful? React with 👍 / 👎.
roboomp
left a comment
There was a problem hiding this comment.
P2: broad opt-in strict structured-output feature; maintainer sign-off still needed for the new eval/SDK contract surface.
Found one blocking behavior regression: the eval bridge maps any SingleResult.failure into AgentSchemaViolationError, so default permissive agent(..., schema=...) calls can now raise the strict-only typed error.
Verification: bun test ./test/tools/yield.test.ts ./test/tools/output-schema-validator.test.ts passed (65 tests). The broader eval-focused test command failed before assertions because src/export/html/index.ts could not resolve ./tool-views.generated.js in this worktree.
Thanks for the thorough coverage and clear strict/permissive split in the implementation.
| })(); | ||
|
|
||
| if (result.exitCode !== 0 || result.error || result.aborted) { | ||
| const strictSchemaFailure = result.failure !== undefined; |
There was a problem hiding this comment.
blocking: result.failure is not strict-mode-only. finalizeSubprocessOutput() sets it for any parent-side schema rejection, including the default schemaMode: "permissive" fallback path when agent(prompt, { schema }) returns JSON that fails the schema. This branch then returns { schemaViolation }, and every language prelude raises AgentSchemaViolationError, so omitted schemaMode no longer has the documented permissive behavior. Gate this on schemaMode === "strict" (and let permissive failures continue through the existing ToolError/subprocess failure path).
94feb17 to
9e4826c
Compare
- Added opt-in permissive and strict schema modes across subagent execution, eval runtimes, persistence, and follow-up turns. - Failed strict invocations before dispatch or after bounded correction attempts with typed schema-violation diagnostics. - Preserved permissive fallback behavior and covered assembly, abort, revival, and runtime boundaries. Fixes can1357#5278
- Allowed permissive runs with unusable schemas to retain the legacy raw-output fallback.\n- Added regression coverage for malformed schemas without yield data.
- Kept provider and transport failures intact when strict subagents exit before yielding.\n- Added regression coverage preventing schema-violation relabeling.
- Reset consecutive schema retries after accepted data-bearing yields.\n- Routed typed eval schema failures only for strict invocations while retaining generic permissive errors.
- Made caller-supplied eval schemas fail closed without a model-facing mode argument.\n- Kept task-agent native and inherited handoff schemas permissive across nested spawns.\n- Removed schema mode from JavaScript, Python, Ruby, and Julia eval helper surfaces.
9e4826c to
5a0d679
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a0d679a74
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
| } else if (!hasOutputSchema && allowFallback && rawOutput.trim().length > 0) { | ||
| } else if ( | ||
| (prepared.normalized === undefined || prepared.error) && |
There was a problem hiding this comment.
Preserve raw fallback for unconstrained schemas
When a subagent/session uses the valid boolean outputSchema: true contract and exits cleanly with plain text but no yield, prepared.normalized is true, resolveFallbackCompletion returns null for non-JSON text, and this branch no longer treats the run as loose. It falls through to the missing-yield failure even though absent schemas and malformed permissive schemas still accept non-empty raw output; include the normalized === true unconstrained case here so intentionally loose agents do not regress to exit code 1.
Useful? React with 👍 / 👎.
Accepted non-empty plain-text completion when a permissive task uses the valid boolean true JSON Schema and exits without yielding. Added regression coverage for the no-yield path.
Summary
evalagent(..., schema=...)contracts fail closed automatically, without exposing a model-facing strictness flagschema_violationresults across JavaScript, Python, Ruby, and Julia eval helpersVerification
bun test— 243 passed, 3 skipped, 0 failed across 19 focused structured-output, executor, eval, persistence, and Vibe files; 927 assertionsbun run checkinpackages/coding-agent— passed$refpreflight, and already-aborted precedenceCloses #5278