From f1a89dcaf3f7b704b2db1280166a946e128e2cbc Mon Sep 17 00:00:00 2001 From: mohsen-uipath <301015030+mohsen-uipath@users.noreply.github.com> Date: Thu, 23 Jul 2026 20:54:21 -0700 Subject: [PATCH 1/3] =?UTF-8?q?fix(early-stop):=20add=20stop=5Fwhen=20'aut?= =?UTF-8?q?o'=20=E2=80=94=20per-instance=20arming=20+=20pass-armed-subset?= =?UTF-8?q?=20stop=20rule?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under v0.8.9 any-engagement scoring, each skill_triggered instance can decide only one polarity mid-run (a positive can only live-pass, a distractor only live-fail), so 'decided' — the only arming value that fit a dataset-fanned suite — was rejected at validation for every instance. - stop_when: auto arms whichever polarities the instance can decide (live_decidable_polarities); a dead arm is a hard error at resolution. - Pass-stop now requires only the pass-armed subset to live-pass, with a vacuous guard so a row with zero pass-armed criteria never pass-stops. Fail-stop is unchanged and evaluated first. - The watcher resolves arming once into _armed_polarities; the stop rule never compares raw stop_when. - Precision trade-off (pass-stop can hide a later distractor misfire) documented in module docstring, CLAUDE.md, and the task guide. --- .claude/harness-candidates.md | 16 +++ CLAUDE.md | 4 +- docs/TASK_DEFINITION_GUIDE.md | 41 ++++--- src/coder_eval/models/criteria.py | 9 +- src/coder_eval/orchestration/early_stop.py | 83 +++++++++++--- tests/test_early_stop.py | 119 ++++++++++++++++++++- 6 files changed, 241 insertions(+), 31 deletions(-) diff --git a/.claude/harness-candidates.md b/.claude/harness-candidates.md index cbfb4970..7a4d61bf 100644 --- a/.claude/harness-candidates.md +++ b/.claude/harness-candidates.md @@ -27,6 +27,22 @@ Deferred lint/test guardrails surfaced during reviews. Promote to a `CExxx` rule reachable (needs a live run). - [ ] CE-rule: `type: Literal[...]` fields on models in `coder_eval/models/` must declare their tag default (`type: Literal["x"] = "x"`) — a member without the default degrades `validate_registry` diagnostics (PydanticUndefined in expected_types) and breaks direct construction. Nothing guards it today; needs a rule-design call (second violation class inside CE024 vs. a new CExxx at the next free id), and the failure is already double-caught by the MINIMAL_PAYLOADS parity test + direct-construction tests — caught in the 2026-07-03 top5-review-fixes run (Phase 1 quality review). +## From 2026-07-23 stop_when:auto early-stop review + +- [ ] CE-rule: the early-stop watcher stop rule must decide polarity via the + resolved `_armed_polarities`, never a raw `criterion.stop_when` comparison — + forbid `.stop_when` attribute reads inside `EarlyStopWatcher._evaluate` / + `_resolve_armed_polarities`'s callers in `orchestration/early_stop.py`. This + diff *was* the fix for exactly that class of bug (the old rule compared + `stop_when in ("pass","decided")` and so vetoed every mixed `auto` pass-stop). + Deferred, not cheap: existing CE rules scope by file/module, not by a specific + method, so a method-scoped attribute-ban needs a new AST-walk shape (and risks + false positives on the legitimate `is not None` membership reads elsewhere in + the file). Claim the next free id in `tests/lint/rules/`. Caught in the + 2026-07-23 stop_when:auto review; the behavior itself is guarded by + `test_auto_mixed_pass_stops_ignoring_undecided_distractors` + + `test_mixed_static_arming_pass_stops_ignoring_fail_armed`. + ## From 2026-07-03 open-source docs cleanup - [ ] **Dead-relative-link checker for `docs/**/*.md`** — resolve every relative diff --git a/CLAUDE.md b/CLAUDE.md index 87b531a6..85e8088a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -141,7 +141,7 @@ action.yml # Published composite GitHub Action (coder-ev - **Reconciliation message (stream self-reconciles to the turn total)**: The per-message stream consistently under-reports the authoritative turn total — a fixed prompt slice (~512 input tokens on Claude) is billed on no SDK-emitted message, and sub-agent input/cache only partially bubbles up. So `EventCollector.build_turn_record` appends one synthetic `ReconciliationMessage` (`role="reconciliation"`, in the `TranscriptMessage` union) per turn, carrying the per-bucket residual = `token_usage` − Σ(assistant message buckets). The invariant: **summing the four token buckets across `TurnRecord.messages` (assistant + reconciliation) equals `token_usage` exactly**, for both Claude and Codex (Codex's stream is already complete after `_recover_subagent_tool_calls`, so its residual is usually 0 and no entry is emitted). This is what lets the evalboard SUM the message stream as the source of truth instead of reading a separate aggregate ("agent tokens"): `selectTokenTotals` returns the stream sum whenever a reconciliation entry is present, and the timeline renders it as its own row. It is agent-agnostic (booked at the single `EventCollector` seam), carries no cost (cost stays on `token_usage`), and is excluded from generation/turn counts and the cost simulator. The Python `token_usage`/`total_token_usage` aggregate is unchanged and still authoritative for budget/judges/reports. - **sandbox isolation**: Tasks that don't need MCP servers should set `setting_sources: []` in their `agent:` block to isolate the sandbox from the host project's CLAUDE.md and settings. Without this, the host project's CLAUDE.md (often 20 KB+) is injected into every API call, inflating cache-creation tokens and cost significantly. - **Run-time caps (non-criterion enforcement)**: `TaskDefinition.run_limits` (`RunLimits` model) is the single namespace for all run-time caps — `max_turns` / `task_timeout` / `turn_timeout` (structural) and `max_input_tokens` / `max_output_tokens` / `max_total_tokens` / `max_usd` (cumulative budget). Token/USD breaches abort with `FinalStatus.TOKEN_BUDGET_EXCEEDED` or `COST_BUDGET_EXCEEDED` (both `category == "failed"`). Structural caps are set from the CLI via `-D run_limits.max_turns=…` / `-D run_limits.task_timeout=…` / `-D run_limits.turn_timeout=…` (field-merged into `run_limits`); budget caps via `-D run_limits.max_usd=…` etc. or YAML. Layered config uses field-merge — a variant block overrides individual keys without replacing the task's block. -- **Early stop on criterion (opt-in)**: `run_limits.stop_early` (default off) ends a single-shot Claude run early once the run's **armed** criteria are decided, so a raised `max_turns` isn't wasted on the smoke flavor. A criterion is armed by `stop_when: pass|fail|decided`; only criteria that can decide from a partial trajectory may arm (they declare a non-empty `live_stop_polarities` ClassVar and override `live_verdict` on `BaseCriterion` — currently `skill_triggered`, `command_executed`; CE025 enforces the two stay consistent). It uses a cooperative `should_stop` seam on the Claude agent's between-messages guard (tool-call granularity, no SIGKILL) driven by `orchestration/early_stop.py::EarlyStopWatcher` (own `EventCollector` + stop rule). Live verdicts only *trigger* the stop; the standard `check_all` on the frozen trajectory is authoritative. An early-stopped run gates on the **armed subset** (`EvaluationResult.armed_criteria_passed`); a completed run gates on the full set. Every unsupported use (non-observable criterion, non-Claude agent, wrong polarity, no armed criterion, simulation mode) is an error at resolution (plan *and* run), and a runtime verdict bug **fails open** to a full run. Surfaces: `EarlyStopInfo` (reason + deciding criterion + when), report notes/badges, `stopped_early` run.json rows, and `EarlyStopped`/`EarlyStopReason` telemetry dims. Defaults off ⇒ behavior byte-for-behavior unchanged. +- **Early stop on criterion (opt-in)**: `run_limits.stop_early` (default off) ends a single-shot Claude run early once the run's **armed** criteria are decided, so a raised `max_turns` isn't wasted on the smoke flavor. A criterion is armed by `stop_when: pass|fail|decided|auto`; only criteria that can decide from a partial trajectory may arm (they declare a non-empty `live_stop_polarities` ClassVar and override `live_verdict` on `BaseCriterion` — currently `skill_triggered`, `command_executed`; CE025 enforces the two stay consistent). `decided` arms **both** polarities (the instance must be able to decide each); `auto` arms whichever polarities **this instance** can decide (its `live_decidable_polarities`) — the value to use when the decidable polarity is instance-dependent, e.g. `skill_triggered` under any-engagement, where a positive row (`skill_name == expected_skill`) can only live-pass and a distractor can only live-fail, so one static `stop_when` on a dataset-fanned criterion (whose positive/distractor role flips per row) cannot fit every row. The watcher's pass-stop fires when every **pass-armed** criterion live-passes (fail-armed distractors are not required to pass, and a row with zero pass-armed criteria — e.g. a negative row — never pass-stops on turn 0); the fail-stop fires on the first fail-armed criterion that live-fails. Precision trade-off: a pass-stop cuts the run once the positive is decided, so a distractor that would misfire on a *later* tool call is not observed (the frozen row scores as a clean pass) — an intentional precision-for-budget trade of the smoke flavor, so authoritative P/R/F1 must come from a `stop_early: false` run. It uses a cooperative `should_stop` seam on the Claude agent's between-messages guard (tool-call granularity, no SIGKILL) driven by `orchestration/early_stop.py::EarlyStopWatcher` (own `EventCollector` + stop rule). Live verdicts only *trigger* the stop; the standard `check_all` on the frozen trajectory is authoritative. An early-stopped run gates on the **armed subset** (`EvaluationResult.armed_criteria_passed`); a completed run gates on the full set. Every unsupported use (non-observable criterion, non-Claude agent, wrong polarity, no armed criterion, simulation mode) is an error at resolution (plan *and* run), and a runtime verdict bug **fails open** to a full run. Surfaces: `EarlyStopInfo` (reason + deciding criterion + when), report notes/badges, `stopped_early` run.json rows, and `EarlyStopped`/`EarlyStopReason` telemetry dims. Defaults off ⇒ behavior byte-for-behavior unchanged. ## Success Criteria (14 types) @@ -162,7 +162,7 @@ action.yml # Published composite GitHub Action (coder-ev | `llm_judge` | Continuous | LLM grades artifacts + optional trajectory + optional reference; routes through the run's backend (Bedrock / Anthropic) | | `agent_judge` | Continuous | Spawns a Claude Code SDK agent in an isolated sandbox copy; judge uses tools (Bash/Read/Grep/…) to investigate and returns a JSON verdict. Expensive; runs with evaluator credentials — see SECURITY note in the criterion docstring. | -All criteria support `weight` (default 1.0) and `pass_threshold` (default 0.9), plus `stop_when` (`pass`/`fail`/`decided`, default `null`) which arms the criterion for early stop when `run_limits.stop_early` is set (observable criteria only). On dataset-backed tasks, criteria may also set `suite_thresholds: {metric: min_value}` — the suite gate passes iff every listed metric (from the criterion's `aggregate()` output) meets its minimum. +All criteria support `weight` (default 1.0) and `pass_threshold` (default 0.9), plus `stop_when` (`pass`/`fail`/`decided`/`auto`, default `null`) which arms the criterion for early stop when `run_limits.stop_early` is set (observable criteria only; `auto` arms the instance's own decidable polarities). On dataset-backed tasks, criteria may also set `suite_thresholds: {metric: min_value}` — the suite gate passes iff every listed metric (from the criterion's `aggregate()` output) meets its minimum. ## Evaluation Flow diff --git a/docs/TASK_DEFINITION_GUIDE.md b/docs/TASK_DEFINITION_GUIDE.md index d0baa887..e16d7294 100644 --- a/docs/TASK_DEFINITION_GUIDE.md +++ b/docs/TASK_DEFINITION_GUIDE.md @@ -228,23 +228,38 @@ Semantics: - **Opt-in, per run.** With `stop_early: false` (the default) the run behaves exactly as before — `stop_when` is inert and every criterion gates normally. -- **Polarity.** `stop_when: pass` stops the moment all armed criteria are decided - in the pass direction; `stop_when: fail` stops on a definitive wrong-signal - fail; `stop_when: decided` stops on either. Only criteria that can decide from a - partial trajectory (currently `skill_triggered`, `command_executed`) may be - armed — arming any other criterion is a hard error at resolution (plan *and* - run), never a silent no-op. Decidability can also depend on a criterion's own - fields: `command_executed` can live-**pass** only with `max_count` unset and - `min_count > 0`, and live-**fail** only with `max_count` set (which includes - the `min_count: 0, max_count: 0` "must-NOT-run" form). Arming a polarity the - configured criterion can never reach (e.g. `stop_when: pass` alongside a - `max_count`) is likewise a hard error at resolution, not a silent full run. +- **Polarity.** `stop_when: pass` stops the moment all **pass-armed** criteria are + decided in the pass direction; `stop_when: fail` stops on a definitive + wrong-signal fail; `stop_when: decided` stops on either (the criterion instance + must be able to decide **both**). `stop_when: auto` arms whichever polarities + **this instance** can decide — use it when the decidable polarity is + instance-dependent, e.g. a `skill_triggered` activation suite where a positive + row (`skill_name == expected_skill`) can only live-pass and a distractor can only + live-fail, so one static value on a dataset-fanned criterion (whose + positive/distractor role flips per row) cannot fit every row. A **pass-stop** + needs every pass-armed criterion to pass — fail-armed distractors are not + required to, and a row with **zero** pass-armed criteria (e.g. a negative row) + never pass-stops; a **fail-stop** fires on the first fail-armed criterion that + live-fails. Only criteria that can decide from a partial trajectory (currently + `skill_triggered`, `command_executed`) may be armed — arming any other criterion + is a hard error at resolution (plan *and* run), never a silent no-op. + Decidability can also depend on a criterion's own fields: `command_executed` can + live-**pass** only with `max_count` unset and `min_count > 0`, and live-**fail** + only with `max_count` set (which includes the `min_count: 0, max_count: 0` + "must-NOT-run" form). Arming a polarity the configured criterion can never reach + (e.g. `stop_when: pass` alongside a `max_count`, or `auto` on an instance that + can decide neither) is likewise a hard error at resolution, not a silent full + run. - **Verdict.** An early-stopped run is gated on the **armed subset only**; the non-armed criteria become **advisory** and are clearly marked (report badge + per-criterion note + `stopped_early` row). A run that completes naturally is gated on the **full** set, as always. This is what lets one file serve both a `smoke` flavor (`stop_early: true`) and an `e2e` flavor (`stop_early: false`) - with identical verdicts — see [AB_EXPERIMENTS.md](AB_EXPERIMENTS.md). + with identical verdicts — see [AB_EXPERIMENTS.md](AB_EXPERIMENTS.md). Note: on a + **pass-stop** the run is cut once the positive is decided, so a distractor that + would misfire on a *later* tool call is not observed (the frozen row scores as a + clean pass) — the smoke flavor trades some precision completeness for budget, so + authoritative precision/recall belongs on the `stop_early: false` run. - **Fail-safe.** A live-verdict bug **fails open** to a full run (logged loudly) — it can never silently disable a criterion or cause a false early stop. @@ -391,7 +406,7 @@ All criteria share these fields: | `description` | — | Human-readable description (required) | | `weight` | 1.0 | Relative importance for weighted score. `0` = **informational**: excluded from both the score and the pass/fail gate | | `pass_threshold` | 0.9 | Minimum score (0.0–1.0) to pass | -| `stop_when` | `null` | Arms this criterion for early stop (`pass`/`fail`/`decided`); requires `run_limits.stop_early: true` and an observable criterion type (`skill_triggered`, `command_executed`). See [`stop_early`](#stop_early-opt-in-early-stop). | +| `stop_when` | `null` | Arms this criterion for early stop (`pass`/`fail`/`decided`/`auto`); requires `run_limits.stop_early: true` and an observable criterion type (`skill_triggered`, `command_executed`). `auto` arms whichever polarity this instance can decide (for dataset-fanned criteria whose positive/distractor role flips per row). See [`stop_early`](#stop_early-opt-in-early-stop). | **Scoring types:** - **Binary** (1.0 or 0.0): `file_exists`, `run_command`, `file_matches_regex` diff --git a/src/coder_eval/models/criteria.py b/src/coder_eval/models/criteria.py index 8786ac86..2605e1cf 100644 --- a/src/coder_eval/models/criteria.py +++ b/src/coder_eval/models/criteria.py @@ -123,12 +123,17 @@ class BaseSuccessCriterion(BaseModel, ABC): ), ) - stop_when: Literal["pass", "fail", "decided"] | None = Field( + stop_when: Literal["pass", "fail", "decided", "auto"] | None = Field( default=None, description=( "Opt-in early-stop polarity for this criterion (membership in the run's 'armed set'). " "None (default) = not a stop criterion. 'pass' = a live PASS may contribute to a stop; " - "'fail' = a live definitive FAIL may trigger a stop; 'decided' = either. Inert unless " + "'fail' = a live definitive FAIL may trigger a stop; 'decided' = either (the instance " + "must be able to decide BOTH polarities). 'auto' = arm whichever polarities THIS instance " + "can actually decide (its live_decidable_polarities) - use it when the decidable polarity " + "is instance-dependent, e.g. skill_triggered under any-engagement, where a positive row " + "(skill_name == expected_skill) can only live-pass and a distractor can only live-fail, " + "so no single static polarity fits every fanned-out row. Inert unless " "run_limits.stop_early is True. Only valid on criteria observable mid-run (e.g. " "skill_triggered, command_executed); an unobservable armed criterion is rejected at " "resolution time." diff --git a/src/coder_eval/orchestration/early_stop.py b/src/coder_eval/orchestration/early_stop.py index a1d76fb6..6182f97c 100644 --- a/src/coder_eval/orchestration/early_stop.py +++ b/src/coder_eval/orchestration/early_stop.py @@ -26,6 +26,14 @@ Live verdicts only *trigger* the stop; the authoritative scores always come from the standard ``check_all`` on the frozen trajectory after the cut. + +Precision trade-off: a pass-stop cuts the run the instant every *pass-armed* +criterion is decided, so a *fail-armed* criterion (e.g. a distractor) that would +only misfire on a LATER tool call is never observed — the frozen trajectory then +scores that row as a clean pass. This is an intentional precision-for-budget +trade of the opt-in "smoke" flavor (an already-visible misfire still fail-stops, +since fail-stop is evaluated before pass-stop each round); the authoritative +precision/recall must come from a non-early-stop (``stop_early: false``) run. """ from __future__ import annotations @@ -85,6 +93,7 @@ def validate_early_stop(task: TaskDefinition) -> None: then per armed criterion: 3. criterion is not observable mid-run -> error 4. requested ``stop_when`` polarity the criterion cannot decide -> error + (``auto`` errors only on a dead arm: an instance that can decide neither) Raises: EarlyStopConfigError: on any unsupported armed configuration. @@ -150,6 +159,19 @@ def validate_early_stop(task: TaskDefinition) -> None: # arm (a polarity this instance can never fire) would silently degrade to # a full run instead of erroring here. polarities = checker_cls.live_decidable_polarities(c) + if polarity == "auto": + # `auto` arms exactly the polarities THIS instance can decide, so the + # only invalid case is a dead arm (an instance that can decide + # neither) — otherwise there is nothing to arm and it would silently + # never fire. A non-empty decidable set is always fully armable by + # `auto`, so there is nothing further to validate for this criterion. + if not polarities: + raise EarlyStopConfigError( + f"criterion type {c.type!r} is armed (stop_when='auto') but this instance can " + + "decide no polarity mid-run; 'auto' requires at least one live-decidable " + + "polarity (its decidability can depend on the criterion's fields)." + ) + continue needed = {"pass", "fail"} if polarity == "decided" else {polarity} missing = sorted(needed - polarities) if missing: @@ -193,6 +215,15 @@ def __init__( ) -> None: self._task_id = task_id self._armed = armed + # Per-instance resolved arming polarities, aligned with ``_armed``. Static + # for the run: a criterion armed ``pass``/``fail`` resolves to that single + # polarity, ``decided`` to both, and ``auto`` to whatever THIS instance can + # decide (its ``live_decidable_polarities``). The stop rule consults this, + # not the raw ``stop_when`` string, so a distractor armed ``auto`` (fail + # only) is not required to live-pass for a pass-stop. + self._armed_polarities: list[frozenset[str]] = [ + self._resolve_armed_polarities(criterion, checker) for criterion, checker in armed + ] self._max_turns = max_turns self._collector = EventCollector() self._sdk_turn_index = 0 @@ -292,6 +323,25 @@ def disarmed(self) -> bool: # --- Stop rule -------------------------------------------------- # + @staticmethod + def _resolve_armed_polarities(criterion: BaseSuccessCriterion, checker: BaseCriterion[Any]) -> frozenset[str]: + """The polarities this armed instance may fire, resolved from ``stop_when``. + + ``pass``/``fail`` -> that single polarity; ``decided`` -> both (validation + has already guaranteed the instance can decide both); ``auto`` -> the + instance's own ``live_decidable_polarities`` (validation has guaranteed it + is non-empty). ``None`` is never armed, but is mapped to the empty set + defensively so the caller need not special-case it. + """ + sw = criterion.stop_when + if sw == "auto": + return checker.live_decidable_polarities(criterion) + if sw == "decided": + return frozenset({"pass", "fail"}) + if sw in ("pass", "fail"): + return frozenset({sw}) + return frozenset() + def _evaluate(self, in_flight: CommandTelemetry | None = None) -> None: record = self._collector.build_turn_record() if in_flight is not None: @@ -321,22 +371,29 @@ def _evaluate(self, in_flight: CommandTelemetry | None = None) -> None: return # Fail-stop: first armed criterion (criteria order) that live-fails AND - # whose stop_when permits fail decides the run. - for (criterion, _checker), verdict in zip(self._armed, verdicts, strict=True): - if verdict == "fail" and criterion.stop_when in ("fail", "decided"): + # whose resolved arming permits fail decides the run. + for (criterion, _checker), verdict, armed_pol in zip( + self._armed, verdicts, self._armed_polarities, strict=True + ): + if verdict == "fail" and "fail" in armed_pol: self._fire(EarlyStopReason.CRITERION_FAILED, criterion, tool_call_index=tool_call_index) return - # Pass-stop: EVERY armed criterion live-passes AND each permits pass. - all_pass = all(v == "pass" for v in verdicts) - all_permit_pass = all(c.stop_when in ("pass", "decided") for c, _ in self._armed) - if all_pass and all_permit_pass: - # Deciding criterion = the last armed (criteria order) whose verdict - # flipped vs the previous round; fall back to the last armed. - deciding = self._armed[-1][0] - for (criterion, _checker), verdict, prev in zip(self._armed, verdicts, self._prev_verdicts, strict=True): - if verdict != prev: - deciding = criterion + # Pass-stop: every PASS-ARMED criterion live-passes. Fail-armed criteria + # (e.g. distractors armed ``auto`` -> fail only) are NOT required to pass — + # they can never live-pass and only guard the fail side above, so requiring + # them would veto every pass-stop (the mixed-arming bug). Guard the vacuous + # case: with zero pass-armed criteria (a negative row whose criteria are all + # distractors) there is nothing to pass-stop on, so the run must continue to + # the cap rather than firing on turn 0 with an empty ``all()``. + pass_armed = [i for i, pol in enumerate(self._armed_polarities) if "pass" in pol] + if pass_armed and all(verdicts[i] == "pass" for i in pass_armed): + # Deciding criterion = the last pass-armed (criteria order) whose verdict + # flipped vs the previous round; fall back to the last pass-armed. + deciding = self._armed[pass_armed[-1]][0] + for i in pass_armed: + if verdicts[i] != self._prev_verdicts[i]: + deciding = self._armed[i][0] self._fire(EarlyStopReason.CRITERION_PASSED, deciding, tool_call_index=tool_call_index) return diff --git a/tests/test_early_stop.py b/tests/test_early_stop.py index 80998978..ce397670 100644 --- a/tests/test_early_stop.py +++ b/tests/test_early_stop.py @@ -230,7 +230,7 @@ def test_stop_early_settable(self) -> None: def test_stop_when_defaults_none(self) -> None: assert _skill_crit("s", "s").stop_when is None - @pytest.mark.parametrize("value", ["pass", "fail", "decided"]) + @pytest.mark.parametrize("value", ["pass", "fail", "decided", "auto"]) def test_stop_when_accepts_valid_polarities(self, value: str) -> None: assert _skill_crit("s", "s", stop_when=value).stop_when == value @@ -238,6 +238,14 @@ def test_stop_when_rejects_invalid_polarity(self) -> None: with pytest.raises(ValueError): _skill_crit("s", "s", stop_when="maybe") + def test_stop_when_auto_roundtrips(self) -> None: + # The new `auto` value survives model_dump -> model_validate with its + # model_fields_set intact (Pydantic round-trip integrity). + crit = _skill_crit("s", "s", stop_when="auto") + restored = SkillTriggeredCriterion.model_validate_json(crit.model_dump_json()) + assert restored.stop_when == "auto" + assert "stop_when" in restored.model_fields_set + # --------------------------------------------------------------------------- # # skill_triggered live verdict @@ -571,6 +579,42 @@ def test_stacked_activation_criteria_accept(self) -> None: task = _task(criteria=crits, stop_early=True) validate_early_stop(task) # no raise + def test_auto_positive_accepts(self) -> None: + # `auto` on a positive resolves to the pass polarity it can decide. + task = _task(criteria=[_skill_crit("s", "s", stop_when="auto")], stop_early=True) + validate_early_stop(task) # no raise + + def test_auto_distractor_accepts(self) -> None: + # `auto` on a distractor resolves to the fail polarity it can decide. + task = _task(criteria=[_skill_crit("wrong", "s", stop_when="auto")], stop_early=True) + validate_early_stop(task) # no raise + + def test_auto_negative_row_distractor_accepts(self) -> None: + # A negative row's criterion (expected_skill == "") is a distractor -> fail. + task = _task(criteria=[_skill_crit("wrong", "", stop_when="auto")], stop_early=True) + validate_early_stop(task) # no raise + + def test_auto_stacked_activation_accepts(self) -> None: + # The real activation shape: ONE uniform `stop_when: auto` across every + # stacked criterion, which resolves per-instance to pass (the positive) or + # fail (each distractor). This is what a single fanned-out `stop_when` value + # can express and `pass`/`fail`/`decided` cannot, since the role flips per row. + crits = [ + _skill_crit("skill-a", "skill-a", stop_when="auto"), # positive -> pass + _skill_crit("skill-b", "skill-a", stop_when="auto"), # distractor -> fail + _skill_crit("skill-c", "skill-a", stop_when="auto"), # distractor -> fail + ] + task = _task(criteria=crits, stop_early=True) + validate_early_stop(task) # no raise + + def test_auto_dead_arm_rejected(self) -> None: + # `auto` on an instance that can decide NEITHER polarity is a dead arm and + # must be rejected, not silently degrade to a full run. command_executed + # with min_count=0 + max_count=None supports no live polarity. + task = _task(criteria=[_cmd_crit(stop_when="auto", min_count=0, max_count=None)], stop_early=True) + with pytest.raises(EarlyStopConfigError, match="no polarity"): + validate_early_stop(task) + # --------------------------------------------------------------------------- # # Guardrail integration: the plan and run resolution surfaces actually invoke @@ -1011,6 +1055,79 @@ def test_stacked_wrong_skill_fail_stop(self) -> None: assert watcher.info is not None assert watcher.info.reason == EarlyStopReason.CRITERION_FAILED + def test_auto_positive_pass_stops(self) -> None: + # `auto` on a positive resolves to pass-armed: engaging the expected skill + # pass-stops, identically to an explicit stop_when="pass". + watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="auto")]) + _feed(watcher, _skill_events("date-teller")) + assert watcher.should_stop() is True + assert watcher.info is not None + assert watcher.info.reason == EarlyStopReason.CRITERION_PASSED + + def test_auto_mixed_pass_stops_ignoring_undecided_distractors(self) -> None: + # THE mixed-arming fix: one positive + two distractors, all armed `auto`. + # Engaging ONLY the expected skill pass-stops on turn 1 even though the two + # distractors are still "undecided" — fail-armed criteria are not required + # to live-pass. (Under the old "every armed must pass" rule this could never + # fire, since a distractor can never live-pass.) + watcher = _watcher( + [ + _skill_crit("date-teller", "date-teller", stop_when="auto"), # positive -> pass + _skill_crit("weather-teller", "date-teller", stop_when="auto"), # distractor -> fail + _skill_crit("news-teller", "date-teller", stop_when="auto"), # distractor -> fail + ] + ) + _feed(watcher, _skill_events("date-teller")) + assert watcher.should_stop() is True + assert watcher.info is not None + assert watcher.info.reason == EarlyStopReason.CRITERION_PASSED + # The deciding criterion is the positive that flipped to pass. + assert watcher.info.deciding_criterion_description == "date-teller activation" + + def test_auto_negative_row_no_pass_stop_on_benign_call(self) -> None: + # THE vacuous guard: a negative row (expected_skill == "") stacks only + # distractors, so there are ZERO pass-armed criteria. A benign non-skill + # tool call must NOT pass-stop on turn 0 (empty all() would be vacuously + # True); the run continues to the cap as intended. + watcher = _watcher( + [ + _skill_crit("date-teller", "", stop_when="auto"), # distractor -> fail + _skill_crit("weather-teller", "", stop_when="auto"), # distractor -> fail + ] + ) + _feed(watcher, [_agent_start(), _turn_start(), _tool_end(_cmd("Bash", {"command": "echo hi"}))]) + assert watcher.should_stop() is False + assert watcher.info is None + + def test_auto_negative_row_misfire_fail_stops(self) -> None: + # The other half of the asymmetry: a negative row that DOES engage a skill + # is a misfire and fail-stops (the precision signal), even though it can + # never pass-stop. + watcher = _watcher( + [ + _skill_crit("date-teller", "", stop_when="auto"), # distractor -> fail + _skill_crit("weather-teller", "", stop_when="auto"), # distractor -> fail + ] + ) + _feed(watcher, _skill_events("date-teller")) + assert watcher.should_stop() is True + assert watcher.info is not None + assert watcher.info.reason == EarlyStopReason.CRITERION_FAILED + + def test_mixed_static_arming_pass_stops_ignoring_fail_armed(self) -> None: + # The pass-armed-subset rule is not `auto`-specific: an explicit + # pass-positive + fail-distractor mix also pass-stops on the positive alone. + watcher = _watcher( + [ + _skill_crit("date-teller", "date-teller", stop_when="pass"), # pass-armed + _skill_crit("weather-teller", "date-teller", stop_when="fail"), # fail-armed + ] + ) + _feed(watcher, _skill_events("date-teller")) + assert watcher.should_stop() is True + assert watcher.info is not None + assert watcher.info.reason == EarlyStopReason.CRITERION_PASSED + def test_records_turn_and_tool_index(self) -> None: watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="pass")]) _feed(watcher, _skill_events("date-teller")) From 415c910cf74f73f18383552cab8602581a8ccb74 Mon Sep 17 00:00:00 2001 From: mohsen-uipath <301015030+mohsen-uipath@users.noreply.github.com> Date: Fri, 24 Jul 2026 13:40:38 -0700 Subject: [PATCH 2/3] fix(early-stop): defer fail-stop while a pass-armed criterion is undecided MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up (PR #51): a distractor misfire on an early tool call no longer truncates a positive row before its expected signal can appear — cutting there froze a would-be TP as an FN and deflated suite recall/F1 under stop_early. The misfire is latched by the criteria's monotone semantics, so the deferred fail-stop still fires the moment every pass-armed criterion decides (fail-stop keeps precedence over pass-stop, now pinned by a same-round test); a row with zero pass-armed criteria (negative rows) defers nothing and fail-stops on the first misfire. Also from the review: - hoist the stop_when->polarity mapping into one _requested_polarities helper (validator + watcher), terminated by assert_never so widening the Literal without updating the mapping is a type error - name the offending criterion's description in the arming errors and add the concrete field guidance to the auto dead-arm message - replace the unsatisfiable 'stop_when: decided' example/hint with 'auto' (guide + validator hint) - state verdict parity as one-sided (fail-stop verdict-preserving, pass-stop precision-optimistic) in the guide, AB_EXPERIMENTS.md, experiments/early-stop-ab.yaml, RunLimits.stop_early, and a compressed CLAUDE.md bullet --- CLAUDE.md | 2 +- docs/AB_EXPERIMENTS.md | 13 ++- docs/TASK_DEFINITION_GUIDE.md | 18 ++- experiments/early-stop-ab.yaml | 8 +- src/coder_eval/models/limits.py | 15 ++- src/coder_eval/orchestration/early_stop.py | 126 +++++++++++++-------- tests/test_early_stop.py | 49 +++++++- 7 files changed, 162 insertions(+), 69 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 87fe0254..cc00eadc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -141,7 +141,7 @@ action.yml # Published composite GitHub Action (coder-ev - **Reconciliation message (stream self-reconciles to the turn total)**: The per-message stream consistently under-reports the authoritative turn total — a fixed prompt slice (~512 input tokens on Claude) is billed on no SDK-emitted message, and sub-agent input/cache only partially bubbles up. So `EventCollector.build_turn_record` appends one synthetic `ReconciliationMessage` (`role="reconciliation"`, in the `TranscriptMessage` union) per turn, carrying the per-bucket residual = `token_usage` − Σ(assistant message buckets). The invariant: **summing the four token buckets across `TurnRecord.messages` (assistant + reconciliation) equals `token_usage` exactly**, for both Claude and Codex (Codex's stream is already complete after `_recover_subagent_tool_calls`, so its residual is usually 0 and no entry is emitted). This is what lets the evalboard SUM the message stream as the source of truth instead of reading a separate aggregate ("agent tokens"): `selectTokenTotals` returns the stream sum whenever a reconciliation entry is present, and the timeline renders it as its own row. It is agent-agnostic (booked at the single `EventCollector` seam), carries no cost (cost stays on `token_usage`), and is excluded from generation/turn counts and the cost simulator. The Python `token_usage`/`total_token_usage` aggregate is unchanged and still authoritative for budget/judges/reports. - **sandbox isolation**: Tasks that don't need MCP servers should set `setting_sources: []` in their `agent:` block to isolate the sandbox from the host project's CLAUDE.md and settings. Without this, the host project's CLAUDE.md (often 20 KB+) is injected into every API call, inflating cache-creation tokens and cost significantly. - **Run-time caps (non-criterion enforcement)**: `TaskDefinition.run_limits` (`RunLimits` model) is the single namespace for all run-time caps — `max_turns` / `task_timeout` / `turn_timeout` (structural) and `max_input_tokens` / `max_output_tokens` / `max_total_tokens` / `max_usd` (cumulative budget). Token/USD breaches abort with `FinalStatus.TOKEN_BUDGET_EXCEEDED` or `COST_BUDGET_EXCEEDED` (both `category == "failed"`). Structural caps are set from the CLI via `-D run_limits.max_turns=…` / `-D run_limits.task_timeout=…` / `-D run_limits.turn_timeout=…` (field-merged into `run_limits`); budget caps via `-D run_limits.max_usd=…` etc. or YAML. Layered config uses field-merge — a variant block overrides individual keys without replacing the task's block. -- **Early stop on criterion (opt-in)**: `run_limits.stop_early` (default off) ends a single-shot Claude run early once the run's **armed** criteria are decided, so a raised `max_turns` isn't wasted on the smoke flavor. A criterion is armed by `stop_when: pass|fail|decided|auto`; only criteria that can decide from a partial trajectory may arm (they declare a non-empty `live_stop_polarities` ClassVar and override `live_verdict` on `BaseCriterion` — currently `skill_triggered`, `command_executed`; CE025 enforces the two stay consistent). `decided` arms **both** polarities (the instance must be able to decide each); `auto` arms whichever polarities **this instance** can decide (its `live_decidable_polarities`) — the value to use when the decidable polarity is instance-dependent, e.g. `skill_triggered` under any-engagement, where a positive row (`skill_name == expected_skill`) can only live-pass and a distractor can only live-fail, so one static `stop_when` on a dataset-fanned criterion (whose positive/distractor role flips per row) cannot fit every row. The watcher's pass-stop fires when every **pass-armed** criterion live-passes (fail-armed distractors are not required to pass, and a row with zero pass-armed criteria — e.g. a negative row — never pass-stops on turn 0); the fail-stop fires on the first fail-armed criterion that live-fails. Precision trade-off: a pass-stop cuts the run once the positive is decided, so a distractor that would misfire on a *later* tool call is not observed (the frozen row scores as a clean pass) — an intentional precision-for-budget trade of the smoke flavor, so authoritative P/R/F1 must come from a `stop_early: false` run. It uses a cooperative `should_stop` seam on the Claude agent's between-messages guard (tool-call granularity, no SIGKILL) driven by `orchestration/early_stop.py::EarlyStopWatcher` (own `EventCollector` + stop rule). Live verdicts only *trigger* the stop; the standard `check_all` on the frozen trajectory is authoritative. An early-stopped run gates on the **armed subset** (`EvaluationResult.armed_criteria_passed`); a completed run gates on the full set. Every unsupported use (non-observable criterion, non-Claude agent, wrong polarity, no armed criterion, simulation mode) is an error at resolution (plan *and* run), and a runtime verdict bug **fails open** to a full run. Surfaces: `EarlyStopInfo` (reason + deciding criterion + when), report notes/badges, `stopped_early` run.json rows, and `EarlyStopped`/`EarlyStopReason` telemetry dims. Defaults off ⇒ behavior byte-for-behavior unchanged. +- **Early stop on criterion (opt-in)**: `run_limits.stop_early` (default off) ends a single-shot Claude run early once the run's **armed** criteria are decided, so a raised `max_turns` isn't wasted on the smoke flavor. A criterion is armed by `stop_when: pass|fail|decided|auto`; only criteria that can decide from a partial trajectory may arm (non-empty `live_stop_polarities` ClassVar + `live_verdict` override — currently `skill_triggered`, `command_executed`; CE025 keeps the two consistent). `decided` arms **both** polarities; `auto` arms whichever polarities **this instance** can decide — the value for dataset-fanned criteria whose positive/distractor role flips per row. Stop rule: the pass-stop fires when every **pass-armed** criterion live-passes (fail-armed distractors are not required to pass; zero pass-armed ⇒ never pass-stops); the fail-stop fires on the first fail-armed live-fail but is **deferred while any pass-armed criterion is undecided** — a distractor misfire must not truncate a positive row's recall signal, so the latched misfire fires once the positives resolve (or the run continues to the cap). A fail-stop is therefore verdict-preserving; a pass-stop can miss a *later* distractor misfire, so authoritative P/R/F1 comes from a `stop_early: false` run. Driven by `orchestration/early_stop.py::EarlyStopWatcher` through the Claude agent's cooperative `should_stop` seam (tool-call granularity, no SIGKILL); live verdicts only *trigger* the stop — the standard `check_all` on the frozen trajectory is authoritative. An early-stopped run gates on the **armed subset** (`EvaluationResult.armed_criteria_passed`); a completed run gates on the full set. Every unsupported use is a hard error at resolution (plan *and* run), and a runtime verdict bug **fails open** to a full run. Surfaces: `EarlyStopInfo`, report notes/badges, `stopped_early` run.json rows, `EarlyStopped`/`EarlyStopReason` telemetry dims. Worked rationale: docs/TASK_DEFINITION_GUIDE.md § `stop_early`. Defaults off ⇒ behavior byte-for-behavior unchanged. ## Success Criteria (14 types) diff --git a/docs/AB_EXPERIMENTS.md b/docs/AB_EXPERIMENTS.md index 629b93ae..56d483ae 100644 --- a/docs/AB_EXPERIMENTS.md +++ b/docs/AB_EXPERIMENTS.md @@ -292,11 +292,14 @@ flavor) and a `max_turns` generous enough for `e2e`; see [`stop_early`](TASK_DEFINITION_GUIDE.md#stop_early-opt-in-early-stop). This recipe ships as `experiments/early-stop-ab.yaml`. -Expect **identical pass/fail verdicts** between the two variants — an -early-stopped run is gated on the armed subset only, and the non-armed criteria -become advisory (clearly marked in the report), so the `smoke` flavor can't -"pass for free" — with the `smoke` variant significantly lower on turns, -duration, and tokens. +Expect the `smoke` variant significantly lower on turns, duration, and tokens. +Verdict parity with `e2e` is **one-sided**: an early-stopped run is gated on the +armed subset only (non-armed criteria become advisory, clearly marked in the +report), a **fail-stop** is verdict-preserving (it is deferred until every +pass-armed criterion has resolved), but a **pass-stop** cuts the run once the +positives are decided — a distractor that would only misfire on a later tool +call is never observed, so the frozen row scores as a clean pass. Authoritative +precision/recall therefore belongs on the `e2e` (`stop_early: false`) variant. ## Replicates (Statistical Power) diff --git a/docs/TASK_DEFINITION_GUIDE.md b/docs/TASK_DEFINITION_GUIDE.md index c1f494b2..c2de203a 100644 --- a/docs/TASK_DEFINITION_GUIDE.md +++ b/docs/TASK_DEFINITION_GUIDE.md @@ -339,7 +339,7 @@ success_criteria: - type: skill_triggered skill_name: date-teller expected_skill: date-teller - stop_when: decided # arm on pass OR definitive fail + stop_when: auto # arm whichever polarity this instance can decide - type: file_exists # not armed → advisory on an early-stopped run path: report.md ``` @@ -360,7 +360,13 @@ Semantics: needs every pass-armed criterion to pass — fail-armed distractors are not required to, and a row with **zero** pass-armed criteria (e.g. a negative row) never pass-stops; a **fail-stop** fires on the first fail-armed criterion that - live-fails. Only criteria that can decide from a partial trajectory (currently + live-fails, but is **deferred while any pass-armed criterion is still + undecided** — a distractor misfire on an early tool call must not cut a + positive row before its expected signal can appear (that would freeze a + would-be true positive as a false negative and deflate suite recall). The + misfire is latched, so the deferred fail-stop fires the moment every + pass-armed criterion decides; if none ever decides, the run simply continues + to the cap. Only criteria that can decide from a partial trajectory (currently `skill_triggered`, `command_executed`) may be armed — arming any other criterion is a hard error at resolution (plan *and* run), never a silent no-op. Decidability can also depend on a criterion's own fields: `command_executed` can @@ -374,9 +380,11 @@ Semantics: non-armed criteria become **advisory** and are clearly marked (report badge + per-criterion note + `stopped_early` row). A run that completes naturally is gated on the **full** set, as always. This is what lets one file serve both a - `smoke` flavor (`stop_early: true`) and an `e2e` flavor (`stop_early: false`) - with identical verdicts — see [AB_EXPERIMENTS.md](AB_EXPERIMENTS.md). Note: on a - **pass-stop** the run is cut once the positive is decided, so a distractor that + `smoke` flavor (`stop_early: true`) and an `e2e` flavor (`stop_early: false`) — + see [AB_EXPERIMENTS.md](AB_EXPERIMENTS.md). Verdict parity between the flavors + is one-sided: a **fail-stop** is verdict-preserving (the deferral above + guarantees every pass-armed signal was allowed to resolve first), but a + **pass-stop** cuts the run once the positives are decided, so a distractor that would misfire on a *later* tool call is not observed (the frozen row scores as a clean pass) — the smoke flavor trades some precision completeness for budget, so authoritative precision/recall belongs on the `stop_early: false` run. diff --git a/experiments/early-stop-ab.yaml b/experiments/early-stop-ab.yaml index f5116836..ba40e784 100644 --- a/experiments/early-stop-ab.yaml +++ b/experiments/early-stop-ab.yaml @@ -7,9 +7,11 @@ description: | decide when the `smoke` variant cuts off — as soon as the designated criteria are decided — while `e2e` runs every task to completion. - Expect identical pass/fail verdicts between the two variants (the armed subset - gates an early-stopped run; other criteria are advisory and clearly marked), - with the `smoke` variant significantly lower on turns, duration, and tokens. + Expect the `smoke` variant significantly lower on turns, duration, and tokens. + Verdict parity is one-sided: the armed subset gates an early-stopped run + (other criteria are advisory, clearly marked), a fail-stop preserves the e2e + verdict, but a pass-stop can miss a distractor that would only misfire later — + authoritative precision/recall belongs on the e2e variant. variants: - variant_id: e2e diff --git a/src/coder_eval/models/limits.py b/src/coder_eval/models/limits.py index 5a99dfae..abce752b 100644 --- a/src/coder_eval/models/limits.py +++ b/src/coder_eval/models/limits.py @@ -90,11 +90,14 @@ class RunLimits(BaseModel): stop_early: bool = Field( default=False, description=( - "Opt-in master switch for early-stop-on-criterion. When True, the run ends as " - "soon as the armed criteria (those with stop_when set) are decided mid-run - on " - "pass or on a definitive fail - so a raised max_turns is not wasted once the " - "measured signal has happened. Default False keeps behavior identical. Requires a " - "Claude single-shot task with at least one observable armed criterion; every " - "unsupported combination is rejected at resolution time." + "Opt-in master switch for early-stop-on-criterion. When True, the run ends early " + "once the armed criteria (those with stop_when set, incl. per-instance 'auto') " + "are decided mid-run: pass-stop when every pass-armed criterion live-passes, " + "fail-stop on the first fail-armed live-fail (deferred while any pass-armed " + "criterion is undecided, so a misfire never truncates the recall signal) - so a " + "raised max_turns is not wasted once the measured signal has happened. Default " + "False keeps behavior identical. Requires a Claude single-shot task with at least " + "one observable armed criterion; every unsupported combination is rejected at " + "resolution time." ), ) diff --git a/src/coder_eval/orchestration/early_stop.py b/src/coder_eval/orchestration/early_stop.py index 6182f97c..9469007d 100644 --- a/src/coder_eval/orchestration/early_stop.py +++ b/src/coder_eval/orchestration/early_stop.py @@ -31,16 +31,26 @@ criterion is decided, so a *fail-armed* criterion (e.g. a distractor) that would only misfire on a LATER tool call is never observed — the frozen trajectory then scores that row as a clean pass. This is an intentional precision-for-budget -trade of the opt-in "smoke" flavor (an already-visible misfire still fail-stops, -since fail-stop is evaluated before pass-stop each round); the authoritative +trade of the opt-in "smoke" flavor; the authoritative precision/recall must come from a non-early-stop (``stop_early: false``) run. + +Recall, by contrast, is never truncated: the fail-stop is DEFERRED while any +*pass-armed* criterion is still undecided, so a distractor misfire on an early +tool call cannot cut a positive row before its expected signal has had the +chance to appear (which would freeze a would-be TP as an FN and deflate +recall/F1). The misfire is not lost — the observable criteria latch +monotonically, so the deferred fail fires the moment every pass-armed criterion +decides (fail-stop is evaluated before pass-stop each round), and if none ever +decides the run simply continues to the cap. A row with zero pass-armed +criteria (e.g. a negative row stacking only distractors) has nothing to defer +for and fail-stops on the first misfire. """ from __future__ import annotations import logging import time -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, Literal, assert_never from coder_eval.models import EarlyStopInfo, EarlyStopReason from coder_eval.streaming.collector import EventCollector @@ -68,6 +78,30 @@ logger = logging.getLogger(__name__) +def _requested_polarities( + stop_when: Literal["pass", "fail", "decided", "auto"], decidable: frozenset[str] +) -> frozenset[str]: + """The polarities a ``stop_when`` value requests to arm, given what the instance can decide. + + The single source of truth for the ``stop_when`` -> polarity mapping — both + the resolution-time validator and the runtime watcher resolve through here, + so a value can never mean different things in the two places. ``pass``/ + ``fail`` request that single polarity; ``decided`` requests both; ``auto`` + requests exactly the instance's own decidable set (which is why it can + return the empty set: an instance that can decide neither polarity is a + dead arm, and the validator rejects it). ``assert_never`` makes widening the + ``stop_when`` Literal without updating this mapping a type error instead of + a silently inert arm. + """ + if stop_when == "auto": + return decidable + if stop_when == "decided": + return frozenset({"pass", "fail"}) + if stop_when == "pass" or stop_when == "fail": + return frozenset({stop_when}) + assert_never(stop_when) + + class EarlyStopConfigError(ValueError): """Raised when a task arms ``run_limits.stop_early`` in a way v1 cannot honor. @@ -131,7 +165,7 @@ def validate_early_stop(task: TaskDefinition) -> None: if not armed: raise EarlyStopConfigError( "run_limits.stop_early is armed but no success criterion sets stop_when; " - + "arming requires at least one stop criterion (e.g. stop_when: decided)." + + "arming requires at least one stop criterion (e.g. stop_when: auto)." ) # (3)+(4) Per armed criterion: observable, then the requested polarity is @@ -159,25 +193,23 @@ def validate_early_stop(task: TaskDefinition) -> None: # arm (a polarity this instance can never fire) would silently degrade to # a full run instead of erroring here. polarities = checker_cls.live_decidable_polarities(c) - if polarity == "auto": - # `auto` arms exactly the polarities THIS instance can decide, so the - # only invalid case is a dead arm (an instance that can decide - # neither) — otherwise there is nothing to arm and it would silently - # never fire. A non-empty decidable set is always fully armable by - # `auto`, so there is nothing further to validate for this criterion. - if not polarities: - raise EarlyStopConfigError( - f"criterion type {c.type!r} is armed (stop_when='auto') but this instance can " - + "decide no polarity mid-run; 'auto' requires at least one live-decidable " - + "polarity (its decidability can depend on the criterion's fields)." - ) - continue - needed = {"pass", "fail"} if polarity == "decided" else {polarity} - missing = sorted(needed - polarities) + requested = _requested_polarities(polarity, polarities) + # Dead arm: only `auto` can request the empty set (it requests exactly + # the instance's decidable polarities) — an instance that can decide + # neither has nothing to arm and would silently never fire. + if not requested: + raise EarlyStopConfigError( + f"criterion {c.type!r} ({c.description!r}) is armed (stop_when='auto') but this " + + "instance can decide no polarity mid-run; 'auto' requires at least one " + + "live-decidable polarity (its decidability can depend on the criterion's " + + "fields — e.g. command_executed can live-pass only with max_count unset + " + + "min_count>0, and live-fail only with max_count set)." + ) + missing = sorted(requested - polarities) if missing: supported = sorted(polarities) or "no polarities" raise EarlyStopConfigError( - f"criterion type {c.type!r} cannot decide polarity {missing} mid-run " + f"criterion {c.type!r} ({c.description!r}) cannot decide polarity {missing} mid-run " + f"(stop_when={polarity!r}) for this configuration; it supports {supported}. " + "Decidability can depend on the criterion's fields (e.g. command_executed " + "can live-pass only with max_count unset + min_count>0, and live-fail only " @@ -216,11 +248,10 @@ def __init__( self._task_id = task_id self._armed = armed # Per-instance resolved arming polarities, aligned with ``_armed``. Static - # for the run: a criterion armed ``pass``/``fail`` resolves to that single - # polarity, ``decided`` to both, and ``auto`` to whatever THIS instance can - # decide (its ``live_decidable_polarities``). The stop rule consults this, - # not the raw ``stop_when`` string, so a distractor armed ``auto`` (fail - # only) is not required to live-pass for a pass-stop. + # for the run, resolved through ``_requested_polarities`` (the single + # stop_when -> polarity mapping). The stop rule consults this, not the raw + # ``stop_when`` string, so a distractor armed ``auto`` (fail only) is not + # required to live-pass for a pass-stop. self._armed_polarities: list[frozenset[str]] = [ self._resolve_armed_polarities(criterion, checker) for criterion, checker in armed ] @@ -325,22 +356,17 @@ def disarmed(self) -> bool: @staticmethod def _resolve_armed_polarities(criterion: BaseSuccessCriterion, checker: BaseCriterion[Any]) -> frozenset[str]: - """The polarities this armed instance may fire, resolved from ``stop_when``. + """The polarities this armed instance may fire, via ``_requested_polarities``. - ``pass``/``fail`` -> that single polarity; ``decided`` -> both (validation - has already guaranteed the instance can decide both); ``auto`` -> the - instance's own ``live_decidable_polarities`` (validation has guaranteed it - is non-empty). ``None`` is never armed, but is mapped to the empty set - defensively so the caller need not special-case it. + Validation has already guaranteed the resolved set is non-empty and + decidable for every armed criterion. ``None`` is never armed, but is + mapped to the empty set defensively so the caller need not special-case + it. """ sw = criterion.stop_when - if sw == "auto": - return checker.live_decidable_polarities(criterion) - if sw == "decided": - return frozenset({"pass", "fail"}) - if sw in ("pass", "fail"): - return frozenset({sw}) - return frozenset() + if sw is None: + return frozenset() + return _requested_polarities(sw, checker.live_decidable_polarities(criterion)) def _evaluate(self, in_flight: CommandTelemetry | None = None) -> None: record = self._collector.build_turn_record() @@ -370,14 +396,23 @@ def _evaluate(self, in_flight: CommandTelemetry | None = None) -> None: ) return + pass_armed = [i for i, pol in enumerate(self._armed_polarities) if "pass" in pol] + # Fail-stop: first armed criterion (criteria order) that live-fails AND - # whose resolved arming permits fail decides the run. - for (criterion, _checker), verdict, armed_pol in zip( - self._armed, verdicts, self._armed_polarities, strict=True - ): - if verdict == "fail" and "fail" in armed_pol: - self._fire(EarlyStopReason.CRITERION_FAILED, criterion, tool_call_index=tool_call_index) - return + # whose resolved arming permits fail decides the run — but DEFERRED while + # any pass-armed criterion is still undecided. Cutting a positive row on a + # distractor misfire before its expected signal could appear would freeze a + # would-be TP as an FN (truncating the suite's recall); the misfire is + # latched by the criterion's own monotone semantics, so the deferred fail + # still fires the moment every pass-armed criterion decides, and a row with + # zero pass-armed criteria (a negative row) defers nothing. + if not any(verdicts[i] == "undecided" for i in pass_armed): + for (criterion, _checker), verdict, armed_pol in zip( + self._armed, verdicts, self._armed_polarities, strict=True + ): + if verdict == "fail" and "fail" in armed_pol: + self._fire(EarlyStopReason.CRITERION_FAILED, criterion, tool_call_index=tool_call_index) + return # Pass-stop: every PASS-ARMED criterion live-passes. Fail-armed criteria # (e.g. distractors armed ``auto`` -> fail only) are NOT required to pass — @@ -386,7 +421,6 @@ def _evaluate(self, in_flight: CommandTelemetry | None = None) -> None: # case: with zero pass-armed criteria (a negative row whose criteria are all # distractors) there is nothing to pass-stop on, so the run must continue to # the cap rather than firing on turn 0 with an empty ``all()``. - pass_armed = [i for i, pol in enumerate(self._armed_polarities) if "pass" in pol] if pass_armed and all(verdicts[i] == "pass" for i in pass_armed): # Deciding criterion = the last pass-armed (criteria order) whose verdict # flipped vs the previous round; fall back to the last pass-armed. diff --git a/tests/test_early_stop.py b/tests/test_early_stop.py index ce397670..71cb8c7d 100644 --- a/tests/test_early_stop.py +++ b/tests/test_early_stop.py @@ -1042,9 +1042,12 @@ def test_stacked_pass_stop_requires_all(self) -> None: assert watcher.info is not None assert watcher.info.reason == EarlyStopReason.CRITERION_PASSED - def test_stacked_wrong_skill_fail_stop(self) -> None: - # A positive (armed pass) + a distractor (armed fail). Engaging the - # distractor's skill fires the fail-stop while the positive stays undecided. + def test_stacked_wrong_skill_defers_fail_stop_until_positive_decides(self) -> None: + # The recall guard: a positive (armed pass) + a distractor (armed fail). + # The distractor misfiring FIRST must NOT stop — cutting here would freeze + # the would-be TP as an FN and deflate suite recall. The misfire is latched + # by the criterion's monotone semantics, so once the expected skill engages + # (no pass-armed criterion left undecided) the deferred fail-stop fires. watcher = _watcher( [ _skill_crit("date-teller", "date-teller", stop_when="pass"), @@ -1052,8 +1055,48 @@ def test_stacked_wrong_skill_fail_stop(self) -> None: ] ) _feed(watcher, _skill_events("weather-teller")) + assert watcher.should_stop() is False # positive undecided -> fail deferred + assert watcher.info is None + _feed(watcher, [_tool_end(_skill_cmd("date-teller", tool_id="d"))]) + assert watcher.should_stop() is True + assert watcher.info is not None + assert watcher.info.reason == EarlyStopReason.CRITERION_FAILED + assert watcher.info.deciding_criterion_description == "weather-teller activation" + + def test_fail_stop_precedes_pass_stop_same_round(self) -> None: + # Precedence pin (kills the block-swap mutation): ONE tool call engages + # both the expected skill and a distractor via file reads, so the positive + # live-passes and the distractor live-fails in the SAME evaluation round + # with no pass-armed criterion left undecided. Fail-stop is evaluated + # before pass-stop, so the round must record CRITERION_FAILED. + watcher = _watcher( + [ + _skill_crit("date-teller", "date-teller", stop_when="auto"), # positive -> pass + _skill_crit("weather-teller", "date-teller", stop_when="auto"), # distractor -> fail + ] + ) + both = _cmd("Bash", {"command": "cat skills/date-teller/SKILL.md skills/weather-teller/SKILL.md"}) + _feed(watcher, [_agent_start(), _turn_start(), _tool_end(both)]) + assert watcher.should_stop() is True assert watcher.info is not None assert watcher.info.reason == EarlyStopReason.CRITERION_FAILED + assert watcher.info.deciding_criterion_description == "weather-teller activation" + + def test_auto_positive_row_misfire_alone_never_stops(self) -> None: + # A positive row armed `auto` whose agent only ever touches wrong skills: + # the fail-stop stays deferred for the whole run (the positive never + # decides), so the run continues to the cap and full-trajectory scoring — + # never a truncated FN. + watcher = _watcher( + [ + _skill_crit("date-teller", "date-teller", stop_when="auto"), # positive -> pass + _skill_crit("weather-teller", "date-teller", stop_when="auto"), # distractor -> fail + ] + ) + _feed(watcher, _skill_events("weather-teller")) + _feed(watcher, [_turn_start(), _tool_end(_cmd("Bash", {"command": "echo hi"}))]) + assert watcher.should_stop() is False + assert watcher.info is None def test_auto_positive_pass_stops(self) -> None: # `auto` on a positive resolves to pass-armed: engaging the expected skill From b24e9e7ab4a4dbb010a14331addd2c37e5b85b3b Mon Sep 17 00:00:00 2001 From: mohsen-uipath <301015030+mohsen-uipath@users.noreply.github.com> Date: Fri, 24 Jul 2026 13:56:05 -0700 Subject: [PATCH 3/3] fix(early-stop): return assert_never explicitly to satisfy CodeQL CodeQL does not model typing.assert_never's Never return, so the bare call read as an implicit fall-through return mixed with explicit returns. 'return assert_never(...)' keeps the pyright exhaustiveness guarantee and makes every path explicit. --- src/coder_eval/orchestration/early_stop.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/coder_eval/orchestration/early_stop.py b/src/coder_eval/orchestration/early_stop.py index 9469007d..2ca5252f 100644 --- a/src/coder_eval/orchestration/early_stop.py +++ b/src/coder_eval/orchestration/early_stop.py @@ -99,7 +99,9 @@ def _requested_polarities( return frozenset({"pass", "fail"}) if stop_when == "pass" or stop_when == "fail": return frozenset({stop_when}) - assert_never(stop_when) + # `return` is redundant for control flow (assert_never never returns) but + # keeps every path explicit for analyzers that don't model `Never`. + return assert_never(stop_when) class EarlyStopConfigError(ValueError):