diff --git a/.claude/harness-candidates.md b/.claude/harness-candidates.md index d5d28a553..4fc94fc19 100644 --- a/.claude/harness-candidates.md +++ b/.claude/harness-candidates.md @@ -31,7 +31,7 @@ Deferred lint/test guardrails surfaced during reviews. Promote to a `CExxx` rule - [ ] 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` / + forbid `.stop_when` attribute reads inside `TurnMonitor._evaluate_impl` / `_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). @@ -744,7 +744,7 @@ divergences, so the deferred-work record is one place. Measurements in `AgentStartEvent`**, which resets only `_agent_end`. Pre-existing and NOT introduced by the timing work. Blast radius is narrower than it first looks: the persisted record, the reports and `max_turns` all read the AGENT's - collector, which is fresh per `communicate()`. Only `EarlyStopWatcher`'s + collector, which is fresh per `communicate()`. Only `TurnMonitor`'s long-lived collector accumulates — where carrying a turn's whole engagement across retry attempts is arguably what a live "did it engage the skill" verdict wants, and `_check_round`'s docstring already reasons about crashed @@ -965,3 +965,8 @@ re-derive from scratch. - [ ] A `TaskDefinition` serialized for a later reload (docker `_stage_inputs`, Harbor `environment/task.yaml`) must dump `agent` with `exclude_unset=True`, or the reload marks model defaults as set and the harness contract check rejects the task — two round-trip tests guard today's two sites, but nothing flags a third `task.model_dump(` written for reload; needs a call-site classifier, not a name match — caught in the harness-contract final review. - [ ] A real-SDK Antigravity policy test: run `policy.enforce(agent._policies(real_policy))` to prove deny-beats-allow and `finish` approval against the installed SDK instead of a SimpleNamespace fake — nothing exercises the SDK's own bucket precedence; needs study of the hook-evaluation API — caught in the harness-contract Phase 3 review. - [ ] OpenCode: warn when an inherited `OPENCODE_CONFIG_CONTENT` `permission` / `instructions` value is not a dict / list and is replaced — today it is dropped silently; small, but needs a decision on warn vs. keep — caught in the harness-contract Phase 3 review. +- [ ] CE070 blind spot: an adapter that counts `ToolEndEvent`s (or tokens) under a new name to cap or stop a run itself — the rule matches identifiers only; needs a data-flow check that a counter in `agents/` feeds a break or an end status — caught in the central-enforcement plan (Phase 5). +- [ ] CE070 blind spot: an adapter that re-grows a skill scanner through `glob("*.md")`, `rglob`, or a file name built from parts — the rule matches the literal `"SKILL.md"` only; needs a filesystem-walk classifier scoped to `agents/` — caught in the central-enforcement plan (Phase 5). +- [ ] Every harness's `TurnEndEvent.tokens` must be a per-report DELTA: over a turn, their sum per bucket must not exceed `AgentEndEvent.usage` (the TurnMonitor latches budgets on the sum) — nothing checks it; the golden-stream runners return only the TurnRecord, so each of the five `run_*_scenario` helpers needs an event sink first — caught in the central-enforcement final review (Claude re-reported an interleaved message id's tokens). +- [ ] Live tests (`-m live`) are neither run nor type-checked in `make verify`, so an SPI signature change (`communicate(max_turns=)`, bool `should_stop`) leaves them broken until someone runs them with credentials — needs pyright over `tests/*_live.py` or an import-time signature smoke test — caught in the central-enforcement live verification (Phase 6). + diff --git a/.claude/notes/agents.md b/.claude/notes/agents.md index 6c2a5e2f3..75f36ed37 100644 --- a/.claude/notes/agents.md +++ b/.claude/notes/agents.md @@ -59,34 +59,11 @@ intentionally brief and out of scope; trimming for DISPLAY belongs in the render ## Harness run-limit parity -- **Harness run-limit parity**: a shared `BaseAgentConfig` field must mean the same - thing on every backend, so a divergence is either fixed or documented — never silent. - **`run_limits.max_turns` on Codex/Antigravity counts VISIBLE turns** (resolved tool - calls, read live off the shared `EventCollector.visible_turn_count`, the same list - `TurnRecord.commands` holds) because one `communicate()` is a single SDK turn on both, - so a native counter would clamp at 1; claude-code keeps its native SDK cap, whose unit - (an agent-loop turn) absorbs arbitrarily many parallel calls — the same number is NOT - the same budget across harnesses. OpenCode and Pi each keep a native unit too, because - their CLIs stream a real multi-step loop per `communicate()` - (`step_start`/`step_finish`, `turn_start`/`turn_end`). The cap is enforced on the same - loop boundary as the cooperative early stop and finalizes cleanly as - `max_turns_exhausted` (no crash, no retry); on Antigravity that boundary lives in - `_drain()`, so the background-work poll loop honors it too. - - The **known unfixed divergences** — which config fields each harness does and does not - enforce, and the per-harness `agent.plugins[].path` depth (claude-code REQUIRES a - plugin root holding `skills/` and silently loads NOTHING from a bare skills directory, - which is the costly direction: no error, every positive row of an activation suite - scores 0, and the suite reports recall 0.0, reading exactly like a skill that never - triggers; held to the plugin-root shape for `SKILL_SOURCE_PATH` by CE045) — are the - table's to state, not this file's. Full table + rationale: - docs/agents/HARNESS_PARITY.md. - - The agent-field half of parity is now the `HarnessContract` each agent class declares: - a field, `permission_mode` value or tool name a harness cannot honor is a resolution - error, and `make parity-table` renders the contract (CE069 checks it), so the page can no - longer drift from the adapters. The run-limit half is still the hand-written table above; - Plan 2 moves it onto the contract. +A shared field must mean the same thing on every backend. Both halves are generated +tables in docs/agents/HARNESS_PARITY.md (`make parity-table`, CE069): `run_limits` from +`RunLimits` and each agent's `HarnessContract`, the agent fields from the contract. Every +cap and budget is the `TurnMonitor`'s (orchestration.md § The watcher became the +TurnMonitor); CE070 keeps adapters from counting one again. ## Shared turn lifecycle @@ -111,17 +88,18 @@ failure is not swallowed. `finalize` is total: an unmapped future member raises loudly instead of silently bucketing to COMPLETED. -Status precedence is the same everywhere: timeout > stopped_early > max_turns_exhausted > +Status precedence is the same everywhere: timeout > stopped_early > tool_calls_exhausted > completed. `stopped_early` outranks the cap because an armed criterion deciding the -outcome is the more specific reason to have cut the run, and every loop checks it first. +outcome is the more specific reason to have cut the run; the `TurnMonitor` evaluates the +armed criteria before the cap, so an armed stop wins a tie and the first latched reason is +final. ## Why a post-stop exception is not a crash -Once the loop has broken on purpose — a cooperative stop or the turn cap — an exception -raised while tearing the stream down must NOT be escalated. Escalating triggers the -orchestrator's retry with the watcher's decision still latched, so the retry stops at turn -0 having spent nothing useful; a cap-break is the same shape, where the retry burns the -budget again and re-hits the cap. `ended_cleanly` is the guard. +Once the loop has broken on purpose — any `should_stop` reason, including the tool-call +cap — an exception raised while tearing the stream down must NOT be escalated. Escalating +triggers the orchestrator's retry with the monitor's decision still latched, so the retry +stops at its first poll having spent nothing useful. `ended_cleanly` is the guard. ## Why the constructors declare every kwarg @@ -261,13 +239,13 @@ matter how much the run actually billed. So the CLI harnesses crash rather than Crashing routes it to `FinalStatus.ERROR`, which is excluded from outcomes. - **A CLI that closed its stream but would not exit** within the grace period. -Every arm is gated on `stopped_early` / `max_turns_exhausted`, because an intentional cut +Every arm is gated on `stopped_early` / `tool_calls_exhausted`, because an intentional cut can land before the clearing event arrives. Pi's error case shows why: `error_message` is set at an error `turn_end` and cleared only by a LATER non-error `turn_end`, but a -`max_turns` / `should_stop` cut can fire at the next `turn_start`, leaving a stale error +`should_stop` cut (an early stop or the tool-call cap) can fire at the next `turn_start`, leaving a stale error from a turn Pi was still retrying. Without the guard that clean, budget-exhausted cut would crash and burn retries, contradicting the documented "finalizes cleanly as -`max_turns_exhausted`, no crash" contract. +`tool_calls_exhausted`, no crash" contract. OpenCode has one escape hatch, `require_token_telemetry`, for a provider or auth mode that reports no usage at all — where crashing every turn makes the harness unusable rather than @@ -335,8 +313,9 @@ The Claude SDK's own `costUSD` is a client-side estimate assuming Anthropic pric is wrong for an open-weight model behind LiteLLM and is repriced from the token buckets at the model's real rate. The buckets are untouched, so the reconciliation invariant holds — only the cost scalar changes. An unpriced model sets the cost to `None` (an honest N/A) -**and warns**, because a silent `None` makes the orchestrator skip the `max_usd` gate with -no diagnostic. +**and warns**. When the task sets `max_usd`, the `TurnMonitor` then raises +`BudgetUnenforceableError` at the turn end, so the row finishes `ERROR` and is never a +silent skip. ## Codex rollout rebuild @@ -638,8 +617,7 @@ tempdir is still reclaimed. `_TERM_GRACE_SECONDS` is re-declared at the same value in both nd-JSON harnesses rather than shared: the CLI-driver hoist that would unify their teardown constants and reducers is -a tracked follow-up. The shared plugin→skills resolver already lives in `agents/_skills.py`, -and `STDOUT_LINE_LIMIT_BYTES`, which IS canonical, is imported. +a tracked follow-up. `STDOUT_LINE_LIMIT_BYTES`, which IS canonical, is imported. ## The system_prompt_semantics marker @@ -663,36 +641,60 @@ cannot disagree with what was sent. ## Skills, per harness -A `plugins:` entry is a Claude-plugin root, and only the SKILLS half of it is honored -anywhere — a plugin's agents, hooks, commands and MCP servers have no equivalent outside -claude-code and are dropped. The manifest's `skills` field is read rather than `skills/` -being hardcoded, so a plugin that relocates its skills keeps working. - -- **OpenCode** maps each root to `skills.paths` via `OPENCODE_CONFIG_CONTENT`, which the - CLI merges as a final local-scope layer. That was chosen over writing - `/.opencode/skills/` because it writes nothing into the sandbox that is later - preserved as a run artifact and inspected by file criteria, and does not depend on how - the CLI resolves a project root from `--dir`. Verified orthogonal to `--pure`, which - skips external *plugins*, not configured skill paths. An inherited value is appended to - rather than clobbered, since the host may legitimately configure OpenCode the same way. -- **Pi** passes each as `--skill `. -- **Codex** symlinks (or copies, on Windows) each skill dir into `.agents/skills/`, which - the CLI auto-discovers from the working directory upward. -- **Antigravity** takes search paths natively via `skills_paths` — but those only drive - DISCOVERY. The file-tool allowlist is `workspaces` alone, so the skill roots must appear +`orchestration/plugin_staging.py` stages every `plugins:` entry into one canonical root, +`/plugin_root`, before `Agent.start`. Each harness then receives the SAME layout: +`.claude-plugin/plugin.json` and `skills/` links. The staging exists because each +adapter used to scan the authored path its own way. claude-code loaded nothing from a bare +skills directory, with no error, so an activation suite scored recall 0.0 and read exactly +like a skill that never triggers. + +- **A plugin root is read the way Claude Code reads it.** The default `skills/` is always + scanned, and each path the manifest's `skills` field declares ADDS to it; a declared path + may parent skills or be one skill (it holds `SKILL.md`). A root holding `SKILL.md` is a + single-skill plugin. If a root yields nothing that way, it is read as a bare skills + directory. A skill's name is its frontmatter `name`, else its directory name: Claude + Code invokes the frontmatter name, so a gate keyed on the directory name would refuse a + skill that loads. Confirmed by the plugins reference ("Adds to the default: `skills`") + and a CLI 2.1.273 spike on 2026-09-16; the moved reader had treated the manifest as a + REPLACEMENT, which dropped the default `skills/` of any plugin that declared extras. +- **Only skills are staged.** A plugin's agents, hooks, commands and MCP servers are + dropped on every harness, claude-code included. That also removes a confound: a project + subagent beside `skills/` can no longer answer the request the skill should answer. +- **The staged manifest is `{"name": "coder-eval-plugins"}` and nothing else.** A + 2026-09-17 spike with `claude -p --plugin-dir` showed a staged root whose manifest declared + `"skills": ["skills"]` load no skill; a 2026-09-16 spike on CLI 2.1.273 loaded a real + `["./skills"]` fine. The name-only manifest loads the `skills/` default either way. +- **Refusal is at resolution.** `validate_plugins` runs in `validate_resolved_task`, so a + path with no skill, an unresolvable path, one skill name from two sources, or a + `skill_triggered` `skill_name` the plugins do not offer fails `plan` before the run is paid + for. A name still holding a `${row...}` placeholder is checked on its expanded row. The + cost: a task whose skill under test comes from a template or `setting_sources` while it + also sets `agent.plugins` is refused, because only plugin skills are offered. +- **`skills_offered` is recorded** in `environment_info` and passed to the checker. + `skill_triggered` still raises `CheckerMisuseError` when its `skill_name` is not offered. + Resolution catches every new run first; the checker gate remains for a detached grade of + a recorded run, where resolution does not re-run. + +Delivery, per harness: + +- **Claude Code** takes the root as an SDK `{"type": "local", "path": plugin_root}` plugin. +- **OpenCode** appends `/skills` to `skills.paths` via + `OPENCODE_CONFIG_CONTENT`, which the CLI merges as a final local-scope layer. That was + chosen over writing `/.opencode/skills/` because it writes nothing into the + sandbox that is later preserved as a run artifact and inspected by file criteria, and + does not depend on how the CLI resolves a project root from `--dir`. Verified orthogonal + to `--pure`, which skips external *plugins*, not configured skill paths. An inherited + value is appended to rather than clobbered. The staged `skills/` holds only skill links, + so the recursive scan no longer walks a repo root's self-referential symlinks. +- **Pi** passes `--skill /skills`. +- **Codex** links each `/skills/` into `.agents/skills/` with + `link_or_copy`, which the CLI auto-discovers from the working directory upward. +- **Antigravity** takes `/skills` in `skills_paths` — but those only drive + DISCOVERY. The file-tool allowlist is `workspaces` alone, so the same path must appear there too, or the agent discovers a skill and every read of its `SKILL.md` is denied as out-of-workspace. -A bare skills directory is used as-is only when the root declares no `skills/` subdir. -That is deliberately not a fallback for a root that HAS one: `skills.paths` is scanned -recursively and a repo root can contain self-referential symlinks (`UiPath/skills` has -`plugins/uipath -> ..`), which resolves skills through an arbitrary path and silently drops -duplicate names. - -Every way this can come up empty is logged loudly — an unresolved env var, a missing dir, -a root with no `/SKILL.md` under it. A plugin whose skills never reach the agent -still *looks* like a normal run, which is precisely the failure the logging closes: the -run measures the model WITHOUT the skill under test. +`plugin_tools_dir` is not a skills source on any harness. ## Why the registry rejects a re-registration diff --git a/.claude/notes/contracts.md b/.claude/notes/contracts.md index 980f3edba..26401378c 100644 --- a/.claude/notes/contracts.md +++ b/.claude/notes/contracts.md @@ -41,7 +41,7 @@ answers pass or fail, it answers the same for every longer prefix); `undecided` verdict allowed to change on a later call. Both properties are stated at the definition site in `criteria/base.py`, because they are the contract an author has to satisfy. -`EarlyStopWatcher`'s deferred fail-stop and its pass/fail flip-attribution are correct ONLY +`TurnMonitor`'s deferred fail-stop and its pass/fail flip-attribution are correct ONLY because the two shipped implementations honor them. A non-monotonic or non-deterministic override compiles, passes CE025, and silently corrupts the stop logic. @@ -92,7 +92,7 @@ matches, but the same haystacks feed `exclude_pattern` and the `max_count` gate, normalized form can newly satisfy an exclusion or trip a cap — a command that counted on the raw text alone can stop counting. -It is memoized because the early-stop watcher re-scans the whole accumulated trajectory on +It is memoized because the `TurnMonitor` re-scans the whole accumulated trajectory on every tool-call event, normalizing the same command many times per run. The regex search window is capped to bound ReDoS on a large command string, and @@ -190,6 +190,14 @@ is genuinely a gating 0.0. `reference_file` is confined to the reference directo judge's author-written `files:` entry, because it names one file of the solution being compared against and traversal out of the staged copy is always a mistake. +`skill_triggered` escalates the same way when its `skill_name` is not among the skills +`agent.plugins` offered (`CheckContext.skills_offered`). Resolution refuses the same task +first (`validate_plugins`), so this gate fires only on a detached grade of a recorded run. The agent was never offered the +skill, so the positive control cannot run. Scored as 0.0, every positive row of an +activation suite would read as a skill that never triggers. The gate applies only when the +task sets plugins: with `skills_offered` `None` the criterion scores as before, so a skill +the harness finds by other means still counts. + Grading time is accumulated at the checker, not at the four orchestrator call sites, so a fifth site cannot be added without it — the same reason the tool subtraction lives at one collector seam. It is monotonic, booked in a `finally` so a grade that raises still records diff --git a/.claude/notes/isolation.md b/.claude/notes/isolation.md index c9e2957d5..5afbdff92 100644 --- a/.claude/notes/isolation.md +++ b/.claude/notes/isolation.md @@ -26,18 +26,18 @@ `FinalStatus.is_execution_fact` (TIMEOUT / ERROR / BUILD_FAILED / the budget stops) is **preserved**, never overwritten: grading may only move `NOT_GRADED` to SUCCESS/FAILURE, since it neither repeated nor observed the agent phase. - **`MAX_TURNS_EXHAUSTED` is deliberately NOT one of them — anywhere**. + **`TOOL_CALLS_EXHAUSTED` is deliberately NOT one of them — anywhere**. `_EXECUTION_FACT_STATUSES` maps it to `False`, and the table and the chain that reads it must agree: it shipped as `True` while `_terminal_status`'s own docstring argued - the opposite, and the disagreement pinned a re-graded max-turns row at - MAX_TURNS_EXHAUSTED *while holding `weighted_score` 1.000* and exit 1 — a combination + the opposite, and the disagreement pinned a re-graded capped row at + TOOL_CALLS_EXHAUSTED *while holding `weighted_score` 1.000* and exit 1 — a combination `run` can never produce for the same trajectory. Under `execute`: `_terminal_status` puts the `grade=False` arm ABOVE it, because on the graded path it is subordinate to - the verdict — `run` returns SUCCESS for a max-turns trajectory whose criteria pass — + the verdict — `run` returns SUCCESS for a capped trajectory whose criteria pass — so it is not knowable without grading. Consuming it first made it terminal AND permanent (the `is_execution_fact` arm then pinned it), so identical agent output - scored SUCCESS/1.0 under `run` and MAX_TURNS_EXHAUSTED under `execute` → `evaluate`. - The fact survives on `result.max_turns_exhausted`, which `_seed_from_prior_result` + scored SUCCESS/1.0 under `run` and TOOL_CALLS_EXHAUSTED under `execute` → `evaluate`. + The fact survives on `result.tool_calls_exhausted`, which `_seed_from_prior_result` carries, so the detached grade walks the identical chain. The CLI must also branch on WHERE a status came from, not on its value: a preserved TIMEOUT exited 0 under "All criteria passed" (a CI wrapper reading the exit code went green on a row run.json @@ -676,6 +676,16 @@ never `source_yaml`, because the raw on-disk text predates `--model` and `-D` mu container must see. `source_yaml` is forwarded separately so `task.json`'s audit trail matches the in-process driver's. +### Plugin staging under docker + +The in-container orchestrator stages `agent.plugins` itself, under `/work/output`, which is +the host run dir bind-mounted. Each `plugins[].path` is dumped into the container's +`task.yaml` as the absolute host path (best-effort: a path that does not resolve on the +dumping host is left as authored, because a detached grade never uses it), and that +path is auto-mounted read-only at the same absolute path, together with any staged +skill whose resolved source sits outside every plugin root. So the stage's symlinks +resolve inside the container and, afterwards, on the host. + ## Trusting what the container sends back ### The stdout line limit @@ -741,7 +751,7 @@ verdict for work it never looked at and billing the model for it. Nothing else c Both are keyed on EVIDENCE, not on the label. For `grade`, "did it grade" is `success_criteria_results` or a non-None `weighted_score`: exempting every execution-fact -status let a stale image return a fully graded MAX_TURNS_EXHAUSTED row — criteria vector, +status let a stale image return a fully graded TOOL_CALLS_EXHAUSTED row — criteria vector, weighted score and all — unchallenged, because that exemption exists for statuses a *fresh* image also produces, and a fresh one produces them with neither. For `regrade`, a container that honored the request seeds from `prior` and never runs the agent, so a DIFFERENT diff --git a/.claude/notes/orchestration.md b/.claude/notes/orchestration.md index 800adb40b..121099eee 100644 --- a/.claude/notes/orchestration.md +++ b/.claude/notes/orchestration.md @@ -17,7 +17,7 @@ - **Generic CLI overrides (`-D`/`--set`)**: Layer 5 is a thin wrapper (`orchestration/overrides.py`) over the resolver above. `coder-eval run -D - agent.model=opus -D run_limits.max_turns=30` overrides any field on the resolved + agent.model=opus -D run_limits.max_tool_calls=30` overrides any field on the resolved `TaskDefinition` (`agent`/`run_limits`/`sandbox` roots), schema-validated with did-you-mean. Only `--model` (→ `agent.model`) and `--driver` (→ `sandbox.driver`) survive as active thin aliases that emit the equivalent `-D` entry; an alias and `-D` @@ -57,7 +57,7 @@ and `VariantAggregate.pass_rate` divide by `tasks_graded` (`tasks_run - tasks_not_graded`), and `tasks_not_graded` is part of the sum-to-`tasks_run` invariant, not a `tasks_failed` sub-counter. **Only SUCCESS/FAILURE collapse into it** - — `ERROR`, `TIMEOUT`, `BUILD_FAILED`, `MAX_TURNS_EXHAUSTED` and the budget stops are + — `ERROR`, `TIMEOUT`, `BUILD_FAILED`, `TOOL_CALLS_EXHAUSTED` and the budget stops are facts about the *run*, not about grading, and still apply (so `execute` still exits non-zero on a crash). The switch is `BatchRunConfig.grade` → `Orchestrator(grade=...)` → the **four** grading call sites (single-shot, evaluate-only, the simulation dialog @@ -88,15 +88,15 @@ grading switch was threaded in. Its ORDER is load-bearing at every step. observed. Without that first arm, a crashed run re-graded against its half-finished workspace reports SUCCESS — with the original `error_message` still attached. -**The NOT_GRADED arm sits ABOVE `max_turns_exhausted`, and that order is what makes -`execute` + `evaluate` equal a single `run`.** MAX_TURNS_EXHAUSTED reads like an execution +**The NOT_GRADED arm sits ABOVE `tool_calls_exhausted`, and that order is what makes +`execute` + `evaluate` equal a single `run`.** TOOL_CALLS_EXHAUSTED reads like an execution fact but is not one: on the graded path it is subordinate to the verdict — `run` returns -SUCCESS for a max-turns trajectory whose criteria pass, and only falls through to -MAX_TURNS_EXHAUSTED when they do not — so it is not knowable under `grade=False`. +SUCCESS for a capped trajectory whose criteria pass, and only falls through to +TOOL_CALLS_EXHAUSTED when they do not — so it is not knowable under `grade=False`. Consuming it first made it terminal AND permanent, so the same agent output scored -SUCCESS/1.0 under `run` and MAX_TURNS_EXHAUSTED under `execute` → `evaluate`; being +SUCCESS/1.0 under `run` and TOOL_CALLS_EXHAUSTED under `execute` → `evaluate`; being category `failed`, `run --resume` then called the row complete and left it forever -unscored. Nothing is lost by deferring: the fact lives on `result.max_turns_exhausted`, +unscored. Nothing is lost by deferring: the fact lives on `result.tool_calls_exhausted`, which the seeding carries. The statuses that ARE execution facts differ in kind — they abort the run before a verdict is reachable, so preserving them overturns nothing. @@ -279,8 +279,8 @@ silent. A missing stamp (a run predating the feature) is tolerated. - **Early stop on criterion (opt-in, per-criterion arming)**: a `stop_early:` block (`StopEarlyPolicy`) on a criterion ends a single-shot run early once the run's - **armed** criteria decide the outcome, so a raised `max_turns` isn't wasted on the - smoke flavor. The block's PRESENCE is the arming and alone activates the watcher — + **armed** criteria decide the outcome, so a raised `max_tool_calls` isn't wasted on the + smoke flavor. The block's PRESENCE is the arming and alone arms the monitor — there is **no run-level master switch**: `run_limits.stop_early: false` is the run-level KILL SWITCH that force-disarms every block (the one-line experiment-variant/`-D` override for an authoritative full run), and @@ -313,8 +313,9 @@ silent. A missing stamp (a run predating the feature) is tolerated. never freezes a sibling `on_pass: continue` criterion's signal out of the trajectory). A fail-stop is therefore verdict-preserving; a pass-stop can miss a *later* distractor misfire, so authoritative P/R/F1 comes from a kill-switched (`stop_early: false`) run. - Driven by `orchestration/early_stop.py::EarlyStopWatcher` (built when - `early_stop_active(task)`: ≥1 armed criterion, kill switch not thrown) through the + Driven by `orchestration/turn_monitor.py::TurnMonitor` (built by `_build_monitor` in + `_setup` on every run; its criteria are armed when `early_stop_active(task)`: ≥1 armed + criterion, kill switch not thrown, and grading on) through the 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. Gating is **FIRED-ONLY**: a run the watcher actually cut @@ -359,11 +360,14 @@ is never assigned there, so an armed simulation task gates strict-AND on a possi truncated trajectory. Wiring the dialog path through it means also setting `early_stop` there; until then the limit is stated rather than implied. -The watcher is built ONCE, in `_setup`, so its turn/tool counters and wall-clock origin -accumulate across retry attempts. It is built before the evaluate-only early return, so an -armed evaluate-only re-grade builds an inert, never-fed watcher — harmless, and one -creation point. Under `execute` it is armed but stays disabled: there is no outcome to -decide and the trajectory is the deliverable, so an armed criterion must not truncate it. +The `TurnMonitor` is built ONCE, by `_build_monitor` in `_setup`, on every run, so its +tool-call counters and wall-clock origin accumulate across retry attempts and dialog turns +(that is what makes `run_limits.max_tool_calls` cumulative per task). It is built before +the evaluate-only early return, so an evaluate-only re-grade builds an inert, never-fed +monitor — harmless, and one creation point. Under `execute` its criteria are not armed +(`arm=self.grade`): there is no outcome to decide and the trajectory is the deliverable, so +an armed criterion must not truncate it. The tool-call cap still applies there, because it +is a run limit, not a verdict. ### Verdicts latch, and the decision happens on the CALL @@ -454,6 +458,23 @@ the pass-stop each round — and if none ever decides, the run simply continues A row with zero pass-capable armed criteria (a negative row stacking only distractors) has nothing to defer for and fail-stops on the first misfire. +### The watcher became the TurnMonitor + +`EarlyStopWatcher` answered one question on the `should_stop` channel. Every harness +also counted its own turn cap in its own unit, and the budgets were checked by the +orchestrator after a turn had already spent the money. `TurnMonitor` answers all four +reasons (`EARLY_CRITERION`, `TOOL_CALL_CAP`, `TOKEN_BUDGET`, `USD_BUDGET`) from ONE +collector, so a cap means the same number of resolved tool calls on every harness and a +budget stops the agent at its next poll. It is cumulative because one instance serves +every retry attempt and every dialog turn of a task: the cap, the budgets and +`expected_tool_calls` all measure the task, not an attempt. On one round the armed stop +wins, then the cap, then the token budgets, then USD, and the first latched reason is +final, so the status an adapter finalizes with cannot flip after the fact. Fail-open +covers only the armed criteria: a raising `live_verdict` is agent-output-dependent code, +while the cap and budgets read counters and must keep running on a run that has lost its +criteria. `result.tool_calls_exhausted` still comes from the turn's end status, not the +latch, because a cap latched after the agent's last poll stopped nothing. + ### Inert triggers are by design, and the watcher fails open A trigger whose polarity an instance can never decide is INERT, not an error — one @@ -462,14 +483,15 @@ distractor rows (fail live, pass and timeout inert) without per-row conditionals why the validator carries NO per-instance polarity guards. Arming an unobservable criterion is structurally impossible, since the block exists only on `LiveSuccessCriterion`, so a `file_exists` criterion carrying one is an `extra='forbid'` error at load. An armed-but- -empty set needs no guard either: with no blocks present there is simply no watcher. +empty set needs no guard either: with no blocks present the monitor has nothing armed and +only its run-limit cap can stop the run. -The watcher keeps its OWN `EventCollector`, independent of the one the agent builds its +The monitor keeps its OWN `EventCollector`, independent of the one the agent builds its returned `TurnRecord` from, so each `live_verdict` sees a fresh single-element partial trajectory. -**Fail-open:** a `live_verdict` that raises disarms the watcher, logs loudly, and degrades -to a full run. Because live verdicts are triggers and not truth, this can never produce a +**Fail-open:** a `live_verdict` that raises disarms the armed criteria, logs loudly, and +degrades to a full run. The tool-call cap reads counters, so it keeps running. Because live verdicts are triggers and not truth, this can never produce a FALSE early stop — it only ever errs toward running more. ### Why the guardrails are not model validators diff --git a/.claude/notes/reporting.md b/.claude/notes/reporting.md index 5c0607cfa..823d940a1 100644 --- a/.claude/notes/reporting.md +++ b/.claude/notes/reporting.md @@ -15,16 +15,16 @@ - **Run-time caps (non-criterion enforcement)**: `TaskDefinition.run_limits` (`RunLimits` model) is the single namespace for all *task-level* run-time caps — - `max_turns` / `task_timeout` / `turn_timeout` (structural) and `max_input_tokens` / + `max_tool_calls` / `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.max_tool_calls=…` / `-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. The one *per-criterion* cap, `stop_early.decide_within`, deliberately lives on `LiveSuccessCriterion` instead - (see [orchestration.md](orchestration.md) § Early stop on criterion) — the watcher + (see [orchestration.md](orchestration.md) § Early stop on criterion) — the monitor must attribute a decision-step timeout to a specific criterion, which `RunLimits` (task-scoped, criterion-agnostic) cannot express. @@ -332,10 +332,12 @@ means the whole bill. ### The claims the reports do NOT make -An early-stopped row does not advertise "N turns avoided". That derived from +An early-stopped row does not advertise "N turns avoided". That claim once derived from `max_turns - sdk_turn_index`, and on harnesses where one `communicate()` is a single SDK turn it advertised dozens of avoided turns when all that was cut was a tool-call tail. The -upper bound is still persisted, labelled as the bound it is. +upper bound is still persisted as `tool_calls_remaining_at_stop` +(`max_tool_calls - tool_call_index`, null when the cap is unset), labelled as the bound it +is. Missing spend is worded cause-agnostically, because an unpriced turn and a hard kill reach the same conclusion and the report cannot always tell which applied. diff --git a/.claude/notes/timing.md b/.claude/notes/timing.md index 49a3ad769..9d58a81c8 100644 --- a/.claude/notes/timing.md +++ b/.claude/notes/timing.md @@ -188,7 +188,7 @@ terminal event as `AgentEndEvent(messages=list(...))` — that copies the LIST, message objects — so writing in place would reach back into the agent's own live state from the collector, which is exactly the layering "the collector is the sole capture seam" exists to prevent. It is also unconditionally safe for a caller that builds a record -twice: `EarlyStopWatcher` holds one collector across a turn's tool-call rounds and calls +twice: `TurnMonitor` holds one collector across a task's tool-call rounds and calls `build_turn_record` on every one. Grouping by identical bounds rather than `message_id`: Codex splits one window across two diff --git a/.claude/shared/run-layout.md b/.claude/shared/run-layout.md index c00a82c69..c006e9352 100644 --- a/.claude/shared/run-layout.md +++ b/.claude/shared/run-layout.md @@ -16,6 +16,7 @@ runs/////{task.json, task.log, artifacts/} - `task.json.graded` — present only after `coder-eval execute --driver docker` refused a container's verdict: the runtime image predated `execute` and graded anyway, so the runner quarantines the graded record here rather than leaving it readable as `task.json`, where a later `--resume` / `aggregate` would fold in exactly the row it declined to publish. Diagnostic-only; `rglob("task.json")` consumers do not match it. - `grade.log` — present only after a DETACHED grade over this directory (`coder-eval run --resume`). The grading pass's own log. It is a separate file because the log handler truncates whatever file it opens, so writing to `task.log` would destroy the agent trajectory log the run already paid for. - `task.log` — the human-readable task log; `artifacts/` — files the agent produced. +- `plugin_root/` — the staged plugin root the agent was handed; symlinks into the authored plugin; present only when the task sets `agent.plugins`. **Scope-marker files** (used to detect what a given path represents): diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 8eec69a15..935c3e4d4 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -798,10 +798,9 @@ jobs: - name: Run cost-budget smoke (max_usd → COST_BUDGET_EXCEEDED via DirectRoute) # Verifies the cost path end-to-end on DirectRoute (Anthropic reports # per-turn cost). A regression that broke per-turn cost reporting would - # log "max_usd budget configured but no turn reported cost; skipping - # cost check" and the task would land in tasks_failed via the - # placeholder criterion instead of COST_BUDGET_EXCEEDED. The - # final_status assertion below catches that silently-broken case. + # finish the task ERROR ("run_limits.max_usd could not be enforced") + # instead of COST_BUDGET_EXCEEDED. The final_status assertion below + # catches that case. run: | # Inverted exit semantics: coder-eval exits 1 when a task fails. We want that. if .venv/bin/coder-eval run tasks/smoke_cost_budget_exceeded.yaml \ diff --git a/.github/workflows/verify-published-action.yml b/.github/workflows/verify-published-action.yml index 3079e354e..27540943a 100644 --- a/.github/workflows/verify-published-action.yml +++ b/.github/workflows/verify-published-action.yml @@ -344,10 +344,10 @@ jobs: # caps are ~10x what the one-file task needs; tripping the spend or wall-clock # one produces a COST_BUDGET_EXCEEDED / TIMEOUT row that the gate's run-limit # branch prints and fails on (both statuses report as "failed", so nothing else - # in the gate would). MAX_TURNS_EXHAUSTED is the one deliberate exception: it is + # in the gate would). TOOL_CALLS_EXHAUSTED is the one deliberate exception: it is # the classic model-quality outcome, tolerated like an unmet criterion. run_limits: - max_turns: 5 + max_tool_calls: 5 task_timeout: 300 max_usd: 0.25 diff --git a/CLAUDE.md b/CLAUDE.md index 2ad1f2f0a..aebcd9e02 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -85,12 +85,14 @@ Each entry is a pointer. Full rationale: `.claude/notes/` (index: `.claude/notes - **Reference solutions are directory-only** and chmod-shielded during `communicate`. Defense-in-depth, not a boundary — the known gaps are documented in the notes. Authoring reference: [Reference Solutions](docs/TASK_DEFINITION_GUIDE.md#reference-solutions). -- **Harness run-limit parity**: a shared config field must mean the same thing on every - backend, or the divergence is documented. Every agent declares a `HarnessContract`; a - base field the harness marks unsupported is rejected at resolution. Table: - [Run-Limit Parity](docs/agents/HARNESS_PARITY.md) § Agent-field contract - (generated). Caps are authored under - [Run Limits](docs/TASK_DEFINITION_GUIDE.md#run-limits). +- **Harness run-limit parity**: every structural cap and budget is one `TurnMonitor` + answer on the `should_stop` channel, in tool calls or tokens, on every harness; + `run_limits` and agent-field meanings are both generated tables in + [Run-Limit Parity](docs/agents/HARNESS_PARITY.md). Every agent declares a + `HarnessContract`; a base field the harness marks unsupported is rejected at + resolution. Caps are authored under [Run Limits](docs/TASK_DEFINITION_GUIDE.md#run-limits). +- **Plugin staging**: `stage_plugins` hands every harness one canonical plugin root; + `skills_offered` is the positive control `skill_triggered` checks. - **Execute vs. run**: `execute` is `run` with grading off — rows finalize as `NOT_GRADED` and leave both sides of every rate. Per-command behaviour: [CLI Commands](docs/USER_GUIDE.md#cli-commands). @@ -134,8 +136,9 @@ CLI → ExperimentRunner (task × variant, 5-layer merge) → run_batch → Orch Per-task (single iteration; simulation mode runs a multi-turn dialog): 1. Orchestrator._communicate_with_retry(prompt, iteration) → TurnRecord - (wraps agent.communicate with retry, per-attempt turn_timeout, and - on_attempt_error → preserves crashed=True partial TurnRecords) + (wraps agent.communicate with retry, per-attempt turn_timeout, the task's + TurnMonitor as the should_stop poll, and on_attempt_error → preserves + crashed=True partial TurnRecords) 2. SuccessChecker.check_all_async() → List[CriterionResult] Cleanup: stop agent, save EvaluationResult, generate reports. @@ -164,7 +167,7 @@ make evalboard-verify # the JS half: tsc --noEmit + vitest + next build make docs-indexes # README/docs index tables from the mkdocs nav (CE028) make plugin-reference # the plugin's criteria reference from the models (CE033) make pricing-mirror # the evalboard's rate table from pricing.py (CE065) -make parity-table # the agent-field contract tables from the agent classes (CE069) +make parity-table # the run-limit and agent-field tables from RunLimits and the agent classes (CE069) make docs-budget # per-file comment budget + docstring essay check (fails `make verify`) ``` @@ -220,8 +223,11 @@ A few rules constrain routine edits, so they are worth knowing before you start: `stats.py` and `run_record.py`. - **CE068** keeps `orchestration/`, `streaming/` and `timing.py` free of concrete agent config classes and `AgentKind` members (except `UNKNOWN`); ask the registry instead. -- **CE069** diffs the generated contract tables in `docs/agents/HARNESS_PARITY.md` against - the agent classes. Regenerate with `make parity-table`. +- **CE069** diffs the generated run-limit and contract tables in `docs/agents/HARNESS_PARITY.md` + against `RunLimits` and the agent classes. Regenerate with `make parity-table`. +- **CE070** keeps agent adapters from counting caps (`max_tool_calls`, `RunLimits`, + `tool_calls_exhausted`, …) or scanning for `SKILL.md`: the `TurnMonitor` owns caps and + `orchestration/plugin_staging.py` owns skill discovery. **Docs index SSOT.** `nav:` plus `extra.docs_index` in `mkdocs.yml` are the single source of truth for `README.md`'s Documentation table, `docs/index.md`'s "Where to go diff --git a/Makefile b/Makefile index 4f619730b..514890d65 100644 --- a/Makefile +++ b/Makefile @@ -39,7 +39,7 @@ plugin-reference: ## Regenerate the plugin's bundled criteria reference from th pricing-mirror: ## Regenerate the evalboard's rate table from pricing.py (SSOT) uv run python -m tests.lint.pricing_mirror -parity-table: ## Regenerate the agent-field contract table from the agent classes (SSOT) +parity-table: ## Regenerate the agent-field, tool-name and run-limit tables (SSOT) uv run python -m tests.lint.harness_parity docs-budget: ## Report the docstring/comment prose budget and check it against the baseline diff --git a/docs/AB_EXPERIMENTS.md b/docs/AB_EXPERIMENTS.md index eee67bf81..ade3cef5b 100644 --- a/docs/AB_EXPERIMENTS.md +++ b/docs/AB_EXPERIMENTS.md @@ -99,7 +99,7 @@ it (the unification invariant). The per-field strategy is: **most lists** (`allowed_tools`, `disallowed_tools`, `plugins`, …) — **replace** (last layer wins; a variant's `allowed_tools: ["Read"]` replaces the lower list entirely). `run_limits` is per-field replace, so a variant setting - `run_limits.max_turns` leaves the task's `task_timeout` intact. + `run_limits.max_tool_calls` leaves the task's `task_timeout` intact. - **nested models** (`sandbox.docker`, `python`, `node`, `limits`) and **free-form dicts** (`agent.sdk_options`) — **deep**-merge: a higher layer touching one sub-key (e.g. `docker.network`) preserves siblings set below it (e.g. @@ -152,7 +152,7 @@ From `ExperimentVariant` (`coder_eval/models/experiment.py`): | `prompt_mutations` | list | Ordered mutations applied to `initial_prompt` | | `initial_prompt` | str | Full prompt replacement (mutually exclusive with the two below) | | `initial_prompt_file` | str | Prompt replacement loaded from a file | -| `run_limits` | block | Per-key cap overrides (`max_turns`, `task_timeout`, token/USD budgets) | +| `run_limits` | block | Per-key cap overrides (`max_tool_calls`, `task_timeout`, token/USD budgets) | | `driver` | `tempdir`/`docker` | Sandbox driver — enables tempdir-vs-docker arms | | `checker_context` | dict | Backend/model override for the judge side (llm_judge, agent_judge) — has no bearing on the simulator; see [Checker Context](TASK_DEFINITION_GUIDE.md#checker-context); **not** currently `-D`-reachable | @@ -205,7 +205,7 @@ variants: agent: plugins: - type: "local" - path: ".." # PLUGIN ROOT holding skills/ — see note below + path: ".." # plugin root or bare skills directory — see note below ``` Notes: @@ -217,14 +217,12 @@ Notes: it. Pair the experiment with a [`skill_triggered`](TASK_DEFINITION_GUIDE.md#skill_triggered) criterion to measure _whether it fired_ alongside your real success criteria that measure _whether outcomes improved_. -- **`path` must be a plugin ROOT — a directory holding `skills/`** — so the skill - resolves at `/skills//SKILL.md`. Point one level deeper, at the - directory of skill directories, and claude-code loads **nothing**: the `with-skill` - arm then silently matches the baseline and the A/B compares two identical arms. - Codex and Antigravity accept either depth, so this fails on claude-code alone — - see [Harness parity](agents/HARNESS_PARITY.md#agentpluginspath-accepts-different-depths-per-harness). +- `path` names a plugin root (`/skills//SKILL.md`) or a bare skills + directory (`//SKILL.md`). Both are staged. A path with no skill fails + `plan`, so the `with-skill` arm cannot silently match the baseline. See + [Plugin staging](agents/HARNESS_PARITY.md#plugin-staging). - Plugin paths are environment-dependent. The shipped example expects a - `$PLUGIN_PATH` env var pointing at your plugin **root**. See + `$PLUGIN_PATH` env var pointing at your plugin root or skills directory. See `experiments/plugin-comparison.yaml`. Run it: @@ -309,7 +307,7 @@ that define "the interesting thing happened" with `stop_early:` blocks in the task file; the `smoke` variant cuts off as soon as they're decided, while `e2e` runs to completion. Because the field merge is per-key, the variant sets only -`stop_early` (the run-level kill switch) without disturbing the task's `max_turns`. +`stop_early` (the run-level kill switch) without disturbing the task's `max_tool_calls`. ```yaml experiment_id: early-stop-ab @@ -324,7 +322,7 @@ variants: ``` The task file supplies the arming (`stop_early:` blocks on the criteria that gate the -flavor) and a `max_turns` generous enough for `e2e`; see +flavor) and a `max_tool_calls` 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`. @@ -390,8 +388,8 @@ if any listed metric is below its minimum. | `--driver tempdir\|docker` | Override sandbox driver for all tasks. | | `-j, --max-parallel N` | Run up to N tasks concurrently. | | `-t, --tags` / `--exclude-tags` | Filter which tasks run. | -| `-D path=value` / `--set` | Generic layer-5 override of any resolved task-config field, applied to **every** variant — e.g. `-D agent.model=opus -D run_limits.max_turns=30`. Repeatable; schema-validated. | -| `--model`, `--driver` | Thin aliases for `-D` (`--model` ≡ `-D agent.model`, `--driver` ≡ `-D sandbox.driver`). All other task-config knobs (permission mode, turn/timeout limits, tools, plugins, SDK options) are set via `-D`. Layer-5 overrides apply to **every** variant (use sparingly — they erase the contrast between arms). | +| `-D path=value` / `--set` | Generic layer-5 override of any resolved task-config field, applied to **every** variant — e.g. `-D agent.model=opus -D run_limits.max_tool_calls=30`. Repeatable; schema-validated. | +| `--model`, `--driver` | Thin aliases for `-D` (`--model` ≡ `-D agent.model`, `--driver` ≡ `-D sandbox.driver`). All other task-config knobs (permission mode, tool-call/timeout limits, tools, plugins, SDK options) are set via `-D`. Layer-5 overrides apply to **every** variant (use sparingly — they erase the contrast between arms). | | `--type` | Dedicated flag for agent type, applied to every variant (re-parses the agent discriminated union). | Layer-5 flags win over variant config, so overriding the very thing you're diff --git a/docs/DIALOG_MODE.md b/docs/DIALOG_MODE.md index 0f18bdd28..98c6d4c3d 100644 --- a/docs/DIALOG_MODE.md +++ b/docs/DIALOG_MODE.md @@ -122,8 +122,11 @@ After each exchange the driver evaluates the stop conditions **in this order**, 2. **`stop_on_criteria_pass`** (`criteria_passed`) — every success criterion passes. Requires per-turn checking (`check_criteria: every_turn` or `both`); pairing it with the default `end_of_dialog` is rejected at load time, since there would be nothing to check against. -3. **`max_turns`** (`max_turns`) — the hard cap on exchanges. The agent exhausting its *own* inner - `max_turns` mid-exchange ends the dialog with the same reason. +3. **`max_turns`** (`max_turns`) — the hard cap on exchanges. The agent reaching + [`run_limits.max_tool_calls`](TASK_DEFINITION_GUIDE.md#run-limits) mid-exchange ends the dialog + with its own reason, `tool_call_cap`, which wins over `max_turns`, the budget and the stop token + on that turn (only a criteria pass outranks it). That cap is cumulative across every dialog turn: + it counts the agent's resolved tool calls over the whole dialog, not per exchange. 4. **`max_total_tokens`** (`budget`) — the dialog-wide budget across simulator **and** agent. The dialog ends and the task is **still scored** — unlike [`run_limits.max_total_tokens`](TASK_DEFINITION_GUIDE.md#run-limits), which covers the subject @@ -131,8 +134,8 @@ After each exchange the driver evaluates the stop conditions **in this order**, 5. **`stop_token`** (`stop_token`) — only if none of the above fired is the simulator asked for another message; the sentinel token in *that fresh utterance* ends the dialog. This is the workhorse in practice — the simulator decides, in character, that it got what it wanted — but it - is evaluated **last**, so a turn that trips `max_turns` or the budget never gets the chance to - produce it. + is evaluated **last**, so a turn that trips `max_turns`, the tool-call cap or the budget never + gets the chance to produce it. A simulator call that raises ends the dialog with `error` (and increments `simulation.simulator_failures`). The reason is recorded as `simulation.stop_reason` on the result, @@ -188,8 +191,9 @@ with them. That guard is what keeps a chatty simulator from poisoning the grade. ## What it costs -Budget roughly `max_turns × n_trials` agent turns per (task, variant) — the worst case, since the -dialog usually stops on the stop token first. On top of that: +Budget roughly `simulation.max_turns × n_trials` agent turns per (task, variant) — the worst case, +since the dialog usually stops on the stop token first. `run_limits.max_tool_calls` bounds the tool +calls of one trial's whole dialog, not of each exchange. On top of that: - **Simulator tokens**, one generation per turn. Small next to the agent's, but not free, and they are reported separately as `simulation.simulator_input_tokens` / `simulator_output_tokens`. diff --git a/docs/EXTENDING.md b/docs/EXTENDING.md index c4307de56..995f6d85c 100644 --- a/docs/EXTENDING.md +++ b/docs/EXTENDING.md @@ -48,7 +48,7 @@ signature. from coder_eval.spi import SPI_VERSION, AgentRegistry def register(registry: type[AgentRegistry]) -> None: - assert SPI_VERSION == 1, f"my-agent supports coder_eval SPI 1, not {SPI_VERSION}" + assert SPI_VERSION == 2, f"my-agent supports coder_eval SPI 2, not {SPI_VERSION}" # Bind type string → config class → agent class. registry.register("my-agent", MyAgentConfig)(MyAgent) # Optionally contribute pricing here too (see §3): @@ -99,7 +99,7 @@ at resolution, so `coder-eval plan` fails before any run. This is a JSONL CLI ag that appends a system prompt and honors `plan` and tool lists natively: ```python -from coder_eval.spi import Agent, Enforcement, HarnessContract, PermissionMode, ToolNameMap +from coder_eval.spi import Agent, Enforcement, HarnessContract, PermissionMode, ToolNameMap, UsageGranularity # native tool name -> canonical (Claude) name; also used for telemetry _TOOL_NAME_MAP = {"bash": "Bash", "read": "Read", "write": "Write", "edit": "Edit", "task": "Agent"} @@ -114,6 +114,7 @@ class MyAgent(Agent[MyAgentConfig]): allowed_tools=Enforcement.ENFORCED, disallowed_tools=Enforcement.ENFORCED, cooperative_stop=True, + usage_granularity=UsageGranularity.STEP, ) tool_names = ToolNameMap.from_inverse( _TOOL_NAME_MAP, @@ -129,8 +130,8 @@ class MyAgent(Agent[MyAgentConfig]): - `tool_names` is required exactly when a tool-list row is `ENFORCED`. It must map every canonical name; list a name your harness has no tool for in `no_equivalent`. - Set `cooperative_stop=True` only if your `communicate()` honors `should_stop` - (needed for criterion-level `stop_early:` arming). `False` means early stop is - rejected at resolution for your agent. + (needed for criterion-level `stop_early:` arming and for `run_limits.max_tool_calls` + to cut a turn). `False` means early stop is rejected at resolution for your agent. ### The `Agent` ABC — implementation checklist @@ -140,10 +141,26 @@ it on every LiteLLM route. Implement these three abstract methods: -- [ ] `async def start(self, working_directory, *, env_path_prepend=None, plugin_tools_dir=None) -> None` -- [ ] `async def communicate(self, user_input, *, stream_callback=None, timeout=None, max_turns=None, should_stop=None) -> TurnRecord` +- [ ] `async def start(self, working_directory, *, env_path_prepend=None, plugin_tools_dir=None, plugin_root: Path | None = None) -> None` +- [ ] `async def communicate(self, user_input, *, stream_callback=None, timeout=None, should_stop: Callable[[], StopReason | None] | None = None) -> TurnRecord` - [ ] `async def stop(self) -> None` +`plugin_root` is the staged plugin root (`/skills//SKILL.md`), or `None` when +the task sets no plugins. Deliver it the harness's native way; do not scan for skills. + +`should_stop` is the run's single stop poll. The `TurnMonitor` owns it: it reads your +event stream and decides every stop (armed criteria, the tool-call cap, the token and USD +budgets). Your agent does not count or cap anything. The budgets read +`TurnEndEvent.tokens` as a per-report DELTA and `AgentEndEvent.usage` as the attempt's +authoritative total, so never report cumulative tokens on a `TurnEndEvent`. Declare how +often you report them as `usage_granularity`. With `cooperative_stop=True`: + +- [ ] Call `should_stop()` at each safe boundary (for example, after each resolved + tool call, before you pull the next unit of work). +- [ ] When it returns a `StopReason`, stop pulling work and remember the reason. +- [ ] Finalize the turn with `AgentEndStatus` `end_status_for(reason)` (both names + come from `coder_eval.spi`), with `crashed=False`. Do not raise. + Optional overrides (sensible defaults exist): `kill()`, `kill_sync()` (called from a non-asyncio watchdog thread — must **not** await), `discard_pending_turn()`. @@ -270,14 +287,14 @@ Notes: own fields — no `turn_records`, no checker instance), and override the checker's `live_verdict(...)`. `LiveSuccessCriterion` subclassing is the single source of truth for "is this criterion type live-observable" — - `validate_early_stop`/`EarlyStopWatcher` check `isinstance(c, + `validate_early_stop`/`TurnMonitor` check `isinstance(c, LiveSuccessCriterion)` directly, no separate checker-side flag. A lint rule (`tests/test_custom_lint.py::TestCE025LiveVerdictConsistency`) keeps the model subclassing and the checker's `live_verdict` override paired. - Your `live_verdict` must be **deterministic** (a pure function of the `turn_records` prefix — no wall-clock, randomness, or hidden instance state) and **monotonic** (once it returns `"pass"`/`"fail"` for some prefix, every - longer prefix returns that same verdict) — `EarlyStopWatcher`'s verdict + longer prefix returns that same verdict) — `TurnMonitor`'s verdict latching and deferred stops silently depend on both. Lint rule CE036 (`tests/lint/live_verdict_contract.py`) enforces this by replaying each live criterion against every prefix of recorded trajectories, and **fails until diff --git a/docs/PLUGIN.md b/docs/PLUGIN.md index 3c3536129..97901ce5e 100644 --- a/docs/PLUGIN.md +++ b/docs/PLUGIN.md @@ -106,18 +106,18 @@ It then: One prerequisite the suite cannot infer: the evaluated agent runs in a fresh sandbox holding none of your files, so it is offered no skills unless the task says where they live. The template reads that location from an environment -variable — point it at a **plugin root**: a directory holding a `skills/` -subdirectory, so the skill sits at `/skills//SKILL.md`. For -`.claude/skills/pdf-forms/SKILL.md` that root is `.claude`, not `.claude/skills`: +variable. Point it at a plugin root (`/skills//SKILL.md`) or at a +bare skills directory (`//SKILL.md`). Both are staged. For +`.claude/skills/pdf-forms/SKILL.md`, `.claude` works and `.claude/skills` works too: ```bash export SKILL_SOURCE_PATH="$(pwd)/.claude" ``` -Leave it unset and the skill is simply absent, every positive row scores 0, and -the result is indistinguishable from a skill that never fires. It stays an -environment variable rather than a path baked into the YAML so the suite is -portable — it is committed and re-run on other machines, and in CI. +Leave it unset, or point it at a directory with no skill, and `coder-eval plan` +fails with a config error. It stays an environment variable rather than a path +baked into the YAML so the suite is portable — it is committed and re-run on other +machines, and in CI. ### A low-recall result has three causes, not one diff --git a/docs/REPORT_SCHEMA.md b/docs/REPORT_SCHEMA.md index b1f1a8cd1..57aedd042 100644 --- a/docs/REPORT_SCHEMA.md +++ b/docs/REPORT_SCHEMA.md @@ -83,12 +83,28 @@ including: `task_id`, `replicate_index`, `variant_id`, `status` `judge_cost_usd` / `simulator_cost_usd` slices and the `cost_complete` flag), `expected_commands`, `actual_commands`, `commands_efficiency`, `agent_config`, `sdk_options`, -`installed_tools`, turn accounting (`total_turns`, `visible_turns`, `expected_turns`, -`max_turns_exhausted`, `has_final_reply`), and early-stop fields (`stopped_early`, -`early_stop_reason`, `turns_remaining_at_stop`). `iterations` here is a **reduced** +`installed_tools`, turn accounting (`total_turns`, `visible_turns`, `expected_tool_calls`, +`expected_tool_calls_overage`, `tool_calls_exhausted`, `has_final_reply`), and early-stop fields (`stopped_early`, +`early_stop_reason`, `tool_calls_remaining_at_stop`). `iterations` here is a **reduced** turn digest (`{iteration, duration_seconds, command_count, assistant_turn_count, crashed, crash_reason}`) — the full transcript is in `task.json`. +> **Historical spellings.** Runs written before the tool-call rename carry +> `max_turns_exhausted`, `expected_turns`, `expected_turns_overage` and the status +> `MAX_TURNS_EXHAUSTED` instead of `tool_calls_exhausted`, `expected_tool_calls`, +> `expected_tool_calls_overage` and `TOOL_CALLS_EXHAUSTED`. The evalboard reads both. +> The Python side does not: there is no alias. A `task.json` with the old flag loads with +> the fact `false`; one whose `final_status` is `MAX_TURNS_EXHAUSTED`, or whose recorded +> config sets `run_limits.expected_turns`, does not load. `run --resume` then runs that +> row again, and `evaluate ` cannot re-grade it from its recorded config. +> +> Runs written before the tool-call cap replaced the turn cap carry +> `turns_remaining_at_stop` instead of `tool_calls_remaining_at_stop`, in both +> `EarlyStopInfo` and the `run.json` row. No reader maps the old key. Their recorded +> config also sets `run_limits.max_turns`, which no longer validates, so +> `evaluate ` re-grades such a run from the source task YAML and prints its +> fallback warning. + ### Missing cost is never fatal Pricing degrades; the evaluation does not. A model absent from the rate card, a turn @@ -128,7 +144,7 @@ The authoritative per-replicate record. | --- | --- | --- | | `final_status` | [`FinalStatus`](#finalstatus) | Terminal status. | | `weighted_score` | `float \| null` | Weighted average of criterion scores, 0.0–1.0. | -| `max_turns_exhausted` | `bool` | Ran out of turns. | +| `tool_calls_exhausted` | `bool` | The tool-call cap ended an iteration before the agent completed on its own. | | `iteration_count` | `int` | Number of turns. | | `success_criteria_results` | `list[CriterionResult]` | Per-criterion results — see [below](#criterionresult). | | `post_failure_criteria_results` | `list[CriterionResult]` | Diagnostic artifact evidence collected after a terminal agent failure. It does not affect `final_status`, `weighted_score`, gating, or suite aggregation. | @@ -158,6 +174,8 @@ each of `system_prompt`, `plugin_skills`, `permission_mode`, `allowed_tools` and `disallowed_tools`, `"enforced"` or `"unsupported"`, plus `system_prompt_semantics` (the class default), `cooperative_stop`, and `permission_modes` (the sorted `permission_mode` values the harness honors, or `null`). +`environment_info.skills_offered` is the list of skill names the staged plugin root +offered to the agent. It is absent when the task sets no `agent.plugins`. `sdk_options.system_prompt` is a `SystemPromptPreset` dict (`{type: "preset", preset: "claude_code", exclude_dynamic_sections: true, append?: str}`) on append-mode Claude Code runs and a plain string only in replace mode — it is @@ -211,7 +229,7 @@ canonical score remains 0.0. (`list[ProviderCallCost]` — one row per real upstream call with its ACTUAL cost + cache buckets, captured proxy-side on the LiteLLM open-weight backend and rendered by the evalboard as a per-call table; empty on every other -backend), `num_turns`, `max_turns_exhausted`, +backend), `num_turns`, `tool_calls_exhausted`, `result_summary` (`{is_error, subtype, stop_reason, result}`), `crashed`, `crash_reason`. @@ -233,7 +251,8 @@ criterion timed out undecided past its `stop_early.decide_within`; it gates thro the same weighted armed gate as a native fail), `deciding_criterion_type`, `deciding_criterion_description`, `armed_criteria`, `sdk_turn_index`, `tool_call_index` (1-based, includes the in-flight call), -`elapsed_seconds`, `turns_remaining_at_stop`, `gate_threshold` (the +`elapsed_seconds`, `tool_calls_remaining_at_stop` (`max_tool_calls − tool_call_index`, +floored at `0`; `null` when `run_limits.max_tool_calls` is unset), `gate_threshold` (the `run_limits.stop_early_gate_threshold` in effect for this stop; default `1.0`). --- @@ -320,7 +339,7 @@ String enum values and their reporting category: | `SUCCESS` | succeeded | `+` | | `FAILURE` | failed | `-` | | `TIMEOUT` | failed | `T` | -| `MAX_TURNS_EXHAUSTED` | failed | `M` | +| `TOOL_CALLS_EXHAUSTED` | failed | `C` | | `TOKEN_BUDGET_EXCEEDED` | failed | `#` | | `COST_BUDGET_EXCEEDED` | failed | `$` | | `ERROR` | error | `!` | @@ -341,7 +360,7 @@ crash, timeout, or budget breach under `execute` reports `ERROR` / `TIMEOUT` / `TOKEN_BUDGET_EXCEEDED` and `COST_BUDGET_EXCEEDED` are produced by the cumulative budget caps under `run_limits:` (`max_input_tokens` / `max_output_tokens` / `max_total_tokens`, and `max_usd` -respectively), checked after each completed agent turn — see +respectively), enforced live by the `TurnMonitor` — see [Task Definition Guide → Run Limits](TASK_DEFINITION_GUIDE.md#run-limits). --- diff --git a/docs/TASK_DEFINITION_GUIDE.md b/docs/TASK_DEFINITION_GUIDE.md index c4e1126fb..3013dee01 100644 --- a/docs/TASK_DEFINITION_GUIDE.md +++ b/docs/TASK_DEFINITION_GUIDE.md @@ -258,8 +258,8 @@ valid and an empty block is legal — every field defaults to "no limit". ```yaml run_limits: # Structural caps - max_turns: 20 # hard cap on agent inner-loop turns per iteration - expected_turns: 8 # SOFT efficiency budget (visible turns) — never aborts + max_tool_calls: 20 # hard cap on resolved tool calls across the whole task + expected_tool_calls: 8 # SOFT efficiency budget (visible tool calls) — never aborts task_timeout: 300 # wall-clock cap for the full run envelope, seconds turn_timeout: 300 # per-communicate() timeout, seconds @@ -273,8 +273,8 @@ run_limits: | Field | Default | Constraint | Description | |-------|---------|------------|-------------| -| `max_turns` | *unset* | `> 0` | Hard cap on agent inner-loop turns per iteration. Unset uses the SDK default. | -| `expected_turns` | *unset* | `>= 1` | **Soft** target for cumulative visible turns. Exceeding it warns and badges the report; it never aborts. See [`expected_turns`](#expected_turns-soft-efficiency-budget). | +| `max_tool_calls` | *unset* | `> 0` | Hard cap on resolved tool calls across the whole task: every retry attempt and every dialog turn count. The TurnMonitor enforces it at the agent's next poll boundary, on every harness. The round that reaches the cap is processed whole, so tool calls already in flight can still land after it. The run finalizes cleanly as `tool_calls_exhausted`, and the criteria are still checked. Unset means no cap. | +| `expected_tool_calls` | *unset* | `>= 1` | **Soft** target for cumulative visible tool calls. Exceeding it warns and badges the report; it never aborts. See [`expected_tool_calls`](#expected_tool_calls-soft-efficiency-budget). | | `task_timeout` | *unset* | `>= 30` | Max seconds for the full run envelope, including agent work, grading, and post-run work. | | `turn_timeout` | *unset* | `>= 10` | Max seconds for the agent's single `communicate()` iteration. | | `max_input_tokens` | *unset* | `>= 1` | Max cumulative input (prompt) tokens. | @@ -297,17 +297,22 @@ model. **Budget-cap semantics:** -- **Checked after each completed agent turn**, and **cumulative** across all of the task's turns. - There is no mid-turn enforcement, so a single runaway turn can overshoot the cap before the - between-turns check sees it. Size caps with headroom for one turn. +- **Enforced live**, and **cumulative** across all of the task's turns. The `TurnMonitor` stops the + agent at its next poll once a cap is crossed. The overshoot is bounded by one usage report plus + any tool calls in flight; how often a harness reports usage is its `usage_granularity` in + [Run-Limit Parity](agents/HARNESS_PARITY.md). A harness that reports usage only once per turn is + checked at the turn end. Size caps with that headroom. - **Subject agent only.** Judge (`llm_judge` / `agent_judge`) and user-simulator token spend are **not** counted against these caps. - A breach aborts the task with `FinalStatus.TOKEN_BUDGET_EXCEEDED` (any of the three token caps) or `FinalStatus.COST_BUDGET_EXCEEDED` (`max_usd`). Both categorize as `failed` — see [Report Schema](REPORT_SCHEMA.md). -- **`max_usd` needs per-turn cost from the SDK.** If no turn reports a cost, the check is **skipped - with a one-shot warning per task**, not failed. A run can therefore blow past `max_usd` silently - on a backend that doesn't report cost — don't rely on it as your only guardrail. +- **`max_usd` is priced from the harness's reported cost**, else from the rate card in + `coder_eval.pricing` for the model the harness reports (then `agent.model`). A turn with no usage + costs nothing. A run that can price a turn neither way finishes **`ERROR`** at that turn's end with + the message "run_limits.max_usd could not be enforced". It is never skipped. Add a rate with + `register_pricing`, pin a priced model, or remove `max_usd`. Mid-turn usage reports rarely carry a + cost, so when the model has no rate the USD cap is checked once the turn's reported cost arrives. - **Cached-read and cache-creation tokens are excluded by default.** `count_cache_creation: true` is what makes an input-token budget meaningful for **Codex**, which buckets its fresh (full-price) prompt slice into `cache_creation`; with the default `false`, a Codex token budget effectively @@ -317,7 +322,7 @@ model. `run_limits` without disturbing the task's other caps: ```bash -coder-eval run task.yaml -D run_limits.max_turns=30 -D run_limits.task_timeout=900 +coder-eval run task.yaml -D run_limits.max_tool_calls=30 -D run_limits.task_timeout=900 coder-eval run task.yaml -D run_limits.max_usd=2.50 -D run_limits.max_total_tokens=200000 ``` @@ -331,18 +336,21 @@ coder-eval run task.yaml -D run_limits.max_usd=2.50 -D run_limits.max_total_toke > **No longer supported:** `max_turns` / `turn_timeout` (and top-level > `task_timeout`) under `agent:` or at the task top level are rejected — > the agent model's `extra="forbid"` raises a clear validation error. -> They must live under `run_limits:`. (A deprecation shim hoisted them -> automatically until it was removed on 2026-06-01.) +> `turn_timeout` and `task_timeout` must live under `run_limits:`. (A +> deprecation shim hoisted them automatically until it was removed on +> 2026-06-01.) `max_turns` under `run_limits:` is rejected too: use +> `run_limits.max_tool_calls`, which counts resolved tool calls, not agent +> inner-loop turns. -### `expected_turns` (soft efficiency budget) +### `expected_tool_calls` (soft efficiency budget) -`run_limits.expected_turns` is a **soft target**, not a cap: the run is never -aborted for exceeding it (use `max_turns` for a hard limit). It's the budget the +`run_limits.expected_tool_calls` is a **soft target**, not a cap: the run is never +aborted for exceeding it (use `max_tool_calls` for a hard limit). It's the budget the dashboard's **"Within Expected Turns"** metric divides by — a task counts as "within budget" when it succeeds *and* its turn count stays within **1.5×** -`expected_turns`. The run-level headline reports the share of **budgeted** tasks +`expected_tool_calls`. The run-level headline reports the share of **budgeted** tasks that did: a budgeted task that failed counts as over budget, while tasks with no -`expected_turns` budget are excluded entirely (success or fail). +`expected_tool_calls` budget are excluded entirely (success or fail). The count compared against the budget is **visible turns** — one per tool call plus one for the agent's final reply — *not* the SDK's `total_turns` (which @@ -356,7 +364,7 @@ default) to exclude a task from the metric entirely. ### `stop_early` (opt-in early stop) Early stop ends a single-shot run **early** once the run's **armed** criteria -decide the outcome — so you can raise `max_turns` for the full-run flavor +decide the outcome — so you can raise `max_tool_calls` for the full-run flavor without paying for turns the smoke flavor doesn't need. A criterion is *armed* by attaching a **`stop_early:` block** to it — the block's presence IS the arming, and it alone activates the run's watcher; there is **no run-level @@ -380,7 +388,7 @@ under the weighted ceiling rule — plus two knobs inside the block: ```yaml run_limits: - max_turns: 30 + max_tool_calls: 30 success_criteria: - type: skill_triggered skill_name: date-teller @@ -518,7 +526,7 @@ Semantics: cannot doom the gate is absorbed, and the run continues). The timeout is checked after the criterion's own verdict each round, so one that decides on that very step is never penalized. `None` (default) = no timeout; the run - relies solely on `run_limits.max_turns`. The step count is **cumulative + relies solely on `run_limits.max_tool_calls`. The step count is **cumulative across every retry attempt** of the turn — including an attempt that crashed or timed out before this criterion's own investigation even began — so size the budget with that headroom in mind. @@ -528,8 +536,8 @@ compares a truncated run against a full one): | Surface | Field / marker | |---------|----------------| -| `run.json` row | `stopped_early`, `early_stop_reason`, `turns_remaining_at_stop` | -| `run.md` | `> **NOTE:** […] stopped early (); <= N turn(s) avoided …` | +| `run.json` row | `stopped_early`, `early_stop_reason`, `tool_calls_remaining_at_stop` | +| `run.md` | `> **NOTE:** […] stopped early (); ` | | `task.html` | header badge `stopped early ()` + `advisory — not gated` markers | | Telemetry | `EarlyStopped` / `EarlyStopReason` dimensions on `CoderEval.Task.End` | @@ -1350,6 +1358,8 @@ Observed label is `"yes"` when either signal is found, else `"no"`. Expected lab **Requires agent telemetry.** This criterion reads `turn_records`, so it only works against a real agent run (not a static check). With no turn records it reports `score=0.0` and an `error`. +**The skill must be offered.** When the task sets `agent.plugins`, coder-eval stages the skills those paths offer (a plugin root or a bare skills directory) and records their names (the `SKILL.md` frontmatter `name`) in `environment_info.skills_offered`. A `skill_name` that is not among them fails `coder-eval plan` before the run is paid for: the positive control cannot run. A `skill_name` taken from a dataset row is checked on each expanded row. This applies even when the skill reaches the agent another way (for example a template's `.claude/skills/`): with `agent.plugins` set, put the skill under test in a plugin path. Re-grading a recorded run whose `skill_name` was not offered finishes `ERROR`, not `0.0`. A plugin path that offers no skill also fails `coder-eval plan`. See [Plugin staging](agents/HARNESS_PARITY.md#plugin-staging). + **Classification metrics.** `skill_triggered` returns a `ClassificationCriterionResult`, so on a [dataset-backed task](#dataset) the suite aggregator computes accuracy / precision / recall / F1 / confusion matrix across all rows. Gate the suite with `suite_thresholds` using any of: `accuracy`, `macro_f1`, `weighted_f1`, `micro_f1`, or per-label `precision.