Skip to content

Don't score slow-but-correct submissions as wrong - #3

Open
davidoj wants to merge 1 commit into
mainfrom
fix/verification-timeout-budget
Open

Don't score slow-but-correct submissions as wrong#3
davidoj wants to merge 1 commit into
mainfrom
fix/verification-timeout-budget

Conversation

@davidoj

@davidoj davidoj commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

cut timeout (af88072) dropped the verification budget to 1s total and 1s per test, and removed the max(10, ...) floor. That budget is smaller than the honest runtime of real problems, so correct solutions are graded incorrect.

Reproduction

One correct submission, default settings, graded four times in a row against the same problem:

before:  call 1 FAILED 3.76s | call 2 FAILED 1.00s | call 3 FAILED 1.00s | call 4 FAILED 1.00s
after:   call 1 PASSED 3.82s | call 2 PASSED 3.09s | call 3 PASSED 3.11s | call 4 PASSED 3.10s

A real instance from the v0.9 train set: bamboo_toctou_overwrite_004_30's ground truth needs ~3.7s and is scored wrong.

Two distinct defects

1. The budget was too small to be a per-problem limit.

Restored to 60s as a backstop against a wedged daemon, with a 5s clamp that engages once a daemon has answered successfully. Steady-state requests therefore don't wait the full backstop, but a legitimately slow solution is no longer scored wrong.

Worth noting how this presented: the first request of a daemon's life gets a 15s startup grace (_daemon_startup_slack), so under the old cap the same submission could pass on call 1 and fail on calls 2+, depending only on how warm the daemon happened to be. In a batch grade that reads as nondeterministic wrongness rather than a systematic limit. (The per-test cap of 1s means call 1 often fails too, as above.)

2. An exceeded budget was reported as FAILED / CRASHED.

FAILED on the secure path, CRASHED on the insecure one — both indistinguishable from "the submission is wrong", so a harness limit silently became a data point. Timeouts now return TIMED_OUT, which VerificationStatus already defines and which callers can filter on. Feedback states explicitly that this is a harness limit, not a wrong answer.

Why this is worth more than a tuning argument

Any dataset built from these verdicts inherits the mislabelling, and the failure is invisible because a timeout looks exactly like a wrong answer. The second half of this PR stands on its own even if you disagree with 60s: a too-tight budget you can see is a tuning decision, whereas a too-tight budget reporting FAILED quietly writes bad labels.

If the intent behind af88072 was to bound wall-clock on a large grading run, the post-warmup clamp is the knob to turn (_post_warmup_timeout), not the total budget — it bounds steady-state cost without putting correctness at risk.

Testing

New djinn/tests/test_timeout_budget.py, 4 tests: timeout-error classification, budget sanity, verdict stability across warmup, and TIMED_OUT rather than FAILED on an exceeded budget. Probe workload is sized to sit well above the old 1s per-test budget and well below the restored clamp so it can't flake on a loaded machine.

Independent of #2 — both branch off main and can merge in either order.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MYYDDvJbdup3v5LPnkcYMW

`cut timeout` (af88072) dropped the verification budget to 1s total and 1s per
test, and removed the max(10, ...) floor. That budget is smaller than the honest
runtime of real problems, so correct solutions are graded incorrect.

Reproduction -- one correct submission, default settings, graded four times in a
row against the same problem:

  before:  call 1 FAILED 3.76s | call 2 FAILED 1.00s | call 3 FAILED 1.00s | ...
  after:   call 1 PASSED 3.82s | call 2 PASSED 3.09s | call 3 PASSED 3.11s | ...

Two distinct defects, fixed separately:

1. The budget was too small to be a per-problem limit. It is restored to 60s as
   a backstop against a wedged daemon, with a 5s clamp that kicks in once a
   daemon has answered successfully. Steady-state requests therefore don't wait
   the full backstop, but a legitimately slow solution is no longer scored wrong.
   Note the first request of a daemon's life gets a 15s startup grace, so under
   the old cap the SAME submission could pass on call 1 and fail on calls 2+,
   depending only on how warm the daemon happened to be -- which reads as
   nondeterministic wrongness in a batch grade.

2. An exceeded budget was reported as FAILED on the secure path and CRASHED on
   the insecure path. Both are indistinguishable from "the submission is wrong",
   so a harness limit silently became a data point. Timeouts now return
   TIMED_OUT, which VerificationStatus already defines and which callers can
   filter on. Feedback says explicitly that this is a harness limit.

This matters beyond a slow test: any dataset built from these verdicts inherits
the mislabelling, and the failure is invisible because a timeout looks exactly
like a wrong answer.

Covered by djinn/tests/test_timeout_budget.py.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MYYDDvJbdup3v5LPnkcYMW
@davidoj

davidoj commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up: #4 fixes what I now believe is the cause of the pressure behind af88072.

A child that exits without writing a result leaves its pipe at EOF; poll() reports EOF as readable, recv() raises EOFError, and the daemon swallows it without replying — so the parent blocks its whole budget. That was 88% of grading wall clock (211s → 25.1s on a 160-call sweep).

Cutting the budget to 1s caps that stall, which is very likely why it was done — but it treats the symptom and creates the false-FAILED problem this PR is about. Recommend merging #4 first: with the stall gone, a generous budget costs little because it is rarely reached, which removes most of the objection to restoring 60s here.

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