Skip to content

The eval-journal corroboration hook is gated behind runError, so it never runs for the failure it targets #332

Closed

Description

@aviggiano

Follow-up from the #320 work. The corroboration hook added on branch fix/diagnostics-corroboration reads the eval journal to settle model_work_started, but it is gated so that it cannot run for the failure it was written for.

The gate

public-worker.ts calls the hook only when runError === undefined. But:

  • packages/cli/src/commands/eval/run.ts:63 — ok: result.failed === 0 && result.incomplete === 0
  • packages/cli/src/command-shared.ts:118-122 — emitCommandResult sets process.exitCode = 1 when !result.ok
  • public-worker.ts — runCommand throws OperationalDispositionError on any nonzero exit

So any failed or incomplete row makes ultrafuzz eval run exit 1, which makes runError defined, which skips the hook. Verified by execution:

HOOK CALLED WHEN eval run EXITS NONZERO: false | runError: true

Consequences

  1. A journal containing a non-launched row can never reach the predicate in production. The failed row that would produce a none verdict is the same row that skips the hook. When the hook does run, failed === 0, so every record is launched and the answer is "launched" regardless of the diagnostics rule.
  2. The only production-reachable none is a zero-record journal (empty matrix), where none is correct.
  3. model_work_started forces strict public-eval diagnostics on a sandbox exit that did no model work #320's stated goal is not delivered. If the point was to restore the pre-model retry for genuinely pre-model failures, this wiring does not do it: all rows fail pre-model → exit 1 → hook skipped → flag stays raised → resume-required / action: "none" → no pre-model retry.
  4. It also means the WORKFLOW_SUBMISSION_FAILED hazard fixed on that branch was latent rather than live, and the conservatism cost of that fix is likewise unreachable today. Both traces that demonstrated the hazard drove the API directly and bypassed the runError gate.

What to decide

Either move the corroboration read to a point that observes the journal regardless of exit status, or accept that model_work_started is never lowered in practice and say so at the raise site — but do not leave code whose comment claims a guarantee the gating prevents.

Related, lower severity

launchEvalRow can throw between a successful launch and appendJsonLine (packages/evals/src/runner.ts:243-249, readRunFingerprints / resolveTerminalReportPath), dropping a launched row from the journal entirely. If the gate above is ever relaxed, a surviving all-failed runs.jsonl would then read none for a run that did spend tokens.

Also: the catch guarding the hook is silent, so a persistently throwing assertSafeEvalId would be invisible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions