Skip to content

feat(early-stop)!: per-criterion arming via stop_early blocks on live criteria - #78

Merged
akshaylive merged 5 commits into
mainfrom
akshaya/earlystop-per-criterion-arming
Aug 4, 2026
Merged

feat(early-stop)!: per-criterion arming via stop_early blocks on live criteria#78
akshaylive merged 5 commits into
mainfrom
akshaya/earlystop-per-criterion-arming

Conversation

@akshaylive

@akshaylive akshaylive commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

Replaces the run-level early-stop master arm with per-criterion arming: a stop_early: block (StopEarlyPolicy) on a live-observable criterion (skill_triggered, command_executed) alone activates the EarlyStopWatcher — there is no run-level master switch anymore.

  • Arming is the block's presence. stop_early: {} arms with the implicit fail trigger; on_pass: stop opts into pass-stops; decide_within: N latches an effective fail (decision_budget_exceeded) after N tool-call steps, fed through the ordinary weighted gate (not a force-fail).
  • run_limits.stop_early is demoted to a kill switch: false force-disarms every block (the one-line e2e / authoritative-P/R/F1 override for experiment variants); true (the removed master arm) is a hard EarlyStopConfigError at resolution (plan exits 1).
  • Weighted stop rules: fail-stop fires when the armed set's ceiling can no longer reach stop_early_gate_threshold (default 1.0 = strict AND); pass-stop fires when the on_pass: stop subset's floor already meets it. Both stops defer while a pass-capable armed criterion outside the deciding subset is undecided — recall is never truncated, and mixed arming (on_pass: stop + a sibling's decide_within) composes safely.
  • Fired-only gating: a run the watcher actually cut gates on the weighted armed subset; a naturally completing run — armed or not — gates strict-AND over the full criteria set. Adding a block never changes the verdict of a run it didn't cut.
  • Verdicts latch on resolved rounds; polarity-inert arming supports dataset fan-out (one YAML line serves positive and distractor rows); a raising live_verdict fails open to a full run.

Hardening (from two structured review passes)

  • Symmetric pass-stop deferral (fixes a silent FAILURE-truncation of would-be passes under mixed arming)
  • allow_inf_nan=False on criterion weight (.inf no longer NaN-poisons the weighted math)
  • Single-sourced DEFAULT_STOP_EARLY_GATE_THRESHOLD; shared early_stop_gate_note() for md+html renderers
  • on_event fully behind the fail-open disarm wrapper; _prev_verdicts latched on resolved rounds only
  • Explicit ValueError instead of a -O-stripped narrowing assert; clearer unregistered-agent guardrail message

Breaking changes

  • run_limits.stop_early: true is now a hard resolution error (use per-criterion stop_early: blocks)
  • Criterion key renamed early_stop:stop_early:; CriterionEarlyStopStopEarlyPolicy
  • decision_budget_exceeded is now an ordinary weighted fail, not a force-FAILURE

Test plan

  • make verify green: 3850 passed, 3 skipped, coverage 91.34%
  • 218 early-stop tests incl. real Orchestrator._setup activation seam, in-flight decide_within expiry, mixed-arming pass-stop deferral, load_experiment over experiments/early-stop-ab.yaml, fail-closed gate pins
  • coder-eval plan resolves all fixtures + the 2-variant early-stop-ab experiment; stop_early: true fails plan with exit 1

Breaking-change blast radius (run_limits.stop_early: true)

run_limits.stop_early: true — previously the ONLY documented arming lever, and -D-reachable — now raises EarlyStopConfigError at resolution (aborts run, flips plan's exit code) with no deprecation window. Verified no in-repo config sets it (grep -rn stop_early .github/ experiments/ tasks/ shows no true). External callers (anything using the composite action's extra-args with -D run_limits.stop_early=true, downstream experiment YAMLs, workflows pinned to the moving v<major> tag) should grep for the key stop_early: true / -D run_limits.stop_early=true and migrate to per-criterion stop_early: blocks. No shipped nightly suite is armed today, so no pass-rate trend step is expected from the fired-only gating flip.

🤖 Generated with Claude Code

@akshaylive
akshaylive force-pushed the akshaya/earlystop-per-criterion-arming branch from 067dba1 to 73b8240 Compare August 4, 2026 17:57
@UiPath UiPath deleted a comment from github-actions Bot Aug 4, 2026

@uipreliga uipreliga left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review: coder_eval — pr:78 (25 files) all 8 axes

Scope: pr:78 (25 files) all 8 axes · branch akshaya/earlystop-per-criterion-arming · 067dba1 · 2026-08-04T17:26Z · workflow variant

Change class: complex — replaces the run-level early-stop master arm with per-criterion stop_early: blocks, rewriting the arming/gating control flow (weighted ceiling/floor stop rules, deferral, fired-only gating) plus breaking schema renames; correctness requires reasoning about partial-trajectory verdict semantics

Architecture, security, API surface and error handling are excellent (three axes at 10/10, zero criticals), but the real risk is concentrated in one confirmed harness-correctness defect where a decide_within fail-stop can gate a task SUCCESS that pre-PR reported FAILURE for byte-identical agent output, compounded by YAML-order-dependent early-stop reason attribution and a structurally broken CLAUDE.md that silently lost four unrelated sections — so the bottom line is that this is a strong 9.4/10 change that should not ship until the gating invariant and the doc regression are fixed, with the remaining style/test-hygiene items safe as follow-ups.

Summary

Axis Score 🔴 🟠 🟡 🔵 Top Issue
1. Code Quality & Style 7.4 / 10 0 1 3 1 PR collaterally deletes four unrelated CLAUDE.md sections (~67 lines: Success Criteria table, Evaluation Flow, Development Commands/lint-loop, ## Configuration heading)
2. Type Safety 9.3 / 10 0 0 1 2 # type: ignore[call-arg] masks a nonexistent kwarg name, making test_guardrail3_unobservable_criterion_unrepresentable (tests/test_early_stop.py:814-824) vacuous
3. Test Health 9.5 / 10 0 0 1 0 validate_early_stop's absent/unregistered-agent guard is uncovered, and its only reachable input (no agent block) yields a misleading diagnosis
4. Security 10 / 10 0 0 0 0
5. Architecture & Design 10 / 10 0 0 0 0
6. Error Handling & Resilience 9.9 / 10 0 0 0 1 Cooperative-stop capability probed via stringly-typed getattr instead of the declared ClassVar on the Agent ABC
7. API Surface & Maintainability 10 / 10 0 0 0 0
8. Evaluation Harness Quality 8.9 / 10 0 1 0 1 A decide_within-driven fail-stop is not verdict-preserving: it can truncate a run whose armed criterion would score 1.0 (gating SUCCESS where main FAILED), and the docs still claim otherwise in 3 places

Overall Score: 9.4 / 10 · Weakest Axis: Code Quality & Style at 7.4 / 10
Totals: 🔴 0 · 🟠 2 · 🟡 5 · 🔵 5 across 8 axes.

Blockers

  1. [Axis 1] PR collaterally deletes four unrelated CLAUDE.md sections (~67 lines: Success Criteria table, Evaluation Flow, Development Commands/lint-loop, ## Configuration heading) (CLAUDE.md:146) — git diff origin/main...pr-78 -- CLAUDE.md shows the two rewritten early-stop bullets replacing a 75-line block that also contained four sections unrelated to early stop: ## Success Criteria (14 types) (the 14-row criteria table + the weight/pass_threshold/suite_thresholds paragraph), ## Evaluation Flow, ## Development Commands (the "MANDATORY: Run after every implementation phase" make block, the "could a custom lint rule have prevented this?" standing rule, the CE030 doc-parity note, the Docs-index-SSOT note, and the anchor-slugger convention), and the ## Configuration heading. Only the early-stop and run-time-caps bullets needed to change. The result is structurally broken: PR CLAUDE.md:146 is now - **ruff**: line-length=120, target py313, select E/F/I/N/W/UP/B/SIM/RUF sitting directly after the early-stop bullet inside ## Key Architectural Patterns (heading at line 128), followed by - **pyright**… / - **pytest**… / - **Coverage threshold**: 80% (147-149) and then ## Extension Points (151) — the ruff/pyright/pytest/coverage settings now read as architectural patterns and the file no longer documents make verify at all. Compare git show origin/main:CLAUDE.md | grep -n '^#\+ ' (17 headings) with the PR's (12). uv run pytest tests/test_custom_lint.py -m lint is 158 passed, so no lint rule guards this. Fix: restore the four sections verbatim and keep only the two intended bullet rewrites.
  2. [Axis 8] A decide_within-driven fail-stop is not verdict-preserving: it can truncate a run whose armed criterion would score 1.0 (gating SUCCESS where main FAILED), and the docs still claim otherwise in 3 places (src/coder_eval/orchestrator.py:1586) — The new single-gate branch justifies itself with a claim the watcher cannot honor: # decision-budget stop is just a fail-stop whose deciding criterion / # timed out (the watcher only fires once the weighted ceiling / # proves the armed gate cannot pass), and the frozen-trajectory / # scores agree by construction (orchestrator.py:1582-1587). For a native live-fail that holds (monotone checkers). For a decide_within timeout it does not: early_stop.py:536-538 manufactures the fail out of undecidedbudget_expired = verdict == "undecided" and budget is not None and tool_call_index >= budget / if budget_expired: verdict = "fail" — and _ceiling then pins that criterion at 0, so the ceiling is NOT an upper bound on the authoritative score. The two trajectories genuinely differ: the watcher drops unresolved tool ends (early_stop.py:437-438: if event.status == ToolEndStatus.UNRESOLVED: return, docstring 423-424 "keeps a crashed attempt's orphan tools out of the retry-persistent partial trajectory") and deliberately does not latch an in-flight round's verdict (early_stop.py:539: if in_flight is None and verdict != "undecided":), while check_all_async(turn_records=self.result.iterations) scores over the drained crashed-attempt partials, which DO contain those force-closed commands (EventCollector.on_event: elif isinstance(event, ToolEndEvent): self._commands[event.tool.tool_id] = event.tool, no status filter). Failure scenario on the shipped fixture tasks/early_stop_decision_budget_exceeded.yaml: attempt 1 dispatches python app.py, then the turn crashes/times out before the tool result arrives; the partial turn is drained into result.iterations, but the watcher's collector never recorded it and its tool_call_index keeps counting (documented as cumulative across retries). On attempt 2 the criterion reads undecided, tool_call_index >= 3 latches the synthetic fail, _ceiling == 0.0 < 1.0 fires reason decision_budget_exceeded — yet armed_criteria_passed scores the same criterion 1.0 from the drained command, the armed gate passes, and the advisory file_exists: app.py criterion is excluded from the gate, so the task reports SUCCESS even with no app.py. Pre-PR the identical output was forced to FAILURE. Fix: either treat a _budget_expired stop as gate-relevant only after re-checking the authoritative score (i.e. do not claim guaranteed failure), or make the watcher's trajectory the same one the checker will score (record unresolved tool ends in the watcher's collector, as the agent's collector already does). At minimum delete the false "agree by construction" claim and add a test where the timed-out criterion's authoritative score is 1.0 and an unarmed criterion is 0.0 — tests/test_early_stop.py:2330 test_decision_budget_exceeded_gates_through_armed_gate scores every criterion 1.0, so it never exercises the advisory-demotion consequence.

Non-blocking, but please consider before merge

  1. [Axis 1] EarlyStopWatcher.init grows to 7-8 index-aligned parallel lists (radon A(3) -> C(14)), with derivable/projected members (src/coder_eval/orchestration/early_stop.py:296) — Seven lists are keyed positionally to self._armed: _decidable (296), _pass_trigger (308), _fail_trigger (314), _budget (318), _latched (336), _budget_expired (337), _prev_verdicts (342). git show origin/main:src/coder_eval/orchestration/early_stop.py has only two (_armed_polarities, _prev_verdicts), and uv run radon cc -s measures EarlyStopWatcher.__init__ at C (14) on the PR vs A (3) on main — the coupling is what this PR adds. They are re-zipped strict=True at lines 309, 319, 321, 473, 583, 642 and indexed by i at 519, 536, 539-541, 556-564, 604, 651. Collapse them into one small _ArmedState dataclass per armed criterion (criterion, checker, decidable, pass_trigger, budget, latched, budget_expired, prev_verdict) and iterate for st in self._armed: — the positional-list-vs-list coupling this file now has is the exact shape the repo's own review rubric calls out as a candidate CEnnn lint rule. Also drop _fail_trigger outright: line 314 is self._fail_trigger: list[bool] = ["fail" in pol for pol in self._decidable] — a derived list carrying zero information beyond _decidable, read at exactly one site (line 604 self._fail_trigger[i]), which can read "fail" in self._decidable[i] directly.
  2. [Axis 1] _collect_verdicts discards the exact budget_expired fact it just computed, forcing _budget_drove (early_stop.py:545-565) to re-derive it heuristically with an always-True final conjunct (src/coder_eval/orchestration/early_stop.py:545) — This PR reclassified DECISION_BUDGET_EXCEEDED into an ordinary weighted fail — results.py:418-428 now says "every reason gates identically through armed_criteria_passed's weighted gate. DECISION_BUDGET_EXCEEDED is a reporting label only" — yet the machinery to derive it grew: a persistent parallel latch (_budget_expired, line 337, written at 541) plus a 21-line second derivation _budget_drove (545-565) that re-computes the same fact for the not-yet-latched in-flight round, consulted twice per fail-stop (604, 611). Its final condition, and "fail" not in self._decidable[index] (line 564), is unreachable-as-a-discriminator today: _budget[i] is non-None only when "pass" in pol (line 318-320), and both in-tree implementations of live_decidable_polarities() return a set holding at most one polarity (criteria.py:800-806 returns frozenset({"pass"}) or frozenset({"fail"}); criteria.py:697-703 adds "pass" only when max_count is None and "fail" only when it is not) — so "pass" in pol already implies "fail" not in pol and the term is always True. It is speculative generality, and in the hypothetical it anticipates (a criterion decidable on both polarities) it is also wrong: on an in-flight round _budget_expired[i] is still False, so a budget-driven fail would be mislabeled CRITERION_FAILED. Since the label is the only remaining consumer, record it once where the conversion happens (line 537-538) — e.g. latch "fail_budget" as the verdict state, or have _collect_verdicts return (verdict, budget_driven) pairs — and delete _budget_drove.
  3. [Axis 1] Early-stop tests still driven through removed API vocabulary (max_steps_to_decide, stop_on_fail), with stop_on_fail=True a provable no-op producing duplicate tests (tests/test_early_stop.py:170) — _block(*, stop_on_pass=False, stop_on_fail=False, max_steps_to_decide=None) (lines 170-182) translates the removed pre-PR vocabulary into the new schema, and _skill_crit/_cmd_crit (185-229) forward it, so ~200 call sites in the suite that documents stop_early: never show a StopEarlyPolicy literal. Two concrete costs. (a) stop_on_fail cannot affect the built object — the body only maps on_pass="stop" if stop_on_pass else "continue" and decide_within=max_steps_to_decide; it participates solely in the if not stop_on_pass and not stop_on_fail and max_steps_to_decide is None: return None armed/unarmed guard. grep -c "stop_on_fail=True" = 48, of which grep -n "stop_on_fail=True" | grep -E "stop_on_pass=True|max_steps_to_decide=" = 32 also set another flag (so the kwarg is inert), and 29 are the literal pair stop_on_pass=True, stop_on_fail=True, byte-identical to stop_on_pass=True alone — which reads as if a separate fail arm still existed, the exact belief this PR removed. (b) max_steps_to_decide is the deleted field name, still in 40+ places including test names like test_max_steps_to_decide_inert_on_fail_only_criterion (line 706) that assert on decide_within. Rename the kwargs to the shipped keys (on_pass, decide_within) or pass StopEarlyPolicy(...) / stop_early={} directly, and drop stop_on_fail in favour of an explicit armed=True.
  4. [Axis 2] # type: ignore[call-arg] masks a nonexistent kwarg name, making test_guardrail3_unobservable_criterion_unrepresentable (tests/test_early_stop.py:814-824) vacuous (tests/test_early_stop.py:823) — The test's stated contract (lines 814-817: 'an armed unobservable criterion cannot even be constructed (extra=forbid)') is not what it asserts. It passes stop_on_pass=True, # type: ignore[call-arg] (line 823) — but stop_on_pass is only a test-helper kwarg name (_block(*, stop_on_pass=...), line 171); grep -rn "stop_on_pass\|stop_on_fail" src returns zero hits, so it has never been a model field. Verified at PR HEAD that the assertion cannot distinguish the intended case from any typo: FileExistsCriterion(..., stop_on_pass=True) -> stop_on_pass Extra inputs are not permitted [extra_forbidden] and FileExistsCriterion(..., totally_bogus_kwarg=True) -> totally_bogus_kwarg Extra inputs are not permitted [extra_forbidden] — identical. The # type: ignore is what suppresses the one signal that would have surfaced the wrong kwarg, and because pyright's exclude lists tests (pyproject.toml:216) nothing else does. Change line 823 to stop_early=StopEarlyPolicy(), (the form test_block_unrepresentable_on_unobservable_criterion at line 390 already uses correctly) and assert the error message mentions stop_early. Note also that all six # type: ignore[...] comments added by this diff use mypy-style error codes while the project checks with pyright and excludes tests/, so they are decorative.
  5. [Axis 3] validate_early_stop's absent/unregistered-agent guard is uncovered, and its only reachable input (no agent block) yields a misleading diagnosis (src/coder_eval/orchestration/early_stop.py:231) — Line 231 is the only statement in the 673-line rewrite that no test reaches (confirmed: uv run pytest tests/test_early_stop.py --cov=coder_eval.orchestration.early_stop --cov-branch167 2 64 5 96.97% 231, 306, 436->443, 660->exit, 665->667). The uncovered guard is:
228:    if registration is None:
229:        # Not the same failure as an agent that opted out of cooperative stop:
230:        # an unregistered type usually means a plugin is not installed/loaded.
231:        raise EarlyStopConfigError(
232:            f"criterion-level stop_early arming requires a registered agent type; {agent_type!r} is "

TestValidateEarlyStop (tests/test_early_stop.py:649) pins the other three guards by name and message — test_master_arm_true_rejected (:655, "has been removed"), test_guardrail5_simulation_rejected (:785, "simulation"), test_guardrail1_non_supporting_agent_rejected (:791, "cooperative stopping"), test_gate_threshold_zero_rejected (:688, "must be > 0.0") — and even pins the raise ORDER (test_raise_order_simulation_before_agent, :826). Guard (3)'s first half is the gap. Add two cases: (a) an armed task with agent_type="no-such-agent" asserting EarlyStopConfigError matching "not registered"; (b) task.agent = Noneagent: ResolvedAgentConfig | None (src/coder_eval/models/tasks.py:343) makes this reachable, and it currently produces "...requires a registered agent type; None is not registered (is the providing plugin installed and loaded?)", which a test would either pin or expose as misleading for an agentless task.

Nits

  1. [Axis 1] Gratuitous function-local import of early_stop_gate_note in reports_html._render_header with no import cycle to avoid (src/coder_eval/reports_html.py:348) — Line 348 is from .reports import early_stop_gate_note inside _render_header, with no comment naming a cycle. There is none: grep -n '^from \.' src/coder_eval/reports.py shows reports.py imports only .models (11) and .path_utils (22), and grep -rn reports_html src/coder_eval/ shows nothing in reports.py; python -c "import coder_eval.reports_html, coder_eval.reports" succeeds. Move it to the module-level import block beside from coder_eval.models import FinalStatus, … (line 20) — the repo's convention is that a function-local import means a real cycle (compare reports_stats.py:3-4, which documents its cycle-avoidance reason), so an undocumented one misleads the next reader into thinking reportsreports_html is circular.
  2. [Axis 2] EarlyStopInfo.gate_threshold keeps a hardcoded default=1.0 although this PR introduces DEFAULT_STOP_EARLY_GATE_THRESHOLD and results.py already imports it (src/coder_eval/models/results.py:479) — This PR adds DEFAULT_STOP_EARLY_GATE_THRESHOLD: Final[float] = 1.0 (models/limits.py:9) whose docstring says it exists 'so the field default below, the watcher's for_task fallback, and the orchestrator's finalize fallback can never drift apart', and results.py imports it at line 24 and uses it at line 680 (def armed_criteria_passed(self, criteria, gate_threshold: float = DEFAULT_STOP_EARLY_GATE_THRESHOLD)). But the persisted-record default four lines away still reads gate_threshold: float = Field( / default=1.0, (results.py:478-479) — the one remaining literal, on the field that makes task.json self-describing. Change line 479 to default=DEFAULT_STOP_EARLY_GATE_THRESHOLD, so a future change to the constant cannot leave persisted records describing a threshold the code no longer uses.
  3. [Axis 2] Two new test helpers in the 1125-line early-stop test module carry no annotations, in a file pyright does not check (tests/test_early_stop.py:921) — def _resolve_surface(task_file: Path, tmp_path: Path, *, overrides: dict[str, Any] | None = None): (line 921) has no return annotation even though it returns resolve_all_tasks(...)'s two-tuple and 8+ tests destructure it as resolved, skipped = _resolve_surface(...); and def counting(self_, criterion, records): (line 1984) — the replacement installed onto type(checker).live_verdict at line 1988 — has no parameter or return annotations at all, so it silently need not match BaseCriterion.live_verdict(self, criterion: C, turn_records: list[TurnRecord]) -> LiveVerdict. These are the only 2 of ~300 defs in the module missing annotations (grep -c "^\s*\(async \)\?def .*):\s*$" tests/test_early_stop.py -> 2), and because pyproject.toml:216 excludes tests from pyright nothing will ever flag them. Annotate both (-> tuple[list[ResolvedTask], list[SkippedTask]], and (self_: SkillTriggeredChecker, criterion: SkillTriggeredCriterion, records: list[TurnRecord]) -> LiveVerdict) so the monkeypatch stays honest against the real signature.
  4. [Axis 6] Cooperative-stop capability probed via stringly-typed getattr instead of the declared ClassVar on the Agent ABC (src/coder_eval/orchestration/early_stop.py:236) — early_stop.py:236 reads the capability by name with a silent default:
if not bool(getattr(registration.agent_class, "supports_cooperative_stop", False)):

registration.agent_class is statically typed type[Agent[Any]] (agents/registry.py:32) and Agent.supports_cooperative_stop: ClassVar[bool] = False is declared on the ABC (agent.py:80), with True on all three built-ins (claude_code_agent.py:657, codex_agent.py:640, antigravity_agent.py:190). So the getattr(..., default) + bool() buys nothing and defeats pyright: renaming the ClassVar would leave this reading False and every armed task would hard-fail with the actively misleading message "agent type 'claude-code' does not [support cooperative stopping]". Replace with the direct attribute access if not registration.agent_class.supports_cooperative_stop:. The same error string also hardcodes "(claude-code, codex, antigravity)", which will drift from the registry as soon as a plugin agent supports the seam — derive it from AgentRegistry instead.
5. [Axis 8] EarlyStopReason attribution between criterion_failed and decision_budget_exceeded is now YAML criterion-order dependent (was deterministic precedence on main) (src/coder_eval/orchestration/early_stop.py:600) — The fail-stop now picks its deciding criterion with a single order-scan — candidate_index = next((i for i, v in enumerate(verdicts) if v == "fail" and (self._fail_trigger[i] or self._budget_drove(i, verdicts, tool_call_index))), None) (early_stop.py:600-607) — and derives the reason from that same index (early_stop.py:609-613). On main the budget check was a separate loop that ran after the native fail-stop, so a native live-fail always won regardless of ordering. Failure scenario: a row with a latched distractor misfire (native fail) and a criterion whose decide_within expired on the same round reports criterion_failed if the distractor is listed first in success_criteria and decision_budget_exceeded if it is listed second — for identical agent behaviour. That value is persisted (EarlyStopInfo.reason), rendered (reports.py:456), and emitted as the EarlyStopReason telemetry dimension (orchestrator.py:264), so reordering criteria in a YAML shifts dashboard counts. No verdict impact (both reasons gate identically). Fix: restore explicit precedence — prefer a native live-fail candidate over a budget-driven one — and add a test with both orderings asserting the same reason.

What's Missing

Daily/nightly:

  • 🟠 🟠 Pipeline blast radius of the hard error is unstated: run_limits.stop_early: true was the ONLY documented arming lever until this PR and is -D-reachable, yet it now raises EarlyStopConfigError at resolution (aborts run, flips plan's exit code) with no deprecation window. action.yml's extra-args input explicitly advertises -D overrides (action.yml:30) and release.yml maintains a moving v<major> tag, so any external caller (the coder-eval-uipath / eval-runner nightly, downstream experiment YAMLs, a workflow pinned to the moving tag) fails hard at resolution rather than degrading. Verified no in-repo config sets it (grep -rn stop_early .github/ experiments/ tasks/ is clean of true) — the PR should say exactly that, plus name the external key to grep for. (trigger: src/coder_eval/models/limits.py)
  • 🟡 🟡 The fired-only gating flip changes final_status for identical agent output and its trend impact is unstated: an armed run that completes naturally now gates strict-AND over the full criteria set (orchestrator.py:1600-1603), where origin/main gated the weighted armed subset for any stop_early-armed task whether or not the watcher fired. Pass rates for any armed suite therefore step discontinuously in the evalboard / App Insights series, and nothing in run.json versions or annotates the semantic change (gate_threshold is null on exactly those runs, indistinguishable from an unarmed run). The PR body should state the expected direction of the shift for the nightly, or note that no shipped nightly suite is armed today. (trigger: src/coder_eval/orchestrator.py)

Tests:

  • 🟠 🟠 No test covers a pass-stop truncating a fail-only-decidable armed sibling, and the rewritten deferral rule does not protect it. _evaluate_impl's outside_pass_capable_undecided (early_stop.py:641-645) defers only on siblings whose live_decidable_polarities() contains "pass", but a command_executed with min_count>=1 AND max_count set is decidable {"fail"} only (criteria.py:677-702) while its authoritative score still requires the command to run. Reproduced against the PR tree: A=min_count:1,max_count:None,on_pass:stop, B=min_count:1,max_count:3,stop_early:{}, both weight 1.0 — one python app.py ToolEnd fires criterion_passed at tool call 1, and armed_criteria_passed([A,B],1.0) on the frozen trajectory returns False, i.e. an unearned FAILURE on a run a full trajectory would have passed. This is exactly the outcome the deferral comment (early_stop.py:636-640: "an unearned fail on the armed gate that a full run would not have produced") claims to prevent. Pre-existing in shape, but the predicate was rewritten here; either defer on "not yet satisfied" rather than "pass-decidable", or pin the gap with a test. (trigger: src/coder_eval/orchestration/early_stop.py) (restates: Axis 8: decide_within-driven fail-stop is not verdict-preserving)
  • 🔵 🔵 The new shared early_stop_gate_note is exercised through the markdown renderer for both branches (test_early_stop.py:2624/2631) but through the HTML renderer only for criterion_passed (test_html_header_shows_early_stop_badge, :2649). The decision_budget_exceeded tooltip — and the newly added _esc(title) escaping at reports_html.py:352 — has no assertion; one extra line in that test would pin both. (trigger: src/coder_eval/reports_html.py)

Parallel paths:

  • 🟡 🟡 The two lint registries that guard every sibling task-YAML config model were not extended for the new user-authored StopEarlyPolicy: tests/lint/doc_schema_parity.py::DOCUMENTED_MODELS (CE030 — lists TaskDefinition/RunLimits/Dataset/SimulationConfig) and tests/lint/dead_config_fields.py::CONSUMED_MODELS (CE031 — SimulationConfig/RunLimits/Dataset). Both would pass today (on_pass and decide_within are documented as inline code in docs/TASK_DEFINITION_GUIDE.md and read by name at early_stop.py:308/318), so this is a free 2-line addition that stops the next stop_early: key from shipping undocumented or dead. (trigger: src/coder_eval/models/criteria.py)
  • 🟡 🟡 validate_early_stop deletes all five instance-level dead-arm guards that origin/main raised (unobservable type, undecidable requested polarity, empty auto, budget on a fail-only instance, "armed but no criterion sets stop_when") in favour of "inert by design", but no user-visible replacement surface was added. The only signal that an armed criterion can never fire is logger.debug("all armed stop triggers are inert for this row") at early_stop.py:321, and plan — which still calls validate_early_stop (plan_command.py:138) — prints nothing about arming. So stop_early: {decide_within: 5} on a fail-only distractor, or a block on command_executed(min_count: 0, max_count: None) (decides neither polarity), is now silent dead config on a non-fanned task, the class CE031 exists to prevent. Fix: warn (not debug) when the whole armed set is inert, and/or surface armed criteria + live triggers in plan output. (trigger: src/coder_eval/orchestration/early_stop.py)
  • 🔵 🔵 experiments/default.yaml — the baseline that deliberately enumerates every run_limits cap, including commented-out optional ones (max_usd, count_cached_input), so "users know the baseline" — was not extended with commented stop_early (kill switch) / stop_early_gate_threshold entries, even though the kill switch is now the primary run-level lever the A/B recipe depends on. (trigger: experiments/early-stop-ab.yaml)

Downstream consumers:

  • 🟡 🟡 The suite-level classification path was not updated for the new arming model: reports.py::write_suite_rollups and the suite_thresholds gate (which drives the CLI exit code) aggregate per-row criterion results with zero early-stop awareness — no stopped_early row count, no flag on SuiteRollup. A pass-stopped row biases recall/precision, and nothing rejects or warns when the same criterion carries both stop_early: and suite_thresholds:. Under the old design a run-level switch had to be typed to reach this; now a task file alone arms every consumer, so a dataset suite that reuses an armed task silently gates CI on truncated metrics. The caveat currently lives only in prose (early_stop.py:82, docs/AB_EXPERIMENTS.md:302). (trigger: src/coder_eval/models/criteria.py)
  • 🟡 🟡 Arming became reachable from exactly one layer and no override surface was extended to compensate: stop_early: lives on success_criteria, which ExperimentVariant cannot override (its fields are variant_id/description/agent/simulation/repeats/template_sources/prompt_mutations/initial_prompt[_file]/run_limits/driver), and -D only reaches the agent/run_limits/sandbox roots. So after this PR you can DISARM from a variant or the CLI (-D run_limits.stop_early=false) but there is no way to ARM without editing the task YAML — asymmetric with the old -D run_limits.stop_early=true lever, and it means every suite sharing an armed task file inherits early stop plus fired-only gating by default. (trigger: src/coder_eval/models/limits.py)

Display & mapping dicts:

  • 🟡 🟡 The only exhaustiveness guard over EarlyStopReason was deleted and not replaced: assert_never(reason) (and the from typing import assert_never import) is gone from orchestrator.py, and grep -rn assert_never src/coder_eval/ now returns nothing. The single remaining reason-dependent surface is reports.py:169 early_stop_gate_note(reason: str), which special-cases "decision_budget_exceeded" and falls through to a generic sentence for everything else — including run.json's literal "unknown" (reports.py:456). A 4th EarlyStopReason member would therefore render the wrong gate prose in both the markdown note and the HTML badge tooltip with no pyright or lint signal. Cheap fix: type the parameter as EarlyStopReason and dispatch via match + assert_never. (trigger: src/coder_eval/orchestrator.py)
  • 🟡 🟡 No evalboard surface consumes the early-stop keys this PR keeps writing into run.json. eval_result_to_task_dict emits stopped_early / early_stop_reason / turns_remaining_at_stop / gate_threshold (reports_experiment.py:208-216), but evalboard/lib/runs.ts's RawTaskRow reads expected_turns / has_final_reply / visible_turns and has no early-stop field at all — a truncated run is rendered, averaged and trended identically to a full one in the dashboard the nightly is read from. Pre-existing gap, but this PR removes the run-level opt-in that made truncation a deliberate, remembered act, so the misread risk is now much higher; a stopped early pill next to the existing badges would close it. (trigger: src/coder_eval/reports_experiment.py)

Harness & Lint Improvements

Static checks (lint / type):

  • [ce-lint] CE034 — CLAUDE.md structural parity (doc-surface test class in tests/test_custom_lint.py, alongside CE027–CE031; helper in tests/lint/claude_md_parity.py). Three mechanical invariants over the repo-root CLAUDE.md: (a) a checked-in manifest of required ## sections (Project Overview, Directory Structure, Key Architectural Patterns, Success Criteria, Evaluation Flow, Development Commands, Configuration, Extension Points, Task Definition, Dependencies, Design Principles, Notes for AI Assistants) must all be present — deleting one then requires deleting its manifest entry, a visible reviewable act; (b) the ## Success Criteria (N types) heading's N and the table's | \type` |row count must equal the member count of theSuccessCriterionunion inmodels/criteria.py(main has 14 rows / 14 members — verified bygit show origin/main:CLAUDE.md | grep -c '^| `'→ 14); (c) everymake named in the## Development Commandsblock must exist as a Makefile target, and every Makefile target carrying a## help string (help/install/format/check/lint/typecheck/test/test-cov/verify/docs-indexes/...) must appear in CLAUDE.md. Numbering note: implemented ids stop at CE031, but .claude/harness-candidates.mdreserves CE026/CE032/CE033, so claim CE034+ (the id-uniqueness assert intests/lint/runner.pyis the SSOT). _Prevents:_ A1/A5/A7 high —CLAUDE.md:146: 67 lines collaterally deleted (Success Criteria table → caught by (b), Development Commands incl. make verifyand the lint-rule standing rule → caught by (c), Evaluation Flow + the## Configurationheading → caught by (a)).uv run pytest tests/test_custom_lint.py -m lint` is 158 passed on the PR, i.e. nothing guards CLAUDE.md today.
  • [pyright] Type-check tests/ with enableTypeIgnoreComments = false. Remove "tests" from [tool.pyright] exclude (pyproject.toml:216), add it to include, and set enableTypeIgnoreComments = false so mypy-style # type: ignore[...] comments (which pyright silently honors while ignoring the bracketed code) stop suppressing real errors. Measured on the PR worktree with exactly the repo's current baseline settings plus that flag: tests/test_early_stop.py alone yields 26 errors, including verbatim 823: reportCallIssue No parameter named "stop_on_pass", 390: No parameter named "stop_early", 409: No parameter named "on_fail", and 1124: Declaration "should_stop" is obscured by a declaration of the same name — i.e. the check fires precisely on the masked defects. Phase it: land the flag plus a tests/-scoped executionEnvironments entry, burn down the existing errors (or gate new/changed test modules first). Do NOT try ruff ANN for the annotation half — measured 4417 ANN001/ANN201/ANN202 hits across tests/, not adoptable. Prevents: A2 medium — tests/test_early_stop.py:823: # type: ignore[call-arg] masks a nonexistent kwarg, making test_guardrail3_unobservable_criterion_unrepresentable vacuous (it cannot distinguish the intended contract from any typo). Also the 5 decorative mypy-style ignores this diff adds, and the A2-low unannotated-helper/monkeypatch-signature issue at :1984.
  • [ce-lint] CE035 — no index-aligned parallel instance lists (and no sibling-derived instance lists) in one __init__. New BaseRule in tests/lint/rules/ce035_no_parallel_instance_lists.py, wired into ALL_RULES in tests/lint/runner.py, scoped to src/coder_eval/. Two violation classes: (1) a single __init__ assigns ≥4 list-typed self._x attributes and the class shows positional coupling (a zip(self._a, self._b, ..., strict=True) or ≥2 distinct self._x[i] subscripts sharing an index name) — measured repo-wide: fires only on EarlyStopWatcher.__init__ (7 lists; zip(..., strict=True) at 309/319/473/583/642), while _CodexTurnState.__init__ and _AntigravityTurnState.__init__ (4 lists each, no positional coupling) stay clean; (2) an instance list assigned as a comprehension over a sibling attribute assigned in the same __init__ — measured 3 hits repo-wide, all inside that same cluster (_pass_trigger 308, _fail_trigger 314, _budget 318 over _decidable). Message points at the fix: one _ArmedState-style record per element. # noqa: CE035 escape. Note ruff C901 is NOT a substitute — I measured it and mccabe never flags this __init__ (radon's C(14) comes from comprehensions, which mccabe does not count); at max-complexity 12 it produces 27 unrelated hits. Prevents: A1/A5 medium — orchestration/early_stop.py:296: 7 index-aligned parallel lists (radon A(3)→C(14)); and the derived-_fail_trigger half of the A1-medium finding at :545 (_fail_trigger carries zero information beyond _decidable).
  • [ce-lint] CE036 — never probe a declared first-party capability flag by string. New BaseRule (tests/lint/rules/ce036_no_stringly_capability_probe.py): AST-parse src/coder_eval/agent.py for names annotated ClassVar[...] on Agent (today supports_cooperative_stop, supports_cost_log_tags) and flag any getattr(x, "<that name>", default) / hasattr(x, "<that name>") anywhere in src/coder_eval/ — the attribute is statically declared on the ABC, so direct access is both type-checked and rename-safe. Measured: exactly 1 hit repo-wide (orchestration/early_stop.py:236), so it is adoptable as-is with no noqa debt. A blanket getattr-literal ban is not viable (145 legitimate duck-typing hits over SDK objects), which is why the name set is derived from the ABC. Prevents: A2/A6 low — orchestration/early_stop.py:236: bool(getattr(registration.agent_class, "supports_cooperative_stop", False)) defeats pyright; a rename would silently read False and hard-fail every armed task with the misleading "'claude-code' does not support cooperative stopping".
  • [ce-lint] CE037 — a literal Field(default=...) must not duplicate an already-imported DEFAULT_* constant. New BaseRule (tests/lint/rules/ce037_field_default_uses_constant.py): collect module-level DEFAULT_* constants and their literal values across src/coder_eval/ (4 today: DEFAULT_LOG_TAIL_MAX_BYTES, DEFAULT_JUDGE_MODEL, DEFAULT_SIMULATION_STOP_TOKEN, DEFAULT_STOP_EARLY_GATE_THRESHOLD); in any module that imports one of them, flag a Field(default=<literal>) whose value equals that constant's value (same type). Measured repo-wide: exactly 1 hit, models/results.py:478 — zero false positives (the model: str | None = Field(default=None) fields do not collide with DEFAULT_JUDGE_MODEL's string value, which is why value-equality rather than name-tail matching is the right predicate). Prevents: A2 low — models/results.py:479: EarlyStopInfo.gate_threshold keeps default=1.0 although this PR adds DEFAULT_STOP_EARLY_GATE_THRESHOLD (whose own docstring says the field default must not drift from it) and results.py:24 already imports it.
  • [ce-lint] CE038 — agent-kind name literals may not appear in src/coder_eval/ outside agents/ and models/enums.py. New BaseRule (or fold as a second class into CE036): flag string literals equal to a registered agent type ("claude-code", "codex", "antigravity", "none") — including inside f-strings — outside src/coder_eval/agents/**, models/enums.py, and doctest/docstring bodies; the fix is to derive the list from AgentRegistry. This mechanizes the agnostic-core litmus CLAUDE.md already states ("grep -ri <agent-name> src/coder_eval/ outside the agent's own package should be ~zero"). Measured: 1 real hit (orchestration/early_stop.py:239) plus the enum definitions and one docstring example, both exemptible by scope. Prevents: A6 low (second half) — orchestration/early_stop.py:239 hardcodes "(claude-code, codex, antigravity)" in the guard's error message; it silently drifts the moment a plugin agent supports the cooperative-stop seam.
  • [ce-lint] CE039 — a second reducer over the event stream may not apply a ToolEndStatus filter that EventCollector does not. New BaseRule (tests/lint/rules/ce039_no_divergent_tool_end_filter.py): inside an isinstance(event, ToolEndEvent) handler located outside src/coder_eval/streaming/ and src/coder_eval/agents/, flag any comparison against ToolEndStatus.* (or event.status early-return/continue) unless a # noqa: CE039 names why the divergence is safe. Rationale: streaming/collector.py records unresolved/force-closed tool ends as commands, so any other reducer that drops them builds a strictly smaller trajectory than the one check_all_async(turn_records=...) later scores — the exact mechanism behind the Axis-8 high. Measured: 1 hit (orchestration/early_stop.py:437-438). Prevents: A8 high — orchestrator.py:1586 / early_stop.py:437: the watcher's trajectory ⊂ the checker's trajectory, so a decide_within fail-stop is NOT verdict-preserving (armed criterion scores 1.0 on the frozen trajectory → run gated SUCCESS with advisory criteria demoted, where main forced FAILURE).
  • [ce-lint] CE040 — tests must not rebind a method on a class object directly. New BaseRule scoped to tests/: flag assignment to an ast.Attribute whose value is a type(...) call (or a bare class reference) — e.g. type(checker).live_verdict = counting — and require monkeypatch.setattr(...) (auto-restoring, raising=True validates the target exists) or mock.patch.object(..., autospec=True) (validates the replacement's signature against the real one). Measured: exactly 2 hits repo-wide, both introduced by this PR (tests/test_early_stop.py:1988, :1996), against 32 test modules already using monkeypatch.setattr — so the convention is established and the rule lands with zero debt. Prevents: A2 low — tests/test_early_stop.py:1984: the unannotated def counting(self_, criterion, records) installed onto type(checker).live_verdict need not match BaseCriterion.live_verdict(self, criterion, turn_records) -> LiveVerdict; autospec=True makes the signature mismatch fail loudly, and pyright does not cover tests/.
  • [ce-lint] CE041 — removed-schema-name residue gate. Whole-tree text check wired as a tests/test_custom_lint.py class (CE027-family, not a BaseRule): compute each coder_eval.models model's field-name set at the merge base (git show origin/main:src/coder_eval/models/*.py, AST-parsed — no imports needed) and at HEAD; any field name that disappeared must not survive as an identifier, kwarg, YAML key, or docs mention in src/, tests/, docs/, tasks/ unless listed in a small RETIRED_NAMES allowlist with a reason. Offline fallback (no merge base available in the sandbox): a checked-in tests/lint/retired_names.py list the renaming PR must extend — the same shape as the deferred "retired-token grep gate" already in .claude/harness-candidates.md, generalized from subsystem tokens to model fields. Prevents: A1/A3/A7 medium — tests/test_early_stop.py:170: max_steps_to_decide (this PR's deleted CriterionEarlyStop field, renamed to decide_within) survives at 47 occurrences including the test name at :706, and the _block(...) helper keeps a whole removed vocabulary alive across 162 call sites.
  • [ruff] Enable PLC0415 (import-outside-top-level) with the repo's existing debt-marker convention. Add "PLC0415" to [tool.ruff.lint] select and bootstrap the 107 existing hits with ruff check --add-noqa, exactly as PLR0915/PLR0912 are already framed in pyproject.toml:187-192 ("gates NEW growth past these bounds … existing offenders are tracked"). The value is that every future function-local import must either move to the top or carry a visible # noqa: PLC0415, which is where the cycle reason belongs (compare reports_stats.py:3-4, which documents its cycle). If 107 noqas is judged too much churn, the narrower alternative is a CE rule scoped to first-party function-local imports (from .x / from coder_eval.x inside a function body) requiring an adjacent comment or noqa. Prevents: A1/A5 low — reports_html.py:348: from .reports import early_stop_gate_note inside _render_header with no cycle to avoid (reports.py imports only .models and .path_utils), which misleads readers into believing reportsreports_html is circular.

Harness improvements (not statically reachable):

  • Trajectory-parity test between the two event reducers. Feed one synthetic event sequence — including two ToolStartEvents emitted before either ToolEndEvent (Claude's parallel-tool-call shape, agents/claude_code_agent.py:344-376) and one force-closed status=UNRESOLVED end — into both streaming.EventCollector and EarlyStopWatcher, then assert the command set the watcher scores over equals the command set that lands in the TurnRecord the checker will score (or that every deliberate difference is enumerated in the test). Parametrize over every ToolEndStatus member so a new status can't quietly diverge one reducer from the other. Why not static: CE039 can forbid the status filter, but the invariant being protected is set-equality of two reductions over an interleaved stream (parallel starts, retry-drained crashed partials) — that needs an actual event sequence and both reducers running, not an AST shape. Prevents: A8 high — the decide_within fail-stop that is not verdict-preserving (orchestrator.py:1586, early_stop.py:437-438, :536-539).
  • Gate-consequence matrix test for every EarlyStopReason. Add a test where the armed criterion's authoritative score is 1.0 while an unarmed/advisory criterion scores 0.0, per reason (criterion_failed, decision_budget_exceeded, ceiling-bound), asserting the resulting FinalStatus. Today tests/test_early_stop.py:2330 scores every criterion 1.0 and :2377 scores the armed one 0.0, so the branch where armed_criteria_passed passes while all_criteria_passed fails — i.e. the advisory-demotion consequence of the single-gate rewrite — is never exercised. Why not static: The gap is a missing combination of runtime scores flowing through Orchestrator finalize; no lint rule can tell that an existing test's fixture happens to make both gates agree. Prevents: A8 high (the SUCCESS-where-main-FAILED outcome) and the stale "agree by construction" comment at orchestrator.py:1582-1587.
  • Criterion-order permutation invariance test. Run the same event stream twice with success_criteria in both orders (a latched fail-armed distractor plus a decide_within-expired criterion resolving on the same round) and assert EarlyStopInfo.reason — which is persisted in task.json, rendered at reports.py:456, and emitted as the EarlyStopReason telemetry dimension at orchestrator.py:264 — is identical. Worth generalizing into a small "YAML-order invariance" helper for any persisted label derived from a first-match scan over criteria. Why not static: The order dependence lives in a runtime next((i for i, v in enumerate(verdicts) ...)) scan whose outcome depends on which verdicts co-occur on a round; a static rule can't distinguish a legitimate first-match scan from a precedence bug. Prevents: A8 low — early_stop.py:600-613: reason attribution flips between criterion_failed and decision_budget_exceeded purely on YAML criterion order, shifting dashboard counts for identical agent behaviour.
  • Diff-coverage gate in make verify and pr-checks.yml. Add diff-cover coverage.xml --compare-branch=origin/main --fail-under=100 (or an equivalent changed-lines gate) after the existing coverage step. The repo only enforces a global --cov-fail-under=80 (Makefile:57, pr-checks.yml:144), which cannot see 2 unexecuted statements inside a 673-line insertion: a full-suite run of this PR reports early_stop.py … 97.40% missing 231, 306, 436->443, 665->667. A changed-lines gate turns "new code is untested" into a red CI check instead of a reviewer catch, and forces the author to either test line 231 or mark it # pragma: no cover with a reason (which itself surfaces that it is unreachable from any validated TaskDefinition). Why not static: Needs runtime coverage data plus a git comparison against the merge base; unreachability of a defensive raise under a registry-validated model is a semantic property, not an AST shape. Prevents: A3/A6 medium — the uncovered registration is None guard at early_stop.py:231 (and the untested defensive raise at :306), whose only reachable input (agent=None) yields the misleading "is the providing plugin installed and loaded?" diagnosis.
  • Fail-open invariant telemetry at the early-stop/finalize seam. When a fail-stop fired, have Orchestrator compare the watcher's live verdicts against the authoritative check_all_async result at finalize and log a WARNING (with reason + criterion) whenever they disagree — e.g. a stopped-early run whose armed gate then passes. Promote the same comparison to a hard assert under an opt-in env flag (CODER_EVAL_STRICT_INVARIANTS=1) exercised by a nightly job, so the divergence shows up as one dashboard-visible counter instead of a silently mis-gated task. Why not static: The claim being audited ("the ceiling is an upper bound on the authoritative score") is only checkable by scoring the frozen trajectory at runtime; the harness already prefers fail-open behavior for verdict bugs, so this is an observability guard, not a gate. Prevents: A8 high — makes the broken verdict-preserving invariant self-reporting the first time it fires in the nightly, rather than surfacing as an unexplained SUCCESS.
  • Require every documented invariant claim to name its enforcing test. The stop_early docs assert verdict-preservation in 3 places (docs/TASK_DEFINITION_GUIDE.md:432 and the CLAUDE.md early-stop bullet) with nothing tying the prose to a test. Add a small checked-in claim→test map (docs/invariants.yaml: claim text anchor → tests/…::test_name) plus a lint-family test asserting each named test id actually exists (and, via --collect-only, is collected). Renaming or deleting the guard test then breaks the build instead of leaving a doc claim the code no longer honors. Why not static: Whether prose is true of the code is semantic judgment; the mechanizable part is only the existence/collection of the cited test, which requires a pytest collection pass. Prevents: A8 high (grouped doc half) — the three surviving "verdict-preserving" claims that the decide_within path contradicts.
  • Extend the CE-id uniqueness assert to the reserved-id ledger. tests/lint/runner.py already asserts no two implemented rules share an id, but .claude/harness-candidates.md reserves CE026/CE032/CE033 for deferred candidates, so "next number after the last file in tests/lint/rules/" (CE026) collides. Have the runner (or the CE-family test) parse the candidates file and fail if an implemented rule claims a reserved id, and print the true next-free id. Why not static: It is a static check, but it guards the review/authoring workflow rather than product code — listing it here so it isn't confused with a defect-preventing rule. Prevents: Pre-empts an id collision while landing CE034–CE041 above (the candidates file explicitly warns that two in-flight branches claiming one number is the likely failure).

Top 5 Priority Actions

  1. Fix the broken verdict-preserving invariant behind the single-gate branch at src/coder_eval/orchestrator.py:1586 — the watcher drops UNRESOLVED tool ends (src/coder_eval/orchestration/early_stop.py:437-438) and never latches an in-flight round (:539) while check_all_async scores the fuller frozen trajectory, so a decide_within timeout (:536-538) can pin a criterion at 0, fire decision_budget_exceeded, and still let armed_criteria_passed score it 1.0 and report SUCCESS with advisory criteria demoted (reproduced with parallel tool calls on tasks/early_stop_decision_budget_exceeded.yaml); make the watcher record unresolved tool ends like src/coder_eval/streaming/collector.py:80-81 does (or re-check the authoritative score before gating), delete the false "agree by construction" comment at orchestrator.py:1582-1587 plus the matching claim at docs/TASK_DEFINITION_GUIDE.md:432, and add the missing test where the timed-out criterion scores 1.0 and an unarmed criterion scores 0.0.
  2. Restore deterministic precedence in the fail-stop candidate scan at src/coder_eval/orchestration/early_stop.py:600-613 so a native live-fail always wins over a budget-driven one, because the deciding index is currently picked by success_criteria YAML order and that value is persisted in EarlyStopInfo.reason, rendered at src/coder_eval/reports.py:456 and emitted as the EarlyStopReason telemetry dimension (src/coder_eval/orchestrator.py:264), so a harmless criterion reorder shifts dashboard counts for identical agent behaviour.
  3. Restore the ~67 lines of CLAUDE.md deleted collaterally around CLAUDE.md:146 — the ## Success Criteria (14 types) table, ## Evaluation Flow, all of ## Development Commands (including make verify, the "could a lint rule have prevented this?" standing rule, the CE030 doc-parity note, the docs-index SSOT note and the anchor-slugger convention) and the ## Configuration heading, whose loss now leaves ruff/pyright/pytest settings reading as architectural patterns, keeping only the two intended early-stop/run-limits bullet rewrites and ideally adding a CEnnn heading-integrity rule since make lint does not catch this.
  4. De-vacuum the guardrail tests: tests/test_early_stop.py:823 passes a nonexistent stop_on_pass=True # type: ignore[call-arg] whose extra_forbidden error is indistinguishable from any typo, so the test cannot fail — switch it to stop_early=StopEarlyPolicy() with pytest.raises(ValidationError, match="stop_early") as line 389 already does, and add the one genuinely missing guard test (armed criteria with task.agent = None, the only reachable path to src/coder_eval/orchestration/early_stop.py:231, since an unregistered agent.type is rejected earlier by the registry validator).
  5. Cut the accidental complexity this PR added in src/coder_eval/orchestration/early_stop.py by collapsing the seven index-aligned parallel lists keyed to self._armed (:296, :308, :314, :318, :336, :337, :342 — __init__ went from radon A(3) to C(14)) into one mutable _ArmedState dataclass, dropping the derived _fail_trigger, and having _collect_verdicts return (verdict, budget_driven) pairs so the provably always-True conjunct at :564 disappears — while keeping _budget_drove's behaviour, which is load-bearing eligibility logic at :604 rather than a mere label; alongside, rename the stale test-helper kwargs max_steps_to_decide/stop_on_fail at tests/test_early_stop.py:170 to the shipped decide_within/armed vocabulary, move the undocumented function-local import at src/coder_eval/reports_html.py:348 to module scope, use DEFAULT_STOP_EARLY_GATE_THRESHOLD at src/coder_eval/models/results.py:479, and replace the stringly-typed getattr(..., "supports_cooperative_stop", False) probe at early_stop.py:236 with direct attribute access.

Stats: 0 🔴 · 2 🟠 · 5 🟡 · 5 🔵 across 8 axes reviewed.

@uipreliga
uipreliga requested review from Copilot and uipreliga August 4, 2026 18:22

@uipreliga uipreliga left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Fix what you agree with and 🚢

Copilot AI 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.

Pull request overview

This PR refactors early-stop so that arming is defined per criterion via a stop_early: block on live-observable criteria, while run_limits.stop_early becomes a kill switch (false to force-disarm; true is now a resolution error). This aligns early-stop behavior, gating semantics (“fired-only”), and reporting across the orchestrator, models, tasks, tests, and docs.

Changes:

  • Replace stop_when / max_steps_to_decide with stop_early: StopEarlyPolicy (on_pass, decide_within) on LiveSuccessCriterion, plus is_stop_armed as the uniform arming predicate.
  • Update early-stop validation/activation (early_stop_active, validate_early_stop) and orchestrator finalize gating to apply the armed weighted gate only when the watcher fired.
  • Unify early-stop report wording across Markdown + HTML via early_stop_gate_note() and update shipped tasks/experiment/docs accordingly.

Reviewed changes

Copilot reviewed 24 out of 25 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_threshold_enforcement.py Updates validation coverage for zero-weight + early-stop arming via StopEarlyPolicy.
tests/test_early_stop.py Reworks extensive test matrix for per-criterion arming, kill switch, fired-only gating, and timeout semantics.
tasks/early_stop_weighted_low_weight_absorbed.yaml Migrates example task to stop_early: blocks and documents fired-only gating + weighted absorption.
tasks/early_stop_weighted_high_weight_kills_run.yaml Same migration for the “high-weight distractor dooms the gate” example.
tasks/early_stop_decision_budget_exceeded.yaml Migrates “decision budget exceeded” example to stop_early.decide_within.
src/coder_eval/reports.py Adds early_stop_gate_note() and uses it in runtime notes to avoid renderer drift.
src/coder_eval/reports_html.py Uses shared gate note for tooltip text and ensures it is HTML-escaped.
src/coder_eval/reports_experiment.py Updates early-stop surface commentary to reflect criterion-level arming.
src/coder_eval/orchestrator.py Activates watcher via early_stop_active() and applies fired-only gating in finalize.
src/coder_eval/orchestration/experiment.py Clarifies early-stop validation timing relative to kill-switch overrides.
src/coder_eval/orchestration/early_stop.py Implements criterion-level arming, inert-by-design triggers, latching, symmetric deferrals, and activation predicate.
src/coder_eval/models/results.py Updates early-stop reason semantics/docs; defaults armed-gate threshold via constant.
src/coder_eval/models/limits.py Adds DEFAULT_STOP_EARLY_GATE_THRESHOLD; changes RunLimits.stop_early to `bool
src/coder_eval/models/criteria.py Introduces StopEarlyPolicy, removes stop_when, adds is_stop_armed, and wires weight/arming validation.
src/coder_eval/models/init.py Re-exports StopEarlyPolicy and DEFAULT_STOP_EARLY_GATE_THRESHOLD.
src/coder_eval/cli/plan_command.py Updates plan-surface comments to reflect per-criterion arming.
experiments/early-stop-ab.yaml Updates “smoke vs e2e” recipe: smoke uses task arming; e2e uses kill switch.
docs/tutorials/04-writing-a-task.md Updates tutorial link text to new arming/kill-switch semantics.
docs/TASK_DEFINITION_GUIDE.md Updates the early-stop guide and run_limits table to the new model and semantics.
docs/REPORT_SCHEMA.md Updates schema semantics for decision_budget_exceeded gating.
docs/EXTENDING.md Updates agent extension docs for cooperative stop requirement under criterion-level arming.
docs/DIALOG_MODE.md Updates dialog-mode note and kill-switch guidance.
docs/agents/CLAUDE_CODE.md Updates agent doc wording to criterion-level arming.
docs/AB_EXPERIMENTS.md Updates the smoke vs e2e recipe to use kill switch rather than a master arm.
CLAUDE.md Updates the repository guide to reflect the new early-stop architecture and semantics.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/coder_eval/orchestrator.py
Comment thread src/coder_eval/reports.py Fixed
akshaylive and others added 3 commits August 4, 2026 12:57
… criteria

Replace the run-level early-stop master arm with per-criterion arming: a
stop_early: block (StopEarlyPolicy) on a live-observable criterion
(skill_triggered, command_executed) alone activates the EarlyStopWatcher.
run_limits.stop_early is demoted to a run-level kill switch — false
force-disarms every block (the one-line e2e/authoritative-P/R/F1 override);
true (the removed master arm) is a hard EarlyStopConfigError at resolution.

Semantics:
- StopEarlyPolicy keys: on_pass (stop|continue, default continue) and
  decide_within (N tool-call steps; expiry latches an effective fail fed
  through the weighted ceiling fail-stop as decision_budget_exceeded).
- Weighted stop rules: fail-stop fires when the armed set's ceiling can no
  longer reach run_limits.stop_early_gate_threshold (default 1.0 = strict
  AND); pass-stop fires when the on_pass:stop subset's floor already meets
  it. BOTH stops defer while a pass-capable armed criterion outside the
  deciding subset is undecided, so recall is never truncated.
- FIRED-ONLY gating: a run the watcher actually cut gates on the weighted
  armed subset (EvaluationResult.armed_criteria_passed); a naturally
  completing run gates strict-AND over the full set — adding a block never
  changes the verdict of a run it didn't cut.
- Verdicts latch on resolved rounds; polarity-inert arming supports dataset
  fan-out (one YAML line for positive and distractor rows); a raising
  live_verdict fails open to a full run.

Hardening: allow_inf_nan=False on criterion weight; single-sourced
DEFAULT_STOP_EARLY_GATE_THRESHOLD; shared early_stop_gate_note() for both
report renderers; on_event fully behind the fail-open disarm wrapper;
_prev_verdicts latched on resolved rounds only.

Tests: 218 early-stop tests incl. real Orchestrator._setup activation,
in-flight decide_within expiry, mixed-arming pass-stop deferral,
load_experiment over experiments/early-stop-ab.yaml, fail-closed pins.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…inism, doc restore

Review blockers:
- Restore the four CLAUDE.md sections (Success Criteria table, Evaluation
  Flow, Development Commands, Configuration heading) collaterally deleted by
  the early-stop bullet rewrite; only the criteria-support paragraph's
  stop_when mention is updated to the stop_early vocabulary.
- Trajectory parity: the watcher now RECORDS UNRESOLVED tool ends into its
  collector (still never counted or evaluated on), so it reduces the same
  trajectory check_all_async scores — a decide_within timeout can no longer
  latch an effective fail on a criterion the frozen trajectory scores as a
  pass. The orchestrator's "agree by construction" comment now cites the
  parity mechanism instead of asserting it.

Also from the review:
- Deterministic fail-stop reason: a native live-fail candidate always wins
  over a budget-driven one, so EarlyStopInfo.reason no longer flips between
  criterion_failed and decision_budget_exceeded on YAML criterion order.
- validate_early_stop: distinct agentless-task diagnosis (was the misleading
  "None is not registered" plugin hint); direct ClassVar access instead of a
  stringly getattr probe; supporting-agent list derived from AgentRegistry
  instead of a hardcoded literal.
- early_stop_gate_note dispatches via exhaustiveness-checked match over
  EarlyStopReason (restores the assert_never guard the rewrite dropped).
- EarlyStopInfo.gate_threshold field default uses
  DEFAULT_STOP_EARLY_GATE_THRESHOLD instead of a duplicate literal.
- reports_html imports early_stop_gate_note at module scope (no cycle).
- All-inert armed set logs at WARNING (visible dead-config signal).
- Orchestrator distinguishes fail-open disarm from natural completion in the
  full-gate log line (Copilot inline comment).
- De-vacuumed test_guardrail3_unobservable_criterion_unrepresentable (was
  asserting on a nonexistent kwarg's extra_forbidden error) and added tests:
  trajectory parity, budget-timeout-vs-orphan verdict preservation, reason
  order-invariance, advisory-demotion gate consequence, agentless/unregistered
  guard coverage (early_stop.py:231), decision-budget HTML tooltip, and a
  pinned documented-gap test for pass-stop cutting an undecided
  fail-only-decidable sibling (with a TASK_DEFINITION_GUIDE caveat).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eQL py/mixed-returns)

CodeQL does not model assert_never's Never return, so the match arm read as
an implicit-None fall-through mixed with explicit returns. Bind the note per
arm and return once; the assert_never exhaustiveness guard is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@akshaylive
akshaylive force-pushed the akshaya/earlystop-per-criterion-arming branch from 7fcbe4f to eb9fe79 Compare August 4, 2026 19:58
Comment thread src/coder_eval/reports.py Fixed
akshaylive and others added 2 commits August 4, 2026 13:02
… every path

The mixed-returns fix traded alert 58 for py/uninitialized-local-variable
(alert 59): CodeQL does not model the assert_never arm as raising, so
`return note` looked reachable with `note` unbound. Default to the generic
note before the match; exhaustiveness checking is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@akshaylive
akshaylive merged commit 4cf8092 into main Aug 4, 2026
13 checks passed
@akshaylive
akshaylive deleted the akshaya/earlystop-per-criterion-arming branch August 4, 2026 23:02
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.

4 participants