fix(criteria): stop ignore_flags re-opening the guard false-PASS

ignore_flags was folded into the value-bearing set to keep `--output json` from
leaving `json` in the positionals. That made every ignored flag value-bearing,
including switches: with ignore_flags: ["verbose"], `delete --verbose proj-1`
bound verbose=proj-1, emptied the positionals, and a max_count: 0 guard on
positional: [proj-1] returned 1.0 on the delete it forbade — the same failure as
the --yes bug, through the last field where binding was still guessed.

Value binding is now declarations only (flag predicates that need a value, plus
value_flags, which defaults to ["output"]). An ignored flag that takes a value
says so in value_flags.

Also from the review:

- min_count: 0 with no max_count is satisfied by every log, so the criterion
  could never fail; rejected alongside the other vacuities.
- A missed positive reported only counts. Failure details now echo up to three
  recorded argvs, for a criterion whose purpose is "what did it actually run".
- CLAUDE.md said 14 criterion types and omitted cli_called.
- Documented that bundled short flags are not split (-rf is one flag named rf,
  so absent: true on f passes despite it).
- Trimmed Field descriptions to behaviour; the rationale lives in comments.

68 criterion tests. Differential against the downstream suite still 7/7.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: alexandrujircan

CLAUDE.md

@@ -34,7 +34,7 @@ coder_eval/
 ├── models/                        # Pydantic data models (subpackage)
 │   ├── __init__.py                # Unified exports for all models
 │   ├── enums.py                   # AgentKind, AgentState, FinalStatus, ApiBackend
-│   ├── criteria.py                # 14 success criterion types + base + union
+│   ├── criteria.py                # 15 success criterion types + base + union
 │   ├── experiment.py              # ExperimentDefinition, ExperimentVariant, ResolvedTask, result models
 │   ├── judge_defaults.py          # DEFAULT_JUDGE_MODEL constant (cycle-free leaf)
 │   ├── mutations.py               # PromptMutation variants (prefix/suffix/replace/template)
@@ -144,7 +144,7 @@ action.yml                         # Published composite GitHub Action (coder-ev
 - **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 (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_async` 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)
+## Success Criteria (15 types)
 
 | Type | Scoring | Description |
 |------|---------|-------------|
@@ -156,6 +156,7 @@ action.yml                         # Published composite GitHub Action (coder-ev
 | `file_matches_regex` | Binary | Regex match on file |
 | `reference_comparison` | Continuous | AST/token/complexity similarity |
 | `command_executed` | Fractional | Agent tool usage verification |
+| `cli_called` | Binary | Structured match over a JSON Lines invocation log: verb / positional / per-flag predicates, with min_count/max_count bounds |
 | `commands_efficiency` | Continuous | Agent tool-call efficiency relative to expected budget |
 | `uipath_eval` | Fractional | UiPath agent evaluation results |
 | `classification_match` | Binary | File-based label match (observed vs expected) with `(none)`/`(other)` sentinels; emits `ClassificationCriterionResult` for suite-level P/R/F1 |

docs/TASK_DEFINITION_GUIDE.md

@@ -822,6 +822,10 @@ Flags the criterion does not mention are ignored, so an extra `--output json` ne
 
 Defaulting to "switch" is deliberate: `--yes` / `--force` / `-y` before the target is how destructive CLIs are invoked, so guessing that the flag swallows its neighbour is precisely how a `max_count: 0` guard ends up passing on the call it exists to forbid. The equals form (`--offset=-1`) is unambiguous and always binds directly, and a declared value flag binds even a dash-leading value (`--limit -1`).
 
+`ignore_flags` drops a flag from matching but does **not** make it value-bearing — an ignored flag that takes a value must also appear in `value_flags` (as `output` does by default). Otherwise `ignore_flags: ["verbose"]` on `delete --verbose proj-1` would let `--verbose` eat `proj-1`.
+
+**Limitation: bundled short flags are not split.** `-rf` parses as one flag named `rf`, so a predicate on `f` will not see it — including `absent: true`, which passes despite `-rf` being present. Assert on the long spelling, or add the bundled form via `aliases`. Likewise a bare negative number in flag position (`seek -1`) is read as a flag named `1`.
+
 **Negative guards want the FEWEST facets that capture the forbidden act.** This is the opposite of a positive assertion, and it is easy to get backwards. `max_count: 0` passes when *nothing matches*, so every facet you add is another way for the real invocation to slip past the pattern and report a false PASS.
 
 In the delete example above, it is tempting to also assert `--yes`. Don't:

src/coder_eval/criteria/cli_called.py

@@ -3,6 +3,7 @@
 import json
 import logging
 import re
+import shlex
 from typing import TYPE_CHECKING, Any
 
 from coder_eval.criteria.base import BaseCriterion, CheckContext, register_criterion
@@ -23,34 +24,17 @@ def _split_flags(
 ) -> tuple[list[str], dict[str, list[str]]]:
     """Split ``argv`` into non-flag arguments and a flag map.
 
-    Value binding is DECLARED, not guessed. ``value_flags`` names the flags that
-    consume a following token; every other flag is a switch whose following token
-    stays positional. The criterion supplies its own ``flags:`` keys plus
-    ``value_flags:`` as that set, so the author's assertion doubles as the grammar.
-
-    This replaced a heuristic ("the next token is the value unless it starts with
-    ``-``") that silently swallowed a positional after a boolean switch. For
-    ``uip fields delete --yes proj-1`` it bound ``yes=proj-1`` and dropped
-    ``proj-1`` from the positionals, so a ``max_count: 0`` guard on
-    ``positional: [proj-1]`` reported a PASS while the log proved the delete had
-    happened. ``--yes``/``--force``/``-y`` before the target is how destructive
-    CLIs are invoked, which is exactly the shape a negative guard exists to catch,
-    so the default resolves ambiguity toward keeping the token positional.
-
-    Other normalizations, each a case a flat-string regex gets wrong:
-
-    - ``--flag=value`` binds directly. The equals form is unambiguous, so it is
-      never re-run through any value/switch decision: ``--offset=-1`` keeps ``-1``
-      instead of dropping it and inventing a flag named ``1``.
-    - A declared value flag consumes its next token even when that token starts
-      with ``-``, so ``--limit -1`` binds ``-1``.
-    - Repeated flags accumulate, so ``--field a --field b`` keeps both values.
-    - Names in ``ignore`` are dropped along with their values.
-
-    A bare ``--`` terminates flag parsing (POSIX convention): everything after it
-    is positional, and the separator itself is dropped so it never has to be
-    written into a ``positional:`` expectation. A lone ``-`` is positional too
-    (the stdin convention), not a flag.
+    Only flags in ``value_flags`` consume a following token; everything else is a
+    switch. Guessing instead (``--yes proj-1`` binding ``yes=proj-1``) let a
+    ``max_count: 0`` guard pass on the delete it forbade, so ambiguity resolves
+    toward keeping the token positional.
+
+    ``--flag=value`` binds directly, being unambiguous. Repeated flags accumulate.
+    ``ignore`` names are dropped with their values. ``--`` ends flag parsing and is
+    itself dropped; a lone ``-`` is positional.
+
+    Known limitation: bundled short flags are not split, so ``-rf`` is one flag
+    named ``rf`` and a predicate on ``f`` will not see it.
     """
     positional: list[str] = []
     flags: dict[str, list[str]] = {}
@@ -134,18 +118,14 @@ def _record_matches(criterion: CliCalledCriterion, argv: list[str], record: dict
     if criterion.tool is not None and record.get("tool") != criterion.tool:
         return False
 
-    # The criterion's own flag predicates declare which flags carry a value;
-    # `value_flags` covers the rest (a flag whose value must not be mistaken for a
-    # positional even though nothing asserts on it).
-    ignore = frozenset(criterion.ignore_flags)
+    # Declarations only. Folding `ignore_flags` in here made ignored SWITCHES
+    # value-bearing, which swallowed the next positional and reopened the guard
+    # false-PASS; an ignored flag that takes a value declares it in value_flags.
     positional, flags = _split_flags(
         argv,
-        ignore,
-        # Ignored flags are value-bearing too: dropping `--output` while leaving
-        # `json` in the positionals would defeat the point of ignoring it.
+        frozenset(criterion.ignore_flags),
         frozenset(n for name, p in (criterion.flags or {}).items() if p.needs_value for n in (name, *p.aliases))
-        | frozenset(criterion.value_flags)
-        | ignore,
+        | frozenset(criterion.value_flags),
     )
 
     offset = 0
@@ -165,9 +145,8 @@ def _record_matches(criterion: CliCalledCriterion, argv: list[str], record: dict
 
     if criterion.flags:
         for name, predicate in criterion.flags.items():
-            # One flag, several spellings: gather values across every name it owns.
-            # [] means the flag was absent under all of them, which _flag_matches
-            # distinguishes from "present with an empty value" (a switch, [""]).
+            # [] means absent under every spelling, which _flag_matches
+            # distinguishes from a switch's "present with empty value" ([""]).
             collected = [v for n in (name, *predicate.aliases) for v in flags.get(n, [])]
             if not _flag_matches(predicate, collected or None):
                 return False
@@ -201,9 +180,8 @@ def _check_impl(
             Result with binary score (1.0 when the match count is within
             [min_count, max_count], 0.0 otherwise)
         """
-        # Compile every flag regex up front so a bad pattern reports itself as
-        # such, instead of surfacing as a generic caught exception once the first
-        # record happens to reach that predicate.
+        # Up front so a bad pattern names its flag, rather than surfacing as a
+        # generic caught exception when some record first reaches that predicate.
         for name, predicate in (criterion.flags or {}).items():
             if predicate.matches_regex is None:
                 continue
@@ -218,10 +196,8 @@ def _check_impl(
                 )
 
         if not sandbox.file_exists(criterion.log):
-            # A missing log is a harness fault, not agent behaviour: the mock never
-            # ran or wrote elsewhere. Failing (rather than treating it as "zero
-            # matching calls") is what stops a negative guard — max_count: 0 —
-            # from passing vacuously against a log that does not exist.
+            # Harness fault, not agent behaviour. Failing stops a max_count: 0
+            # guard passing vacuously against a log that never existed.
             return CriterionResult(
                 criterion_type=criterion.type,
                 description=criterion.description,
@@ -252,12 +228,8 @@ def _check_impl(
             usable.append((argv, parsed))
 
         if unusable:
-            # Same footing as a missing log, and for the same reason: a record we
-            # cannot read might BE the invocation a max_count: 0 guard forbids, so
-            # scoring it as "did not match" would let the guard pass on the very
-            # call it exists to catch. Skipping these silently (the previous
-            # behaviour) contradicted the fail-loud missing-log path above and the
-            # sibling precedent in json_check.
+            # A record we cannot read might BE the call a max_count: 0 guard
+            # forbids, so scoring it "did not match" would let the guard pass.
             logger.warning(
                 f"cli_called: {unusable} unusable record(s) in '{criterion.log}'"
                 + " (unparseable line, non-object line, or argv that is not a list of strings)"
@@ -299,9 +271,14 @@ def _check_impl(
         if score == 1.0:
             details = f"{count} invocation(s) matched ({wanted}); satisfies {bound}"
         elif not within_lower:
+            # A bare count sends the reader to the sandbox; this criterion exists
+            # to answer "what did it actually run".
+            sample = "; ".join(shlex.join(argv)[:120] for argv, _ in usable[:3])
+            more = f" (+{len(usable) - 3} more)" if len(usable) > 3 else ""
+            recorded = f" Recorded: {sample}{more}" if sample else ""
             details = (
                 f"{count} invocation(s) matched ({wanted}); needs {bound}. "
-                f"{len(records)} invocation(s) recorded in '{criterion.log}'"
+                f"{len(records)} invocation(s) recorded in '{criterion.log}'.{recorded}"
             )
         else:
             details = f"{count} invocation(s) matched ({wanted}) but {bound} forbids it"