Skip to content

fix(eval): support expected terminal error outcomes - #5272

Merged
kwakayama merged 5 commits into
mainfrom
fix/eval-expected-error-outcome
Oct 18, 2026
Merged

kwakayama merged 5 commits into
mainfrom
fix/eval-expected-error-outcome

Conversation

@kwakayama

@kwakayama kwakayama commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Negative-path agent evals currently fail on the mandatory record.error gate even when every assertion passes. This blocks the invalid-reference, missing-text, and command-failure cases described in Outlook PR #89.

Add expectedOutcome: "error" to individual dataset examples. An error satisfies that expectation only when the adapter confirms a terminal target error with outcome: "error". The local agent and tool adapters and the AG-UI agent-service adapter provide that marker. Custom adapters must opt in explicitly. Unexpected success, transport failures, incomplete streams, timeouts, grading errors, and blocking assertions still fail. Existing cases retain the strict success default. Completion, error, and trace data remain available to checks and report consumers.

Use one outcome gate across summary, progress, runtime metrics, Markdown, and JUnit. Preserve expectations in timeout records and Studio source documents and patches. Outcome-only tool errors produce failed synthetic tool traces. Update public types, hash-input descriptions, and the eval concept guide. Generated API references were regenerated with pinned Deno 2.9.7 and remain unchanged.

Validation:

  • Regression fails against the original implementation and passes with the fix.
  • deno task test:file src/eval cli/commands/eval: 39 suites, 395 steps pass.
  • deno task lint: passes.
  • env -u FORCE_COLOR NO_COLOR=1 deno task lint:ci: passes.
  • deno task typecheck: passes.
  • deno check src/eval/runner.test.ts src/eval/run-report.test.ts: passes.
  • deno task fmt:check: passes.
  • deno task docs: passes.

Hacks or workarounds: none. No CI gates or configuration changed.

@github-actions

Copy link
Copy Markdown

📦 Client bundle boundary

Entrypoint Modules Source size Server leaks
src/index.client.ts 65 362 KiB ✅ 0
src/react/components/Head.tsx 5 74 KiB ✅ 0
src/react/router/index.tsx 5 74 KiB ✅ 0
src/react/context/index.tsx 5 74 KiB ✅ 0
src/react/fonts/index.ts 6 81 KiB ✅ 0
src/react/components/ui/index.ts 111 717 KiB ✅ 0
src/react/components/ui/adapter/contract.ts 1 18 KiB ✅ 0
src/react/components/ui/icons/index.ts 3 17 KiB ✅ 0
src/chat/index.ts 301 2017 KiB ✅ 0
src/chat/ag-ui.ts 61 457 KiB ✅ 0
src/chat/protocol.ts 1 11 KiB ✅ 0
src/chat/types.ts 45 242 KiB ✅ 0
src/chat/message-prep.ts 65 509 KiB ✅ 0
src/markdown/index.ts 6 30 KiB ✅ 0
src/mdx/index.ts 2 2 KiB ✅ 0
src/agent/identity-contracts.ts 1 2 KiB ✅ 0
src/schemas/index.ts 42 216 KiB ✅ 0
src/platform/env.ts 5 53 KiB ✅ 0
src/workflow/claude-code/react/index.ts 9 91 KiB ✅ 0
src/events/index.ts 31 568 KiB ✅ 0
src/events/ag-ui/public.ts 30 561 KiB ✅ 0
src/run-events/index.ts 53 317 KiB ✅ 0

A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in scripts/lint/client-bundle-baseline.json to burn down.

@kwakayama

Copy link
Copy Markdown
Contributor Author

Codex independent review of 362dd8722acdedc05f7a37ff8a1a909b38cb6318

No actionable findings.

The implementation adds an explicit per-example expectedOutcome: "error" opt-in while keeping the default success gate strict. A terminal error must carry the adapter's confirmed outcome: "error" marker. Adapter exceptions, incomplete execution, grading failures, timeouts, and model-access denials do not acquire or retain that marker. Contradictory adapter results (outcome: "error" with completed: true) are rejected, and an error outcome with omitted completion normalizes to completed: false.

Reporting uses the same outcome gate for summaries, progress, runtime metrics, Markdown, and JUnit. Dataset normalization and hashing include the declared expectation, and local agent, tool, and hosted AG-UI adapters supply the marker only on their terminal error paths.

Verification performed against this exact SHA with pinned Deno 2.9.7:

  • git diff --check: passed
  • deno check for all 12 modified TypeScript files: passed
  • Focused runner suite: 46 steps passed
  • Focused report, agent-service, run-report, datasets, CLI, and exporter suites: passed
  • Broader repository run reported by the implementation lane: 391 steps passed
  • Documentation generation and checks: passed with no generated changes

Score: 98/100

  • Correctness and completeness: 39/40
  • Regression tests and verification: 20/20
  • Reliability and security: 15/15
  • Repository standards and maintainability: 14/15
  • Scope, documentation, and rollout clarity: 10/10

Verdict: APPROVE. This is a plain review comment and does not submit a GitHub approval review.

Review-Gate:
Reviewer: Codex
Reviewed-SHA: 362dd87
Score: 98/100
Actionable-Findings: 0
Verdict: APPROVE

@codecov

codecov Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.82759% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/eval/runner.ts 83.33% 2 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@kwakayama
kwakayama marked this pull request as ready for review October 18, 2026 07:05
@github-actions

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-18T07:31:31.282400Z 27599e7 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@kwakayama
kwakayama added this pull request to the merge queue Oct 18, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 362dd8722a

ℹ️ 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".

Comment thread src/eval/runner.ts
Comment thread src/eval/runner.ts
Comment thread src/eval/types.ts
Comment thread src/eval/report.ts
@kwakayama
kwakayama marked this pull request as draft October 18, 2026 07:12
@kwakayama
kwakayama removed this pull request from the merge queue due to a manual request Oct 18, 2026
@kwakayama

Copy link
Copy Markdown
Contributor Author

Codex independent re-review of 27599e7f8e99696bb2134330936afdeeada24bc0

No actionable findings remain.

This head addresses the four prior gaps at their owning boundaries:

  • Synthetic direct-tool traces now classify an outcome-only terminal error as a failed tool call, so behavior gates cannot report the failed invocation as healthy.
  • The pre-target timeout record preserves the example's declared expectedOutcome while clearing any confirmed target outcome, so the timeout remains a failure without losing report context.
  • Studio source documents and source patches preserve and validate explicit "success", explicit "error", and omitted expectations.
  • The dataset-hash documentation now names expectedOutcome among the hashed fields.

I also swept the sibling EvalExample and EvalRecord constructors, projections, schemas, exporter clones, timeout branches, reporting paths, and tool-trace normalization. I found no remaining path that drops the declaration or turns an outcome-only error into a successful synthetic trace.

Verification against this exact SHA with pinned Deno 2.9.7:

  • git diff --check: passed
  • Type checking for all modified TypeScript files: passed
  • Eval runner: 49 steps passed
  • Studio: 6 steps passed
  • Eval exporter: 15 steps passed
  • Broader eval and CLI verification reported by the implementation lane: 395 steps passed
  • Formatting, test-type, documentation generation, and documentation checks: passed
  • Full lint:ci: still running at comment time

Score: 99/100

  • Correctness and completeness: 40/40
  • Regression tests and verification: 20/20
  • Reliability and security: 15/15
  • Repository standards and maintainability: 14/15
  • Scope, documentation, and rollout clarity: 10/10

Verdict: APPROVE. This is a plain PR comment and does not submit a GitHub approval review.

Review-Gate:
Reviewer: Codex
Reviewed-SHA: 27599e7
Score: 99/100
Actionable-Findings: 0
Verdict: APPROVE

@kwakayama
kwakayama marked this pull request as ready for review October 18, 2026 07:29
@github-actions

Copy link
Copy Markdown

@codex review

@kwakayama
kwakayama added this pull request to the merge queue Oct 18, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

Reviewed commit: 27599e7f8e

ℹ️ 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".

@sonarqubecloud

Copy link
Copy Markdown

Merged via the queue into main with commit 71357de Oct 18, 2026
456 checks passed
@kwakayama
kwakayama deleted the fix/eval-expected-error-outcome branch October 18, 2026 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant