Skip to content

fix(early-stop): add stop_when 'auto' — per-instance arming + pass-armed-subset stop rule#51

Merged
mohsen-uipath merged 4 commits into
mainfrom
fix/early-stop-auto-polarity
Jul 24, 2026
Merged

fix(early-stop): add stop_when 'auto' — per-instance arming + pass-armed-subset stop rule#51
mohsen-uipath merged 4 commits into
mainfrom
fix/early-stop-auto-polarity

Conversation

@mohsen-uipath

Copy link
Copy Markdown
Contributor

Problem

Since v0.8.9, skill_triggered scores any engagement of the target skill, which narrows each criterion instance to a single mid-run-decidable polarity: a positive instance (skill_name == expected_skill) can only live-pass, and a distractor instance can only live-fail.

That broke early-stop arming for dataset-fanned suites. The criteria are written once in the task YAML but expanded per row, and a given instance's positive/distractor role flips from row to row — so no static stop_when value fits every row:

  • decided requires the instance to be able to decide both polarities → now rejected at validation for every instance.
  • pass / fail each fit only one role → wrong (and rejected) for the rows where the role flips.

Net effect: activation-style suites (e.g. the skills-repo suite, 23 stacked skill_triggered criteria × ~1,150 rows) could not use early stop at all.

Change

  • stop_when: auto — arms whichever polarities this instance can actually decide (its live_decidable_polarities). On a positive row the criterion arms pass; on a distractor row it arms fail. An auto that resolves to zero decidable polarities (a dead arm) stays a hard error at resolution, same as the other values.
  • Pass-stop rule now uses the pass-armed subset — the stop fires when every pass-armed criterion live-passes; fail-armed distractors are no longer required to pass. A vacuous guard ensures a row with zero pass-armed criteria (e.g. a negative row) never pass-stops on turn 0. Fail-stop is unchanged and still evaluated first.
  • Watcher resolves arming once into _armed_polarities at construction; the stop rule never inspects raw stop_when values.
  • Documented precision trade-off: a pass-stop freezes the run once the positive is decided, so a distractor that would misfire on a later tool call is not observed — an intentional precision-for-budget trade of the smoke flavor. Authoritative P/R/F1 must come from a stop_early: false run. Noted in the module docstring, CLAUDE.md, and TASK_DEFINITION_GUIDE.

Defaults unchanged: stop_early remains opt-in; pass/fail/decided semantics are untouched.

Testing

  • Unit coverage extended in tests/test_early_stop.py (+119 lines): auto-arming resolution per instance, pass-armed-subset stop rule, vacuous-pass guard on negative rows, dead-arm resolution errors.
  • Live validation against the downstream skill-activation suite (23 skill_triggered criteria, stop_when: auto, max_turns: 3, real claude-code agent): 8/8 positive rows pass-stopped at SDK turn 1 / tool call 1 with verdicts identical to a full run (23/23 criteria, score 1.000, ~$0.05/row); the negative row correctly never armed a pass-stop and ran to its caps with a clean 23/23 classification.

…med-subset stop rule

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.
@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown

Claude finished @mohsen-uipath's task in 1m 18s —— View job


Code Review in Progress

Todo List:

  • Read code review guidelines (.github/code_review.md)
  • Read project conventions (CLAUDE.md)
  • Analyze the full diff (git diff origin/main...HEAD)
  • Read and review each changed file:
    • .claude/harness-candidates.md
    • CLAUDE.md
    • docs/TASK_DEFINITION_GUIDE.md
    • src/coder_eval/models/criteria.py
    • src/coder_eval/orchestration/early_stop.py
    • tests/test_early_stop.py
  • Check cross-file consistency
  • Analyze design-level decisions
  • Complete review feedback

@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:51 (6 files) axis:1,2,3,4,5,6,7,8

Scope: pr:51 (6 files) axis:1,2,3,4,5,6,7,8 · branch fix/early-stop-auto-polarity (PR #51 head; local main at 947acd3) · f1a89dc · 2026-07-24T17:32Z · workflow variant

Change class: complex — adds a new stop_when: auto arming value and rewrites the early-stop pass-stop rule to a pass-armed subset, changing when a run is cut and therefore which frozen trajectory gets scored

coder_eval is in strong health — type safety, security, and error handling score a clean 10/10 and the architecture (single declarative merge resolver, plugin SPI, one EventCollector capture seam) is genuinely well-factored — but the real risk is concentrated in the new opt-in early-stop path, where stop_when: auto lets a fail-stop truncate a positive row into a false negative and lets truncated rows silently feed the suite_thresholds CI gate, so identical agent output can score differently under stop_early: true; bottom line: the code is production-grade, and what needs fixing is the harness's own measurement integrity, not its quality.

Summary

Axis Score 🔴 🟠 🟡 🔵 Top Issue
1. Code Quality & Style 9 / 10 0 0 2 0 stop_when: decided is unsatisfiable for every shipping criterion, yet it is the guide's only copy-paste stop_early example and the validator's own suggested fix (hard resolution error)
2. Type Safety 10 / 10 0 0 0 0
3. Test Health 9.5 / 10 0 0 1 0 New early-stop arming/stop-rule branches lack mutation-killing tests: decided arm never executed, pass_armed index remap only tested at index 0, fail-before-pass precedence untested
4. Security 10 / 10 0 0 0 0
5. Architecture & Design 9.8 / 10 0 0 0 2 Rewritten Verdict bullet asserts "identical verdicts" and negates it in the next sentence
6. Error Handling & Resilience 10 / 10 0 0 0 0
7. API Surface & Maintainability 9.7 / 10 0 0 0 3 New auto dead-arm EarlyStopConfigError identifies the criterion only by c.type (and drops the sibling error's concrete fix hint), so it is unactionable in stacked/fanned suites
8. Evaluation Harness Quality 8 / 10 0 2 0 0 stop_when: auto fail-arms every distractor instance, so on a positive row an exploratory read of a wrong skill fail-stops the run before the expected skill can load — the frozen row scores TP→FN and per-skill recall.yes/f1.yes in the suite rollup is deflated (effectively reverting the any-engagement policy to first-engagement whenever stop_early: true)

Overall Score: 9.5 / 10 · Weakest Axis: Evaluation Harness Quality at 8 / 10
Totals: 🔴 0 · 🟠 2 · 🟡 3 · 🔵 5 across 8 axes.

Blockers

  1. [Axis 8] stop_when: auto fail-arms every distractor instance, so on a positive row an exploratory read of a wrong skill fail-stops the run before the expected skill can load — the frozen row scores TP→FN and per-skill recall.yes/f1.yes in the suite rollup is deflated (effectively reverting the any-engagement policy to first-engagement whenever stop_early: true) (src/coder_eval/orchestration/early_stop.py:378) — auto resolves to live_decidable_polarities, which for every skill_triggered distractor instance (skill_name != expected_skill) is exactly frozenset({"fail"}) (src/coder_eval/criteria/skill_triggered.py:210-211: return frozenset({"pass"}) if expected_yes else frozenset({"fail"})). The stop rule then fires on it unconditionally: if verdict == "fail" and "fail" in armed_pol: (early_stop.py:378), evaluated BEFORE the pass-stop. On the PR's motivating activation suite (~23 stacked criteria per row, all armed with one fanned stop_when: auto), a positive row where the agent reads .agents/skills/<wrong-skill>/SKILL.md at tool call 1 fail-stops the run right there — _engaged_skill_names matches any skills/<name>/ substring in any string parameter, and live_verdict returns "fail" for that distractor. The frozen trajectory then scores the row's POSITIVE criterion score=0.0, observed_label='no', expected_label='yes' (skill_triggered.py:145-152), i.e. a false negative, where the same trajectory under stop_early: false would have reached the expected skill and scored 1.0 / TP. overlay_classification_metrics computes recall = tp / (tp + fn) (criteria/_classification_aggregate.py:62) from exactly those labels, so per-skill recall/F1 in the suite rollup collapses in the smoke flavor. This directly defeats the documented intent of the criterion the feature was built for — skill_triggered.py's live_verdict docstring states "a wrong skill engaged first no longer live-fails a positive row — the run keeps going so the expected skill can still load" — and it is NOT covered by the new trade-off note, which discloses only the pass-stop precision loss (early_stop.py:30-36). Fix options (pick one, the choice is an open question): (a) suppress the fail-stop while any pass-armed criterion on the row is still undecided, so the recall signal is always allowed to resolve; (b) give auto a narrowing form (e.g. auto-pass) so a fanned suite can arm per-instance on the pass side only; (c) at minimum, extend the module docstring + guide to state that a fail-stop truncates the recall signal and record deciding + the undecided pass-armed criteria in EarlyStopInfo so those rows can be excluded from recall.
  2. [Axis 8] suite_thresholds gate (drives CLI exit code) pools truncated early-stopped rows with complete rows and carries no suite-level marker — the trade-off auto newly makes reachable is recorded only in prose (early_stop.py:30-36, TASK_DEFINITION_GUIDE.md:259-274) (src/coder_eval/orchestration/early_stop.py:30) — The new note says "the frozen trajectory then scores that row as a clean pass ... the authoritative precision/recall must come from a non-early-stop (stop_early: false) run" (early_stop.py:30-36), but nothing mechanically records or separates truncated rows at the aggregate level, and auto is precisely what first makes an armed dataset-fanned suite possible (pre-PR, one static stop_when on a fanned criterion whose role flips per row was rejected at resolution for most rows — see tests/test_early_stop.py:488-493). Verified: _compute_suite_rollup feeds EVERY row into checker.aggregate(criterion, per_rows) with no early-stop filter (src/coder_eval/reports.py:778-805), SuiteRollup has no truncated-row field (src/coder_eval/models/results.py:786-814), and SuiteRollup.passed is documented as "Drives CLI exit code for dataset-backed tasks" (results.py:808-813) and is consumed as such (failed_gates = [r for r in rollups if not r.passed], src/coder_eval/cli/run_command.py:700-708). So a suite_thresholds: {precision.yes: 0.9} gate can go green on a smoke run that a full run fails. Compounding it, the armed-subset verdict gates on criteria that were never observed: armed_criteria_passed filters on c.stop_when is not None (results.py:656-658), so on a pass-stop the ~22 fail-armed distractors are gating and trivially score 1.0, and reports_html._render_criteria marks only NON-armed criteria advisory (elif early_stop is not None and f"{cr.criterion_type}: {cr.description}" not in armed:, reports_html.py:574-582), so an unobserved distractor renders as a genuine armed pass. Add a suite-level surface (e.g. SuiteRollup.rows_stopped_early + a CriterionAggregate metric/marker, mirrored in suite.md) and either exclude truncated rows from classification metrics or fail the gate when a threshold-gated criterion was unobserved on any row; the guide's own observability table (docs/TASK_DEFINITION_GUIDE.md:266-274, "every early-stopped run is flagged everywhere so analysis never compares a truncated run against a full one") lists only per-row markers and should be extended.

Non-blocking, but please consider before merge

  1. [Axis 1] stop_when: decided is unsatisfiable for every shipping criterion, yet it is the guide's only copy-paste stop_early example and the validator's own suggested fix (hard resolution error) (src/coder_eval/models/criteria.py:126) — Both observable checkers narrow live_decidable_polarities to at most ONE polarity per instance — skill_triggered.py:210-211 returns frozenset({"pass"}) if expected_yes else frozenset({"fail"}) and command_executed.py:61-67 adds "pass" xor "fail" (or neither). So needed = {"pass", "fail"} if polarity == "decided" else {polarity} (early_stop.py:175) can never be satisfied: stop_when: decided ALWAYS raises EarlyStopConfigError today, which the PR's own suite asserts (tests/test_early_stop.py:488-491, :519, :523-525). Consequences the diff leaves in place: (a) the guardrail message at early_stop.py:132-135 still advertises the guaranteed-invalid value — "arming requires at least one stop criterion (e.g. stop_when: decided)." — so a user following the error hits a second error; (b) the widened Literal now ships four values of which, for every in-tree criterion instance, exactly one is valid and auto always resolves to it (pass/fail differ from auto only as a strict-intent assertion, decided is dead). Per CLAUDE.md's greenfield rule ("No worries about backward compatibility") the KISS option is to redefine decided as the per-instance decidable set (i.e. what auto now does) and drop the fourth member; if auto is kept, at minimum change the example in the early_stop.py:134 message to stop_when: auto and note in the stop_when description + docs/TASK_DEFINITION_GUIDE.md:233 that no shipped criterion instance can satisfy decided.
  2. [Axis 1] stop_when → armed-polarity mapping duplicated between validate_early_stop and EarlyStopWatcher._resolve_armed_polarities (and validate_early_stop grows to D(21)) (src/coder_eval/orchestration/early_stop.py:162) — radon on the PR-head file reports F 81:0 validate_early_stop - D (21) (was C (19) on main) — the new auto block at lines 162-174 with its continue is what tips it over. The same block also creates a second source of truth for the polarity mapping: the validator computes needed = {"pass", "fail"} if polarity == "decided" else {polarity} (line 175) while the watcher re-derives the identical mapping in _resolve_armed_polarities (lines 336-343: if sw == "auto": ... if sw == "decided": return frozenset({"pass", "fail"}) ... if sw in ("pass", "fail"): return frozenset({sw})). Adding or renaming a polarity value now requires editing both. Hoist one module-level helper, e.g. def _requested_polarities(stop_when: str, decidable: frozenset[str]) -> frozenset[str] returning decidable for auto, and have the validator do requested = _requested_polarities(polarity, polarities) then if not requested: <dead-arm error> / if requested - polarities: <undecidable error> — that removes the auto special-case branch (dropping the function back under D) and makes EarlyStopWatcher._armed_polarities a call to the same function. It also removes the third restatement of the mapping in prose: the same rule is currently written out in the __init__ comment (lines 218-223) and again in the _resolve_armed_polarities docstring (lines 328-335) directly above the eight lines of code that say it.
  3. [Axis 3] New early-stop arming/stop-rule branches lack mutation-killing tests: decided arm never executed, pass_armed index remap only tested at index 0, fail-before-pass precedence untested (tests/test_early_stop.py:1117) — The new module docstring at early_stop.py:34-35 makes the ordering load-bearing for the documented trade-off: trade of the opt-in "smoke" flavor (an already-visible misfire still fail-stops, / since fail-stop is evaluated before pass-stop each round), and CLAUDE.md + docs/TASK_DEFINITION_GUIDE.md repeat it. Nothing pins it: physically swapping the fail-stop block (early_stop.py:373-380) with the pass-stop block (:382-398) still passes 129/129 in tests/test_early_stop.py. The uncovered case is a single _evaluate round in which a pass-armed criterion live-passes AND a fail-armed criterion live-fails — under the swapped order that row records EarlyStopReason.CRITERION_PASSED instead of CRITERION_FAILED, corrupting EvaluationResult.early_stop.reason, the report badge/notes, and the EarlyStopReason telemetry dim for exactly the misfire rows the activation suite exists to count. The nearest existing tests do not reach it: test_stacked_wrong_skill_fail_stop (line 1045) leaves the positive undecided, and test_auto_negative_row_misfire_fail_stops (line 1102) has zero pass-armed criteria, so in both the pass-stop branch cannot fire regardless of order. Add a test where both polarities are decided in one round (criteria [_skill_crit("date-teller","date-teller",stop_when="auto"), _skill_crit("weather-teller","date-teller",stop_when="auto")], fed so both skills are engaged before the deciding evaluation) and assert reason == EarlyStopReason.CRITERION_FAILED.

Nits

  1. [Axis 5] Rewritten Verdict bullet asserts "identical verdicts" and negates it in the next sentence (docs/TASK_DEFINITION_GUIDE.md:258) — The bullet keeps the pre-PR invariant claim — lines 256-258: "This is what lets one file serve both a smoke flavor (stop_early: true) and an e2e flavor (stop_early: false) with identical verdicts" — and then immediately contradicts it in the sentence the PR appended: "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)". Those cannot both hold: a smoke row that pass-stops scores PASS where the e2e row would score FAIL. The claim was true before this change for the observable criteria — a distractor can never live-pass, so the old all_permit_pass = all(c.stop_when in ("pass", "decided") ...) veto meant a mixed pass/fail arming could only ever fail-stop, and a fail-stop is verdict-preserving under the monotone any-engagement latch. The new pass-armed-subset rule is what breaks it (and not only for auto — the PR's own test_mixed_static_arming_pass_stops_ignoring_fail_armed shows static pass + fail arming now pass-stops where it previously could not, contradicting the PR description's "pass/fail/decided semantics untouched"). Delete or qualify the "identical verdicts" clause so the guide states one rule: verdict-preserving on a fail-stop, precision-optimistic on a pass-stop.
  2. [Axis 5] Third index-aligned parallel list in EarlyStopWatcher, with raw positional indexing replacing the zip(strict=True) guard (src/coder_eval/orchestration/early_stop.py:224) — The watcher now carries three lists that must stay positionally aligned — self._armed (line 217), self._armed_polarities (line 224, built by a comprehension over armed), and self._prev_verdicts (line 236, ["undecided"] * len(armed)) — plus the locally built verdicts. The fail-stop loop keeps the safety net (zip(self._armed, verdicts, self._armed_polarities, strict=True), lines 375-377), but the new pass-stop branch drops it for raw index arithmetic: line 389 pass_armed = [i for i, pol in enumerate(self._armed_polarities) if "pass" in pol], then verdicts[i], self._prev_verdicts[i], self._armed[i][0] and self._armed[pass_armed[-1]][0] (lines 390-396). Lengths are structurally guaranteed today (all three derive from armed, and _prev_verdicts is only reassigned to a same-length verdicts at line 401), so there is no bug — but this is the list[i] ⟷ list[i] coupling shape this repo treats as a lint-rule-worthy smell, and it is now three-wide with no strict=True on the hot branch. Collapse to one list of records (a small frozen dataclass or NamedTuple _ArmedEntry(criterion, checker, polarities) plus a mutable prev_verdict), so pass_armed becomes a filter over entries and the alignment is unrepresentable-if-wrong rather than assert-if-remembered.
  3. [Axis 7] New auto dead-arm EarlyStopConfigError identifies the criterion only by c.type (and drops the sibling error's concrete fix hint), so it is unactionable in stacked/fanned suites (src/coder_eval/orchestration/early_stop.py:169) — The new branch raises 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)." (early_stop.py:169-173). Executed against a task stacking two command_executed criteria — a valid one (description="ran pytest", min_count=1) and a dead arm (description="ran ruff (BAD ARM)", min_count=0, max_count=None), both stop_when: auto — the message emitted is exactly: criterion type 'command_executed' 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). It names neither the offending criterion's description (a required field on every criterion, and the natural identifier — the watcher already uses f"{c.type}: {c.description}" for EarlyStopInfo.armed_criteria, early_stop.py:412) nor its index, so with stacked same-type criteria the author must bisect by hand. It is also less actionable than the pre-existing polarity error 12 lines below, which does spell out the fix ("command_executed can live-pass only with max_count unset + min_count>0, and live-fail only with max_count set", early_stop.py:182-184). Fix: include c.description!r in the auto message and append the same concrete field guidance (e.g. "set min_count>0 with max_count unset to make pass decidable, or set max_count to make fail decidable").
  4. [Axis 7] _resolve_armed_polarities's non-exhaustive fall-through silently returns an empty (inert) polarity set instead of failing loud (src/coder_eval/orchestration/early_stop.py:343) — _resolve_armed_polarities ends with a bare return frozenset() (early_stop.py:343) after an if-chain over "auto" / "decided" / ("pass", "fail"), documented as covering only None ("None is never armed, but is mapped to the empty set defensively", :333-334). Because the parameter is a stringly-typed read of criterion.stop_when into frozenset[str], pyright gives no exhaustiveness check: adding a fifth value to Literal["pass", "fail", "decided", "auto"] (models/criteria.py:126) plus a validate_early_stop acceptance branch — precisely the two-site edit this PR just performed — and forgetting this mapper yields an empty polarity set, so the criterion is armed, passes validation, appears in EarlyStopInfo.armed_criteria, and can never fire either branch ("fail" in armed_pol at :378 and "pass" in pol at :389 are both False). That is exactly the silent no-op the module's own contract forbids ("Rejects every configuration v1 cannot honor as a hard error, so an unsupported arming is never a silent no-op", :7-9). Fix: replace the trailing return frozenset() with an explicit if sw is None: return frozenset() followed by typing.assert_never(sw) (or a raise naming the unhandled value), so a future value fails loudly at the point of omission instead of degrading to a dead arm.
  5. [Axis 7] The CLAUDE.md early-stop bullet grew to 2589 chars — 1.9x the next-longest bullet — becoming a fourth near-verbatim copy of the polarity semantics with no single source of truth (CLAUDE.md:144) — Measured at PR HEAD: awk 'NR==144{print length($0)}' CLAUDE.md = 2589 chars, up from 1438 on main (+80%), against 1395 for the next-longest bullet in the file (line 141). The polarity/pass-armed-subset/precision-trade-off text now exists in four places that must be kept in sync by hand: models/criteria.py:128-146 (the field description, the repo's declared SSOT), CLAUDE.md:144, docs/TASK_DEFINITION_GUIDE.md:231-262, and the early_stop.py:30-36 module docstring. Finding #2 is direct evidence the drift is real, not hypothetical — this PR updated three of those copies and left the downstream "identical verdicts" restatements stale in the same commit. Since CLAUDE.md is prepended to every assistant session, the cost is also per-call context. Fix: compress CLAUDE.md:144 back toward its neighbours' length — keep the invariant (armed subset gates an early-stopped run; auto resolves per instance; pass-stop needs only the pass-armed subset; authoritative P/R needs stop_early: false) and delegate the worked skill_triggered positive/distractor rationale to the single detailed home at docs/TASK_DEFINITION_GUIDE.md#stop_early-opt-in-early-stop, which the bullet should link rather than restate.

What's Missing

Parallel paths:

  • 🟠 The PR added the pass-stop precision caveat to docs/TASK_DEFINITION_GUIDE.md, CLAUDE.md and the early_stop.py docstring but left the two A/B surfaces that make the opposite promise untouched: docs/AB_EXPERIMENTS.md:259-264 still says "Expect identical pass/fail verdicts between the two variants ... so the smoke flavor can't 'pass for free'", and the shipped recipe experiments/early-stop-ab.yaml:10-12 repeats it verbatim in its description. Under the new pass-armed-subset rule a mixed-arming row pass-stops with its fail-armed distractors unobserved (scoring 1.0), so the smoke flavor CAN pass for free — both surfaces need the same qualification (verdict-preserving on a fail-stop, precision-optimistic on a pass-stop). (trigger: docs/TASK_DEFINITION_GUIDE.md) (restates: Axis 5: Rewritten Verdict bullet asserts "identical verdicts" and negates it in the next sentence)
  • 🟡 RunLimits.stop_early's own field description (src/coder_eval/models/limits.py:90-99) — the user-facing SSOT rendered in schema/-D help and a fifth copy of the semantics — was not updated: it still says 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", which no longer describes the pass-armed-subset pass-stop, and it never mentions auto. The PR updated four of the five copies (criteria.py description, CLAUDE.md, the guide, the module docstring) and missed this one — concrete evidence that the duplicated prose drifts on every edit. (trigger: src/coder_eval/models/criteria.py) (restates: Axis 7: The CLAUDE.md early-stop bullet grew to a fourth near-verbatim copy of the polarity semantics with no single source of truth)
  • 🟡 coder-eval plan was not taught about dataset fan-out, even though auto exists specifically for fanned suites: plan_command.py:105-138 calls load_taskresolve_task_for_variantvalidate_early_stop on the unexpanded task, whereas the run path validates post-expansion (orchestration/experiment.py:625 then :673). Verified empirically: a fanned skill_triggered with skill_name: ${row.skill} / expected_skill: ${row.expected_skill} resolves to live_decidable_polarities == frozenset({'fail'}) because the two literal placeholders differ — so plan validates a fictitious distractor. Consequences: plan can never catch a per-row dead arm nor preview which polarity a row will arm (the one thing an author of a 1150-row suite needs), and a fanned criterion armed with static pass spuriously fails plan with exit 1 while the real run is valid. (trigger: src/coder_eval/orchestration/early_stop.py)

Tests:

  • 🟠 tests/test_early_stop.py contains zero occurrences of dataset (verified grep -c dataset = 0), so the PR's entire motivating scenario — ONE criterion fanned over rows whose positive/distractor role flips per row — has no test. The closest new case, test_auto_stacked_activation_accepts (:597), hand-stacks static criteria in a single task. Missing: an expand_dataset → per-row validate_early_stop → per-row EarlyStopWatcher._armed_polarities test asserting the SAME criterion resolves pass-armed on a positive row and fail-armed on a distractor row (the exact behavior auto was added to provide, and the only place the placeholder-vs-expanded distinction above would be caught). (trigger: tests/test_early_stop.py)
  • 🟡 auto makes the documented live_decidable_polarities ⊆ live_stop_polarities invariant (criteria/base.py:247) load-bearing — _resolve_armed_polarities arms straight from the per-instance set, bypassing the class-level live_stop_polarities gate at early_stop.py:150 — yet the invariant is still only asserted by two hand-written per-criterion loops (tests/test_early_stop.py:418 for command_executed, :449 for skill_triggered). No registry-wide parametrized test over every registered checker was added, so a future/plugin criterion whose override widens the set would arm a polarity its live_verdict never emits: validation accepts it and the arm is a silent no-op, precisely what the module contract forbids. (trigger: src/coder_eval/orchestration/early_stop.py)
  • 🟡 TestOrchestratorEarlyStopWiring (tests/test_early_stop.py:1355-1500) gained no case using auto or mixed arming, so nothing pins the end-to-end consequence of the new pass-stop rule: on a pass-stop the never-observed fail-armed distractors are still counted as gating passes by armed_criteria_passed (models/results.py:656-664) and render as genuine armed passes in HTML (reports_html.py:582, which marks only NON-armed criteria advisory). The new behavior is asserted only at watcher level (verdict + reason), never at the verdict/report level where it actually changes a row's meaning. (trigger: tests/test_early_stop.py) (restates: Axis 8: suite_thresholds gate pools truncated early-stopped rows with complete rows and carries no suite-level marker)

Downstream consumers:

  • 🟠 The pass-armed-subset rule changes which trajectories the classification labels are computed from, but no consumer of those labels was updated: _compute_suite_rollup still feeds every row into checker.aggregate with no early-stop filter (reports.py:775-807), overlay_classification_metrics computes recall = tp/(tp+fn) / precision from the truncated labels (criteria/_classification_aggregate.py:62), and SuiteRollup.passed — documented as driving the CLI exit code (models/results.py:808-814) — is consumed unchanged at cli/run_command.py:701. So a suite_thresholds gate can go green (or red) on truncated evidence with nothing in suite.json / suite.md indicating any row was cut. (trigger: src/coder_eval/orchestration/early_stop.py) (restates: Axis 8: suite_thresholds gate pools truncated early-stopped rows with complete rows and carries no suite-level marker)
  • 🟡 Arming is now a two-notion concept and only one notion was updated: the watcher resolves per-instance polarities into _armed_polarities (early_stop.py:224), while the frozen-trajectory gate still derives "armed" from a raw polarity-blind c.stop_when is not None read (models/results.py:656-657), as does EarlyStopWatcher.for_task (:264) and the HTML advisory set (reports_html.py:574). The gate therefore cannot tell that a fail-armed distractor was structurally unobservable at the cut — the same raw-.stop_when-read class the PR's own new harness-candidate proposes banning inside the stop rule, left in place in the gate that decides final_status. (trigger: src/coder_eval/orchestration/early_stop.py) (restates: Axis 8: suite_thresholds gate pools truncated early-stopped rows with complete rows and carries no suite-level marker)

Display & mapping dicts:

  • 🟡 No display surface was extended for the new per-instance arming, so auto is invisible everywhere after the run: EarlyStopInfo.armed_criteria is still a flat list of "type: description" strings (models/results.py:425-427) with no polarity, run.json rows carry only stopped_early + early_stop_reason (reports_experiment.py:145-146), and the run.md note (reports.py:421-427), HTML badge (reports_html.py:348) and advisory marker (reports_html.py:574-582) all treat "armed" as one undifferentiated set. A grep for stop_when outside models/criteria.py + early_stop.py finds no renderer at all, so there is no way — in any report, JSON row, or telemetry dim — to see which polarity a criterion actually armed or that a listed armed criterion was never observable. (trigger: src/coder_eval/orchestration/early_stop.py)

Daily/nightly:

  • 🟠 The stated motivation is the out-of-tree nightly activation suite (~23 criteria × ~1150 rows), yet the blast radius on that pipeline is unstated and unwired: no in-tree task or experiment adopts stop_when: auto (grep over tasks/ finds no stop_when at all; experiments/early-stop-ab.yaml only flips stop_early), so nothing exercises the new value through the run path, and the PR does not say whether the nightly switches to stop_early: true + auto. It matters because the run-record schema is unchanged while the meaning of the classification columns changes for stop_early runs — every positive row whose agent touches a wrong skill first now fail-stops and books TP→FN, deflating the per-skill recall/F1 the nightly rollup (and any suite_thresholds CI gate reading it) reports. (trigger: CLAUDE.md) (restates: Axis 8: stop_when: auto fail-arms every distractor instance, deflating per-skill recall/f1)

Harness & Lint Improvements

Static checks (lint / type):

  • [ce-lint] CE032 — doc YAML examples must survive the RESOLUTION-time validators, not just Pydantic. Extend tests/lint/doc_examples.py (today's CE029 only calls model_validate) with a second pass: any fenced yaml block containing stop_when:/stop_early: is synthesized into a minimal TaskDefinition (inject placeholder task_id/description/initial_prompt, a per-criterion description, and agent: {type: claude-code} when absent — the guide's snippets are fragments, which is why CE029 skips them today) and then run through orchestration/early_stop.validate_early_stop; any raise is a lint failure. Wire as TestCE032DocExamplesResolve in tests/test_custom_lint.py beside TestCE029DocYamlExamples (tests/lint/runner.py is AST/.py-only, so this belongs in the doc-surface pytest family with CE027–CE031). EMPIRICALLY VERIFIED against PR head f1a89dc: exactly one block fires — docs/TASK_DEFINITION_GUIDE.md:214-225 -> "criterion type 'skill_triggered' cannot decide polarity ['fail'] mid-run (stop_when='decided') … it supports ['pass']" — and zero other doc blocks, so false-positive load is 0. Prevents: Finding 1 (stop_when: decided is unsatisfiable for every in-tree criterion, yet it is the guide's only copy-pasteable stop_early example). Generalizes to any future config key whose real gate is a resolution-time validator rather than a Pydantic validator.
  • [ce-lint] CE033 — an advertised config value must be satisfiable by some in-tree criterion (arming matrix as SSOT). Add a curated arming-matrix fixture (observable criterion shapes x stop_when values: skill_triggered positive AND distractor, command_executed with min_count>0 / max_count-set / neither) that derives the satisfiable stop_when set by running validate_early_stop; then scan user-facing message string literals in src/ and docs prose for the stop_when: <value> pattern and fail when a named value is not in that set. Would have caught src/coder_eval/orchestration/early_stop.py:134 — "arming requires at least one stop criterion (e.g. stop_when: decided)." — i.e. the validator's own suggested fix is a guaranteed second error. DESIGN NOTE: the existing MINIMAL_PAYLOADS dict (tests/test_success_criterion_union.py:31, already parity-asserted against the union) is NOT sufficient alone — its command_executed payload decides no polarity and its skill_triggered payload is a positive, so fail (legitimately satisfiable by a distractor) would false-positive; hence the curated matrix, which also serves as CE032's fixture set. Prevents: Finding 1's error-message half (early_stop.py:134 advertising decided), plus any future stop_when/backend/driver value named in an error hint or doc that no shipped implementation can honor.
  • [ce-lint] CE034 — Literal-field string dispatch must be centralized in one resolver. Registry-driven AST rule (same shape as CE018's closed-enum denylist rule): given a table of (model Literal field -> owning function), forbid any ==/!=/in (...) comparison of that field, or of a local directly assigned from it, against a string constant outside the owner. Seed entry: BaseSuccessCriterion.stop_when -> orchestration/early_stop._requested_polarities (the hoisted helper Finding 2 recommends). At PR head this fires on early_stop.py:162 (polarity == "auto"), :175 (polarity == "decided") and :336-341 (_resolve_armed_polarities's chain); after the DRY fix only the owner remains. stop_when is not None arming filters (orchestrator.py:1428, models/results.py:657, early_stop.py:130/251) are identity checks and unaffected. New file tests/lint/rules/ce034_literal_dispatch_centralized.py, wired in tests/lint/runner.py + the [tool.ruff.lint].external list. CONSIDERED AND REJECTED alongside it: a ruff C901 gate for the radon D(21) growth the finding cites — measured, ruff scores validate_early_stop at 10 (up from 8) while the tree max is 24 (_build_argv), so no tree-passing threshold would fire; the duplication rule is the enforceable part. Prevents: Finding 2 (stop_when->polarity mapping duplicated between validate_early_stop and EarlyStopWatcher._resolve_armed_polarities, plus two prose restatements). Also a precondition for the pyright check below: it guarantees exactly ONE exhaustive chain exists to terminate in assert_never.
  • [pyright] Terminate the single stop_when resolver with typing.assert_never — no config change required — and optionally add reportMatchNotExhaustive = "error" to [tool.pyright] if it is written as a match. Replace the bare return frozenset() fall-through at src/coder_eval/orchestration/early_stop.py:343 with an explicit if sw is None: return frozenset() followed by assert_never(sw). EMPIRICALLY VERIFIED with this repo's own pyright and typeCheckingMode = "standard": the current bare-return form still type-checks clean after adding a 5th member to the Literal, while the assert_never form errors immediately — 'Argument of type "Literal['newvalue']" cannot be assigned to parameter "arg" of type "Never" in function "assert_never"'. So the exhaustiveness gate is available today at zero config cost. Prevents: Finding 7 (non-exhaustive fall-through silently returns an inert empty polarity set — a criterion that is armed, passes validation, is listed in EarlyStopInfo.armed_criteria, and can never fire, exactly the silent no-op the module docstring forbids). Also guards the two-site edit pattern this PR performed (widen the Literal + add a validator branch, forget the mapper).
  • [ce-lint] CE035 — no two parallel containers indexed by the same integer variable. AST rule: inside one function, flag >=2 distinct container expressions subscripted by the same Name when that Name is bound as an integer index (a range()/enumerate() loop target, or a loop over a list comprehended from an enumerate index — the one-hop dataflow needed to see pass_armed = [i for i, pol in enumerate(...)] then for i in pass_armed). MEASURED false-positive load on main: exactly 1 site — src/coder_eval/agents/claude_code_agent.py:99 _distribute_output_tokens (floors/order/out/raw/weights all indexed by i), a genuine local parallel-array algorithm that takes a # noqa: CE035 debt marker. On PR head it additionally fires on src/coder_eval/orchestration/early_stop.py:345 _evaluate (self._armed / self._prev_verdicts / verdicts indexed by i). This is the complement to bugbear B905 (already active via select = ["B"]), which guards zip() without strict= but is silent the moment code abandons zip for raw index arithmetic — precisely what the new pass-stop branch did. Prevents: Finding 5 (third index-aligned parallel list in EarlyStopWatcher: the fail-stop loop keeps zip(..., strict=True) but the new pass-stop branch drops it for verdicts[i] / self._prev_verdicts[i] / self._armed[i][0]). Pushes the fix toward one record type where misalignment is unrepresentable.
  • [ce-lint] CE036 — a validation error raised while iterating user-authored items must name the offending item. AST rule: a raise inside a for over success_criteria (direct, or via a name bound from a comprehension over it) whose message interpolates neither the item's .description nor its index is a violation. MEASURED: fires on 4 in-tree sites at PR head — src/coder_eval/orchestration/early_stop.py:151, :169 (the new auto dead-arm message), :179, and src/coder_eval/models/tasks.py:572 (success_criteria[{c.type!r}].suite_thresholds requires a dataset) — 3 pre-existing plus the new one, and every one is unactionable in a stacked or dataset-fanned suite where N same-type criteria coexist (the watcher already uses f"{c.type}: {c.description}" as the identity at early_stop.py:412, so the identifier exists). Slots in as tests/lint/rules/ce036_validation_error_names_item.py. Prevents: Finding 6 (the new stop_when='auto' dead-arm EarlyStopConfigError identifies the criterion only by c.type, forcing hand-bisection in stacked suites), plus the same defect in the 3 pre-existing messages it drags in.
  • [ce-lint] CE037 — cap CLAUDE.md top-level bullet length so detail is delegated, not duplicated. Markdown check: fail when any line starting with - in CLAUDE.md exceeds ~1500 chars, with a message telling the author to keep the invariant in CLAUDE.md and link the docs page for the worked rationale. MEASURED: main's longest bullet is 1434 chars, PR head's is 2577 (CLAUDE.md:144, +80%), against 1390 for the next-longest — so a 1500 cap is green today and fires exactly on this change. Wire as a lint-marked pytest class beside CE028's docs-index test (markdown, not AST). STRETCH (noisier, propose separately): n-gram overlap detection between CLAUDE.md and docs/*.md to flag near-verbatim prose duplication — that is the mechanical form of the 4-copy drift, but the length cap is the deterministic part. Prevents: Finding 10 (the early-stop bullet became a fourth near-verbatim copy of the polarity semantics, 1.9x the next-longest bullet, in a file prepended to every session). Indirectly reduces the drift surface behind Findings 1 and 4, where this PR updated three of four copies and left the fourth stale.

Harness improvements (not statically reachable):

  • Smoke-vs-e2e parity replay test with an explicitly declared divergence table. Take a recorded event-stream fixture for an activation-shaped dataset-fanned suite (positive + distractors), drive EarlyStopWatcher over it to produce the truncated trajectory, then run SuccessChecker.check_all + overlay_classification_metrics + reports._compute_suite_rollup over BOTH the truncated and the full trajectory, and assert every per-criterion label flip appears in a checked-in declared-divergence table — failing on any UNdeclared flip. This makes the parity claim executable instead of prose. Note a FIFTH copy of that claim the review did not flag: experiments/early-stop-ab.yaml still says "Expect identical pass/fail verdicts between the two variants", which the new pass-armed-subset rule falsifies — and that experiment file is the natural fixture for this test. Why not static: The divergence is a runtime property of a specific trajectory: it requires replaying an event stream through the watcher's latch and then through the whole scoring/aggregation path. No AST or grep can see that a truncated trajectory scores a row TP->FN. Prevents: Findings 8 (auto fail-arms distractors -> TP->FN, recall/F1 deflation), 9 (truncated rows pooled into the suite gate), and 4 (docs + experiment YAML assert "identical verdicts" while the appended note negates it).
  • Measurement-integrity guard on the suite gate: (a) surface truncation at the aggregate level — SuiteRollup.rows_stopped_early plus a CriterionAggregate marker, mirrored into suite.json/suite.md — since _compute_suite_rollup (reports.py:775-807) currently feeds every row into aggregate() with no early-stop notion while SuiteRollup.passed drives the CLI exit code (cli/run_command.py:701); (b) either exclude truncated rows from classification metrics or fail the gate when a suite_thresholds-gated criterion was unobserved on any row; (c) record the still-undecided pass-armed criteria in EarlyStopInfo so consumers can exclude those rows; (d) make run_limits.stop_early + suite_thresholds an explicit resolution-time error or loud warning in validate_early_stop, backed by a test asserting a stop_early: true run cannot report a green precision.yes gate on truncated evidence. Why not static: Truncation is per-row runtime state, and the suite that motivated the feature lives out-of-tree (the activation suite is in coder-eval-uipath), so a lint rule over in-tree tasks/*.yaml would reach nothing — measured: zero in-tree YAML files set stop_early and suite_thresholds together today (experiments/early-stop-ab.yaml sets only the former, tasks/sentiment_classification.yaml only the latter). Only a runtime guardrail plus a report/model field protects an external consumer. Prevents: Findings 9 (gate can go green on truncated evidence with no suite-level marker; armed-subset verdict counts never-observed distractors as armed passes) and 8 (deflated recall.yes/f1.yes gated in CI).
  • Add a curated ordering/precedence mutation smoke gate — e.g. make mutate-smoke — that applies a small hand-written set of source mutations to hot invariant-bearing modules and fails if the module's own test file stays green. Seed mutations for orchestration/early_stop.py: swap the fail-stop block (:373-380) with the pass-stop block (:382-398); off-by-one on the pass_armed index remap; delete the pass_armed and … vacuity guard. The reviewer ran mutation #1 by hand and tests/test_early_stop.py stayed at 129/129 (tests/test_threshold_enforcement.py at 15/15) — the docstring's load-bearing claim "fail-stop is evaluated before pass-stop each round" has no test owner. Pair it with the one missing direct test: both polarities decided in a SINGLE _evaluate round via one tool call engaging both skills, asserting reason == EarlyStopReason.CRITERION_FAILED. Why not static: Mutation survival is a property of the test suite's assertions, not of the source AST — no lint rule can tell whether an existing test would notice a semantically different but syntactically valid reordering. Prevents: Finding 3 (new arming/stop-rule branches exercised only in shapes where mutations survive: the decided arm is never executed, the pass_armed index remap is tested only at index 0, and fail-before-pass precedence is untested).
  • Give every prose-asserted behavioral invariant an executable owner: for claims like "identical verdicts", "fail-stop is evaluated before pass-stop each round", and "never a silent no-op", require the prose to name the test that pins it (see tests/test_early_stop.py::test_…) and add a cheap collect-only check that every referenced test id exists. The existence half is mechanical (a lint-marked test); the truth half is the named test itself. This converts the current failure mode — an invariant stated in four places, updated in three — into a build break the moment behavior diverges from the claim. Why not static: Detecting that a paragraph asserts an invariant and then negates it two sentences later is semantic judgment; only the back-reference existence check is grep-able, and the invariant's truth can only be established by running the behavior. Prevents: Findings 4 ("identical verdicts" asserted and immediately contradicted), 3 (precedence ordering with no test owner), and the drift dimension of 1 and 10 (four hand-synced copies of the polarity semantics).

Top 5 Priority Actions

  1. Stop auto's fail-stop from truncating the recall signal at src/coder_eval/orchestration/early_stop.py:378 — suppress the fail-stop while any pass-armed criterion on the row is still undecided (or add an auto-pass-only narrowing) — because today a positive row where the agent reads a wrong skill's SKILL.md first freezes as observed_label='no', flipping a TP to an FN and deflating per-skill recall.yes/f1.yes for identical agent output.
  2. Make truncated rows visible and non-gating in the suite rollup: add a rows_stopped_early marker to SuiteRollup (src/coder_eval/models/results.py:778) and either exclude early-stopped rows from checker.aggregate(...) at src/coder_eval/reports.py:791 or fail the gate when a threshold-gated criterion was unobserved, since SuiteRollup.passed drives the CLI exit code (src/coder_eval/cli/run_command.py:701) and can go green on a smoke run that a full run fails.
  3. Tighten the armed-subset verdict so never-observed criteria cannot score a pass — armed_criteria_passed filters only on c.stop_when is not None (src/coder_eval/models/results.py:656), so on a pass-stop the ~22 unobserved fail-armed distractors gate the run at a trivial 1.0 and render as genuine armed passes in the HTML report (src/coder_eval/reports_html.py:582).
  4. Fix the two surfaces that hand users a guaranteed-invalid value by switching stop_when: decided to stop_when: auto in the guide's only copy-paste stop_early snippet (docs/TASK_DEFINITION_GUIDE.md:222) and in the guardrail hint at src/coder_eval/orchestration/early_stop.py:134, because no in-tree criterion instance can satisfy decided and following the error message produces a second EarlyStopConfigError.
  5. Pin the load-bearing fail-before-pass precedence with a test that decides both polarities in one _evaluate round (tests/test_early_stop.py:1117 — physically swapping the blocks at early_stop.py:373-398 leaves all 129 tests green), and while in that file collapse the duplicated stop_when→polarity mapping (early_stop.py:162 vs :336) into one _requested_polarities helper, replace the silent return frozenset() fall-through at early_stop.py:343 with assert_never, and name the offending criterion's description in the auto dead-arm error at early_stop.py:169.

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

mohsen-uipath pushed a commit that referenced this pull request Jul 24, 2026
…cided

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
@mohsen-uipath

Copy link
Copy Markdown
Contributor Author

Addressed the review in 5a9ba3b (plus a merge of main for the required up-to-date check):

Blocker 1 (recall truncation) — fixed. The fail-stop is now deferred while any pass-armed criterion is still undecided (EarlyStopWatcher._evaluate). A distractor misfire on an early tool call can no longer cut a positive row before its expected signal appears — the misfire stays latched and fires the moment every pass-armed criterion decides; a row with zero pass-armed criteria (negative rows) defers nothing and still fail-stops immediately. Fail-stops are now verdict-preserving; the pass-stop precision caveat remains and is documented as one-sided parity.

Also from the review:

  • Fail-before-pass precedence is now pinned by a same-round test (test_fail_stop_precedes_pass_stop_same_round — one tool call engaging both skills), killing the block-swap mutation; plus deferral tests for the misfire-then-positive and misfire-only paths.
  • stop_when → polarity mapping hoisted into one _requested_polarities helper (validator + watcher), terminated with typing.assert_never so widening the Literal without updating the mapping is a type error instead of a silently inert arm.
  • The unsatisfiable stop_when: decided example/hint replaced with auto (guide snippet + validator error hint); arming errors now name the offending criterion's description.
  • The one-sided verdict-parity statement (fail-stop verdict-preserving, pass-stop precision-optimistic) propagated to TASK_DEFINITION_GUIDE.md, AB_EXPERIMENTS.md, experiments/early-stop-ab.yaml, RunLimits.stop_early's field description, and a compressed CLAUDE.md bullet (2589 → ~1900 chars, worked rationale delegated to the guide).

Blocker 2 (suite-rollup truncation marker / gate on truncated evidence) is deferred to a follow-up PR — it's a separate surface (SuiteRollup, reports, CLI gate) and this feature is opt-in and default-off.

Validation: make verify green (3554 passed, 91% coverage); live re-run of the downstream activation suite on this commit — 5/5 sampled positive rows pass-stop at SDK turn 1 with verdicts identical to the pre-change baseline (23/23, score 1.000, ~$0.05/row), and the 4-row demo (incl. the negative row, which correctly never stops early) also matches baseline row-for-row.

Comment thread src/coder_eval/orchestration/early_stop.py Fixed
…cided

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
@mohsen-uipath
mohsen-uipath force-pushed the fix/early-stop-auto-polarity branch from 5a9ba3b to 415c910 Compare July 24, 2026 20:40
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.
@mohsen-uipath
mohsen-uipath merged commit 08e21e6 into main Jul 24, 2026
13 checks passed
@mohsen-uipath
mohsen-uipath deleted the fix/early-stop-auto-polarity branch July 24, 2026 21:00
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