fix(orchestrator): grade a forced-kill timeout instead of discarding it

[joeysbase]

Summary

A structural timeout used to discard the agent's work. TaskTimeoutError / TurnTimeoutError set a status and threw the trajectory away, so a task that timed out but had already satisfied its success criteria was reported TIMEOUT. Both handlers now grade what the agent produced and finalize SUCCESS (plain, error_message cleared) when the criteria pass.

Three things ride along, each needed to make that safe:

• Antigravity's poll loop is rebounded. A poll cycle's cost is bimodal — against a backgrounded job the connection is idle and receive_steps() returns immediately (5s/cycle); against a wedged connection every re-drain burns the full 30s per-step timeout. One bound can't cover both, so _MAX_BACKGROUND_POLLS (120 × 5s = 600s) sizes the cheap mode and a new _MAX_BACKGROUND_POLL_WALL_SECONDS sizes the wedged one.
• A real TurnTimeoutError on the "nothing settled" exit, instead of silently finalizing as an ordinary COMPLETED turn — that's what gives the orchestrator's grading path a chance to run.
• CE022 generalized to a (file, function, cap) table, since this change adds two # noqa: PLR0915 sites.

Behavior change worth flagging to consumers

FinalStatus.TIMEOUT no longer means "every timed-out run". A timed-out task whose criteria pass now finalizes SUCCESS, so anything reading TIMEOUT as a proxy for "hit the wall" (dashboards, error-rate rollups) will see a shift. CLAUDE.md, docs/agents/HARNESS_PARITY.md and docs/agents/ANTIGRAVITY.md are updated to say so.

Safety properties of the grading pass

• Commits the fallback status before its first await and only ever upgrades to SUCCESS, so a BaseException mid-grade can't leave the row at the constructor default.
• Quiesces the agent first. On a TurnTimeoutError nothing has torn the harness down yet — Antigravity's kill_sync() is intent-only and _cleanup() runs later in run()'s finally — so without this the criteria could read a sandbox a backgrounded build was still writing.
• Wall-clock bounded (60s), never raises, and honors the same FIRED-ONLY early-stop gate as a normal run.
• Re-grades rather than reusing a stale snapshot. The simulation loop rewrites success_criteria_results every turn under check_criteria: every_turn, so a non-empty list alone doesn't mean the grade covers the trajectory; a _graded_iteration_count stamp gates the shortcut.

Known limitation (not fixed here)

With a configured turn_timeout, the poll deadline is 0.8 × turn_timeout measured from turn start (it has to be turn-anchored to win its race with the watchdog). At the repo default turn_timeout: 300 that's 240s — below the 300s worst backgrounded job on record. Raising it is a defaults change rather than a constants change, so it's asserted explicitly in test_background_poll_budget_still_covers_the_worst_observed_backgrounded_job: changing the default deliberately trips the test.

Testing

make verify — 4149 passed, coverage 91.68%; make lint — 340 passed.

One pre-existing failure remains, unrelated to this branch: test_effective_model_prefers_config_then_default asserts _effective_model() falls through to _DEFAULT_MODEL, which fails on any machine whose .env sets ANTIGRAVITY_MODEL (pydantic-settings reads .env). It passes in CI, which has no .env.

New coverage includes the forced-kill grading matrix (pass → SUCCESS, fail → TIMEOUT, already-graded shortcut, stale-snapshot re-grade, 60s budget expiry, CancelledError mid-grade, agent quiesce + quiesce failure), both branches of the FIRED-ONLY gate, and the poll-budget bounds.

Review

Two rounds of multi-model review (three Opus reviewers each; the multi MCP server was unavailable, so this used the documented fallback). Round 2 reviewed round 1's fixes and found three High issues in those fixes — a grading/live-agent race, a poll budget that could be zero, and a lost TIMEOUT classification under BaseException — all fixed here.

🤖 Generated with Claude Code

[uipreliga]

Review: coder_eval — pr:117

Scope: pr:117 · branch fix/timeout-discards-grading (PR #117 head; local checkout is feat/plugin-optimize-skill) · d720719 · 2026-08-17T08:16Z · workflow variant

Change class: complex — rewires the orchestrator's forced-kill/timeout path to grade instead of discard, adding new asyncio cancellation, quiesce and partial-turn control flow plus a new FinalStatus-adjacent grading branch; correctness requires reasoning about cancellation ordering and turn-record preservation

Architecture, typing and security are in excellent shape (9.5/9.8/10) and the new forced-kill grading path is unusually well-reasoned in prose, but the same feature quietly opens two ways for identical agent output to land a different final_status — grading a sandbox that surviving Bash children are still mutating (orchestrator.py:778, claude_code_agent.py:1343 kills the CLI pid only, no process group) and Antigravity's 0.8×turn_timeout poll deadline turning a quiet 30s step-fetch into an unretried TurnTimeoutError (antigravity_agent.py:795) — while the branch that actually decides that status is untested, a cancellable kill() can orphan a localharness process, and four doc/prose surfaces now contradict the shipped code; bottom line: strong codebase, land the determinism and teardown fixes plus the missing tests before merge, then clean up the prose.

Summary

Axis	Score	🔴	🟠	🟡	🔵	Top Issue
1. Code Quality & Style	7.9 / 10	0	1	2	1	Complexity pushed further past the configured ceilings in the two hot modules this PR touches (3 functions, incl. communicate CC 43→49 / 86 statements behind a new # noqa: PLR0915)
2. Type Safety	9.8 / 10	0	0	0	2	pyright reportOptionalMemberAccess on endpoint.options — isinstance cannot narrow against an untyped SDK class
3. Test Health	8.4 / 10	0	0	3	1	Three of the four new forced_kill consumers ship with zero assertions, and neither hand-maintained dict projection has a model_fields parity guard
4. Security	10 / 10	0	0	0	0	—
5. Architecture & Design	9.5 / 10	0	0	1	0	_await_pending_grade sits after _run_post_run_commands, so post-run shell commands interleave with the still-running criterion the barrier was written to protect
6. Error Handling & Resilience	7.8 / 10	0	1	2	2	New 15s quiesce wait_for is the tree's only cancellable kill() site; AntigravityAgent._teardown nulls _exit_stack before aclose(), so an interrupted localharness reap is never retried
7. API Surface & Maintainability	8.3 / 10	0	0	3	2	HARNESS_PARITY.md documents a "120 poll cycles" parallel bound this PR deleted (and two in-code comments at antigravity_agent.py:753/:801 are stale too)
8. Evaluation Harness Quality	9 / 10	0	1	0	0	Forced-kill grading races orphaned tool-call children on claude-code too, but the RESIDUAL comment scopes the gap to Antigravity — a previously deterministic TIMEOUT row becomes race-dependent

Overall Score: 8.8 / 10 · Weakest Axis: Error Handling & Resilience at 7.8 / 10
Totals: 🔴 0 · 🟠 3 · 🟡 11 · 🔵 8 across 8 axes.

Blockers

1. [Axis 1] Complexity pushed further past the configured ceilings in the two hot modules this PR touches (3 functions, incl. communicate CC 43→49 / 86 statements behind a new # noqa: PLR0915) (src/coder_eval/agents/antigravity_agent.py:587) — Measured with radon at PR HEAD vs origin/main (I re-ran radon on both trees; automated/radon.txt lines 389, 209, 212 agree):

2. AntigravityAgent.communicate — antigravity_agent.py:587 — F (49) at HEAD vs F (43) on main (+6), 274 lines (587–860). The PR acknowledges the growth by adding a suppression instead of decomposing: async def communicate( # noqa: PLR0915 — the new step_fetch_timed_out post-loop branch pushed it over the cap; decomposing this poll-loop-plus-finalize method is out of scope for this fix. ruff check --select PLR0915 --ignore-noqa reports Too many statements (86 > 80).

3. Orchestrator.run — orchestrator.py:460 — D (23) at HEAD vs D (21) on main (+2), from the new except TurnTimeoutError arm plus the if self.result.forced_kill or self.result.final_status in {...} allowlist at line 648.

4. Orchestrator._grade_after_forced_kill — orchestrator.py:722 — new C (14), 165 lines, with two nested try/except layers, an eager status commit, a shortcut return, a shield+wait_for and a fallback handler in one method.

Both are hot modules (Axis-1 anchor: "CC > 20 in a hot module"; the axis brief names agent as a key module). Concrete decompositions that need no design change: in communicate, the new post-loop timeout arm (lines 795–829) is self-contained — lift it to _finalize_stalled_poll_exit(state, conversation, timeout, turn_start_time, poll_count); the poll loop (712–762) is likewise liftable to _poll_for_background_work(...). In _grade_after_forced_kill, the already-graded shortcut (820–825) and the fresh-grade block (829–878) are two separable units. Note the growth is now invisible to make check: ruff's statement check is suppressed and CE022's cap for this function is 81 against a measured 77 (4 statements of headroom), so the next equivalent addition just bumps the cap again.
2. [Axis 6] New 15s quiesce wait_for is the tree's only cancellable kill() site; AntigravityAgent._teardown nulls _exit_stack before aclose(), so an interrupted localharness reap is never retried (src/coder_eval/orchestrator.py:798) — _grade_after_forced_kill introduces the first cancellable kill() call site in the tree:

798:                await asyncio.wait_for(self.agent.kill(), timeout=_QUIESCE_TIMEOUT_SECONDS)

asyncio.wait_for cancels the inner coroutine on expiry. AntigravityAgent.kill() (antigravity_agent.py:867-873) awaits conversation.cancel() and then stop() -> _teardown(), and _teardown is NOT cancellation-safe — it drops the stack reference before awaiting the close:

901:    async def _teardown(self) -> None:
902:        """Close the SDK Agent context (reaps the localharness subprocess)."""
903:        stack = self._exit_stack
904:        self._exit_stack = None
905:        self._sdk_agent = None
906:        if stack is not None:
907:            with contextlib.suppress(Exception):
908:                await stack.aclose()

The exit stack holds SdkAgent(cfg) (antigravity_agent.py:492-493), whose __aexit__ reaps the localharness subprocess. If the 15s cancel lands inside stack.aclose(), contextlib.suppress(Exception) does not catch CancelledError, the remaining callbacks never run, and _exit_stack is already None — so _cleanup()'s later await self.agent.stop() (orchestrator.py:2574) re-enters _teardown, sees stack is None, and no-ops. The localharness process is never reaped, once per timed-out quiesce, for the life of the batch process. Antigravity is the only affected agent: ClaudeCodeAgent.kill() is self.kill_sync() (claude_code_agent.py:1299) and CodexAgent.kill() is _interrupt_active_turn() + a fully synchronous stop() (codex_agent.py:992-995), so neither has an await point to cancel.

Fix either side: (a) make _teardown cancellation-safe (try: await stack.aclose() finally: self._exit_stack = None, or only null it after a successful close), or (b) bound the hang where it actually is — put the wait_for around Antigravity's own unbounded await conversation.cancel() (antigravity_agent.py:872) instead of around the whole kill(), leaving teardown uninterruptible.
3. [Axis 8] Forced-kill grading races orphaned tool-call children on claude-code too, but the RESIDUAL comment scopes the gap to Antigravity — a previously deterministic TIMEOUT row becomes race-dependent (src/coder_eval/orchestrator.py:778) — _grade_after_forced_kill reads the sandbox with check_all_async after a best-effort quiesce, and the code scopes the leftover hazard to one backend: "# RESIDUAL (not closed by this): on Antigravity the harness backgrounds / any command over ~10s ... So a / criterion can still read a tree a backgrounded build is mutating." (orchestrator.py:778-781). That understates the exposure — the quiesce is await asyncio.wait_for(self.agent.kill(), timeout=_QUIESCE_TIMEOUT_SECONDS) (orchestrator.py:798), and on the nightly's default backend ClaudeCodeAgent.kill() -> kill_sync() -> _kill_transport() ends at proc.kill() (claude_code_agent.py:1343), which signals the CLI pid ONLY — no killpg, no start_new_session. Any child a Bash tool call spawned (pytest, npm run build, even the sleep 300 in tasks/smoke_task_timeout.yaml) survives and keeps writing into the tree that file_exists / file_contains / run_command criteria now grade. Before this PR nothing read the sandbox on the timeout path, so the row was deterministically TIMEOUT; now the same task with the same agent behaviour can finalize SUCCESS or TIMEOUT depending on which side of the race wins, and weighted_score moves with it. Fix: reap the process group (spawn the agent CLI with start_new_session=True and os.killpg(os.getpgid(pid), SIGKILL) in _kill_transport) so the quiesce actually quiesces; at minimum correct the RESIDUAL comment to say the gap applies to claude-code too, and add a determinism assertion (grade the same frozen trajectory twice) rather than leaving the invariant to prose.

Non-blocking, but please consider before merge

1. [Axis 1] _grade_after_forced_kill re-inlines the extracted _run_dialog_criteria_check sequence, and the new _graded_iteration_count invariant is hand-written at 4 unguarded sites (src/coder_eval/orchestrator.py:829) — _run_dialog_criteria_check (orchestrator.py:1954) already IS the extracted form of this block — its docstring says so: The block lifted verbatim from the three identical sites (per-turn, budget-gate fallback, end-of-dialog): (re)load the reference, run ``check_all_async``, fold this turn's judge usage into the dialog-wide accumulator, store the results, and recompute the weighted score. Lines 1969–1983 are exactly load_reference(...) → check_all_async(...) → _accumulate_judge_usage(...) → success_criteria_results = ... → _graded_iteration_count = len(...) → calculate_weighted_score(...).

_grade_after_forced_kill re-inlines that same sequence at lines 829–864 (differing only in the ensure_future/shield/wait_for wrapper) instead of calling the helper, so the block now exists at four sites: 1791–1803 (evaluate-only), 1850–1862 (single-shot), 1969–1983 (dialog helper), 829–864 (forced kill).

The new bookkeeping line is the part that will actually rot: self._graded_iteration_count = len(self.result.iterations) now appears at orchestrator.py:862, 1803, 1862 and 1982 — four hand-maintained writes of one invariant that the field's own comment (line 418-423) declares is "len(result.iterations) at the moment success_criteria_results was last written". A fifth grading site that forgets the line silently changes the forced-kill path's already-graded shortcut at line 820. Fix: give _run_dialog_criteria_check an optional wait budget (or extract its load-reference + check_all_async core) and call it from _grade_after_forced_kill, so the counter is written in exactly one place.
2. [Axis 1] Explanatory prose in reports_experiment.py contradicts the code it documents in 5 places (incl. _cost_complete's docstring disclaiming the final_status is TIMEOUT disjunct it keeps at L67), plus one stale measured line number (src/coder_eval/reports_experiment.py:60) — Each of these was read at the PR HEAD and is wrong as written:

1. reports_experiment.py:60-62 — docstring: NOT keyed on ``final_status is TIMEOUT`` -- such a run is graded after the kill and can finalize SUCCESS. Line 67 is if result.forced_kill or result.final_status is FinalStatus.TIMEOUT: — it is keyed on it. (The disjunct is what keeps the untouched tests/test_cost_accounting_paths.py:126 green, since that test sets only final_status; say so, or drop the disjunct — _grade_after_forced_kill(fallback_status=FinalStatus.TIMEOUT) at orchestrator.py:720 is the tree's only FinalStatus.TIMEOUT producer and it always sets forced_kill = True first at line 766.)
2. agents/antigravity_agent.py:801 — new comment says Poll budget exhausted (poll_deadline or the cycle cap), but this PR deleted _MAX_BACKGROUND_POLLS; line 717-723 in the same function says the cycle cap was tried and removed.
3. agents/antigravity_agent.py:753 — same stale cycle cap) reference, left unupdated by the removal.
4. tests/test_antigravity_agent.py:971 — # Exited via the poll_deadline (well under the 120-cycle cap); there is no 120-cycle cap any more.
5. tests/test_custom_lint.py:391-393 — CE022 bounds the regrowth of every function that keeps a ``# noqa: PLR0915`` (currently ``_simulation_dialog_loop``, ``run``, and ``communicate``). _TARGETS registers two entries (ce022_dialog_loop_statement_cap.py:57-58) and grep -rn 'noqa: PLR09' src/ returns exactly two hits — no run anywhere.

Also stale-by-4: ce022_dialog_loop_statement_cap.py:24-25 records communicate as 77 CE022-stmts ≙ 82 ruff-stmts; ruff check --select PLR0915 --ignore-noqa at HEAD reports 86.
3. [Axis 3] Three of the four new forced_kill consumers ship with zero assertions, and neither hand-maintained dict projection has a model_fields parity guard (src/coder_eval/reports_experiment.py:203) — EvaluationResult.forced_kill (models/results.py:520) grew four consumers in this PR; only ONE is tested.

Tested: _cost_complete — tests/test_timeout_orchestrator.py:824 asserts _cost_complete(orchestrator.result) is False.

Untested:

1. reports_experiment.py:203 — "forced_kill": result.forced_kill, in eval_result_to_task_dict. This is the cross-repo contract row the external coder-eval-uipath pipeline reads; tests/test_reports_experiment.py has 12 eval_result_to_task_dict assertions and none touches this key.
2. orchestrator.py:285 — "ForcedKill": result.forced_kill, in build_task_event. tests/test_orchestrator_telemetry.py:163 test_build_task_event_passes_driver_and_buckets_status asserts Driver/VariantId/Category/Status and stops there.
3. orchestrator.py:648 — if self.result.forced_kill or self.result.final_status in {...} gating error_log_tail. The comment above it claims "the tail is the only in-task.json evidence of the kill once error_message/error_details are cleared on that upgrade", but tests/test_orchestrator_error_log_tail.py has no forced-kill case; its test_error_log_tail_none_on_success (line 95) covers only the ordinary SUCCESS path.

Self-evidencing: grep -rn '\"ForcedKill\"\|\[\"forced_kill\"\]' tests/ returns 0 lines, and grep -rn model_fields over tests/test_reports_experiment.py tests/test_orchestrator_telemetry.py tests/test_reports_junit.py returns nothing — so there is no field-parity test that would have caught a fifth serializer being missed either.

Fix: assert eval_result_to_task_dict(result)["forced_kill"] is True and build_task_event(result, ...)[1]["ForcedKill"] is True for a forced-kill result that finalized SUCCESS; add a run()-level assertion that such a result carries a non-None error_log_tail; and add a parity test over EvaluationResult.model_fields for the two hand-maintained dict projections.
4. [Axis 3] The forced-kill grading path's new behaviours ship untested — two never-exercised branches (incl. the shortcut arm that decides final_status), the teardown barrier call, the CancelledError re-raise, and the judge-cost fold all pass when deleted (src/coder_eval/orchestrator.py:822) — The routed coverage run (automated/pytest.txt, orchestrator.py at 91.39%) lists 822->825 as a partial branch and 931-933 as missed. Both are new code from this PR.

1. orchestrator.py:820-825 — the already-graded shortcut:

            if self.result.success_criteria_results and self._graded_iteration_count == len(self.result.iterations):
                self.result.final_status = FinalStatus.SUCCESS if self._gate_passed() else fallback_status
                if self.result.final_status == FinalStatus.SUCCESS:

The 822->825 partial means the False edge — shortcut taken, gate FAILS, status stays fallback_status — is never executed. tests/test_timeout_orchestrator.py:727 test_grade_after_forced_kill_skips_regrade_when_already_graded covers only the passing arm (score=1.0 → SUCCESS). This is precisely the "gate that turns a gap into a score" shape: the branch that keeps a covering-but-failing grade at TIMEOUT is untested, and it is the branch whose whole purpose is to avoid re-spending an llm_judge/agent_judge criterion.

2. orchestrator.py:931-933 — _log_graded_after_forced_kill's length-mismatch guard:

        except ValueError:
            logger.warning("[%s] Graded after forced kill (tally unavailable)", self.task.task_id)
            return

This handler is the entire reason the helper was extracted (its docstring: "so a zip(..., strict=True) length mismatch in a log line can never undo the status its caller already committed") and it is never entered.

Fix: (1) copy test_grade_after_forced_kill_skips_regrade_when_already_graded with score=0.0 and assert both check_all_async.assert_not_awaited() and final_status == FinalStatus.TIMEOUT; (2) call _log_graded_after_forced_kill with a results list shorter than task.success_criteria and assert it returns without raising while result.final_status is untouched.
5. [Axis 3] The 600s poll budget's load-bearing SDK premise is asserted only in prose, and the arithmetic assertion at tests/test_antigravity_agent.py:1903 is implied by line 1900 (the configured-timeout assertions at 1910-1911 are the test's only independent constraints) (tests/test_antigravity_agent.py:1867) — _MAX_BACKGROUND_POLL_WALL_SECONDS = 600.0 is justified entirely by a claim about the vendored SDK (antigravity_agent.py:159-163): "the installed SDK's receive_steps() returns immediately (if self.is_idle and self._processor.step_queue.empty(): return, connections/local/local_connection.py) and the cycle costs just _BACKGROUND_POLL_INTERVAL_SECONDS = 5s -> 120 cycles inside the budget." If a future google-antigravity bump makes an idle receive_steps() block instead, every cycle costs 30 + 5 = 35s, 600s buys ~17 cycles instead of 120, and the docstring's own words apply: it "cut the idle budget to 85s and re-opened d3f1432's bug."

Nothing asserts that SDK behaviour. grep -rn "is_idle\|step_queue" tests/ returns exactly ONE hit — tests/test_antigravity_agent.py:1871, inside a docstring. The file already establishes the right convention two hundred lines earlier: test_installed_sdk_still_exposes_the_env_seam (line 1275) uses pytest.importorskip to pin the SDK half of a contract precisely because "all of them would still pass while … silently stopped".

Meanwhile the test that claims to pin the budget adds no second constraint. At lines 1900-1903:

    assert antigravity_agent._MAX_BACKGROUND_POLL_WALL_SECONDS >= _WORST_OBSERVED_BACKGROUNDED_JOB_SECONDS
    cycles = antigravity_agent._MAX_BACKGROUND_POLL_WALL_SECONDS / antigravity_agent._BACKGROUND_POLL_INTERVAL_SECONDS
    assert cycles >= _WORST_OBSERVED_BACKGROUNDED_JOB_SECONDS / antigravity_agent._BACKGROUND_POLL_INTERVAL_SECONDS

the second assertion is the first divided on both sides by the same positive constant, so it can never fail independently. (test_per_step_timeout_is_not_aliased_to_the_poll_interval at line 1845 has the same redundancy: its != assertion is implied by the >= 6 * one.)

Fix: add an importorskip-guarded test asserting the real LocalConnection.receive_steps() returns promptly on an idle connection with an empty queue (mirroring line 1275's pattern), and replace line 1903 with a constraint the first assertion does not already imply — e.g. that the wedged-mode cycle cost _RECEIVE_STEPS_PER_STEP_TIMEOUT_SECONDS + _BACKGROUND_POLL_INTERVAL_SECONDS still divides the budget into more than one cycle.
6. [Axis 5] _await_pending_grade sits after _run_post_run_commands, so post-run shell commands interleave with the still-running criterion the barrier was written to protect (src/coder_eval/orchestrator.py:637) — The barrier's stated purpose (orchestrator.py:431) is "Awaited before sandbox teardown so cleanup can never race a live criterion", and its docstring names the hazard: "Deleting or moving the sandbox out from under one is a real corruption risk (a run_command criterion may still be writing)". But the finally block orders it second: line 629 await self._run_post_run_commands() then line 637 await self._await_pending_grade() then line 638 await self._cleanup(). _run_post_run_commands (line 2558) executes "post-run commands inside the sandbox" via _run_command_list — the same tree the detached criterion thread is reading. Failure scenario: a task with a post_run: block times out; grading exceeds _GRADE_AFTER_FORCED_KILL_TIMEOUT_SECONDS, so _pending_grade is left set with a file_exists/run_command criterion still on an asyncio.to_thread worker; the post-run command then creates or removes files in the sandbox while that criterion evaluates them. The verdict for identical agent output becomes timing-dependent — the exact class the barrier was added to close, just against the wrong mutator. Move await self._await_pending_grade() to the top of the finally block, before _refresh_runtime_tool_versions()/_run_post_run_commands(); it is unconditional and best-effort, so nothing in post-run depends on it running first. Cross-axis: this is also an error-handling (6) and scoring-determinism (8) issue — flag for escalation there.
7. [Axis 6] _await_pending_grade (orchestrator.py:915) suppresses CancelledError without recording it, so a cancel arriving in run()'s teardown is dropped instead of folded into teardown_interrupt and re-raised (src/coder_eval/orchestrator.py:915) — Commit 4 of this PR was "don't swallow CancelledError in the pre-grading quiesce", but the same swallow remains one function over — and here it is by name:

915:        with contextlib.suppress(Exception, asyncio.CancelledError):
916:            await grade

The finish teardown, don't abort intent is right; what is missing is the second half of the pattern established 15 lines earlier in the same finally:

630:                except (Exception, asyncio.CancelledError) as e:
631:                    teardown_interrupt = e
...
657:                if teardown_interrupt is not None:
658:                    raise teardown_interrupt

Because _await_pending_grade (called at line 637) drops the exception instead of recording it, a cancel delivered while awaiting the over-budget grading worker is permanently lost: teardown completes, _finalize_result runs, and run() returns a normal EvaluationResult at line 660. A coroutine that swallows CancelledError and returns normally completes with a result rather than as cancelled (_must_cancel is unset once the error was thrown in), so a Ctrl-C / loop-shutdown cancel aimed at that task is silently ignored — the task looks like it finished cleanly. Make _await_pending_grade return BaseException | None (or take/return the interrupt slot) and have run() fold it into teardown_interrupt so it is re-raised after cleanup, exactly as post-run failures already are. There is no test for this path — test_over_budget_grading_is_awaited_before_sandbox_teardown (tests/test_timeout_orchestrator.py:756) covers only the non-cancelled case.
8. [Axis 6] Antigravity: 0.8 x turn_timeout poll_deadline became the effective turn ceiling for any turn in poll mode with no ACTIVE tool call (>=30s quiet step-fetch), forfeiting the last 20% of budget as a TurnTimeoutError (src/coder_eval/agents/antigravity_agent.py:795) — The poll loop's entry condition now ORs in the new per-step-timeout flag:

716:                        and (state.has_orphaned_tool_call() or state.step_fetch_timed_out)
...
724:                        and time.monotonic() < poll_deadline

with poll_deadline = turn_start_time + timeout * _POLL_DEADLINE_TIMEOUT_FRACTION (line 668, anchored at TURN START). So if a single steps.__anext__() goes quiet for _RECEIVE_STEPS_PER_STEP_TIMEOUT_SECONDS = 30.0 (line 549) at any point after 0.8 x turn_timeout has already elapsed, the while head fails immediately, the loop body never runs, has_orphaned_tool_call() is False, state.timeout_hit is still False (the watchdog fires at 1.0 x timeout), and control falls into the new branch:

795:            elif (
796:                state.step_fetch_timed_out
797:                and not state.has_orphaned_tool_call()
...
829:                self._finalize_and_raise_timeout(state.finalize, timeout if timeout is not None else elapsed)

The turn is aborted with TurnTimeoutError at >=0.8 x turn_timeout, forfeiting the last 20% of the configured budget. step_fetch_timed_out is set even when earlier steps in the same _drain call landed (line 534-540 states this explicitly), so this fires on a turn that was producing content and merely paused — before this change _drain simply blocked on __anext__ and the turn could still finish inside its real budget. Combined with the orchestrator change this now also flips final_status (TIMEOUT/SUCCESS-with-forced_kill instead of a clean completion) for that turn. Either exclude the no-orphan case from the tighter poll_deadline (fall back to the ThreadedWatchdog at 1.0 x timeout, which is what a genuine turn timeout is), or re-anchor the deadline at poll-loop entry for this signal the way the timeout=None backstop already is (line 710-711).
9. [Axis 7] HARNESS_PARITY.md documents a "120 poll cycles" parallel bound this PR deleted (and two in-code comments at antigravity_agent.py:753/:801 are stale too) (docs/agents/HARNESS_PARITY.md:96) — Line 95–100 states: "a task that sets no timeout falls back to a flat 600s wall-clock backstop. A second bound, 120 poll cycles, applies in parallel — whichever trips first wins. Two bounds because a cycle's cost is bimodal". No such second bound exists in the shipped code — the same PR removed it. _MAX_BACKGROUND_POLLS is gone (replaced by _MAX_BACKGROUND_POLL_WALL_SECONDS = 600.0, antigravity_agent.py:174) and the loop condition now carries the opposite comment: "ONE bound: the deadline. A parallel cycle cap was tried and removed -- it silently overrode a large configured turn_timeout (a task asking for 960s of polling got 120*5s = 600s)". grep -rn "noqa: PLR\|_MAX_BACKGROUND_POLLS" src/ confirms no cycle cap remains. A reader sizing turn_timeout: 1200 will compute a 600s polling ceiling that the code does not impose. Rewrite the paragraph to the single-deadline model CLAUDE.md:145 already describes correctly, and fix the sibling stale comment at src/coder_eval/agents/antigravity_agent.py:801 ("Poll budget exhausted (poll_deadline or the cycle cap)").
10. [Axis 7] REPORT_SCHEMA.md's forced_kill edit: field missing from both enumerations, gloss wrong, and the inserted paragraph splits the FinalStatus table (orphaning ERROR/BUILD_FAILED) (docs/REPORT_SCHEMA.md:298) — Two defects on the same new field. (a) Omission: eval_result_to_task_dict now emits "forced_kill": result.forced_kill (reports_experiment.py:203) into run.json task_results[] and experiment.json, and EvaluationResult persists it into task.json (models/results.py:520) — but neither field enumeration was updated: the task.json Results table still ends at | max_turns_exhausted | bool | Ran out of turns. | (line 127) and the task_results[] key list still reads "turn accounting (total_turns, visible_turns, expected_turns, max_turns_exhausted, has_final_reply)" (line 82). A consumer building a parser from the field tables — the stated purpose of this page ("Field-level reference … for anyone consuming a run") — never learns the field exists. (b) Imprecision: line 298 says 'To ask "did this task blow its budget?", read the durable forced_kill flag on the row, not the status.' forced_kill is set only by _grade_after_forced_kill (orchestrator.py:766), i.e. only for TaskTimeoutError/TurnTimeoutError; TOKEN_BUDGET_EXCEEDED and COST_BUDGET_EXCEEDED rows — literal budget breaches — carry forced_kill: false. Reword to the field's own accurate description ("a structural timeout hard-killed this run") and add it to both tables. CLAUDE.md:147 repeats the same "blew its budget" phrasing and needs the same correction.
11. [Axis 7] forced_kill reaches run.json/telemetry but no human-facing report surface, and the HTML error section meant to evidence it is suppressed (src/coder_eval/reports_experiment.py:203) — This PR adds "forced_kill": result.forced_kill, to the one task-dict builder (good — orchestration/batch.py:673 and reports_experiment.py:692 both route through it), but the parallel renderers that already surface exactly this class of structural marker were not touched: reports.py::_runtime_notes_lines emits > **WARNING:** [{task_id}] max_turns exhausted for max_turns_exhausted (line 495) and notes for expected_turns_overage / stopped_early, with no forced_kill branch; reports_html.py's task header badges (lines 361–368: status, score, cost, expected_turns_badge, early_stop_badge) likewise have none; reports_junit.py carries no attribute either. Net effect: a task that blew task_timeout yet passed its criteria renders in run.md as a bare SUCCESS row and in task.html as a green SUCCESS badge, with the kill visible only inside raw JSON — the opposite of the field's stated purpose. Add a forced_kill note in _runtime_notes_lines and a warning badge in reports_html.py, or state in the docstring why the two markdown/HTML surfaces deliberately omit it.

Nits

8 🔵 findings (stale comments, a redundant asyncio import at tests/test_antigravity_agent.py:516, a dead store, wording) — see tmp/code-review-260817-0116/00-summary.md and the per-axis files.

What's Missing

Parallel paths:

• 🟠 The BudgetExceededError arm three lines below the two it fixes (orchestrator.py:574-591) still discards everything the agent produced: TOKEN_BUDGET_EXCEEDED / COST_BUDGET_EXCEEDED rows keep success_criteria_results=[] and weighted_score=None, get no quiesce, no forced_kill marker and no error_log_tail-independent evidence — even though a cumulative-budget breach is the same class of mid-run hard abort from the same run_limits namespace (and the generic except Exception/AgentCrashError -> ERROR path is a third). Either route the budget arm through _grade_after_forced_kill(fallback_status=...) too, or state on the method why a timeout is gradable and a budget breach is not. (trigger: src/coder_eval/orchestrator.py)
• 🟡 The dialog path's own grader _run_dialog_criteria_check (orchestrator.py:1954) was not taught the forced-kill discipline and is not called by it — _grade_after_forced_kill re-implements load-reference + check_all_async + _accumulate_judge_usage + counter + calculate_weighted_score in parallel, so the two graders can now drift (a change to the dialog grader's reference handling or judge fold silently misses the kill path). (trigger: src/coder_eval/orchestrator.py) (restates: Axis 1: _grade_after_forced_kill re-inlines the extracted _run_dialog_criteria_check sequence)

Tests:

• 🟠 No test drives the quiesce wait_for to EXPIRY (orchestrator.py:798) — the new tests cover quiesce success and a kill() that raises only. That expiry is the one path that reaches the "Could not quiesce" warning, the un-reaped AntigravityAgent._teardown cancellation, and grading a sandbox under a live agent; a test with a kill() that sleeps past _QUIESCE_TIMEOUT_SECONDS asserting the warning and that _teardown still reaps costs three lines. (trigger: tests/test_timeout_orchestrator.py) (restates: Axis 6: New 15s quiesce wait_for is the tree's only cancellable kill() site)
• 🟠 No test at the SUITE/report level for the new behaviour: nothing asserts that a graded forced-kill row now appears in build_suite_rollup's criterion_aggregates (previously excluded by the empty-results slice), nor that its weighted_score is a float rather than None in the run.json task_results[] row. Both are the visible, contract-level consequences of the change and both currently pass if the grading is deleted. (trigger: tests/test_timeout_orchestrator.py)

Downstream consumers:

• 🟠 Every consumer that computes rates over ROWS THAT PRODUCED CRITERION RESULTS changes silently: reports.py::_attach_row_accounting derives completion_rate from rows_aggregated / rows_total and a killed row used to be dropped (empty success_criteria_results), so completion_rate now rises and rows_excluded falls; classification suites (recall.yes / precision.yes in plugins/coder-eval/reference/templates/activation.yaml, accuracy / macro_f1 in tasks/sentiment_classification.yaml) now count a mid-run-killed row as a graded negative instead of excluding it, so suite_thresholds gates move in BOTH directions; and reports_stats.py:327 appends the row's weighted_score per replicate, so paired A/B comparisons step for identical agent behaviour. No threshold recalibration, no note in docs/REPORT_SCHEMA.md, no test. (trigger: src/coder_eval/orchestrator.py)
• 🟡 task_timeout no longer bounds a task's wall clock: _QUIESCE_TIMEOUT_SECONDS (15s) + _GRADE_AFTER_FORCED_KILL_TIMEOUT_SECONDS (60s) now run AFTER the cap fires, plus post-run commands, so a timed-out task can consume task_timeout + 75s. docs/TASK_DEFINITION_GUIDE.md:242/257 still calls it the "wall-clock cap across all iterations", and duration_seconds, per-task CI timeout-minutes budgets and any wall-clock capacity planning were sized on the old bound. (trigger: src/coder_eval/orchestrator.py)

Display & mapping dicts:

• 🟡 No rendering surface gained an entry for the new orthogonal marker — grep -c forced_kill returns 0 in reports.py, reports_html.py and reports_junit.py: _runtime_notes_lines has branches for max_turns_exhausted and stopped_early but none for forced_kill, the HTML header badge list has none, _task_case's prop_specs has none, and STATUS_ICON/STATUS_CATEGORY are keyed on FinalStatus alone — so a hard-killed run renders as a plain green SUCCESS in run.md, task.html and the JUnit XML that CI ingests. (trigger: src/coder_eval/models/results.py) (restates: Axis 7: forced_kill reaches run.json/telemetry but no human-facing report surface)
• 🟡 The hand-mirrored JS half was not updated: evalboard/lib/runs.ts:1996's task.json projection type reads final_status / error_message but neither forced_kill nor error_log_tail, so the flag cannot reach the UI at all, and lib/status.ts / lib/pills.tsx map the upgraded status to the ordinary green "succeeded" pill. Since the SUCCESS upgrade clears error_message, the evalboard shows a forced-kill run as indistinguishable from a clean one — make evalboard-verify is a separate gate and nothing there fails. (trigger: src/coder_eval/reports_experiment.py)
• 🔵 The plugin analyze skill's compact per-task jq projection (plugins/coder-eval/skills/analyze/SKILL.md:53) enumerates final_status, weighted_score, max_turns_exhausted … but not forced_kill, so the agent-facing run-analysis surface for runs with >20 tasks cannot tell a hard-killed SUCCESS from a clean one — the same omission as the two REPORT_SCHEMA.md field tables, one surface over. (trigger: src/coder_eval/models/results.py)

Daily/nightly:

• 🟠 BOTH gates of the published GitHub Action change verdict for timeout rows and neither action.yml nor docs/CI_GATE.md was touched: (a) cli/run_command.py:543 exits non-zero on tasks_failed, and TIMEOUT is category failed, so a task that blows task_timeout but passes its criteria now lands in tasks_succeeded and the gate goes GREEN where it was red; (b) the minimum-task-score floor (action.yml:190) explicitly skips rows whose weighted_score is None ("errored rows … coder-eval's own exit code already accounts for") — a killed row used to be exactly that, and now carries a float, so it both enters the floor comparison and can satisfy it. The PR says nothing about what happens to a CI gate or a nightly pass rate. (trigger: src/coder_eval/orchestrator.py)
• 🟠 New unaccounted spend on the production timeout path: the post-kill grade runs the FULL criteria set, so every timed-out row now additionally pays for its llm_judge and agent_judge criteria (the latter spawning an SDK sub-agent over a copytree of the sandbox with evaluator credentials) inside a 60s budget — and an agent_judge routinely exceeds 60s, in which case _await_pending_grade waits for it and DISCARDS the verdict, i.e. pays in full for nothing. Neither the cost delta per timed-out nightly row nor the discard is stated anywhere. (trigger: src/coder_eval/orchestrator.py)
• 🟡 The CI smoke-fail expectation became criterion-driven and only the task file records it: EXPECTED_SMOKE_FAIL_FAILED: "3" (.github/workflows/pr-checks.yml:469) now holds solely because should_never_be_checked.txt can never exist — the PR rewrote the task's comment but left the workflow's rationale ("regression detection for the orchestrator's task_timeout watchdog", pr-checks.yml:464-465) unrevised, and nothing in-repo pins the coupling, so making that criterion satisfiable (or the rename Axis 7 recommends going further) drops the bucket to 2 and reds the smoke job for a reason the comment does not explain. (trigger: tasks/smoke_task_timeout.yaml)
• 🟡 A new dimension ForcedKill was added to the production CoderEval.Task.End event (orchestrator.py:285) with no consumer statement: the surfaces table that documents the sibling EarlyStopped / EarlyStopReason dims (docs/TASK_DEFINITION_GUIDE.md:488) got no row, and the out-of-tree App Insights dashboards-as-code that read these customEvents are not mentioned — so the dim ships with no chart, no doc, and no note on whether the external coder-eval-uipath pipeline needs a schema bump for the new forced_kill key in task_results[]. (trigger: src/coder_eval/orchestrator.py)

Harness & Lint Improvements

13 static-check proposals (CE022 ratchet; new CE055–CE063; a C90 ruff ceiling; a CodeQL PR gate) and 7 harness improvements (diff-coverage gate, forced-kill end-to-end artifact test, scoring-determinism + orphan-reap runtime guards, skip-visibility gate, a behavioural ComputedClaim, constant-perturbation guard) — full text in tmp/code-review-260817-0116/00-summary.md § Harness & Lint Improvements.

Top 5 Priority Actions

1. Reap the agent's process group on the forced-kill path (spawn with start_new_session=True and os.killpg in claude_code_agent.py:1343's _kill_transport; zero killpg/setsid hits exist anywhere in src/) so check_all_async can no longer grade a tree a surviving Bash child is still writing, and correct the RESIDUAL comment at orchestrator.py:778 that wrongly scopes this to Antigravity — this is the one finding that can flip a previously deterministic TIMEOUT row to SUCCESS for identical agent output.
2. Stop the 0.8×turn_timeout poll deadline from becoming the effective turn ceiling: at agents/antigravity_agent.py:795 (and the in-loop break at :730) a ≥30s quiet step_fetch_timed_out with no ACTIVE tool call raises TurnTimeoutError — max_retries=0, forced_kill=True — forfeiting the last 20% of the configured budget that on origin/main completed cleanly, so either exclude the no-orphan case from the tighter deadline or re-anchor it at poll-loop entry.
3. Make the forced-kill teardown cancellation-safe and correctly ordered: wrap antigravity_agent.py:901-908's close as try: await stack.aclose() finally: self._exit_stack = None (or bound the unbounded await conversation.cancel() at :872 instead of the whole kill() at orchestrator.py:798) so an expired 15s quiesce cannot abandon the localharness reap with no retry, hoist await self._await_pending_grade() (orchestrator.py:637) above _run_post_run_commands() so post-run shell commands stop mutating the tree a live criterion is reading, and have _await_pending_grade (orchestrator.py:915) record its swallowed CancelledError into teardown_interrupt instead of dropping it.
4. Cover the new forced-kill decision points that currently pass when deleted: the 822->825 False edge at orchestrator.py:820-825 (shortcut taken, gate fails, status stays fallback_status) and _log_graded_after_forced_kill's except ValueError at orchestrator.py:931-933, plus assertions for the three untested forced_kill consumers (reports_experiment.py:203, orchestrator.py:285, the error_log_tail disjunct at orchestrator.py:648) and a model_fields parity guard over the two hand-maintained dict projections; also collapse the four hand-written _graded_iteration_count writes (862/1803/1862/1982) to one, since a forgotten fifth write silently double-spends every llm_judge/agent_judge criterion.
5. Fix the prose that now contradicts the code: docs/REPORT_SCHEMA.md's inserted forced_kill paragraph splits the FinalStatus table and orphans the ERROR/BUILD_FAILED rows (move it below line 304), the field is missing from both enumerations (lines 82 and 127) and its "blew its budget" gloss is wrong (it means a structural timeout only — CLAUDE.md:147 repeats it), docs/agents/HARNESS_PARITY.md:94-104 documents a 120-poll-cycle bound this PR deleted (mirrored by stale comments at antigravity_agent.py:753/:801 and tests/test_antigravity_agent.py:971), reports_experiment.py:60-62 disclaims the final_status is TIMEOUT disjunct it keeps at line 67, and tests/test_custom_lint.py:391-393 names a third CE022 target (run) that does not exist.

---

Stats: 0 🔴 · 3 🟠 · 11 🟡 · 8 🔵 across 8 axes reviewed.

---

(GitHub caps a comment at 65,536 characters; this review renders to 76,331. The two sections below were replaced with pointers so the Blockers, the full pre-merge list, What's Missing and the Top 5 could be posted intact. Nothing was edited — the omitted text is verbatim in the report files.)