diff --git a/.claude/commands/coder-eval-code-review-full.md b/.claude/commands/coder-eval-code-review-full.md index e7d086cc6..bb5538b37 100644 --- a/.claude/commands/coder-eval-code-review-full.md +++ b/.claude/commands/coder-eval-code-review-full.md @@ -348,7 +348,7 @@ in a value that doesn't match the formula. - Models with the same field across types (e.g. `RunSummary` and `VariantAggregate`, `TaskDefinition` and `ResolvedTask`, `EvaluationResult` and the per-row `CriterionResult`): verify type, default, validator, and field description match. - Parallel orchestration code paths: `orchestration/batch.py` ↔ `orchestration/experiment.py`. A bug fixed in one routinely needs to be fixed in the other (precedent in this codebase: dataset fan-out, run_limits merging, lineage tracking). - Parallel agent paths: `Orchestrator` ↔ any new driver (e.g. `isolation/docker_runner.py`) — does the driver preserve the `pending_turn` / `crashed=True TurnRecord` contract documented in CLAUDE.md? - - Parallel renderers: `reports.py` ↔ `reports_experiment.py` ↔ `reports_html.py` ↔ `reports_stats.py` — if a new field is added to `EvaluationResult`, do all four render it (and if not, is that deliberate)? + - Parallel renderers: `reports/markdown.py` ↔ `reports/experiment.py` ↔ `reports/html.py` ↔ `reports/helpers.py` — if a new field is added to `EvaluationResult`, do all four render it (and if not, is that deliberate)? Flag any divergence as a finding even if the unchanged side is technically still correct in isolation — the divergence itself is the bug, and silent drift between parallel paths is one of the most expensive defects to debug later. 3. **Check exhaustiveness when an enum / Literal / status set changes.** @@ -401,7 +401,7 @@ in a value that doesn't match the formula. - **Parallel code paths not updated**: e.g. `orchestration/batch.py` was changed but `orchestration/experiment.py` wasn't, despite handling the same concept; a fix landed in `Orchestrator` but the parallel `DockerRunner` path was missed. - **Missing tests for new code paths**: every new branch, public function, validator, CLI flag, and report row needs a test. If a report row was added, is there a test asserting non-zero values for it? - **Downstream consumers of changed counting / classification / formula logic**: if a counting formula changed in one place, did all places that compute rates / averages / percentages / pass/fail status from those counts also update? Same for any threshold or scoring change. - - **Display / icon / mapping dicts not extended for new enum values**: if a new `FinalStatus` / `AgentState` / `SnapshotMode` / criterion type / category was added, do all rendering dicts in `reports*.py` (`reports.py` / `reports_experiment.py` / `reports_html.py` / `reports_stats.py`) cover it, or do they fall through to `"?"` / `"unknown"`? + - **Display / icon / mapping dicts not extended for new enum values**: if a new `FinalStatus` / `AgentState` / `SnapshotMode` / criterion type / category was added, do all rendering dicts in `reports/` (`markdown.py` / `experiment.py` / `html.py` / `helpers.py`) cover it, or do they fall through to `"?"` / `"unknown"`? - **Daily/nightly pipeline impact not stated**: if the change touches the production run path (the cron/nightly entrypoint, the `DockerRunner` entrypoint, the `--backend bedrock` judge) or the cross-repo contract consumed by the external `coder-eval-uipath` / eval-runner pipeline (run-record / `task.json` schema, report JSON shape, CLI output), does the PR say what happens to the nightly run? An unstated blast radius on the daily pipeline is itself the gap. This pass is allowed to surface items that are not tied to a single file:line (since the whole point is that the *absence* of a change isn't anchored anywhere). Express each as a short bullet, prefixed with the bucket it falls into, and reference the *changed* file that triggered the expectation. Each bullet should also carry a severity tag (🔴 / 🟠 / 🟡 / 🔵) using the same anchor table — a missing test for a new public function is 🟠 Test Health; a missing entry in a display dict is typically 🟡; a missing parallel-path update that introduces a real divergence is 🟠 Architecture. **Add these severity tags to the per-axis totals** so they show up in Counts and on-screen output. If there's nothing missing, write a single line: "Nothing identified." diff --git a/.claude/harness-candidates.md b/.claude/harness-candidates.md index 4521ec8b3..e550452ad 100644 --- a/.claude/harness-candidates.md +++ b/.claude/harness-candidates.md @@ -450,7 +450,7 @@ with the two `action.yml` items above — one considered change to the action's `verify-published-action.yml` reads `task_results[*].status` / `weighted_score` / `total_tokens`, and `action.yml`'s score gate reads `weighted_score` / `task_id`. These are string keys in shell/YAML that no test or type-checker binds to - `eval_result_to_task_dict` (`reports_experiment.py`), so renaming a key there + `eval_result_to_task_dict` (`run_record.py`), so renaming a key there silently turns an external gate into a no-op — a reviewer here proposed `final_status`, which does not exist in `run.json` and would have made a new assertion dead on arrival. Guard: assert the key set that non-Python consumers @@ -859,7 +859,7 @@ re-derive from scratch. an existing form. Caught in: the turn-timing consolidation, Phase 5 review. - [ ] **Pre-existing, surfaced by the turn-timing final review: - `reports_stats.regularized_incomplete_beta` clamps an out-of-domain `x` + `stats.regularized_incomplete_beta` clamps an out-of-domain `x` instead of raising.** Its docstring says "Raises ValueError outside that domain — returning NaN would let a bad input render as a real-looking statistic downstream", and it does raise for a non-finite `a`/`b`/`x` and for @@ -916,3 +916,49 @@ re-derive from scratch. (AST-comparing every `ClassDef` first line against a base ref) was written and used throughout Phase 6 and is the thing to promote if that access appears. Caught in: prose mass reduction, Phase 6. + +- [ ] A generated surface (`*.generated.*`) has no mechanical guard against being hand-edited + — CE065/CE033/CE028 all catch *drift* (source changed, output not regenerated) but an edit + to BOTH passes cleanly. Guarding it needs a checksum or a git-attribute gate, not a diff, + so it is a different shape of sensor. — caught during the reports consolidation (CE065). +- [ ] No rule resolves file paths named in PROSE (comments, docstrings, Markdown) across + `src/`, `evalboard/`, `litellm/` and `.github/`. That consolidation hand-fixed ~25 stale + module references across five phases, and two reviewers each found more the greps missed. + The plan's Open Questions measured and declined the CLAUDE.md-only variant (its stale refs + live in an ASCII tree, not backticks); a wider variant has the same parsing problem plus + legitimate non-resolving refs (container paths, plugin-relative paths). Recorded because + the recurrence is now the argument, not the idea. — caught during the reports consolidation. +- [ ] The anchored package regex `(?:^|[/\\])src[/\\]coder_eval[/\\]` is compiled + independently across the rule tree — `ce050_no_union_getattr_probe.py:101`, + `ce051_no_driver_override.py:60`, `ce052_process_lethal_must_be_container_gated.py:78`, + `ce053_run_record_filename_literal.py:65`, `ce054_env_info_key_round_trip.py:66`, + `ce056_no_container_env_literal.py:51` and `ce058_no_timing_literal.py:116` — seven + rule modules, to which `_layers.py` adds one more (its `_CLI` and `_REPORTS` derive from it), with the + `agents/`-suffixed variant of the same idiom in + `_model_ctor.py:28` and `ce059_generation_window_is_two_reads.py:45`, plus a near-variant + in `ce037_no_dead_private_helper.py:61` and a `cli/`-suffixed one in + `ce048_no_in_process_typer_command_call.py:68`. `_layers.py` is the designated shared rule-helper + module, though `_model_ctor.py` is an equal peer and a generic src-path regex arguably + belongs in a neutrally named helper rather than one named `_layers`. Not hoisted here + because retargeting seven unrelated rules needs a per-rule verification that its scope + did not shift — a second refactor inside a review-fix plan. The new copies were written + in the established *spelling* deliberately: the defect being fixed was a regex that + disagreed with its siblings, so a new variant would be that defect again. — caught during the reports-consolidation review fixes, Phase 1. +- [x] ~~**CE004 inherits CE066's `reports/` exemption because the two rules share one + predicate.**~~ **DONE.** `_layers.py` now shares the package anchor and the `cli/` + boundary (`is_package_path`, `is_cli_path`) rather than one exemption set. CE066 keeps + `is_core_path` (`{cli, reports}` exempt); CE004's scope is the package minus `cli/`. + Widening CE004 to `reports/` found 0 violations. `test_the_reports_package_is_in_scope` + and `TestCoreLayerMembership.test_ce004_scope_is_every_module_outside_cli` both fail if + CE004 goes back to the core predicate; `test_the_reports_package_itself_stays_exempt` + pins that CE066's scope did not widen with it. — caught in the reports-consolidation + review fixes, Phase 1 quality review. + +- [ ] **A CLAUDE.md Directory Structure bullet naming a path that no longer exists.** + Shipped briefly as CE067 over the fenced `coder_eval/` tree, then removed when that + tree was replaced by `ls` plus selective bullets — the exhaustive half of the rule + became false by design. The surviving half is still real: the bullets name modules + (`result_metrics.py`, `reports/html.py`, `models/container_paths.py`) and a rename + leaves them stale with nothing failing. Needs a backtick-path extractor scoped to + one section, which is the narrow case of the prose-path candidate above. — caught + during the reports consolidation rebase. diff --git a/.claude/notes/orchestration.md b/.claude/notes/orchestration.md index c05196984..733aab932 100644 --- a/.claude/notes/orchestration.md +++ b/.claude/notes/orchestration.md @@ -48,7 +48,7 @@ `run` and `execute` share one body (`run_command.run_pipeline`) and differ solely in that flag, so there is no third code path. Three things are refused rather than degraded: `--junit-xml` (a report of verdicts, and there are none — though - `reports_junit` still emits `` for an ungraded row it encounters), + `reports/junit.py` still emits `` for an ungraded row it encounters), `--allow-host-grading` (it decides how an ungraded row is GRADED, and `execute` grades nothing), and simulation tasks (their turn-continuation logic reads criteria results, so an ungraded dialog would silently change its own stopping behavior). `stop_early:` diff --git a/.claude/notes/reporting.md b/.claude/notes/reporting.md index 4d25c97b8..a694d49a8 100644 --- a/.claude/notes/reporting.md +++ b/.claude/notes/reporting.md @@ -184,11 +184,80 @@ malformed record — which raises while building the per-call breakdown — abor with the run untouched, matching the caller's "keeping static pricing" contract. Spend tagged with an iteration no turn has is surfaced rather than silently dropped. +## The reports package + +`reports/` is a **leaf**: it may import from anywhere in `coder_eval`, and the core layers +may import only its public *writer* entry points. That asymmetry is the whole point, and +**CE066** enforces it. The invariant is not "core must not import reports" — core +legitimately *writes* reports (`orchestrator.py` writes the per-task HTML, +`orchestration/batch.py` drives `ReportGenerator`). It is that a **metric, a statistic, a +serializer or a formatter** must never be reached out of the rendering layer. + +That was the actual shape of the code before the split. `reports_stats.py` was three +unrelated modules sharing a file, and the orchestrator imported `turn_time_buckets` and +`visible_turn_count` from it *during a run* — a number the evaluation loop needs, living in +a reporting module. The three pieces now sit where their consumers are: + +- **`stats.py`** — distribution-free statistics, **dependency-free by contract**: stdlib + only, no `coder_eval` import, direct or relative. A unit test parses its AST and asserts + that, rather than leaving it to convention, because being reasonable-about-in-isolation is + the only reason it is a separate module. Display formatters (`fmt_mean_sd`, `fmt_p`) stay + in `reports/helpers.py`: they return `"N/A"`, `"—"` and `"<0.001"`, which is presentation. +- **`result_metrics.py`** — metrics derived from a finished `EvaluationResult`, consumed by + the orchestrator mid-run as well as by the reporters. Deliberately **not** folded into + `timing.py`, which has no `EvaluationResult` dependency and is imported by every agent + adapter; adding one would widen that surface for everyone. +- **`run_record.py`** — the `run.json` task-row serializer. It is a run-record serializer, + not a report, and its old home inside the experiment reporter was the *only* reason + `orchestration/batch.py` reached into the reports layer at all. Moving it is what lets + CE066's allowlist be purely writers; carrying a serializer on that list would be the rule + documenting a wart instead of the wart being removed. + +**CE066 checks both the absolute and the relative import spelling.** Its first draft matched +only `node.module`, which for `from ..reports import X` holds `"reports"` with the dots in +`node.level` — so it fired on neither of the two real edges in the tree, and its own tests +passed because they used the absolute form. The layer predicates live in +`tests/lint/rules/_layers.py` so CE004 and CE066 cannot drift about where the package or its +`cli/` boundary is. Each rule's scope is an allowlist of what is *exempt*, so a new subpackage +is in scope by default: CE066's core is *everything under `src/coder_eval/` except `cli/` and +`reports/`*, and CE004's scope is *everything except `cli/`*. The two sets differ on purpose — +the reports package runs without the CLI, so it must not import `cli`, but it may reach into +itself. CE004 first borrowed CE066's predicate whole and so inherited the `reports/` +exemption. Both denylist forms before that also leaked: naming only `orchestrator.py` left +`result_metrics.py` exempt (the module CE066's own fix message points at), and its +ten-directory successor never named `isolation/`, leaving the `driver: docker` evaluation path +invisible to both rules. + +**`format_ms` lives in `durations.py`, not `formatting.py`.** `formatting.py` imports +`claude_agent_sdk` for the payload formatters, and the reports package should not reach +through an SDK-shaped module for a 14-line duration formatter. This does *not* make the +package SDK-free — `models/agent_config.py` imports `ClaudeAgentOptions` and every report +module needs `models` — so the tests assert what is true: `durations.py` is SDK-free, and +`reports` no longer imports `coder_eval.formatting`. + +### Rejected: a shared section-data layer + +The markdown and HTML reporters render four "duplicated" sections. All four pairs were read +in full before deciding, and **only one shares an input shape** (command statistics, both +taking `CommandStatistics`); the others take a `list[dict]` row, a `TokenUsage`, an +`EvaluationResult` and a `list[EvaluationResult]` across three different scopes. The +remaining differences are legitimate per-surface presentation, not drift: `:.1f%` vs `:.0f%`, +and an unmeasured average **hidden** in markdown versus **dashed** in HTML — two valid +renderings of the same `None`. Building the adapter would mean normalizing dict-row and +live-model inputs across three scopes, touching the `run.json` contract, to remove about +twenty lines. Rejected on KISS/YAGNI. The two things in those pairs that *were* real — a +literal `50` beside its own `SLOW_PARAMS_PREVIEW_CHARS`, and a hand-rolled +`TokenUsage.total_tokens` — were simply fixed. + +`analysis.py` and `formatting.py` stay top-level on purpose: they are not report modules, +and moving them in would give the package an SDK dependency and force CE066 to exempt the +orchestrator's `analysis` import. + ## Report rollups and the HTML twin -`reports_html.py` is the evalboard's STATIC TWIN: the two render the same run and must +`reports/html.py` is the evalboard's STATIC TWIN: the two render the same run and must agree, so a rule implemented on one side belongs on the other. The arithmetic itself lives -in `reports_stats.py` and the renderers only format it. +in `result_metrics.py` and `stats.py`; the renderers only format it. ### An unmeasured value is never zero diff --git a/.claude/notes/timing.md b/.claude/notes/timing.md index 37992850c..49a3ad769 100644 --- a/.claude/notes/timing.md +++ b/.claude/notes/timing.md @@ -144,7 +144,7 @@ per-harness composition. The span set the generation subtraction, the head and the tail are all measured against, so they cannot disagree about which calls exist. Shared with -`reports_stats.turn_time_buckets`, which answers the same question about a finished +`result_metrics.turn_time_buckets`, which answers the same question about a finished `TurnRecord` — a second typed copy of this rule is how two report surfaces come to publish two different tool totals for one run. (`scripts/timing/decompose_run.py` keeps its own, over raw `task.json` dicts rather than models; that is the sanctioned third reader, and diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 16d4385d1..8eec69a15 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -181,19 +181,21 @@ jobs: echo "📊 All checks passed: formatting, linting, types, security, tests" evalboard: - # The dashboard's own gate. `evalboard/` ships ~460 vitest assertions, - # including the pricing drift guard that asserts lib/pricing.ts still agrees - # with src/coder_eval/pricing.py — and until this job existed NOTHING ran - # them: not a workflow, not a Makefile target, not a pre-commit hook. The - # guard was consequently red on `main` for weeks while a 3x-wrong Opus rate - # and five unpriced in-use models shipped to the board. An unrun assertion is - # documentation, not enforcement. + # The dashboard's own gate. `evalboard/` ships ~460 vitest assertions, and + # until this job existed NOTHING ran them: not a workflow, not a Makefile + # target, not a pre-commit hook. The pricing guard was consequently red on + # `main` for weeks while a 3x-wrong Opus rate and five unpriced in-use models + # shipped to the board. An unrun assertion is documentation, not enforcement. + # + # Rate-table drift is no longer this job's concern: lib/pricing.generated.ts + # is GENERATED from src/coder_eval/pricing.py, and CE065 in `quality-gate` + # fails a reprice that was not regenerated. What runs here is the CONSUMPTION + # half (pricing-generated.test.ts) — a generated file that is missing, empty + # or narrow fails the board's own build. # # Deliberately NOT path-filtered. `paths:` is workflow-scoped in GitHub - # Actions, and the parity guard's whole point is that a reprice in - # src/coder_eval/pricing.py — a pure-Python diff touching no evalboard file — - # must trip it. A `evalboard/**`-only filter would skip exactly the change - # class this job exists to catch. + # Actions, and a skipped required check blocks a PR rather than passing it — + # so the filter buys nothing and costs a merge-blocking pending status. name: Evalboard (Types, Tests, Build) # Fork-PR carve-out — see `quality-gate`. `pnpm install --frozen-lockfile` runs # the PR's own lockfile install scripts, same untrusted-code class. diff --git a/.github/workflows/verify-published-action.yml b/.github/workflows/verify-published-action.yml index 914cbdc07..3079e354e 100644 --- a/.github/workflows/verify-published-action.yml +++ b/.github/workflows/verify-published-action.yml @@ -463,7 +463,7 @@ jobs: # The JUnit report must describe the same run, not merely be parseable: an empty # but well-formed passed the old parse-only check. Trusted, # self-generated input (our writer emits no DTDs/entities), so stdlib ET is fine. - # `>=` not `==`: reports_junit.py also emits synthetic `skipped` / `suite-gates` + # `>=` not `==`: reports/junit.py also emits synthetic `skipped` / `suite-gates` # testsuites, which only ever ADD cases. cases = len(list(ET.parse(os.environ["JUNIT"]).getroot().iter("testcase"))) if cases < len(rows): diff --git a/.gitignore b/.gitignore index 5d52c6eec..e6fbf41bd 100644 --- a/.gitignore +++ b/.gitignore @@ -61,7 +61,9 @@ uipath.json .claude/settings.local.json c/ runs/ -reports/ +# Anchored to the repo root: this is generated RUN OUTPUT, not the +# `src/coder_eval/reports/` package, which a bare `reports/` also matched. +/reports/ /runs/**/artifacts/ .DS_Store tmp/ diff --git a/CLAUDE.md b/CLAUDE.md index 5cf9f61e5..b92f39b64 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -39,9 +39,16 @@ data-driven analysis. - **`path_utils.py`** owns run ids, atomic writes and tree digests — and every run-record filename literal (CE053). - **`models/container_paths.py`** owns `IN_CONTAINER_ENV` (CE056). -- **`pricing.py`** is hand-mirrored by `evalboard/lib/pricing.ts`; a parity test fails on - drift either way. -- **`reports_html.py`** is the evalboard's static twin. +- **`pricing.py`** is the rate SSOT; `evalboard/lib/pricing.generated.ts` is generated + from it by `make pricing-mirror`, and CE065 fails the build on drift. +- **`reports/`** is a LEAF rendering layer (markdown, html, experiment, junit + + helpers): it may import anywhere, and core may import only its public writers + (CE066). `reports/html.py` is the evalboard's static twin. +- **`result_metrics.py`** holds `EvaluationResult` metrics the ORCHESTRATOR reads + mid-run; **`stats.py`** is distribution-free statistics, dependency-free by + contract; **`run_record.py`** is the `run.json` task-row serializer, not a report. +- **`durations.py`** is `format_ms`, split from `formatting.py` so the reports layer + does not reach through an SDK-shaped module for it. - **`isolation/`** is `driver: docker`, one container per task. - **`streaming/`** is the event protocol and `EventCollector`. @@ -154,13 +161,20 @@ make evalboard-verify # the JS half: tsc --noEmit + vitest + next build # Regenerate a generated surface — never hand-edit the output 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 docs-budget # per-file comment budget + docstring essay check (fails `make verify`) ``` -Editing `src/coder_eval/pricing.py` means editing `evalboard/lib/pricing.ts` too — it is -a hand-copied mirror, and `evalboard/lib/__tests__/pricing-parity.test.ts` fails the -build on drift in either direction. +`src/coder_eval/pricing.py` is the single source of truth for rates on both halves of +the repo. The evalboard's table (`evalboard/lib/pricing.generated.ts`) is generated from +it by `make pricing-mirror` — regenerate and commit after a reprice; **CE065** fails the +build on drift. Never hand-edit the generated file. Two sets are omitted from the mirror: +a rate flagged `per_request_billing` (the provider bills per request, so the board shows +the captured actual per-call cost instead of a static estimate), and the ids in +`DELIBERATELY_UNMIRRORED` (`tests/lint/pricing_mirror.py`), which are priced in Python +for the `max_usd` pre-flight but not worth pricing on the board. Adding to that second +set needs a corpus grep first — a stale entry hides a live bug. ## Custom Lint Rules (CE000+) @@ -194,6 +208,13 @@ A few rules constrain routine edits, so they are worth knowing before you start: Renaming an action input means updating that skill too — the user-facing contract is [CI Gate: GitHub Action & JUnit reports](docs/CI_GATE.md). - **CE047** requires every onboarding surface to name every built-in `AgentKind`. +- **CE065** diffs `evalboard/lib/pricing.generated.ts` against `pricing.py`; the table was + a hand-copy whose exemption set let four heavily-used models render `—` for cost. + Regenerate with `make pricing-mirror`; never hand-edit the generated file. +- **CE066** lets the core layer import only the `reports/` package's public *writers*. A + metric, statistic or serializer pulled out of `reports*` is what put `turn_time_buckets` + and the run.json serializer in a rendering module; they now live in `result_metrics.py`, + `stats.py` and `run_record.py`. **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 62b03d52a..44b2d3f44 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: help install format check typecheck test test-live test-smoke verify verify-noextra evalboard-verify clean run lint docs-indexes plugin-reference docs-budget docker-image docker-image-full coder-eval-runtime docker-images +.PHONY: help install format check typecheck test test-live test-smoke verify verify-noextra evalboard-verify clean run lint docs-indexes plugin-reference pricing-mirror docs-budget docker-image docker-image-full coder-eval-runtime docker-images # Single source of the installed coder-eval version (used to tag the docker # images). Referenced lazily inside the docker recipes, so it doesn't run on @@ -36,6 +36,9 @@ docs-indexes: ## Regenerate README/docs indexes from the mkdocs nav (SSOT) plugin-reference: ## Regenerate the plugin's bundled criteria reference from the models (SSOT) uv run python -m tests.lint.plugin_reference +pricing-mirror: ## Regenerate the evalboard's rate table from pricing.py (SSOT) + uv run python -m tests.lint.pricing_mirror + docs-budget: ## Report the docstring/comment prose budget and check it against the baseline uv run python -m tests.lint.prose_budget @@ -76,8 +79,10 @@ evalboard-verify: ## Run the evalboard (Next.js dashboard) checks: tsc + vitest # The JS half of the repo. Not folded into `make verify` because it needs a # Node/pnpm toolchain a Python-only contributor may not have — but it IS # gated in CI by the `evalboard` job, so a red run here is a red PR. - # Includes the pricing drift guard: a reprice in src/coder_eval/pricing.py - # without the matching edit to evalboard/lib/pricing.ts fails right here. + # The rate table is generated (`make pricing-mirror`), so drift between the + # two halves is caught by CE065 in `make lint`, not here. What this gates is + # CONSUMPTION: pricing-generated.test.ts fails if the generated file is + # missing, empty or narrow. cd evalboard && pnpm install --frozen-lockfile && pnpm verify verify-noextra: ## Verify the framework works without the optional [uipath] extra diff --git a/docs/AB_EXPERIMENTS.md b/docs/AB_EXPERIMENTS.md index ab2e4adb8..afe35d66b 100644 --- a/docs/AB_EXPERIMENTS.md +++ b/docs/AB_EXPERIMENTS.md @@ -381,7 +381,7 @@ Each run writes to `runs//`. Per-task artifacts are nested zero-padded replicate index (`00`, `01`, …). There is no `` segment — the experiment-level report lives at the run root. -The reports (generated by `reports_experiment.py`) are: +The reports (generated by `reports/experiment.py`) are: - **`experiment.md` / `experiment.json`** (run root) — the cross-variant summary: each task's per-variant score, the `best_variant`, the `score_spread`, and diff --git a/docs/agents/HARNESS_PARITY.md b/docs/agents/HARNESS_PARITY.md index b3501e081..ec0919ee5 100644 --- a/docs/agents/HARNESS_PARITY.md +++ b/docs/agents/HARNESS_PARITY.md @@ -536,7 +536,7 @@ measurements. ## `max_turns` counts visible turns on Codex and Antigravity A "visible turn" is one entry in the run's timeline: one resolved tool call. It is -the unit `reports_stats.visible_turn_count` reports and the unit that lands in +the unit `result_metrics.visible_turn_count` reports and the unit that lands in `TurnRecord.commands`. Both backends count it live off the shared `EventCollector.visible_turn_count`, so one `max_turns` value means one thing on both. diff --git a/evalboard/lib/__tests__/pricing-generated.test.ts b/evalboard/lib/__tests__/pricing-generated.test.ts new file mode 100644 index 000000000..775c9de3b --- /dev/null +++ b/evalboard/lib/__tests__/pricing-generated.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, test } from "vitest"; +import { PRICING, resolvePricing } from "../pricing"; + +// Consumption-side guard on the GENERATED rate table (lib/pricing.generated.ts). +// CE065 on the Python side proves the file matches src/coder_eval/pricing.py; +// this proves the evalboard actually consumes it, so a truncated, empty or +// wrongly-shaped generated file fails `pnpm verify` rather than silently +// rendering "—" for every cost. That silent-narrowing failure mode is exactly +// what the deleted hand-copy parity test shipped. + +describe("generated pricing table", () => { + test("is not silently narrow", () => { + // 59 built-in Python rows, minus the 3 per_request_billing ones and the + // 4 in DELIBERATELY_UNMIRRORED. + expect(Object.keys(PRICING).length).toBeGreaterThanOrEqual(52); + }); + + test("each field carries its own rate", () => { + // A FROZEN legacy key with four distinct rates: it pins the + // Python-field -> TS-field mapping without re-creating the two-file + // edit that generating the table exists to remove. Never pin a live, + // actively-repriced model here. + expect(PRICING["claude-3-haiku-20240307"]).toEqual({ + inputPerMTok: 0.25, + outputPerMTok: 1.25, + cacheWritePerMTok: 0.3, + cacheReadPerMTok: 0.03, + }); + }); + + test("prefix stripping still resolves through the generated table", () => { + expect(resolvePricing("eu.anthropic.claude-opus-4-8")).not.toBeNull(); + }); + + test("per_request_billing models stay unpriced so runs.ts apportions the real bill", () => { + expect(resolvePricing("moonshotai/kimi-k3")).toBeNull(); + expect(resolvePricing("z-ai/glm-5.2")).toBeNull(); + expect(resolvePricing("deepseek/deepseek-v4-pro")).toBeNull(); + }); + + test("DELIBERATELY_UNMIRRORED models stay unpriced here", () => { + // The second exemption axis, and the one that is a FRONTEND decision + // rather than a fact about the rate: these are priced in pricing.py for + // the Python max_usd pre-flight, but no harness runs them on this board. + // Generating the table must reproduce that, not quietly widen it — + // pricing a model the hand-copy deliberately skipped is a behaviour + // change smuggled in as a refactor. + for (const id of ["gpt-5.4-mini", "gpt-5.4-nano", "gpt-5.4-pro", "gpt-5.5-pro"]) { + expect(resolvePricing(id), `${id} should not be priced`).toBeNull(); + } + }); +}); diff --git a/evalboard/lib/__tests__/pricing-parity.test.ts b/evalboard/lib/__tests__/pricing-parity.test.ts deleted file mode 100644 index 531657cac..000000000 --- a/evalboard/lib/__tests__/pricing-parity.test.ts +++ /dev/null @@ -1,134 +0,0 @@ -import { readFileSync } from "node:fs"; -import { dirname, resolve } from "node:path"; -import { fileURLToPath } from "node:url"; -import { describe, expect, test } from "vitest"; -import { PRICING } from "../pricing"; - -// Drift guard: lib/pricing.ts is a hand-copied mirror of the authoritative -// Python table in src/coder_eval/pricing.py. It exists so the frontend's -// "estimated" USD figures agree with the backend's authoritative Cost on the -// same tokens. -// -// Semantics are SUBSET, not exact-match: every model priced in lib/pricing.ts -// must exist in pricing.py with identical rates (a frontend rate that disagrees -// with the backend, or prices a model the backend doesn't, fails the build). -// The frontend is NOT required to mirror every backend model — it only needs to -// price the ones it displays, and the backend legitimately prices models the -// evalboard never renders. (Exact-match was too strict: it forced unrelated -// backend model additions into this file to keep the build green.) - -const here = dirname(fileURLToPath(import.meta.url)); -const PY_PATH = resolve(here, "../../../src/coder_eval/pricing.py"); - -// Match: "model-id": ModelPricing(1.25, 10.0, 1.25, 0.125), -const ROW_RE = - /"([^"]+)":\s*ModelPricing\(\s*([\d.]+),\s*([\d.]+),\s*([\d.]+),\s*([\d.]+)\s*\)/g; - -function parsePythonTable(): Record< - string, - [number, number, number, number] -> { - const src = readFileSync(PY_PATH, "utf8"); - const out: Record = {}; - for (const m of src.matchAll(ROW_RE)) { - out[m[1]] = [ - Number(m[2]), - Number(m[3]), - Number(m[4]), - Number(m[5]), - ]; - } - return out; -} - -describe("pricing.ts ↔ pricing.py parity", () => { - const py = parsePythonTable(); - - test("parses every ModelPricing row in the Python table", () => { - // A "> 10" floor is not enough: the guard NARROWS silently if the regex - // stops matching some rows (a `ruff format` reflow onto several lines, a - // switch to keyword args), and a narrowed guard stops reporting exactly - // the class of omission this file exists to catch. Count the constructor - // calls in the source and require the parse to have found all of them. - const declared = ( - readFileSync(PY_PATH, "utf8").match(/^\s*"[^"]+":\s*ModelPricing\(/gm) ?? [] - ).length; - expect(declared).toBeGreaterThan(10); - expect( - Object.keys(py).length, - "ROW_RE missed a pricing.py row — the parity guard is narrower than it looks", - ).toBe(declared); - }); - - test("every model in lib/pricing.ts exists in pricing.py", () => { - const orphans = Object.keys(PRICING).filter((m) => !(m in py)); - expect( - orphans, - `priced in lib/pricing.ts but absent from pricing.py: ${orphans.join(", ")}`, - ).toEqual([]); - }); - - test("shared models have identical input/output/cacheWrite/cacheRead rates", () => { - for (const [model, ts] of Object.entries(PRICING)) { - const rates = py[model]; - expect(rates, `not priced in pricing.py: ${model}`).toBeDefined(); - expect([ - ts.inputPerMTok, - ts.outputPerMTok, - ts.cacheWritePerMTok, - ts.cacheReadPerMTok, - ]).toEqual(rates); - } - }); - - // Python-priced models we deliberately do NOT mirror to the frontend: heavy - // frontier variants no harness runs, so pricing them here adds nothing. Kept - // explicit (not a blanket "ignore extras") so a NEW model added to pricing.py - // that ISN'T here and ISN'T in PRICING breaks the build — catching a real - // litellm-relevant omission (e.g. the Bedrock open-weight ids that previously - // rendered "—" for cost). - // - // KEEP THIS SET HONEST. It silences the drift guard, so a stale entry hides a - // live bug rather than a non-issue: `claude-sonnet-5`, `gpt-5.6-sol`, - // `gpt-5.6-terra` and `gpt-5.6-luna` sat here under "the evalboard never runs - // them" while appearing ~32k / ~2k / ~17k / ~2k times in `runs-remote/`, so - // every one of those runs rendered "—" for cost with nothing failing. Before - // adding an id, grep the corpus for it — absence from run data is the ONLY - // justification, and it expires the moment a harness adopts the model. - const DELIBERATELY_UNMIRRORED = new Set([ - "gpt-5.4-mini", - "gpt-5.4-nano", - "gpt-5.4-pro", - "gpt-5.5-pro", - // OpenRouter open-weight models: priced in pricing.py only for the Python - // max_usd static fallback. The evalboard deliberately does NOT statically - // price them — OpenRouter routes per-request, so it shows the captured - // ACTUAL per-call cost instead (see the per-call table, provider_call_costs). - // Same reasoning under the OpenCode harness, which addresses OpenRouter - // natively: it reports the provider's own per-step cost, which the turn - // carries as token_usage.total_cost_usd. - "moonshotai/kimi-k3", - "z-ai/glm-5.2", - "deepseek/deepseek-v4-pro", - ]); - - test("every DELIBERATELY_UNMIRRORED id still exists in pricing.py", () => { - // Stale-membership guard. An exemption silences the drift guard for one - // id forever; once the id leaves pricing.py the entry silences nothing - // and only survives to be copied. Making that a build failure is what - // forces the set to be re-read rather than appended to. - const stale = [...DELIBERATELY_UNMIRRORED].filter((m) => !(m in py)); - expect( - stale, - `exempted from the mirror but no longer priced in pricing.py — drop them from DELIBERATELY_UNMIRRORED: ${stale.join(", ")}`, - ).toEqual([]); - }); - - test("every pricing.py model is mirrored in pricing.ts or explicitly unmirrored", () => { - const missing = Object.keys(py).filter((m) => !(m in PRICING) && !DELIBERATELY_UNMIRRORED.has(m)); - expect( - missing, - `priced in pricing.py but missing from pricing.ts — mirror it or add to DELIBERATELY_UNMIRRORED: ${missing.join(", ")}`, - ).toEqual([]); - }); -}); diff --git a/evalboard/lib/__tests__/pricing.test.ts b/evalboard/lib/__tests__/pricing.test.ts index 7c6ea53c8..66236b060 100644 --- a/evalboard/lib/__tests__/pricing.test.ts +++ b/evalboard/lib/__tests__/pricing.test.ts @@ -40,7 +40,7 @@ describe("resolvePricing", () => { // generations differ 3x, so pin the boundary: an id on the wrong side of // it triples (or thirds) every Opus cost the board renders, which reads // as a plausible number rather than an obvious error. - // pricing-parity.test.ts is the authority on the rates themselves; this + // CE065 (tests/lint/pricing_mirror.py) is the authority on the rates themselves; this // asserts the split survives an edit to the table. expect(resolvePricing("claude-opus-4-8")?.outputPerMTok).toBe(25); expect(resolvePricing("claude-opus-5")?.outputPerMTok).toBe(25); diff --git a/evalboard/lib/__tests__/status-parity.test.ts b/evalboard/lib/__tests__/status-parity.test.ts index 0ade2830f..965baa786 100644 --- a/evalboard/lib/__tests__/status-parity.test.ts +++ b/evalboard/lib/__tests__/status-parity.test.ts @@ -12,7 +12,7 @@ import { statusCategory, type StatusCategory } from "../status"; // // status.test.ts covers the mapping, but it iterates a HAND-MAINTAINED record — // so it can never fail when Python adds a tenth member. This file parses the -// Python table instead, following the lib/__tests__/pricing-parity.test.ts +// Python table instead, following the precedent of the pricing table, now generated // precedent, so the next status added upstream breaks the build here rather // than silently rendering as grey "unknown" and inflating a denominator. diff --git a/evalboard/lib/pricing.generated.ts b/evalboard/lib/pricing.generated.ts new file mode 100644 index 000000000..73ba6c3d5 --- /dev/null +++ b/evalboard/lib/pricing.generated.ts @@ -0,0 +1,65 @@ +// generated by `make pricing-mirror` — do not edit +// Source: src/coder_eval/pricing.py — regenerate with `make pricing-mirror`. + +import type { Pricing } from "./pricing"; + +// Omitted (per_request_billing): the provider bills per request, so a static +// rate here would replace the captured ACTUAL per-call cost with an estimate. +// deepseek/deepseek-v4-pro, moonshotai/kimi-k3, z-ai/glm-5.2 +// Omitted (DELIBERATELY_UNMIRRORED in tests/lint/pricing_mirror.py): priced in +// Python for the max_usd pre-flight, but no harness runs them on this board. +// gpt-5.4-mini, gpt-5.4-nano, gpt-5.4-pro, gpt-5.5-pro +export const PRICING: Record = { + "claude-3-5-sonnet-20240620": { inputPerMTok: 3.0, outputPerMTok: 15.0, cacheWritePerMTok: 3.75, cacheReadPerMTok: 0.3 }, + "claude-3-5-sonnet-20241022": { inputPerMTok: 3.0, outputPerMTok: 15.0, cacheWritePerMTok: 3.75, cacheReadPerMTok: 0.3 }, + "claude-3-7-sonnet-20250219": { inputPerMTok: 3.0, outputPerMTok: 15.0, cacheWritePerMTok: 3.75, cacheReadPerMTok: 0.3 }, + "claude-3-haiku-20240307": { inputPerMTok: 0.25, outputPerMTok: 1.25, cacheWritePerMTok: 0.3, cacheReadPerMTok: 0.03 }, + "claude-3-opus-20240229": { inputPerMTok: 15.0, outputPerMTok: 75.0, cacheWritePerMTok: 18.75, cacheReadPerMTok: 1.5 }, + "claude-3-sonnet-20240229": { inputPerMTok: 3.0, outputPerMTok: 15.0, cacheWritePerMTok: 3.75, cacheReadPerMTok: 0.3 }, + "claude-fable-5": { inputPerMTok: 10.0, outputPerMTok: 50.0, cacheWritePerMTok: 12.5, cacheReadPerMTok: 1.0 }, + "claude-fable-5-1": { inputPerMTok: 10.0, outputPerMTok: 50.0, cacheWritePerMTok: 12.5, cacheReadPerMTok: 0.25 }, + "claude-haiku-3-5": { inputPerMTok: 0.8, outputPerMTok: 4.0, cacheWritePerMTok: 1.0, cacheReadPerMTok: 0.08 }, + "claude-haiku-4-5": { inputPerMTok: 1.0, outputPerMTok: 5.0, cacheWritePerMTok: 1.25, cacheReadPerMTok: 0.1 }, + "claude-haiku-4-5-20251001": { inputPerMTok: 1.0, outputPerMTok: 5.0, cacheWritePerMTok: 1.25, cacheReadPerMTok: 0.1 }, + "claude-opus-4": { inputPerMTok: 15.0, outputPerMTok: 75.0, cacheWritePerMTok: 18.75, cacheReadPerMTok: 1.5 }, + "claude-opus-4-1": { inputPerMTok: 15.0, outputPerMTok: 75.0, cacheWritePerMTok: 18.75, cacheReadPerMTok: 1.5 }, + "claude-opus-4-20250514": { inputPerMTok: 15.0, outputPerMTok: 75.0, cacheWritePerMTok: 18.75, cacheReadPerMTok: 1.5 }, + "claude-opus-4-5": { inputPerMTok: 5.0, outputPerMTok: 25.0, cacheWritePerMTok: 6.25, cacheReadPerMTok: 0.5 }, + "claude-opus-4-5-20251101": { inputPerMTok: 5.0, outputPerMTok: 25.0, cacheWritePerMTok: 6.25, cacheReadPerMTok: 0.5 }, + "claude-opus-4-6": { inputPerMTok: 5.0, outputPerMTok: 25.0, cacheWritePerMTok: 6.25, cacheReadPerMTok: 0.5 }, + "claude-opus-4-7": { inputPerMTok: 5.0, outputPerMTok: 25.0, cacheWritePerMTok: 6.25, cacheReadPerMTok: 0.5 }, + "claude-opus-4-8": { inputPerMTok: 5.0, outputPerMTok: 25.0, cacheWritePerMTok: 6.25, cacheReadPerMTok: 0.5 }, + "claude-opus-5": { inputPerMTok: 5.0, outputPerMTok: 25.0, cacheWritePerMTok: 6.25, cacheReadPerMTok: 0.5 }, + "claude-sonnet-4-20250514": { inputPerMTok: 3.0, outputPerMTok: 15.0, cacheWritePerMTok: 3.75, cacheReadPerMTok: 0.3 }, + "claude-sonnet-4-5": { inputPerMTok: 3.0, outputPerMTok: 15.0, cacheWritePerMTok: 3.75, cacheReadPerMTok: 0.3 }, + "claude-sonnet-4-5-20250929": { inputPerMTok: 3.0, outputPerMTok: 15.0, cacheWritePerMTok: 3.75, cacheReadPerMTok: 0.3 }, + "claude-sonnet-4-6": { inputPerMTok: 3.0, outputPerMTok: 15.0, cacheWritePerMTok: 3.75, cacheReadPerMTok: 0.3 }, + "claude-sonnet-5": { inputPerMTok: 2.0, outputPerMTok: 10.0, cacheWritePerMTok: 2.5, cacheReadPerMTok: 0.2 }, + "codex-mini-latest": { inputPerMTok: 1.5, outputPerMTok: 6.0, cacheWritePerMTok: 1.5, cacheReadPerMTok: 0.375 }, + "deepseek.v3.2": { inputPerMTok: 0.74, outputPerMTok: 2.22, cacheWritePerMTok: 0.74, cacheReadPerMTok: 0.0 }, + "gemini-3-flash-preview": { inputPerMTok: 0.5, outputPerMTok: 3.0, cacheWritePerMTok: 0.5, cacheReadPerMTok: 0.05 }, + "gemini-3-pro-preview": { inputPerMTok: 2.0, outputPerMTok: 12.0, cacheWritePerMTok: 2.0, cacheReadPerMTok: 0.2 }, + "gemini-3.1-flash-lite": { inputPerMTok: 0.25, outputPerMTok: 1.5, cacheWritePerMTok: 0.25, cacheReadPerMTok: 0.025 }, + "gemini-3.1-flash-lite-preview": { inputPerMTok: 0.25, outputPerMTok: 1.5, cacheWritePerMTok: 0.25, cacheReadPerMTok: 0.025 }, + "gemini-3.1-pro-preview": { inputPerMTok: 2.0, outputPerMTok: 12.0, cacheWritePerMTok: 2.0, cacheReadPerMTok: 0.2 }, + "gemini-3.1-pro-preview-customtools": { inputPerMTok: 2.0, outputPerMTok: 12.0, cacheWritePerMTok: 2.0, cacheReadPerMTok: 0.2 }, + "gemini-3.5-flash": { inputPerMTok: 1.5, outputPerMTok: 9.0, cacheWritePerMTok: 1.5, cacheReadPerMTok: 0.15 }, + "gemini-3.5-flash-lite": { inputPerMTok: 0.3, outputPerMTok: 2.5, cacheWritePerMTok: 0.3, cacheReadPerMTok: 0.03 }, + "gemini-3.6-flash": { inputPerMTok: 1.5, outputPerMTok: 7.5, cacheWritePerMTok: 1.5, cacheReadPerMTok: 0.15 }, + "gemini-3.7-flash": { inputPerMTok: 1.5, outputPerMTok: 7.5, cacheWritePerMTok: 1.5, cacheReadPerMTok: 0.15 }, + "gemini-3.8-flash": { inputPerMTok: 1.5, outputPerMTok: 7.5, cacheWritePerMTok: 1.5, cacheReadPerMTok: 0.15 }, + "gpt-5": { inputPerMTok: 1.25, outputPerMTok: 10.0, cacheWritePerMTok: 1.25, cacheReadPerMTok: 0.125 }, + "gpt-5-codex": { inputPerMTok: 1.25, outputPerMTok: 10.0, cacheWritePerMTok: 1.25, cacheReadPerMTok: 0.125 }, + "gpt-5.1-codex": { inputPerMTok: 1.25, outputPerMTok: 10.0, cacheWritePerMTok: 1.25, cacheReadPerMTok: 0.125 }, + "gpt-5.1-codex-max": { inputPerMTok: 1.25, outputPerMTok: 10.0, cacheWritePerMTok: 1.25, cacheReadPerMTok: 0.125 }, + "gpt-5.1-codex-mini": { inputPerMTok: 0.25, outputPerMTok: 2.0, cacheWritePerMTok: 0.25, cacheReadPerMTok: 0.025 }, + "gpt-5.2-codex": { inputPerMTok: 1.75, outputPerMTok: 14.0, cacheWritePerMTok: 1.75, cacheReadPerMTok: 0.175 }, + "gpt-5.3-codex": { inputPerMTok: 1.75, outputPerMTok: 14.0, cacheWritePerMTok: 1.75, cacheReadPerMTok: 0.175 }, + "gpt-5.4": { inputPerMTok: 2.5, outputPerMTok: 15.0, cacheWritePerMTok: 2.5, cacheReadPerMTok: 0.25 }, + "gpt-5.5": { inputPerMTok: 5.0, outputPerMTok: 30.0, cacheWritePerMTok: 5.0, cacheReadPerMTok: 0.5 }, + "gpt-5.6-luna": { inputPerMTok: 0.2, outputPerMTok: 1.2, cacheWritePerMTok: 0.2, cacheReadPerMTok: 0.02 }, + "gpt-5.6-sol": { inputPerMTok: 4.0, outputPerMTok: 20.0, cacheWritePerMTok: 4.0, cacheReadPerMTok: 0.4 }, + "gpt-5.6-terra": { inputPerMTok: 2.0, outputPerMTok: 12.0, cacheWritePerMTok: 2.0, cacheReadPerMTok: 0.2 }, + "moonshotai.kimi-k2.5": { inputPerMTok: 0.72, outputPerMTok: 3.6, cacheWritePerMTok: 0.72, cacheReadPerMTok: 0.0 }, + "zai.glm-5": { inputPerMTok: 1.2, outputPerMTok: 3.84, cacheWritePerMTok: 1.2, cacheReadPerMTok: 0.0 }, +}; diff --git a/evalboard/lib/pricing.ts b/evalboard/lib/pricing.ts index b858f715c..6099dad65 100644 --- a/evalboard/lib/pricing.ts +++ b/evalboard/lib/pricing.ts @@ -1,11 +1,13 @@ -// Per-million-token prices and cost math. Ported from -// src/coder_eval/pricing.py — keep in sync when that table changes. +// Per-million-token prices and cost math. The rate table itself is GENERATED +// from src/coder_eval/pricing.py by `make pricing-mirror` — see ./pricing.generated. // Source: Anthropic / OpenAI / Google public pricing. // -// This is the single source of truth for rates on the frontend: the -// cascade-aware thinking-cost simulator (lib/thinkingSim.ts) and the -// per-message cost column (lib/runs.ts) both price against this table, so a -// model added or repriced here updates both at once. +// This is the frontend's single entry point for rates: the cascade-aware +// thinking-cost simulator (lib/thinkingSim.ts) and the per-message cost column +// (lib/runs.ts) both price through resolvePricing, so a model added or repriced +// in pricing.py reaches both at once once the mirror is regenerated. + +import { PRICING } from "./pricing.generated"; export interface Pricing { inputPerMTok: number; @@ -14,111 +16,22 @@ export interface Pricing { cacheReadPerMTok: number; } -// Exported so a unit test can assert key-and-rate parity against the -// authoritative Python table (src/coder_eval/pricing.py) and fail the -// build on drift — this hand-copied mirror is otherwise guarded only by a -// comment. Not part of the consumer API; use resolvePricing() instead. -export const PRICING: Record = { - // Claude. Opus 4.5 and later are priced at the POST-repricing $5/$25 rates, - // not Opus 4.1's $15/$75 — the two generations differ 3x, so an undated - // alias must never inherit the older tier. Undated aliases each need their - // own key: resolvePricing's fallback only strips a trailing date (dated → - // undated), it cannot invent one. - // Fable 5.1 prices cache hits at 0.025x input, not the 0.1x every other - // Claude model uses. Fable 5 pays $1 on the identical $10 base. - "claude-fable-5-1": p(10, 50, 12.5, 0.25), - "claude-fable-5": p(10, 50, 12.5, 1), - "claude-opus-5": p(5, 25, 6.25, 0.5), - "claude-opus-4-8": p(5, 25, 6.25, 0.5), - "claude-opus-4-7": p(5, 25, 6.25, 0.5), - "claude-opus-4-6": p(5, 25, 6.25, 0.5), - "claude-opus-4-5": p(5, 25, 6.25, 0.5), - "claude-opus-4-5-20251101": p(5, 25, 6.25, 0.5), - "claude-opus-4-1": p(15, 75, 18.75, 1.5), - "claude-opus-4": p(15, 75, 18.75, 1.5), - "claude-opus-4-20250514": p(15, 75, 18.75, 1.5), - // $2/$10, not the $3/$15 that 4.6 and earlier pay. - "claude-sonnet-5": p(2, 10, 2.5, 0.2), - "claude-sonnet-4-6": p(3, 15, 3.75, 0.3), - "claude-sonnet-4-5": p(3, 15, 3.75, 0.3), - "claude-sonnet-4-5-20250929": p(3, 15, 3.75, 0.3), - "claude-sonnet-4-20250514": p(3, 15, 3.75, 0.3), - "claude-haiku-4-5": p(1, 5, 1.25, 0.1), - "claude-haiku-4-5-20251001": p(1, 5, 1.25, 0.1), - "claude-haiku-3-5": p(0.8, 4, 1, 0.08), - "claude-3-7-sonnet-20250219": p(3, 15, 3.75, 0.3), - "claude-3-5-sonnet-20241022": p(3, 15, 3.75, 0.3), - "claude-3-5-sonnet-20240620": p(3, 15, 3.75, 0.3), - "claude-3-opus-20240229": p(15, 75, 18.75, 1.5), - "claude-3-sonnet-20240229": p(3, 15, 3.75, 0.3), - "claude-3-haiku-20240307": p(0.25, 1.25, 0.3, 0.03), - // OpenAI (CodexAgent). cacheWrite == input on every entry below is - // DELIBERATE, not a copy-paste slip: OpenAI bills no separate cache-write - // fee, so the fresh prompt slice is plain input. It is also inert — the - // Codex agent records cache_creation_tokens as 0 (codex_agent.py), so this - // rate always multiplies zero. Rationale mirrored from pricing.py, which - // states it once for the whole block. - "gpt-5-codex": p(1.25, 10, 1.25, 0.125), - "gpt-5": p(1.25, 10, 1.25, 0.125), - "gpt-5.1-codex-max": p(1.25, 10, 1.25, 0.125), - "gpt-5.1-codex": p(1.25, 10, 1.25, 0.125), - "gpt-5.1-codex-mini": p(0.25, 2, 0.25, 0.025), - "codex-mini-latest": p(1.5, 6, 1.5, 0.375), - "gpt-5.3-codex": p(1.75, 14, 1.75, 0.175), - "gpt-5.2-codex": p(1.75, 14, 1.75, 0.175), - "gpt-5.4": p(2.5, 15, 2.5, 0.25), - "gpt-5.5": p(5, 30, 5, 0.5), - // Sol's rate is promotional through at least 2026-11-21. - "gpt-5.6-sol": p(4, 20, 4, 0.4), - "gpt-5.6-terra": p(2, 12, 2, 0.2), - "gpt-5.6-luna": p(0.2, 1.2, 0.2, 0.02), - // Google Gemini (AntigravityAgent). Gemini bills no separate cache-write - // fee (cache_write == input, effectively unused); cache_read is the cached- - // input rate. Pro's >200K-token tier is higher — this flat rate reads low - // for very-large-context runs, fine for typical eval tasks. - "gemini-3-pro-preview": p(2, 12, 2, 0.2), - "gemini-3.1-pro-preview": p(2, 12, 2, 0.2), - "gemini-3.1-pro-preview-customtools": p(2, 12, 2, 0.2), - // 3.6 / 3.7 / 3.8 Flash share one rate card. List rates; Google is - // discounting all three by half through 2026-12-31. - "gemini-3.8-flash": p(1.5, 7.5, 1.5, 0.15), - "gemini-3.7-flash": p(1.5, 7.5, 1.5, 0.15), - "gemini-3.6-flash": p(1.5, 7.5, 1.5, 0.15), - "gemini-3.5-flash": p(1.5, 9, 1.5, 0.15), - "gemini-3.5-flash-lite": p(0.3, 2.5, 0.3, 0.03), - "gemini-3.1-flash-lite": p(0.25, 1.5, 0.25, 0.025), - "gemini-3.1-flash-lite-preview": p(0.25, 1.5, 0.25, 0.025), - "gemini-3-flash-preview": p(0.5, 3, 0.5, 0.05), - // OpenRouter open-weight models (litellm backend) are DELIBERATELY NOT priced - // here. OpenRouter routes per-request, so a static headline rate is wrong (the - // billed rate depends on the provider it landed on), and there is no per-bucket - // rate to show. Instead the harness captures each call's ACTUAL cost proxy-side - // and the detail view renders it per call (TurnRecord.provider_call_costs → - // ProviderCallTableSection); a static estimate here would only reintroduce the - // wrong number. See pricing.py (kept for the Python-side max_usd fallback). - // Bedrock open-weight models (litellm backend, eu-north-1). Mirror of pricing.py. - // These run on Bedrock (fixed rates, like Claude) with NO OpenRouter actual-cost - // capture, so static pricing is correct and required here. - // The recorded model_used arrives prefixed (e.g. "converse/zai.glm-5"), so - // resolvePricing strips the routing/region prefixes before lookup. - "deepseek.v3.2": p(0.74, 2.22, 0.74, 0), - "zai.glm-5": p(1.2, 3.84, 1.2, 0), - "moonshotai.kimi-k2.5": p(0.72, 3.6, 0.72, 0), -}; - -function p( - input: number, - output: number, - cacheWrite: number, - cacheRead: number, -): Pricing { - return { - inputPerMTok: input, - outputPerMTok: output, - cacheWritePerMTok: cacheWrite, - cacheReadPerMTok: cacheRead, - }; -} +// The rate table itself is GENERATED from the authoritative Python table +// (src/coder_eval/pricing.py) by `make pricing-mirror`, and CE065 fails the +// build if the generated file drifts from it. Never hand-edit a rate here or in +// pricing.generated.ts — change pricing.py and regenerate. +// +// Two sets of models are deliberately absent from the generated table, for +// different reasons. A model flagged `per_request_billing` in pricing.py MUST NOT +// be priced here: the provider routes per request, so resolvePricing returns null +// and runs.ts apportions the captured ACTUAL per-call cost instead. A model in +// `DELIBERATELY_UNMIRRORED` (tests/lint/pricing_mirror.py) simply is not worth +// pricing on this board — it is priced in Python for the max_usd pre-flight only. +// +// Re-exported so the rate table has one import path for the whole frontend +// regardless of which file generates it. Not part of the consumer API; use +// resolvePricing() instead. +export { PRICING }; // Strip the LiteLLM/Bedrock routing + region/vendor prefixes back to the bare // pricing key — mirror of src/coder_eval/pricing.py::_normalize_model, since the diff --git a/evalboard/lib/runs.ts b/evalboard/lib/runs.ts index f19ab684d..47b064709 100644 --- a/evalboard/lib/runs.ts +++ b/evalboard/lib/runs.ts @@ -118,7 +118,7 @@ export interface TaskResultSummary { // Per-task token totals from run.json. Null on legacy runs that // don't record per-task token counts. `inputTokens` is the disjoint // uncached slice (run.json `input_tokens` is serialized from - // TokenUsage.uncached_input_tokens — see reports_experiment.py), so it + // TokenUsage.uncached_input_tokens — see run_record.py), so it // sits alongside the cache columns without overlap. inputTokens: number | null; outputTokens: number | null; @@ -431,8 +431,8 @@ function sumMeasured(values: (number | null | undefined)[]): number | null { // Three of the task's four wall-clock buckets, summed over its turns. The // per-turn values are measured by `coder_eval/timing.py` — head and tail by // `decompose_turn`, the tool union at the collector seam — and the summation is -// evalboard-only, mirroring `reports_stats.turn_time_buckets` the way -// `pricing.ts` mirrors `pricing.py`. The arithmetic that consumes it, the +// evalboard-only, a hand-written mirror of `result_metrics.turn_time_buckets` +// (unlike the rate table, which is generated). The arithmetic that consumes it, the // Unaccounted residual in `_sections.tsx`, is the deliberate second // implementation `decompose_turn`'s docstring names. // @@ -493,7 +493,7 @@ export function aggregateSubAgentUsage( export interface RawTaskResult { task_id?: string; // Experiment arm that produced this row (the sub-dir). Written by - // reports_experiment.py on every run; absent on runs that predate it, which + // run_record.py on every run; absent on runs that predate it, which // read as DEFAULT_VARIANT_ID. variant_id?: string | null; // Replicate index of this row (the // sub-dir). Repeated diff --git a/evalboard/lib/thinkingSim.ts b/evalboard/lib/thinkingSim.ts index 0c28d23f3..1ca0f272c 100644 --- a/evalboard/lib/thinkingSim.ts +++ b/evalboard/lib/thinkingSim.ts @@ -57,9 +57,9 @@ // recorded token mix exactly. import type { MessageEvent, TokenTotals } from "@/lib/runs"; -// Rates and resolution live in lib/pricing.ts (the single source of truth, -// ported from src/coder_eval/proxy/pricing.py). Import directly from there; -// this module no longer re-exports them. +// Rates and resolution live in lib/pricing.ts, whose table is GENERATED from +// src/coder_eval/pricing.py by `make pricing-mirror`. Import directly from +// there; this module no longer re-exports them. import { type Pricing, resolvePricing } from "@/lib/pricing"; // Per-tool aggregates for the tool-skip simulator. diff --git a/evalboard/lib/timing.ts b/evalboard/lib/timing.ts index db23a1c1d..1d4982a3e 100644 --- a/evalboard/lib/timing.ts +++ b/evalboard/lib/timing.ts @@ -192,7 +192,7 @@ export function busyMs( // bare duration is the out-of-tree `delegate-sdk`; see // docs/agents/HARNESS_PARITY.md. // The same union, but `null` when NOTHING bounded was recorded — the direct -// twin of `reports_stats._turn_tool_union_ms`, and the one a display cell wants. +// twin of `result_metrics._turn_tool_union_ms`, and the one a display cell wants. // // `toolExecutionMs` above returns `0` for an empty span list because that is // what a UNION of nothing is, and what `coder_eval.timing.union_ms` returns; diff --git a/evalboard/lib/variants.ts b/evalboard/lib/variants.ts index b41763acf..ba2b53ab9 100644 --- a/evalboard/lib/variants.ts +++ b/evalboard/lib/variants.ts @@ -12,7 +12,7 @@ export const DEFAULT_VARIANT_ID = "default"; // A variant id is exactly ONE path segment, so it is held to a stricter rule // than a task id, which may nest (`/` from dataset expansion). -// Mirrors coder_eval's reports_junit._is_safe_component. Restated rather than +// Mirrors coder_eval's reports/junit.py::_is_safe_component. Restated rather than // imported from lib/blob.ts to keep this module free of node built-ins. const VARIANT_ID_RE = /^[\w.-]+$/; diff --git a/litellm/README.md b/litellm/README.md index 6ca8e6b61..628b94a1c 100644 --- a/litellm/README.md +++ b/litellm/README.md @@ -139,7 +139,7 @@ curl -s https://openrouter.ai/api/v1/models -H "Authorization: Bearer $KEY" \ | python3 -c "import sys,json;[print(m['id'],m['pricing']) for m in json.load(sys.stdin)['data'] if 'SEARCH' in m['id'].lower()]" ``` -### 2. Register pricing in **both** tables (rates must match) +### 2. Register pricing (one table, mirrored automatically) - `src/coder_eval/pricing.py` — add to the `_PRICING` dict, keyed on the `model_name` (bare id as passed in `agent.model`): @@ -148,9 +148,10 @@ curl -s https://openrouter.ai/api/v1/models -H "Authorization: Bearer $KEY" \ ``` These implicit-caching providers charge no separate cache-write fee, so `cache_write == input` (unused) and `cache_read` is the discounted rate. -- `evalboard/lib/pricing.ts` — add the same entry so the evalboard cost columns - populate. A test (`lib/__tests__/pricing-parity.test.ts`) fails the build if a - rate here disagrees with `pricing.py`. + That is the only place to add it. The evalboard's table + (`evalboard/lib/pricing.generated.ts`) is GENERATED from `pricing.py` — run + `make pricing-mirror` and commit the result; CE065 fails the build if the two + drift. Never hand-edit the generated file. ### 3. Restart the proxy @@ -193,4 +194,4 @@ subject to this. Bedrock models are single-provider and not affected. | `Invalid model name passed in model=...` | Model added to yaml but proxy not restarted — restart it. | | HTTP 401 / "Unable to locate credentials" | Missing `AWS_BEARER_TOKEN_BEDROCK` / `OPENROUTER_API_KEY` in `.env`, or key mismatch between `LITELLM_AUTH_TOKEN` (client) and the proxy's master key. | | `ModuleNotFoundError: No module named 'proxy_server'` (masked startup death) | fastapi drifted past 0.140.0 (`get_flat_dependant` removed). Use `start-litellm.sh` (it pins the deps), or run with `--with 'fastapi==0.140.0'`. If overriding `LITELLM_SPEC`, bump `LITELLM_FASTAPI_SPEC` to match. | -| evalboard cost column blank for a model | Model missing from `evalboard/lib/pricing.ts`. | +| evalboard cost column blank for a model | Model missing from `_PRICING` in `pricing.py`, or the mirror was not regenerated — add it and run `make pricing-mirror`. (A `per_request_billing` model is blank by design: the board shows its captured actual per-call cost instead.) | diff --git a/pyproject.toml b/pyproject.toml index 6f6c66a58..cafde0ba8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -280,6 +280,7 @@ external = [ "CE038", "CE039", "CE043", + "CE044", "CE045", "CE046", "CE047", @@ -299,6 +300,8 @@ external = [ "CE061", "CE063", "CE064", + "CE065", + "CE066", ] # custom architectural lint rules (tests/lint/) [tool.ruff.lint.pylint] diff --git a/src/coder_eval/agents/codex_agent.py b/src/coder_eval/agents/codex_agent.py index 1264f5fbb..5611038ef 100644 --- a/src/coder_eval/agents/codex_agent.py +++ b/src/coder_eval/agents/codex_agent.py @@ -874,7 +874,7 @@ async def communicate( stream_callback: Optional callback for real-time event streaming timeout: Hard wall-clock deadline in seconds max_turns: Hard cap on VISIBLE turns — tool calls, the unit - ``reports_stats.visible_turn_count`` counts — enforced in-stream on + ``result_metrics.visible_turn_count`` counts — enforced in-stream on the same pump boundary as the cooperative stop. Codex delivers one SDK turn per ``communicate()``, so a native turn counter would cap at 1; see docs/agents/HARNESS_PARITY.md. diff --git a/src/coder_eval/cli/report_command.py b/src/coder_eval/cli/report_command.py index a64d0cf77..c864db57a 100644 --- a/src/coder_eval/cli/report_command.py +++ b/src/coder_eval/cli/report_command.py @@ -7,8 +7,7 @@ from ..models import EvaluationResult from ..path_utils import TASK_JSON_FILENAME -from ..reports import ReportGenerator -from ..reports_html import write_task_html +from ..reports import ReportGenerator, write_task_html from .console import console @@ -62,7 +61,7 @@ def report_command( return if fmt == "junit": - from ..reports_junit import write_junit_xml + from ..reports import write_junit_xml target = output_file if output_file is not None else run_dir / "junit.xml" try: diff --git a/src/coder_eval/cli/run_command.py b/src/coder_eval/cli/run_command.py index 247ac5bda..cad6cff0a 100644 --- a/src/coder_eval/cli/run_command.py +++ b/src/coder_eval/cli/run_command.py @@ -672,7 +672,7 @@ async def _run_all_tasks( # BEFORE the exit-code gate, so a failing run still produces the report. # Rationale: .claude/notes/orchestration.md § What the exit code counts if junit_xml is not None: - from ..reports_junit import write_junit_xml + from ..reports import write_junit_xml written = write_junit_xml(run_dir, junit_xml) console.print(f"[green][OK]JUnit report written to {written}[/green]") @@ -980,7 +980,7 @@ async def _run_with_experiment( load_experiment, resolve_all_tasks, ) # resolve_task_for_variant not needed here - from ..reports_experiment import ExperimentReportGenerator + from ..reports import ExperimentReportGenerator # Load experiments (avoid double-loading when using default) exp_path = experiment_path or DEFAULT_EXPERIMENT_PATH diff --git a/src/coder_eval/durations.py b/src/coder_eval/durations.py new file mode 100644 index 000000000..94b9667b7 --- /dev/null +++ b/src/coder_eval/durations.py @@ -0,0 +1,31 @@ +"""Duration formatting for the report renderers. + +Split out of ``formatting.py`` because that module imports ``claude_agent_sdk`` +for the SDK payload formatters, and the reports package should not reach through +an SDK-shaped module for a 14-line duration formatter. + +Note what this does NOT buy: ``coder_eval.reports`` still pulls the SDK in +transitively, because ``coder_eval.models.agent_config`` imports +``ClaudeAgentOptions`` and every report module needs ``models``. What is true and +tested (``tests/test_reports_package.py``) is that **this** module is SDK-free, so +the formatter is usable without it, and that ``reports`` no longer imports +``coder_eval.formatting``. +""" + +from __future__ import annotations + + +def format_ms(ms: float | None) -> str: + """A duration in ms, or an em dash when it was never measured. + + SHARED by the HTML report and the markdown one. They render the same four + wall-clock buckets from the same `result_metrics.turn_time_buckets` call, so + formatting them twice is how one surface comes to print `0ms` where the + other prints a dash — the `None`-vs-`0.0` distinction CE058 enforces on the + producing side, thrown away at the last step. + """ + if ms is None: + return "—" + if ms < 1000: + return f"{ms:.0f}ms" + return f"{ms / 1000:.2f}s" diff --git a/src/coder_eval/formatting.py b/src/coder_eval/formatting.py index 91f8dd39d..88e707a27 100644 --- a/src/coder_eval/formatting.py +++ b/src/coder_eval/formatting.py @@ -18,22 +18,6 @@ logger = logging.getLogger(__name__) -def format_ms(ms: float | None) -> str: - """A duration in ms, or an em dash when it was never measured. - - SHARED by the HTML report and the markdown one. They render the same four - wall-clock buckets from the same `reports_stats.turn_time_buckets` call, so - formatting them twice is how one surface comes to print `0ms` where the - other prints a dash — the `None`-vs-`0.0` distinction CE058 enforces on the - producing side, thrown away at the last step. - """ - if ms is None: - return "—" - if ms < 1000: - return f"{ms:.0f}ms" - return f"{ms / 1000:.2f}s" - - def format_messages( messages: list[Message], *, diff --git a/src/coder_eval/models/experiment.py b/src/coder_eval/models/experiment.py index ee9c563c4..c72e3ea01 100644 --- a/src/coder_eval/models/experiment.py +++ b/src/coder_eval/models/experiment.py @@ -289,7 +289,7 @@ def pass_rate(self) -> float | None: used to be a claim rather than a fact — this property said "Mirrors RunSummary.pass_rate" while implementing only half of it, so the same 10-task execute run with one crash gave ``None`` in ``run.json`` and - ``0.0`` here, which ``reports_experiment`` rendered as + ``0.0`` here, which ``reports.experiment`` rendered as "Pass Rate: 0.0% (0/1)". Both now route through ``nothing_was_measured``. """ diff --git a/src/coder_eval/models/judge.py b/src/coder_eval/models/judge.py index 4c9e913cd..463d64251 100644 --- a/src/coder_eval/models/judge.py +++ b/src/coder_eval/models/judge.py @@ -60,7 +60,7 @@ def _coerce_rationale(cls, v: Any) -> str: if not isinstance(v, str): raise ValueError(f"rationale must be a string, got {type(v).__name__}") # Collapsed to single spaces because two consumers parse this by LINE: - # ``format_details`` writes it on one, and the HTML report grabs only the + # ``format_details`` writes it on one, and ``reports.html`` grabs only the # first "rationale: " line. The schema asks for a 1-2 sentence headline. collapsed = " ".join(v.split()) if not collapsed: diff --git a/src/coder_eval/orchestration/batch.py b/src/coder_eval/orchestration/batch.py index 0674669a0..3b343e8a1 100644 --- a/src/coder_eval/orchestration/batch.py +++ b/src/coder_eval/orchestration/batch.py @@ -30,7 +30,7 @@ ) from ..path_utils import TASK_JSON_FILENAME, format_task_log_id from ..pricing import unpriced_models -from ..reports_experiment import eval_result_to_task_dict +from ..run_record import eval_result_to_task_dict from ..streaming.callbacks import StreamCallback from ..utils import get_version_info, looks_like_version from .config import BatchRunConfig, resolve_preservation_mode diff --git a/src/coder_eval/orchestrator.py b/src/coder_eval/orchestrator.py index 6d978e89b..cdcfba490 100644 --- a/src/coder_eval/orchestrator.py +++ b/src/coder_eval/orchestrator.py @@ -76,6 +76,7 @@ task_log_path, write_text_atomic, ) +from .result_metrics import turn_time_buckets, visible_turn_count from .sandbox import Sandbox from .simulation import DialogStopReason, SimulatorResult, UserSimulator, evaluate_stop from .streaming.callbacks import CompositeStreamCallback, StreamCallback, TaskScopedCallback, safe_emit @@ -299,8 +300,6 @@ def build_task_event(result: EvaluationResult, *, driver: str, variant_id: str) # The four wall-clock buckets, from the ONE canonical summation — this # function adds nothing up itself. Each is omitted rather than zeroed, for the # reason `Score` above is. - from .reports_stats import turn_time_buckets - buckets = turn_time_buckets(result) for name, value in ( ("StartupMs", buckets.startup_ms), @@ -1155,7 +1154,7 @@ def _finalize_result(self, start_time: float) -> None: # HTML failure must never mask the run outcome — write_task_html logs and # returns None. - from .reports_html import write_task_html + from .reports import write_task_html write_task_html(self.result, self.html_report_path) @@ -1244,7 +1243,6 @@ def _check_expected_turns(self, *, iteration: int) -> None: return if self._expected_turns_warning_emitted: return - from .reports_stats import visible_turn_count total = visible_turn_count(self.result) if total > limits.expected_turns: diff --git a/src/coder_eval/pricing.py b/src/coder_eval/pricing.py index a04b70794..0e4d2fc1b 100644 --- a/src/coder_eval/pricing.py +++ b/src/coder_eval/pricing.py @@ -8,23 +8,31 @@ ``/api/v1/models`` (every row re-verified 2026-09-03, except the Bedrock open-weight block: AWS publishes no eu-north-1 figures for those three). -HAZARD: this table is a HAND-COPIED MIRROR of ``evalboard/lib/pricing.ts``. Editing -one means editing the other; ``evalboard/lib/__tests__/pricing-parity.test.ts`` fails -the build on drift in either direction. +SSOT for rates on both halves of the repo: ``evalboard/lib/pricing.generated.ts`` is +generated from it by ``make pricing-mirror`` and guarded by CE065. Never hand-edit the +generated file. """ -from collections.abc import Iterable +from collections.abc import Iterable, Mapping from dataclasses import dataclass +from types import MappingProxyType @dataclass(frozen=True) class ModelPricing: - """Pricing for a single model (per million tokens).""" + """Pricing for a single model (per million tokens). + + ``per_request_billing`` marks a provider that bills per request, so this + static rate is a last-resort estimate retained for the ``max_usd`` pre-flight + only. A frontend that can show the actual captured per-call cost must not + price such a model statically — an estimate would replace a real figure. + """ input_per_mtok: float output_per_mtok: float cache_write_per_mtok: float # prompt caching write cache_read_per_mtok: float # prompt caching read + per_request_billing: bool = False _PRICING: dict[str, ModelPricing] = { @@ -109,11 +117,12 @@ class ModelPricing: # These providers cache prefixes implicitly, so cache-creation is priced at # input (unused). HEADLINE rates only: OpenRouter routes per request, so the # real bill depends on the provider a call lands on -- which is why the litellm - # path captures actual per-call cost and overrides these. Static fallback. + # path captures actual per-call cost and overrides these. Static fallback, and + # why these three carry per_request_billing (the mirror omits them). # Rationale: .claude/notes/reporting.md § Cost joining - "moonshotai/kimi-k3": ModelPricing(3.0, 15.0, 3.0, 0.30), - "z-ai/glm-5.2": ModelPricing(0.966, 3.036, 0.966, 0.1932), - "deepseek/deepseek-v4-pro": ModelPricing(1.030776, 2.061552, 1.030776, 0.085898), + "moonshotai/kimi-k3": ModelPricing(3.0, 15.0, 3.0, 0.30, per_request_billing=True), + "z-ai/glm-5.2": ModelPricing(0.966, 3.036, 0.966, 0.1932, per_request_billing=True), + "deepseek/deepseek-v4-pro": ModelPricing(1.030776, 2.061552, 1.030776, 0.085898, per_request_billing=True), } @@ -121,6 +130,15 @@ class ModelPricing: _REGISTERED_PRICING: dict[str, ModelPricing] = {} +def builtin_rates() -> Mapping[str, ModelPricing]: + """The built-in rate card, read-only. + + Excludes plugin rates (``register_pricing``), which are not resolvable + outside a running process and are out-of-tree by design. + """ + return MappingProxyType(_PRICING) + + def _lookup_rate(key: str) -> ModelPricing | None: """Resolve a (normalized) pricing key: plugin overlay first, then built-ins. diff --git a/src/coder_eval/reports/__init__.py b/src/coder_eval/reports/__init__.py new file mode 100644 index 000000000..d012654d8 --- /dev/null +++ b/src/coder_eval/reports/__init__.py @@ -0,0 +1,58 @@ +"""Report rendering: markdown, HTML, JUnit XML, and cross-variant experiment reports. + +**This package is a LEAF.** It may import from anywhere in ``coder_eval``; the +core layers may import only its public *writer* entry points. A metric, a +statistic, a serializer or a formatter reached out of here by a core module is a +layering violation — that is what **CE066** enforces, and why +``result_metrics.py``, ``stats.py`` and ``run_record.py`` live outside it. + +Intra-package imports name a sibling module directly (``from .markdown import +X``). A submodule must never do ``from . import X`` or ``from coder_eval.reports +import X``: that re-enters this ``__init__`` mid-initialization. + +``__all__`` is the public surface. Private names are deliberately absent — tests +that need one import it from its submodule, so the package's API is not a +function of its test suite. +""" + +from .experiment import ExperimentReportGenerator +from .helpers import ( + UNGRADED_SCORE_TEXT, + collect_variant_series, + describe_prompt_config, + format_score, + is_env_table_key, +) +from .html import ( + HTMLReportGenerator, + safe_write, + write_experiment_html, + write_task_html, + write_variant_html, +) +from .junit import generate_junit_xml, write_junit_xml +from .markdown import ( + ReportGenerator, + collect_agent_settings_rows, + write_suite_rollups, +) + + +__all__ = [ + "UNGRADED_SCORE_TEXT", + "ExperimentReportGenerator", + "HTMLReportGenerator", + "ReportGenerator", + "collect_agent_settings_rows", + "collect_variant_series", + "describe_prompt_config", + "format_score", + "generate_junit_xml", + "is_env_table_key", + "safe_write", + "write_experiment_html", + "write_junit_xml", + "write_suite_rollups", + "write_task_html", + "write_variant_html", +] diff --git a/src/coder_eval/reports_experiment.py b/src/coder_eval/reports/experiment.py similarity index 73% rename from src/coder_eval/reports_experiment.py rename to src/coder_eval/reports/experiment.py index d8724ffec..2edb32329 100644 --- a/src/coder_eval/reports_experiment.py +++ b/src/coder_eval/reports/experiment.py @@ -4,24 +4,17 @@ import logging from pathlib import Path -from typing import Any -from coder_eval.errors import truncate_crash_message -from coder_eval.models import ( - EvaluationResult, +from ..models import ( ExperimentDefinition, ExperimentResult, - FinalStatus, TaskExperimentSummary, - judge_cost_usd, - simulator_cost_usd, - sum_costs, ) -from coder_eval.path_utils import replicate_subdir_name -from coder_eval.reports import resolve_agent_settings -from coder_eval.reports_stats import ( +from ..path_utils import replicate_subdir_name +from ..run_record import eval_result_to_task_dict +from ..stats import bootstrap_mean_ci, stddev, welch_t_test, wilson_interval +from .helpers import ( VariantSeries, - bootstrap_mean_ci, collect_variant_series, describe_prompt_config, fmt_mean_sd, @@ -30,10 +23,9 @@ is_env_table_key, load_variant_eval_results, paired_comparison, - stddev, - welch_t_test, - wilson_interval, ) +from .html import write_experiment_html, write_variant_html +from .markdown import ReportGenerator, resolve_agent_settings logger = logging.getLogger(__name__) @@ -41,184 +33,6 @@ # Default pass_threshold from BaseSuccessCriterion — used for Wilson pass-rate in replicate stats. _REPLICATE_PASS_THRESHOLD = 0.9 -# Enough to identify a failure without fetching the task artifact, short enough that -# a wholly-errored run doesn't bloat run.json. The full message stays on task.json. -_ROW_ERROR_MESSAGE_MAX_CHARS = 400 - - -def _cost_complete(result: EvaluationResult) -> bool: - """Whether this row's recorded agent spend accounts for everything it spent. - - False means the costs on the row are a floor, not the bill. Two ways in: - - 1. A turn burned tokens the rate card could not price. The card is the fallback - for anything the backend did not price itself, so with no rate those tokens - book no money. - 2. The task was hard-killed by the task-level timeout. Keyed on the status - rather than on emptiness: the watchdog fires while the evaluation loop is - running, so a TIMEOUT row always lost an in-flight turn, even one that - completed earlier turns that do carry costs. - - True for a row that burned nothing: an error before the agent ran genuinely - cost zero, and a slow setup failure is as free as a fast one. - """ - if result.final_status is FinalStatus.TIMEOUT: - return False - return all( - usage.total_cost_usd is not None - for t in result.iterations - if (usage := t.token_usage) is not None and not usage.is_empty() - ) - - -# Build a task_result dict from an EvaluationResult, for the variant reports. - - -def eval_result_to_task_dict( - result: EvaluationResult, - *, - variant_id: str | None = None, - tags: list[str] | None = None, - task_path: str | None = None, - duration_override: float | None = None, - replicate_index: int | None = None, -) -> dict[str, Any]: - """Convert an EvaluationResult to the task_result dict format used by ReportGenerator. - - Args: - result: The evaluation result to convert. - variant_id: Optional variant ID to include in the dict. - tags: Optional tags list (defaults to []). - task_path: Optional path of the task YAML (as supplied to the runner) — - lets downstream consumers (evalboard) derive groupings like skill - from the source folder structure instead of guessing from tags. - duration_override: Optional duration value (defaults to result.duration_seconds). - replicate_index: Replicate index of this row (the ``//`` - sub-dir). Repeated runs of the same task share a ``task_id``, so - without this the row is indistinguishable from its siblings and - downstream consumers (evalboard) collapse them to one. ``None`` when - the caller doesn't track replicates (repeats disabled / legacy). - """ - from coder_eval.reports_stats import expected_turns_overage, turn_time_buckets, visible_turn_count - from coder_eval.reports_stats import has_final_reply as _has_final_reply - - ref_similarity: float | None = None - for cr in result.success_criteria_results: - if cr.criterion_type == "reference_comparison": - ref_similarity = cr.score - break - - overage = expected_turns_overage(result) - - total_turns = sum((t.num_turns or 0) for t in result.iterations) - - # Carried as a row-level boolean so the evalboard can compute the visible turn - # count without re-reading per-task content. - has_reply = _has_final_reply(result) - - agent_cost = result.total_token_usage.total_cost_usd if result.total_token_usage else None - judge_cost = judge_cost_usd(result) - simulator_cost = simulator_cost_usd(result) - row_total_cost = sum_costs(agent_cost, judge_cost, simulator_cost) - - expected_turns_value: int | None = None - if result.task_config is not None: - rl = (result.task_config.resolved or {}).get("run_limits") or {} - if isinstance(rl, dict): - raw = rl.get("expected_turns") - if isinstance(raw, int) and raw >= 1: - expected_turns_value = raw - - _buckets = turn_time_buckets(result) - - d: dict[str, Any] = { - "task_id": result.task_id, - "replicate_index": replicate_index, - "status": result.final_status, - "weighted_score": result.weighted_score, - "duration": duration_override if duration_override is not None else result.duration_seconds, - "iteration_count": result.iteration_count, - "tags": tags if tags is not None else [], - "task_path": task_path, - "iterations": [ - { - "iteration": t.iteration, - "duration_seconds": t.duration_seconds, - "command_count": len(t.commands), - "assistant_turn_count": t.assistant_turn_count, - "crashed": t.crashed, - "crash_reason": t.crash_reason, - } - for t in result.iterations - ], - # Computed ONCE through `turn_time_buckets` and carried as TASK-level keys. - # `iterations` below is a deliberate 6-key projection, so no renderer can - # re-derive them. Each stays `float | None` (CE049). - # Rationale: .claude/notes/reporting.md § Read the stored value, do not re-derive it - "startup_ms": _buckets.startup_ms, - "generation_ms": _buckets.generation_ms, - "tool_ms": _buckets.tool_ms, - "teardown_ms": _buckets.teardown_ms, - "model_used": result.model_used, - "reference_similarity": ref_similarity, - "input_tokens": (result.total_token_usage.uncached_input_tokens if result.total_token_usage else None), - "output_tokens": (result.total_token_usage.output_tokens if result.total_token_usage else None), - "cache_creation_input_tokens": ( - result.total_token_usage.cache_creation_input_tokens if result.total_token_usage else None - ), - "cache_read_input_tokens": ( - result.total_token_usage.cache_read_input_tokens if result.total_token_usage else None - ), - "total_tokens": (result.total_token_usage.total_tokens if result.total_token_usage else None), - # agent + judge + simulator. `total_cost_usd` means the whole bill on every - # surface. None when nothing was priced at all. - "total_cost_usd": row_total_cost, - # Subject-agent spend alone: judge cost is identical across harnesses, so - # leaving it in would make two look closer than they are. - "agent_cost_usd": agent_cost, - # False when the agent spend above is missing money, so it is a floor. - # Rolled up as RunSummary.tasks_cost_incomplete / cost_complete. - "cost_complete": _cost_complete(result), - # The two halves of the eval-machinery bill, rolled up as - # RunSummary.eval_overhead_cost_usd. - "judge_cost_usd": judge_cost, - "simulator_cost_usd": simulator_cost, - # Errors count as misses, so the rollup has to say why it lost those points. - # Without these, triaging an errored run needs one task.json fetch per row. - "error_message": ( - truncate_crash_message(result.error_message, limit=_ROW_ERROR_MESSAGE_MAX_CHARS) - if result.error_message - else None - ), - "error_category": (result.error_details or {}).get("error_category"), - "expected_commands": result.expected_commands, - "actual_commands": result.actual_commands, - "commands_efficiency": result.commands_efficiency, - "agent_config": (result.agent_config.model_dump() if result.agent_config else None), - "sdk_options": result.sdk_options, - "installed_tools": result.environment_info.get("installed_tools"), - "max_turns_exhausted": result.max_turns_exhausted, - "expected_turns_overage": list(overage) if overage is not None else None, - "total_turns": total_turns, - # "Visible turns" (tool calls + final reply) -- what the "within expected - # turns" metric compares against. Distinct from total_turns (SDK num_turns). - "visible_turns": visible_turn_count(result), - "expected_turns": expected_turns_value, - "has_final_reply": has_reply, - # None/False on the default path, so downstream analysis never confuses a - # truncated run with a full one. - "stopped_early": result.early_stop is not None, - "early_stop_reason": (result.early_stop.reason.value if result.early_stop is not None else None), - "turns_remaining_at_stop": ( - result.early_stop.turns_remaining_at_stop if result.early_stop is not None else None - ), - # The threshold in effect for this stop, so a sweep that varies it can tell - # which weighted-gate value produced a given verdict. - "gate_threshold": (result.early_stop.gate_threshold if result.early_stop is not None else None), - } - d["variant_id"] = variant_id - return d - class ExperimentReportGenerator: """Generates markdown reports for experiment results.""" @@ -345,8 +159,9 @@ def _aggregate_count_rows(result: ExperimentResult, show_p_values: bool) -> list row += " | —" lines.append(row + " |") - # Conditional, like the budget sub-rows above. - # Rationale: .claude/notes/reporting.md § The ungraded row in every surface + # Row: Not Graded — conditional, like the budget sub-rows above. Without + # it Tasks Run / Succeeded / Failed / Errors stop summing to tasks_run on + # an ungraded run, with nothing in the table to say where the rest went. if any(result.variant_aggregates[vid].tasks_not_graded > 0 for vid in result.variant_ids): row = "| Not Graded" for vid in result.variant_ids: @@ -544,7 +359,7 @@ def _replicate_stats_lines(result: ExperimentResult) -> list[str]: def _paired_comparison_lines(result: ExperimentResult) -> list[str]: """The ``## Paired Comparison`` block for 2-variant experiments. - Renders :func:`coder_eval.reports_stats.paired_comparison`, which the HTML + Renders :func:`coder_eval.reports.helpers.paired_comparison`, which the HTML reporter renders too. Returns ``[]`` only when the two variants have no scored task in common; when they have exactly one, the section explains why no paired result is shown. @@ -614,8 +429,6 @@ def generate_variant_report(variant_id: str, result: ExperimentResult, run_dir: Returns: Markdown string. """ - from coder_eval.reports import ReportGenerator - agg = result.variant_aggregates[variant_id] pass_rate_str = f"{agg.pass_rate * 100:.1f}%" if agg.pass_rate is not None else "n/a" tokens_str = f"{agg.total_tokens:,}" if agg.total_tokens is not None else "N/A" @@ -780,12 +593,10 @@ def write_reports( (variant_dir / "variant.md").write_text(variant_report, encoding="utf-8") (variant_dir / "variant.json").write_text(agg.model_dump_json(indent=2), encoding="utf-8") - # HTML reports — each write is wrapped by ``safe_write`` so a render - # bug in one report cannot mask the run outcome. - from .reports_html import write_experiment_html, write_variant_html - - # Every variant_id in task_summaries is guaranteed to appear in - # ``result.variant_ids``, so pre-seed with all known variants and extend. + # Build per-variant task link tables from task_summaries. Every + # variant_id in task_summaries is guaranteed to appear in + # ``result.variant_ids`` (the aggregator constructs them from the same + # source), so we pre-seed the dict with all known variants and extend. task_links_by_variant: dict[str, list[tuple[str, str, float | None, str]]] = { vid: [] for vid in result.variant_ids } @@ -796,6 +607,8 @@ def write_reports( (vr.task_id, rel_link, vr.weighted_score, vr.final_status.value) ) + # HTML reports — each write is wrapped by ``safe_write`` so a render + # bug in one report cannot mask the run outcome. for vid in result.variant_ids: agg = result.variant_aggregates.get(vid) if agg is None: diff --git a/src/coder_eval/reports/helpers.py b/src/coder_eval/reports/helpers.py new file mode 100644 index 000000000..14f944afa --- /dev/null +++ b/src/coder_eval/reports/helpers.py @@ -0,0 +1,251 @@ +"""Report-shaped helpers over variant and experiment results. + +What is left here after the split is presentation and report assembly: the +variant/experiment series collectors, the paired-comparison summary, and the +formatters that turn a number into a cell (``fmt_mean_sd``, ``fmt_p``, +``format_score``). The numeric core moved to ``coder_eval.stats`` and the +``EvaluationResult`` metrics to ``coder_eval.result_metrics``. + +**The cycle rationale is live, not historical.** These helpers stay in a module +of their own so ``reports.html`` can consume them without importing +``reports.experiment`` — which imports ``reports.html`` for its HTML-write +helpers. Folding this module into the experiment reporter would close +``experiment -> html -> helpers`` into a cycle. +""" + +from __future__ import annotations + +import logging +from pathlib import Path +from typing import NamedTuple + +from ..models import ( + EvaluationResult, + ExperimentResult, + ExperimentVariant, + TaskExperimentSummary, +) +from ..path_utils import TASK_JSON_FILENAME +from ..stats import cohens_d, mean, paired_t_ci, paired_t_test, stddev + + +logger = logging.getLogger(__name__) + + +# --------------------------------------------------------------------------- +# Display formatters (presentation, not computation — the statistics are in +# coder_eval.stats) +# --------------------------------------------------------------------------- + + +def fmt_mean_sd(values: list[float], fmt: str = ".3f") -> str: + """Format mean ± stddev string. Omits ± when n < 2 (stddev undefined).""" + if not values: + return "N/A" + m = mean(values) + if len(values) < 2: + return f"{m:{fmt}}" + sd = stddev(values) + return f"{m:{fmt}} ± {sd:{fmt}}" + + +def fmt_p(p: float | None) -> str: + """Format p-value for display.""" + if p is None: + return "—" + if p < 0.001: + return "<0.001" + return f"{p:.3f}" + + +# --------------------------------------------------------------------------- +# Aggregate-metric series +# --------------------------------------------------------------------------- + + +class VariantSeries(NamedTuple): + """One variant's numeric series across all tasks — the raw inputs to the + Aggregate Metrics rows and their p-values.""" + + scores: list[float] + durations: list[float] + tokens: list[float] + asst_turns: list[float] + + +# environment_info keys the Environment table must NOT render as ordinary rows. +# `installed_tools` has its own dedicated section; the rest are harness +# bookkeeping the reader did not ask for — `command_base_path` is a full PATH +# string on every row, and the graded_by_* provenance keys only appear on a +# re-graded row where they would read as facts about the run itself. +ENV_TABLE_EXCLUDE = frozenset({"installed_tools", "command_base_path", "reference_digest"}) + + +def is_env_table_key(key: str) -> bool: + """Whether ``key`` belongs in a rendered Environment table.""" + return key not in ENV_TABLE_EXCLUDE and not key.startswith("graded_by_") + + +# What an ungraded row shows where a score would go. Deliberately not "0.000": +# an ungraded task was never measured, and a zero is indistinguishable from a +# task that was measured and scored nothing. +UNGRADED_SCORE_TEXT = "n/a" + + +def format_score(score: float | None) -> str: + """Render a weighted score for a report table, or ``n/a`` when ungraded.""" + return UNGRADED_SCORE_TEXT if score is None else f"{score:.3f}" + + +def collect_variant_series(result: ExperimentResult) -> dict[str, VariantSeries]: + """Per-variant (scores, durations, tokens, assistant-turns) series, keyed by variant id. + + Shared by the markdown and HTML reporters so both render the same numbers. + ``VariantResult.duration_seconds`` is *summed* across replicates, so it is + divided by ``replicate_count`` to give a per-run duration comparable across + variants that ran different replicate counts. + """ + series = {vid: VariantSeries([], [], [], []) for vid in result.variant_ids} + for ts in result.task_summaries: + for vr in ts.variant_results: + s = series.get(vr.variant_id) + if s is None: # a task result for a variant not in variant_ids + continue + # Only the SCORE is dropped when there is none — never the row. + # Duration, tokens and assistant turns are facts about the run that + # grading has nothing to do with, and `execute`'s stated contract is + # that only the verdict is withheld. Skipping the row whole made an + # all-ungraded experiment render `Avg Duration | N/A | N/A` with the + # Tokens and Assistant Turns rows absent entirely. + # + # The series are consumed independently (each statistic reads one + # list), so they need not be index-aligned with each other; + # `paired_comparison` pairs across VARIANTS by task id, not by index + # into these lists. An earlier note here claimed an experiment is + # either entirely graded or entirely ungraded because `grade` is + # run-level — `run --resume` grades rows independently and folds a + # failed one back ungraded, so mixed experiments are real. + if vr.weighted_score is not None: + s.scores.append(vr.weighted_score) + s.durations.append(vr.duration_seconds / vr.replicate_count) + if vr.total_tokens is not None: + s.tokens.append(float(vr.total_tokens)) + if vr.total_assistant_turns is not None: + s.asst_turns.append(float(vr.total_assistant_turns)) + return series + + +class PairedComparison(NamedTuple): + """A 2-variant paired comparison over per-task mean scores. + + ``task_count`` is the number of tasks both variants scored. When it is < 2 + the statistics are all ``None`` — there is nothing to compare, and the + reporters say so rather than rendering an empty section. ``excluded_count`` + is the number of tasks that appeared for at least one variant but could not + be paired (missing or empty on the other side); the reporters surface it so + a silently narrowed sample is visible. + """ + + vid_a: str + vid_b: str + task_count: int + excluded_count: int + mean_diff: float | None + ci_low: float | None + ci_high: float | None + effect_size: float | None + p_value: float | None + + +def paired_comparison(result: ExperimentResult, confidence: float = 0.95) -> PairedComparison | None: + """Pair the two variants' per-task mean scores. Returns None unless the + experiment has exactly 2 variants with at least one commonly-scored task. + + The task is the unit of analysis: replicate slots within a task share the task + effect and are not independent, so pairing them individually would understate + the standard error. Replicate counts need not match — a task's mean score is a + well-defined pair member either way. + """ + if len(result.variant_ids) != 2: + return None + vid_a, vid_b = result.variant_ids[0], result.variant_ids[1] + per_rep_a = result.per_replicate_scores.get(vid_a, {}) + per_rep_b = result.per_replicate_scores.get(vid_b, {}) + common_tasks = sorted(t for t in set(per_rep_a) & set(per_rep_b) if per_rep_a[t] and per_rep_b[t]) + if not common_tasks: + # No shared task, or per_replicate_scores absent (results from before it existed). + return None + + # Tasks seen for at least one variant but not paired (missing or empty on the + # other side) — surfaced so a silently narrowed sample doesn't go unnoticed. + excluded_count = len(set(per_rep_a) | set(per_rep_b)) - len(common_tasks) + + if len(common_tasks) < 2: + return PairedComparison(vid_a, vid_b, len(common_tasks), excluded_count, None, None, None, None, None) + + a_scores = [mean(per_rep_a[task_id]) for task_id in common_tasks] + b_scores = [mean(per_rep_b[task_id]) for task_id in common_tasks] + ci = paired_t_ci(a_scores, b_scores, confidence=confidence) + if ci is None: # non-finite scores + return PairedComparison(vid_a, vid_b, len(common_tasks), excluded_count, None, None, None, None, None) + mean_diff, ci_low, ci_high = ci + return PairedComparison( + vid_a, + vid_b, + len(common_tasks), + excluded_count, + mean_diff, + ci_low, + ci_high, + cohens_d(a_scores, b_scores), + paired_t_test(a_scores, b_scores), + ) + + +# --------------------------------------------------------------------------- +# Prompt config + variant-result loaders +# --------------------------------------------------------------------------- + + +def describe_prompt_config(variant: ExperimentVariant) -> str: + """Return a short description of the variant's prompt configuration. + + Returns strings like ``"(base prompt)"``, ``"(prompt override)"``, or + ``"(2 mutations: prefix, suffix)"``. + """ + if variant.initial_prompt is not None or variant.initial_prompt_file is not None: + return "(prompt override)" + if variant.prompt_mutations: + type_names = [m.type for m in variant.prompt_mutations] + return f"({len(type_names)} mutations: {', '.join(type_names)})" + return "(base prompt)" + + +def load_variant_eval_results( + run_dir: Path, variant_id: str, task_summaries: list[TaskExperimentSummary] +) -> list[EvaluationResult]: + """Load EvaluationResult objects for a variant from disk. + + Walks all ``///NN/task.json`` replicate + subdirs for each task in ``task_summaries`` and returns every result that + loads successfully. + """ + variant_dir = run_dir / variant_id + results: list[EvaluationResult] = [] + + if not variant_dir.is_dir(): + return results + + for ts in task_summaries: + task_dir = variant_dir / ts.task_id + if not task_dir.is_dir(): + continue + for rep_subdir in sorted(task_dir.glob("[0-9][0-9]")): + task_json = rep_subdir / TASK_JSON_FILENAME + if task_json.exists(): + try: + results.append(EvaluationResult.model_validate_json(task_json.read_text(encoding="utf-8"))) + except Exception: + logger.warning("Failed to load %s for variant report", task_json, exc_info=True) + + return results diff --git a/src/coder_eval/reports_html.py b/src/coder_eval/reports/html.py similarity index 98% rename from src/coder_eval/reports_html.py rename to src/coder_eval/reports/html.py index 1364a0a13..6b1bbd6a3 100644 --- a/src/coder_eval/reports_html.py +++ b/src/coder_eval/reports/html.py @@ -21,15 +21,32 @@ from pathlib import Path from typing import TYPE_CHECKING, Any -from coder_eval.formatting import format_ms -from coder_eval.models import FinalStatus, eval_result_total_cost, sum_costs - -from .reports import early_stop_gate_note -from .reports_stats import format_score, is_env_table_key, turn_time_buckets +from ..analysis import calculate_command_statistics +from ..durations import format_ms +from ..models import FinalStatus, eval_result_total_cost, sum_costs +from ..result_metrics import expected_turns_overage, turn_time_buckets +from ..stats import stddev, welch_t_test +from .helpers import ( + collect_variant_series, + describe_prompt_config, + fmt_mean_sd, + fmt_p, + format_score, + is_env_table_key, + load_variant_eval_results, + paired_comparison, +) +from .markdown import ( + SLOW_PARAMS_PREVIEW_CHARS, + collect_agent_settings_rows, + count_partials_by_outcome, + early_stop_gate_note, + group_consecutive_by_iteration, +) if TYPE_CHECKING: - from coder_eval.models import ( + from ..models import ( CommandTelemetry, CriterionResult, EarlyStopInfo, @@ -321,8 +338,6 @@ def _format_params(params: dict[str, Any]) -> str: def _render_header(result: EvaluationResult) -> str: - from .reports_stats import expected_turns_overage - started = result.started_at.isoformat(timespec="seconds") if result.started_at else "—" duration = _format_duration(result.duration_seconds) score_badge = _score_pill(result.weighted_score) if result.weighted_score is not None else "" @@ -683,7 +698,6 @@ def _group_turns_by_iteration( turns: list[TurnRecord], ) -> list[tuple[int, list[TurnRecord]]]: """Group consecutive TurnRecords by iteration as ``(iteration, group)`` tuples for the renderer.""" - from .reports import group_consecutive_by_iteration groups = group_consecutive_by_iteration(turns, lambda t: t.iteration) return [(group[0].iteration, group) for group in groups] @@ -799,7 +813,6 @@ def _render_command_stats(stats: Any | None) -> str: for tool, count in sorted((stats.commands_by_tool or {}).items(), key=lambda x: x[1], reverse=True): rows.append(f"{_esc(tool)}{count}") rows_html = "".join(rows) or "No commands" - from .reports import SLOW_PARAMS_PREVIEW_CHARS slow_rows_list: list[str] = [] for c in stats.slowest_commands or []: @@ -935,7 +948,6 @@ def _format_signed_ms(ms: float | None) -> str: def _render_generation_metrics(result: EvaluationResult) -> str: """Render Generation Metrics — latency, turns, and the four wall-clock buckets.""" - from .reports import count_partials_by_outcome, group_consecutive_by_iteration turns = result.iterations or [] num_turns = len(turns) @@ -952,8 +964,8 @@ def _render_generation_metrics(result: EvaluationResult) -> str: f'
Crashed Partials
' f'
{_esc(breakdown)}
' ) - # The arithmetic is in reports_stats; this only formats it. An unmeasured bucket - # renders as an em dash, never 0ms (CE058). + # The arithmetic is in result_metrics; this only formats it. An unmeasured + # bucket renders as an em dash, never 0ms (CE058). # Rationale: .claude/notes/reporting.md § An unmeasured value is never zero buckets = turn_time_buckets(result) startup = format_ms(buckets.startup_ms) @@ -1009,7 +1021,6 @@ def _render_commands_efficiency(result: EvaluationResult) -> str: def _render_agent_settings(result: EvaluationResult) -> str: """Render Agent Settings section. Prefers sdk_options, falls back to agent_config.""" - from .reports import collect_agent_settings_rows if result.sdk_options: settings: dict[str, Any] = result.sdk_options @@ -1145,7 +1156,6 @@ def _variant_stddev_lines(variant_id: str, result: ExperimentResult | None) -> s """ if result is None: return "" - from .reports_stats import stddev vrs = [vr for ts in result.task_summaries for vr in ts.variant_results if vr.variant_id == variant_id] scores = [vr.weighted_score for vr in vrs if vr.weighted_score is not None] @@ -1171,9 +1181,6 @@ def _variant_rich_sections(variant_id: str, result: ExperimentResult | None, run if result is None or run_dir is None: return "" - from .analysis import calculate_command_statistics - from .reports_stats import load_variant_eval_results - eval_results = load_variant_eval_results(run_dir, variant_id, result.task_summaries) if not eval_results: return "" @@ -1238,7 +1245,7 @@ def _render_variant_token_usage(eval_results: list[EvaluationResult]) -> str: output_tok = sum(u.output_tokens for u in usages) cache_write = sum(u.cache_creation_input_tokens for u in usages) cache_read = sum(u.cache_read_input_tokens for u in usages) - total = input_tok + output_tok + cache_write + cache_read + total = sum(u.total_tokens for u in usages) variant_cost = sum_costs(*(eval_result_total_cost(r) for r in eval_results)) cost_str = f"${variant_cost:.4f}" if variant_cost is not None else "N/A" return f""" @@ -1287,7 +1294,6 @@ def _experiment_prompt_config(experiment: ExperimentDefinition | None, variant_i actually specifies any mutations or overrides.""" if experiment is None: return "" - from .reports_stats import describe_prompt_config has_config = bool(experiment.defaults and experiment.defaults.prompt_mutations) or any( v.prompt_mutations or v.initial_prompt or v.initial_prompt_file for v in experiment.variants @@ -1311,10 +1317,9 @@ def _experiment_prompt_config(experiment: ExperimentDefinition | None, variant_i def _experiment_paired_comparison(result: ExperimentResult) -> str: """Render the Paired Comparison section — the HTML twin of the markdown one. - Both render the same ``reports_stats.paired_comparison`` result, so the two + Both render the same ``reports.helpers.paired_comparison`` result, so the two reports can never disagree about the paired numbers. """ - from .reports_stats import fmt_p, paired_comparison pc = paired_comparison(result) if pc is None: @@ -1353,7 +1358,6 @@ def _experiment_paired_comparison(result: ExperimentResult) -> str: def _experiment_aggregate_metrics(result: ExperimentResult) -> str: """Render the Aggregate Metrics table (with p-values when exactly 2 variants).""" - from .reports_stats import collect_variant_series, fmt_mean_sd, fmt_p, welch_t_test show_p = len(result.variant_ids) == 2 vid_a, vid_b = (result.variant_ids[0], result.variant_ids[1]) if show_p else ("", "") diff --git a/src/coder_eval/reports_junit.py b/src/coder_eval/reports/junit.py similarity index 99% rename from src/coder_eval/reports_junit.py rename to src/coder_eval/reports/junit.py index 890b6414d..901a9712d 100644 --- a/src/coder_eval/reports_junit.py +++ b/src/coder_eval/reports/junit.py @@ -27,9 +27,9 @@ from pathlib import Path, PurePosixPath, PureWindowsPath from typing import Any, Literal -from .evaluation.judge_context import truncate -from .models import FinalStatus, RunSummary, SuiteRollup -from .path_utils import TASK_JSON_FILENAME +from ..evaluation.judge_context import truncate +from ..models import FinalStatus, RunSummary, SuiteRollup +from ..path_utils import TASK_JSON_FILENAME logger = logging.getLogger(__name__) diff --git a/src/coder_eval/reports.py b/src/coder_eval/reports/markdown.py similarity index 98% rename from src/coder_eval/reports.py rename to src/coder_eval/reports/markdown.py index 680fce266..05c3f8581 100644 --- a/src/coder_eval/reports.py +++ b/src/coder_eval/reports/markdown.py @@ -8,25 +8,29 @@ from pathlib import Path, PurePosixPath from typing import TYPE_CHECKING, Any, Literal, assert_never -from .formatting import format_ms -from .models import ( +from ..analysis import calculate_command_statistics +from ..durations import format_ms +from ..models import ( CriterionAggregate, CriterionStats, EarlyStopReason, + EvaluationResult, FailedRowSummary, + RunSummary, SuiteRollup, TaskResult, ThresholdCheck, + TurnRecord, eval_overhead_cost, nothing_was_measured, row_cost_incomplete, sum_costs, ) -from .path_utils import TASK_JSON_FILENAME, build_task_run_dir +from ..path_utils import TASK_JSON_FILENAME, build_task_run_dir if TYPE_CHECKING: - from .models import CommandStatistics, RunSummary + from ..models import CommandStatistics logger = logging.getLogger(__name__) @@ -310,8 +314,9 @@ def _generate_command_statistics_section(stats: CommandStatistics) -> list[str]: ["", "### Slowest Commands", "", "| Tool | Duration | Parameters |", "|------|----------|------------|"] ) for cmd in stats.slowest_commands: - params_str = str(cmd.parameters)[:50] - if len(str(cmd.parameters)) > 50: + params_full = str(cmd.parameters) + params_str = params_full[:SLOW_PARAMS_PREVIEW_CHARS] + if len(params_full) > SLOW_PARAMS_PREVIEW_CHARS: params_str += "..." lines.append(f"| {cmd.tool} | {cmd.duration_ms:.0f}ms | {params_str} |") @@ -359,9 +364,9 @@ def _generate_generation_metrics_section(task_results: list[dict[str, Any]]) -> else: avg_turn_str = "N/A" - # READ, never summed here -- computed once by `turn_time_buckets`. - # `.get()` because an older `run.json` has none of the four, which then - # renders as a dash, not as `0ms`. + # READ, never summed here -- computed once by + # `result_metrics.turn_time_buckets`. `.get()` because an older + # `run.json` has none of the four, which then renders as a dash, not `0ms`. # Rationale: .claude/notes/reporting.md § Read the stored value, do not re-derive it buckets = " | ".join( format_ms(task.get(key)) for key in ("startup_ms", "generation_ms", "tool_ms", "teardown_ms") @@ -742,9 +747,6 @@ def _aggregate_command_statistics(run_dir: Path) -> CommandStatistics | None: Returns: Aggregated CommandStatistics or None if no stats available """ - from .analysis import calculate_command_statistics - from .models import EvaluationResult, TurnRecord - all_turns: list[TurnRecord] = [] # Find all task.json files recursively to handle both flat and nested (experiment) layouts @@ -791,8 +793,6 @@ def load_from_run_dir(run_dir: Path) -> tuple[str, Path]: return report_md_path.read_text(encoding="utf-8"), report_md_path if summary_json_path.exists(): - from .models import RunSummary - summary = RunSummary.model_validate_json(summary_json_path.read_text(encoding="utf-8")) report_md = ReportGenerator.generate_markdown(summary, run_dir=run_dir) return report_md, summary_json_path @@ -874,7 +874,9 @@ def _compute_suite_rollup( ``suite_thresholds`` evaluation. Pass None when unavailable — per-criterion stats still compute but no aggregate/threshold gating happens. """ - from .criteria import CriterionRegistry, init_criteria + # Deferred on purpose: importing coder_eval.criteria runs pkgutil auto-discovery + # with registry side effects, which would land on every `import coder_eval.reports`. + from ..criteria import CriterionRegistry, init_criteria rows_total = len(rows) rows_passed = sum(1 for r in rows if r.result.final_status.category == "succeeded") diff --git a/src/coder_eval/reports_stats.py b/src/coder_eval/reports_stats.py deleted file mode 100644 index bb3b2de6e..000000000 --- a/src/coder_eval/reports_stats.py +++ /dev/null @@ -1,639 +0,0 @@ -"""Shared statistical + prompt-config helpers for the markdown and HTML reporters. - -Kept in a standalone module so ``reports_html`` can consume them without -importing ``reports_experiment`` (which in turn imports ``reports_html`` -for its HTML-write helpers, and would otherwise form a cycle). -""" - -from __future__ import annotations - -import logging -import math -import random -import statistics as _stats -from collections.abc import Iterable -from pathlib import Path -from typing import NamedTuple - -from coder_eval.models import ( - AssistantMessage, - EvaluationResult, - ExperimentResult, - ExperimentVariant, - TaskExperimentSummary, - TurnRecord, -) -from coder_eval.timing import main_thread_tool_spans, union_ms - -from .path_utils import TASK_JSON_FILENAME - - -logger = logging.getLogger(__name__) - - -# --------------------------------------------------------------------------- -# Statistical helpers (stdlib statistics module) -# --------------------------------------------------------------------------- - - -def mean(values: list[float]) -> float: - return _stats.mean(values) if values else 0.0 - - -def stddev(values: list[float]) -> float: - """Sample standard deviation (Bessel-corrected). Returns 0.0 for n < 2.""" - return _stats.stdev(values) if len(values) >= 2 else 0.0 - - -def _betacf(a: float, b: float, x: float) -> float: - """Continued fraction for the regularized incomplete beta (Lentz's method).""" - max_iterations = 200 - eps = 3e-12 - fpmin = 1e-300 - - qab, qap, qam = a + b, a + 1.0, a - 1.0 - c = 1.0 - d = 1.0 - qab * x / qap - if abs(d) < fpmin: - d = fpmin - d = 1.0 / d - h = d - for m in range(1, max_iterations + 1): - m2 = 2 * m - # Even step of the recurrence. - aa = m * (b - m) * x / ((qam + m2) * (a + m2)) - d = 1.0 + aa * d - if abs(d) < fpmin: - d = fpmin - c = 1.0 + aa / c - if abs(c) < fpmin: - c = fpmin - d = 1.0 / d - h *= d * c - # Odd step. - aa = -(a + m) * (qab + m) * x / ((a + m2) * (qap + m2)) - d = 1.0 + aa * d - if abs(d) < fpmin: - d = fpmin - c = 1.0 + aa / c - if abs(c) < fpmin: - c = fpmin - d = 1.0 / d - delta = d * c - h *= delta - if abs(delta - 1.0) < eps: - return h - logger.warning("Incomplete beta continued fraction did not converge for a=%r, b=%r, x=%r", a, b, x) - return h - - -def regularized_incomplete_beta(a: float, b: float, x: float) -> float: - """Regularized incomplete beta function I_x(a, b), for a, b > 0 and x in [0, 1]. - - Raises ValueError outside that domain — returning NaN would let a bad input - render as a real-looking statistic downstream. - """ - if not (math.isfinite(a) and math.isfinite(b) and math.isfinite(x)): - raise ValueError(f"a, b and x must be finite, got a={a!r}, b={b!r}, x={x!r}") - if a <= 0.0 or b <= 0.0: - raise ValueError(f"a and b must be positive, got a={a!r}, b={b!r}") - if x <= 0.0: - return 0.0 - if x >= 1.0: - return 1.0 - ln_front = math.lgamma(a + b) - math.lgamma(a) - math.lgamma(b) + a * math.log(x) + b * math.log1p(-x) - front = math.exp(ln_front) - # Use the continued fraction directly where it converges fast, else via symmetry. - if x < (a + 1.0) / (a + b + 2.0): - return front * _betacf(a, b, x) / a - return 1.0 - front * _betacf(b, a, 1.0 - x) / b - - -def student_t_two_tailed_p(t_stat: float, df: float) -> float: - """Exact two-tailed p-value for Student's t: P(|T| >= |t|) = I_x(df/2, 1/2), x = df/(df + t^2). - - Non-finite inputs fail closed to 1.0 — garbage must never read as significant. - """ - if not math.isfinite(t_stat) or not math.isfinite(df) or df <= 0: - return 1.0 - x = df / (df + t_stat * t_stat) - return regularized_incomplete_beta(df / 2.0, 0.5, x) - - -def welch_t_test(a: list[float], b: list[float]) -> float | None: - """Two-tailed p-value from Welch's unequal-variances t-test (exact t distribution). - - Degrees of freedom via Welch-Satterthwaite; the t CDF is evaluated exactly - through the regularized incomplete beta (stdlib only, no scipy). Returns - None if either group has fewer than 2 observations, or holds a non-finite - value (rendered as "—" rather than a fabricated p-value). - """ - n_a, n_b = len(a), len(b) - if n_a < 2 or n_b < 2: - return None - if not all(math.isfinite(v) for v in (*a, *b)): - return None - - mean_a, mean_b = _stats.mean(a), _stats.mean(b) - var_a = _stats.variance(a) - var_b = _stats.variance(b) - - se_sq = var_a / n_a + var_b / n_b - if se_sq == 0: - # Zero variance in both groups: identical constants (p=1) or a - # deterministic difference (p=0). - return 1.0 if mean_a == mean_b else 0.0 - - t_stat = abs(mean_a - mean_b) / math.sqrt(se_sq) - df = se_sq**2 / ((var_a / n_a) ** 2 / (n_a - 1) + (var_b / n_b) ** 2 / (n_b - 1)) - return student_t_two_tailed_p(t_stat, df) - - -def fmt_mean_sd(values: list[float], fmt: str = ".3f") -> str: - """Format mean ± stddev string. Omits ± when n < 2 (stddev undefined).""" - if not values: - return "N/A" - m = mean(values) - if len(values) < 2: - return f"{m:{fmt}}" - sd = stddev(values) - return f"{m:{fmt}} ± {sd:{fmt}}" - - -def fmt_p(p: float | None) -> str: - """Format p-value for display.""" - if p is None: - return "—" - if p < 0.001: - return "<0.001" - return f"{p:.3f}" - - -# --------------------------------------------------------------------------- -# Replicate statistics helpers (stdlib random + statistics) -# --------------------------------------------------------------------------- - - -def bootstrap_mean_ci( - values: list[float], - n_resamples: int = 1000, - confidence: float = 0.95, - seed: int = 0, -) -> tuple[float, float, float]: - """Percentile-bootstrap confidence interval for the mean. - - Returns (mean, ci_low, ci_high). When ``len(values) < 2``, returns - (values[0], values[0], values[0]) or (0, 0, 0) for empty input. - Uses ``random.Random(seed)`` for determinism. - - Raises ValueError for a ``confidence`` outside (0, 1) or a non-positive - ``n_resamples`` — clamping those would quietly return an interval of the - wrong width, which is worse than refusing. - """ - if not 0.0 < confidence < 1.0: - raise ValueError(f"confidence must be in (0, 1), got {confidence!r}") - if n_resamples < 1: - raise ValueError(f"n_resamples must be >= 1, got {n_resamples!r}") - if not values: - return (0.0, 0.0, 0.0) - m = sum(values) / len(values) - if len(values) < 2: - return (m, m, m) - rng = random.Random(seed) - n = len(values) - resampled_means = sorted(sum(rng.choice(values) for _ in range(n)) / n for _ in range(n_resamples)) - alpha = (1.0 - confidence) / 2.0 - lo = resampled_means[int(alpha * n_resamples)] - hi = resampled_means[int((1.0 - alpha) * n_resamples) - 1] - return (m, lo, hi) - - -def wilson_interval(successes: int, n: int, confidence: float = 0.95) -> tuple[float, float]: - """Wilson score interval for a binomial proportion. Returns (low, high). - - More reliable than the normal approximation at small N and near 0/1. - """ - if n <= 0: - return (0.0, 0.0) - z = _stats.NormalDist().inv_cdf((1.0 + confidence) / 2.0) - p_hat = successes / n - denom = 1.0 + z * z / n - center = (p_hat + z * z / (2.0 * n)) / denom - half = (z * math.sqrt(p_hat * (1.0 - p_hat) / n + z * z / (4.0 * n * n))) / denom - return (max(0.0, center - half), min(1.0, center + half)) - - -def cohens_d(a: list[float], b: list[float]) -> float | None: - """Paired Cohen's d = mean(a_i - b_i) / stddev(a_i - b_i).""" - if len(a) != len(b) or len(a) < 2: - return None - diffs = [ai - bi for ai, bi in zip(a, b, strict=True)] - s = stddev(diffs) - return (sum(diffs) / len(diffs)) / s if s > 0 else None - - -def student_t_critical(confidence: float, df: float) -> float: - """Two-tailed critical value t* with P(|T| >= t*) = 1 - confidence. - - Inverts :func:`student_t_two_tailed_p` by bisection — that p is continuous and - strictly decreasing in |t|, so a plain bracket-and-halve is exact to ~1e-12 and - needs no separate quantile expansion. - """ - if not 0.0 < confidence < 1.0: - raise ValueError(f"confidence must be in (0, 1), got {confidence!r}") - if df <= 0 or not math.isfinite(df): - return math.inf - alpha = 1.0 - confidence - lo, hi = 0.0, 1.0 - while student_t_two_tailed_p(hi, df) > alpha: - lo = hi - hi *= 2.0 - if hi > 1e12: - # Only reachable for a confidence so close to 1 that t* overflows the - # bracket. Warn rather than return a silently wrong-width interval. - logger.warning( - "student_t_critical failed to bracket t* for confidence=%r, df=%r; returning a degraded upper bound", - confidence, - df, - ) - return hi - for _ in range(200): - mid = (lo + hi) / 2.0 - if mid in (lo, hi): - break - if student_t_two_tailed_p(mid, df) > alpha: - lo = mid - else: - hi = mid - return (lo + hi) / 2.0 - - -def paired_t_ci(a: list[float], b: list[float], confidence: float = 0.95) -> tuple[float, float, float] | None: - """Student-t confidence interval for mean(a_i - b_i): mean ± t* · sd/√n. - - Returns (mean_diff, ci_low, ci_high), or None if lengths differ, n < 2, or any - value is non-finite. Shares its distribution with :func:`paired_t_test`, so the - interval and the p-value always agree about whether 0 is excluded. - """ - if len(a) != len(b) or len(a) < 2: - return None - if not all(math.isfinite(v) for v in (*a, *b)): - return None - diffs = [ai - bi for ai, bi in zip(a, b, strict=True)] - n = len(diffs) - mean_diff = sum(diffs) / n - half_width = student_t_critical(confidence, n - 1) * stddev(diffs) / math.sqrt(n) - return (mean_diff, mean_diff - half_width, mean_diff + half_width) - - -def paired_t_test(a: list[float], b: list[float]) -> float | None: - """Two-tailed p-value from a paired t-test on (a_i - b_i), exact t distribution. - - Equivalent to a one-sample t-test of the differences against 0, df = n - 1. - Returns None if lengths differ, n < 2, or any value is non-finite. - """ - if len(a) != len(b) or len(a) < 2: - return None - if not all(math.isfinite(v) for v in (*a, *b)): - return None - diffs = [ai - bi for ai, bi in zip(a, b, strict=True)] - sd = stddev(diffs) - mean_diff = sum(diffs) / len(diffs) - if sd == 0: - # All diffs identical: no difference (p=1) or a deterministic shift (p=0). - return 1.0 if mean_diff == 0 else 0.0 - t_stat = abs(mean_diff) / (sd / math.sqrt(len(diffs))) - return student_t_two_tailed_p(t_stat, len(diffs) - 1) - - -# --------------------------------------------------------------------------- -# Aggregate-metric series -# --------------------------------------------------------------------------- - - -class VariantSeries(NamedTuple): - """One variant's numeric series across all tasks — the raw inputs to the - Aggregate Metrics rows and their p-values.""" - - scores: list[float] - durations: list[float] - tokens: list[float] - asst_turns: list[float] - - -# Keys the Environment table must NOT render as ordinary rows: `installed_tools` has -# its own section, `command_base_path` is a full PATH string, and the graded_by_* -# keys appear only on a re-graded row, where they would read as facts about the run. -ENV_TABLE_EXCLUDE = frozenset({"installed_tools", "command_base_path", "reference_digest"}) - - -def is_env_table_key(key: str) -> bool: - """Whether ``key`` belongs in a rendered Environment table.""" - return key not in ENV_TABLE_EXCLUDE and not key.startswith("graded_by_") - - -# Deliberately not "0.000": a zero is indistinguishable from a task that WAS -# measured and scored nothing. -# Rationale: .claude/notes/reporting.md § An unmeasured value is never zero -UNGRADED_SCORE_TEXT = "n/a" - - -def format_score(score: float | None) -> str: - """Render a weighted score for a report table, or ``n/a`` when ungraded.""" - return UNGRADED_SCORE_TEXT if score is None else f"{score:.3f}" - - -class TurnTimeBuckets(NamedTuple): - """The four wall-clock buckets of a whole run, plus what they leave over. - - Each is ``None`` when NOTHING in the run measured it — a run recorded before - the head and tail were captured has no startup at all, a run that recorded - no bounded tool span has no tool total, and a run with no duration has no - residual. Rendering any of those as ``0ms`` claims a measurement nobody - took (CE058, and the reason the evalboard's ``sumMeasured`` returns - ``null``). A MEASURED zero stays ``0.0`` and renders as ``0ms``. - - DISPLAY AND ARITHMETIC DIFFER HERE, on purpose. An unmeasured bucket renders - as a dash and counts as ``0.0`` toward ``unaccounted``, so the missing time - surfaces as residual rather than vanishing. That is the rule - ``scripts/timing/decompose_run.py::_turn_buckets`` already applies, and - keeping the two the same is what lets a reader compare them. - """ - - startup_ms: float | None - generation_ms: float | None - tool_ms: float | None - teardown_ms: float | None - unaccounted_ms: float | None - - -def turn_time_buckets(result: EvaluationResult) -> TurnTimeBuckets: - """Sum the four timing buckets across a run's turns, and the residual. - - The arithmetic lives HERE rather than in the renderer because this module is - the designated home for shared report statistics: the evalboard, the - markdown report and the HTML report must not each grow their own version. - ``reports_html`` formats what this returns and decides nothing. - - ``unaccounted`` is measured against ``EvaluationResult.duration_seconds`` — - the TASK's wall clock, which is what the card's existing Total Latency uses - and what the evalboard's own Unaccounted cell uses. It therefore legitimately - contains sandbox setup and grading, and is LARGER than the per-turn residual - ``decompose_run.py`` reports. The two are not comparable and the label says - so. - """ - turns = result.iterations or [] - startup = _sum_measured(t.harness_startup_ms for t in turns) - teardown = _sum_measured(t.harness_teardown_ms for t in turns) - # MAIN THREAD ONLY, the same filter the collector and the evalboard apply: a - # sub-agent's generations bubble into the same stream, and the spawning call's - # own interval already spans them. - generation = _sum_measured( - m.generation_duration_ms - for t in turns - for m in t.messages - if isinstance(m, AssistantMessage) and m.parent_tool_use_id is None - ) - # `None` only when NO turn recorded a bounded span: the presence of a SPAN, not - # of a turn, is what decides measured-versus-not. - # Rationale: .claude/notes/reporting.md § An unmeasured value is never zero - per_turn = [_turn_tool_union_ms(t) for t in turns] - tool = _sum_measured(per_turn) if any(ms is not None for ms in per_turn) else None - - # `duration_seconds` defaults to 0.0 with no None arm to write, but a 0.0 - # duration is a run that was never timed, and subtracting real buckets from it - # renders a fabricated negative residual. The evalboard keeps that null too. - unaccounted = ( - result.duration_seconds * 1000.0 - (startup or 0.0) - (generation or 0.0) - (tool or 0.0) - (teardown or 0.0) - if result.duration_seconds > 0.0 - else None - ) - return TurnTimeBuckets(startup, generation, tool, teardown, unaccounted) - - -def _sum_measured(values: Iterable[float | None]) -> float | None: - """Sum what was measured, or ``None`` when nothing was. - - The Python twin of the evalboard's ``sumMeasured``: a run with no measured - value anywhere returns ``None`` (never measured), while a run that measured - a genuine zero returns ``0.0``. - """ - total: float | None = None - for value in values: - if isinstance(value, (int, float)) and math.isfinite(value): - total = (total or 0.0) + value - return total - - -def _turn_tool_union_ms(turn: TurnRecord) -> float | None: - """One turn's tool execution — the UNION of its main-thread command spans. - - PREFERS THE STORED ``TurnRecord.tool_union_ms``, which the collector writes from - the single span set it measures all four buckets against, so this surface and the - collector are guaranteed to agree rather than merely observed to. The derivation - is the LEGACY path for a ``task.json`` written before that field existed. - - The stored value is checked with ``is not None``, never truthiness: a stored - ``0.0`` is a MEASUREMENT and must not fall through to a re-derivation. - - Rationale: .claude/notes/reporting.md § Read the stored value, do not re-derive it - """ - if turn.tool_union_ms is not None: - return turn.tool_union_ms - spans = main_thread_tool_spans(turn.messages, turn.commands) - return union_ms(spans) if spans else None - - -def collect_variant_series(result: ExperimentResult) -> dict[str, VariantSeries]: - """Per-variant (scores, durations, tokens, assistant-turns) series, keyed by variant id. - - Shared by the markdown and HTML reporters so both render the same numbers. - ``VariantResult.duration_seconds`` is *summed* across replicates, so it is - divided by ``replicate_count`` to give a per-run duration comparable across - variants that ran different replicate counts. - """ - series = {vid: VariantSeries([], [], [], []) for vid in result.variant_ids} - for ts in result.task_summaries: - for vr in ts.variant_results: - s = series.get(vr.variant_id) - if s is None: # a task result for a variant not in variant_ids - continue - # Only the SCORE is dropped when there is none, never the row. The - # series are consumed independently, so they need not be index-aligned; - # `paired_comparison` pairs across VARIANTS by task id. Mixed - # graded/ungraded experiments are real -- `run --resume` grades rows - # independently and folds a failed one back ungraded. - # Rationale: .claude/notes/reporting.md § The ungraded row in every surface - if vr.weighted_score is not None: - s.scores.append(vr.weighted_score) - s.durations.append(vr.duration_seconds / vr.replicate_count) - if vr.total_tokens is not None: - s.tokens.append(float(vr.total_tokens)) - if vr.total_assistant_turns is not None: - s.asst_turns.append(float(vr.total_assistant_turns)) - return series - - -class PairedComparison(NamedTuple): - """A 2-variant paired comparison over per-task mean scores. - - ``task_count`` is the number of tasks both variants scored. When it is < 2 - the statistics are all ``None`` — there is nothing to compare, and the - reporters say so rather than rendering an empty section. ``excluded_count`` - is the number of tasks that appeared for at least one variant but could not - be paired (missing or empty on the other side); the reporters surface it so - a silently narrowed sample is visible. - """ - - vid_a: str - vid_b: str - task_count: int - excluded_count: int - mean_diff: float | None - ci_low: float | None - ci_high: float | None - effect_size: float | None - p_value: float | None - - -def paired_comparison(result: ExperimentResult, confidence: float = 0.95) -> PairedComparison | None: - """Pair the two variants' per-task mean scores. Returns None unless the - experiment has exactly 2 variants with at least one commonly-scored task. - - The task is the unit of analysis: replicate slots within a task share the task - effect and are not independent, so pairing them individually would understate - the standard error. Replicate counts need not match — a task's mean score is a - well-defined pair member either way. - """ - if len(result.variant_ids) != 2: - return None - vid_a, vid_b = result.variant_ids[0], result.variant_ids[1] - per_rep_a = result.per_replicate_scores.get(vid_a, {}) - per_rep_b = result.per_replicate_scores.get(vid_b, {}) - common_tasks = sorted(t for t in set(per_rep_a) & set(per_rep_b) if per_rep_a[t] and per_rep_b[t]) - if not common_tasks: - # No shared task, or per_replicate_scores absent (results from before it existed). - return None - - # Tasks seen for at least one variant but not paired (missing or empty on the - # other side) — surfaced so a silently narrowed sample doesn't go unnoticed. - excluded_count = len(set(per_rep_a) | set(per_rep_b)) - len(common_tasks) - - if len(common_tasks) < 2: - return PairedComparison(vid_a, vid_b, len(common_tasks), excluded_count, None, None, None, None, None) - - a_scores = [mean(per_rep_a[task_id]) for task_id in common_tasks] - b_scores = [mean(per_rep_b[task_id]) for task_id in common_tasks] - ci = paired_t_ci(a_scores, b_scores, confidence=confidence) - if ci is None: # non-finite scores - return PairedComparison(vid_a, vid_b, len(common_tasks), excluded_count, None, None, None, None, None) - mean_diff, ci_low, ci_high = ci - return PairedComparison( - vid_a, - vid_b, - len(common_tasks), - excluded_count, - mean_diff, - ci_low, - ci_high, - cohens_d(a_scores, b_scores), - paired_t_test(a_scores, b_scores), - ) - - -# --------------------------------------------------------------------------- -# Prompt config + variant-result loaders -# --------------------------------------------------------------------------- - - -def has_final_reply(result: EvaluationResult) -> bool: - """True iff any iteration emitted a non-empty ResultMessage.result. - - Mirrors the evalboard rendering: a "final reply" is a text answer the - agent produced that becomes the trailing entry in the Turn timeline. - """ - for t in result.iterations: - if t.result_summary is not None: - r = t.result_summary.result - if isinstance(r, str) and r.strip(): - return True - return False - - -def visible_turn_count(result: EvaluationResult) -> int: - """Count of agent actions visible in the timeline so far. - - A "turn" here is one entry rendered in the Turn timeline: each tool - invocation contributes 1, plus 1 for the final assistant reply when - present. This is the canonical metric — distinct from the SDK's - ``num_turns`` which counts assistant *messages* and can bundle tool - use with trailing text into a single turn. - """ - commands = sum(len(t.commands) for t in result.iterations) - return commands + (1 if has_final_reply(result) else 0) - - -def expected_turns_overage(result: EvaluationResult) -> tuple[int, int] | None: - """Return ``(visible_turns, expected)`` when the visible-events turn - count strictly exceeds ``run_limits.expected_turns``; else ``None``. - - Safe against missing ``task_config``, missing ``run_limits``, and - non-int ``expected_turns`` values. - """ - task_cfg = result.task_config - if task_cfg is None: - return None - run_limits = (task_cfg.resolved or {}).get("run_limits") or {} - if not isinstance(run_limits, dict): - return None - expected = run_limits.get("expected_turns") - if not isinstance(expected, int) or expected < 1: - return None - actual = visible_turn_count(result) - if actual > expected: - return actual, expected - return None - - -def describe_prompt_config(variant: ExperimentVariant) -> str: - """Return a short description of the variant's prompt configuration. - - Returns strings like ``"(base prompt)"``, ``"(prompt override)"``, or - ``"(2 mutations: prefix, suffix)"``. - """ - if variant.initial_prompt is not None or variant.initial_prompt_file is not None: - return "(prompt override)" - if variant.prompt_mutations: - type_names = [m.type for m in variant.prompt_mutations] - return f"({len(type_names)} mutations: {', '.join(type_names)})" - return "(base prompt)" - - -def load_variant_eval_results( - run_dir: Path, variant_id: str, task_summaries: list[TaskExperimentSummary] -) -> list[EvaluationResult]: - """Load EvaluationResult objects for a variant from disk. - - Walks all ``///NN/task.json`` replicate - subdirs for each task in ``task_summaries`` and returns every result that - loads successfully. - """ - variant_dir = run_dir / variant_id - results: list[EvaluationResult] = [] - - if not variant_dir.is_dir(): - return results - - for ts in task_summaries: - task_dir = variant_dir / ts.task_id - if not task_dir.is_dir(): - continue - for rep_subdir in sorted(task_dir.glob("[0-9][0-9]")): - task_json = rep_subdir / TASK_JSON_FILENAME - if task_json.exists(): - try: - results.append(EvaluationResult.model_validate_json(task_json.read_text(encoding="utf-8"))) - except Exception: - logger.warning("Failed to load %s for variant report", task_json, exc_info=True) - - return results diff --git a/src/coder_eval/result_metrics.py b/src/coder_eval/result_metrics.py new file mode 100644 index 000000000..10be401ec --- /dev/null +++ b/src/coder_eval/result_metrics.py @@ -0,0 +1,175 @@ +"""Metrics derived from a finished ``EvaluationResult``. + +These are consumed by the orchestrator *during* a run as well as by the +reporters afterwards, which is why they do not live in a ``reports*`` module: +a metric the core layer needs is not a report, and CE066 enforces that +distinction. + +They are equally not part of ``timing.py``. That module operates on +``AssistantMessage`` / ``CommandTelemetry`` / ``TranscriptMessage`` and has no +``EvaluationResult`` dependency; adding one would widen the surface every agent +adapter imports. +""" + +from __future__ import annotations + +import math +from collections.abc import Iterable +from typing import NamedTuple + +from coder_eval.models import AssistantMessage, EvaluationResult, TurnRecord +from coder_eval.timing import main_thread_tool_spans, union_ms + + +class TurnTimeBuckets(NamedTuple): + """The four wall-clock buckets of a whole run, plus what they leave over. + + Each is ``None`` when NOTHING in the run measured it — a run recorded before + the head and tail were captured has no startup at all, a run that recorded + no bounded tool span has no tool total, and a run with no duration has no + residual. Rendering any of those as ``0ms`` claims a measurement nobody + took (CE058, and the reason the evalboard's ``sumMeasured`` returns + ``null``). A MEASURED zero stays ``0.0`` and renders as ``0ms``. + + DISPLAY AND ARITHMETIC DIFFER HERE, on purpose. An unmeasured bucket renders + as a dash and counts as ``0.0`` toward ``unaccounted``, so the missing time + surfaces as residual rather than vanishing. That is the rule + ``scripts/timing/decompose_run.py::_turn_buckets`` already applies, and + keeping the two the same is what lets a reader compare them. + """ + + startup_ms: float | None + generation_ms: float | None + tool_ms: float | None + teardown_ms: float | None + unaccounted_ms: float | None + + +def turn_time_buckets(result: EvaluationResult) -> TurnTimeBuckets: + """Sum the four timing buckets across a run's turns, and the residual. + + The arithmetic lives HERE rather than in the renderer because this module is + the designated home for shared report statistics: the evalboard, the + markdown report and the HTML report must not each grow their own version. + ``reports_html`` formats what this returns and decides nothing. + + ``unaccounted`` is measured against ``EvaluationResult.duration_seconds`` — + the TASK's wall clock, which is what the card's existing Total Latency uses + and what the evalboard's own Unaccounted cell uses. It therefore legitimately + contains sandbox setup and grading, and is LARGER than the per-turn residual + ``decompose_run.py`` reports. The two are not comparable and the label says + so. + """ + turns = result.iterations or [] + startup = _sum_measured(t.harness_startup_ms for t in turns) + teardown = _sum_measured(t.harness_teardown_ms for t in turns) + # MAIN THREAD ONLY, the same filter the collector and the evalboard apply: + # a sub-agent's generations bubble into the same stream, and the spawning + # Agent call's own interval already spans them. + generation = _sum_measured( + m.generation_duration_ms + for t in turns + for m in t.messages + if isinstance(m, AssistantMessage) and m.parent_tool_use_id is None + ) + # `None` only when NO turn recorded a bounded tool span. A turn that ran + # tools and timed none is indistinguishable from a turn that ran none, so + # the presence of a SPAN — not the presence of a turn — is what decides + # measured-versus-not. `_sum_measured` over a list of plain floats could + # never return None, which made this read `0ms` ("measured and instant") + # for a run nobody timed. + per_turn = [_turn_tool_union_ms(t) for t in turns] + tool = _sum_measured(per_turn) if any(ms is not None for ms in per_turn) else None + + # `duration_seconds` is a non-optional float defaulting to 0.0, so there is + # no None arm to write — but a 0.0 duration is a run that was never timed, + # and subtracting real buckets from it renders a fabricated negative + # residual. The evalboard keeps that null for the same reason; so do we. + unaccounted = ( + result.duration_seconds * 1000.0 - (startup or 0.0) - (generation or 0.0) - (tool or 0.0) - (teardown or 0.0) + if result.duration_seconds > 0.0 + else None + ) + return TurnTimeBuckets(startup, generation, tool, teardown, unaccounted) + + +def _sum_measured(values: Iterable[float | None]) -> float | None: + """Sum what was measured, or ``None`` when nothing was. + + The Python twin of the evalboard's ``sumMeasured``: a run with no measured + value anywhere returns ``None`` (never measured), while a run that measured + a genuine zero returns ``0.0``. + """ + total: float | None = None + for value in values: + if isinstance(value, (int, float)) and math.isfinite(value): + total = (total or 0.0) + value + return total + + +def _turn_tool_union_ms(turn: TurnRecord) -> float | None: + """One turn's tool execution — the UNION of its main-thread command spans. + + PREFERS THE STORED ``TurnRecord.tool_union_ms``, which the collector writes from + the single span set it measures all four buckets against, so this surface and the + collector are guaranteed to agree rather than merely observed to. The derivation + is the LEGACY path for a ``task.json`` written before that field existed. + + The stored value is checked with ``is not None``, never truthiness: a stored + ``0.0`` is a MEASUREMENT and must not fall through to a re-derivation. + + Rationale: .claude/notes/reporting.md § Read the stored value, do not re-derive it + """ + if turn.tool_union_ms is not None: + return turn.tool_union_ms + spans = main_thread_tool_spans(turn.messages, turn.commands) + return union_ms(spans) if spans else None + + +def has_final_reply(result: EvaluationResult) -> bool: + """True iff any iteration emitted a non-empty ResultMessage.result. + + Mirrors the evalboard rendering: a "final reply" is a text answer the + agent produced that becomes the trailing entry in the Turn timeline. + """ + for t in result.iterations: + if t.result_summary is not None: + r = t.result_summary.result + if isinstance(r, str) and r.strip(): + return True + return False + + +def visible_turn_count(result: EvaluationResult) -> int: + """Count of agent actions visible in the timeline so far. + + A "turn" here is one entry rendered in the Turn timeline: each tool + invocation contributes 1, plus 1 for the final assistant reply when + present. This is the canonical metric — distinct from the SDK's + ``num_turns`` which counts assistant *messages* and can bundle tool + use with trailing text into a single turn. + """ + commands = sum(len(t.commands) for t in result.iterations) + return commands + (1 if has_final_reply(result) else 0) + + +def expected_turns_overage(result: EvaluationResult) -> tuple[int, int] | None: + """Return ``(visible_turns, expected)`` when the visible-events turn + count strictly exceeds ``run_limits.expected_turns``; else ``None``. + + Safe against missing ``task_config``, missing ``run_limits``, and + non-int ``expected_turns`` values. + """ + task_cfg = result.task_config + if task_cfg is None: + return None + run_limits = (task_cfg.resolved or {}).get("run_limits") or {} + if not isinstance(run_limits, dict): + return None + expected = run_limits.get("expected_turns") + if not isinstance(expected, int) or expected < 1: + return None + actual = visible_turn_count(result) + if actual > expected: + return actual, expected + return None diff --git a/src/coder_eval/run_record.py b/src/coder_eval/run_record.py new file mode 100644 index 000000000..8f99719df --- /dev/null +++ b/src/coder_eval/run_record.py @@ -0,0 +1,198 @@ +"""The run.json task-row serializer. + +``eval_result_to_task_dict`` projects one finished ``EvaluationResult`` into the +row that lands in ``run.json``. It is a **run-record serializer, not a report**: +the batch runner writes these rows during a run, and the reporters read them +afterwards. Its previous home inside the experiment reporter was the only reason +``orchestration/batch.py`` imported from the reports layer at all. + +The keys this function writes are a contract — the evalboard and every archived +run read them — so changing one is a breaking change, not a rename. See +``docs/REPORT_SCHEMA.md``. +""" + +from __future__ import annotations + +from typing import Any + +from coder_eval.errors import truncate_crash_message +from coder_eval.models import EvaluationResult, FinalStatus, judge_cost_usd, simulator_cost_usd, sum_costs +from coder_eval.result_metrics import expected_turns_overage, turn_time_buckets, visible_turn_count +from coder_eval.result_metrics import has_final_reply as _has_final_reply + + +# Cap on the ``error_message`` carried into each run.json row: enough to identify +# a failure without fetching the task artifact, short enough that a wholly-errored +# run doesn't bloat run.json. The untruncated message stays on task.json. +_ROW_ERROR_MESSAGE_MAX_CHARS = 400 + + +def _cost_complete(result: EvaluationResult) -> bool: + """Whether this row's recorded agent spend accounts for everything it spent. + + False means the costs on the row are a floor, not the bill. Two ways in: + + 1. A turn burned tokens the rate card could not price. The card is the fallback + for anything the backend did not price itself, so with no rate those tokens + book no money. + 2. The task was hard-killed by the task-level timeout. Keyed on the status + rather than on emptiness: the watchdog fires while the evaluation loop is + running, so a TIMEOUT row always lost an in-flight turn, even one that + completed earlier turns that do carry costs. + + True for a row that burned nothing: an error before the agent ran genuinely + cost zero, and a slow setup failure is as free as a fast one. + """ + if result.final_status is FinalStatus.TIMEOUT: + return False + return all( + usage.total_cost_usd is not None + for t in result.iterations + if (usage := t.token_usage) is not None and not usage.is_empty() + ) + + +def eval_result_to_task_dict( + result: EvaluationResult, + *, + variant_id: str | None = None, + tags: list[str] | None = None, + task_path: str | None = None, + duration_override: float | None = None, + replicate_index: int | None = None, +) -> dict[str, Any]: + """Convert an EvaluationResult to the task_result dict format used by ReportGenerator. + + Args: + result: The evaluation result to convert. + variant_id: Optional variant ID to include in the dict. + tags: Optional tags list (defaults to []). + task_path: Optional path of the task YAML (as supplied to the runner) — + lets downstream consumers (evalboard) derive groupings like skill + from the source folder structure instead of guessing from tags. + duration_override: Optional duration value (defaults to result.duration_seconds). + replicate_index: Replicate index of this row (the ``//`` + sub-dir). Repeated runs of the same task share a ``task_id``, so + without this the row is indistinguishable from its siblings and + downstream consumers (evalboard) collapse them to one. ``None`` when + the caller doesn't track replicates (repeats disabled / legacy). + """ + + ref_similarity: float | None = None + for cr in result.success_criteria_results: + if cr.criterion_type == "reference_comparison": + ref_similarity = cr.score + break + + overage = expected_turns_overage(result) + + total_turns = sum((t.num_turns or 0) for t in result.iterations) + + # Carried as a row-level boolean so the evalboard can compute the visible turn + # count without re-reading per-task content. + has_reply = _has_final_reply(result) + + agent_cost = result.total_token_usage.total_cost_usd if result.total_token_usage else None + judge_cost = judge_cost_usd(result) + simulator_cost = simulator_cost_usd(result) + row_total_cost = sum_costs(agent_cost, judge_cost, simulator_cost) + + expected_turns_value: int | None = None + if result.task_config is not None: + rl = (result.task_config.resolved or {}).get("run_limits") or {} + if isinstance(rl, dict): + raw = rl.get("expected_turns") + if isinstance(raw, int) and raw >= 1: + expected_turns_value = raw + + _buckets = turn_time_buckets(result) + + d: dict[str, Any] = { + "task_id": result.task_id, + "replicate_index": replicate_index, + "status": result.final_status, + "weighted_score": result.weighted_score, + "duration": duration_override if duration_override is not None else result.duration_seconds, + "iteration_count": result.iteration_count, + "tags": tags if tags is not None else [], + "task_path": task_path, + "iterations": [ + { + "iteration": t.iteration, + "duration_seconds": t.duration_seconds, + "command_count": len(t.commands), + "assistant_turn_count": t.assistant_turn_count, + "crashed": t.crashed, + "crash_reason": t.crash_reason, + } + for t in result.iterations + ], + # Computed ONCE through `turn_time_buckets` and carried as TASK-level keys. + # `iterations` below is a deliberate 6-key projection, so no renderer can + # re-derive them. Each stays `float | None` (CE049). + # Rationale: .claude/notes/reporting.md § Read the stored value, do not re-derive it + "startup_ms": _buckets.startup_ms, + "generation_ms": _buckets.generation_ms, + "tool_ms": _buckets.tool_ms, + "teardown_ms": _buckets.teardown_ms, + "model_used": result.model_used, + "reference_similarity": ref_similarity, + # The UNCACHED slice, not TokenUsage.input_tokens; evalboard/lib/runs.ts + # depends on that reading and the run.json contract fixes the name. + "input_tokens": (result.total_token_usage.uncached_input_tokens if result.total_token_usage else None), + "output_tokens": (result.total_token_usage.output_tokens if result.total_token_usage else None), + "cache_creation_input_tokens": ( + result.total_token_usage.cache_creation_input_tokens if result.total_token_usage else None + ), + "cache_read_input_tokens": ( + result.total_token_usage.cache_read_input_tokens if result.total_token_usage else None + ), + "total_tokens": (result.total_token_usage.total_tokens if result.total_token_usage else None), + # agent + judge + simulator. `total_cost_usd` means the whole bill on every + # surface. None when nothing was priced at all. + "total_cost_usd": row_total_cost, + # Subject-agent spend alone: judge cost is identical across harnesses, so + # leaving it in would make two look closer than they are. + "agent_cost_usd": agent_cost, + # False when the agent spend above is missing money, so it is a floor. + # Rolled up as RunSummary.tasks_cost_incomplete / cost_complete. + "cost_complete": _cost_complete(result), + # The two halves of the eval-machinery bill, rolled up as + # RunSummary.eval_overhead_cost_usd. + "judge_cost_usd": judge_cost, + "simulator_cost_usd": simulator_cost, + # Errors count as misses, so the rollup has to say why it lost those points. + # Without these, triaging an errored run needs one task.json fetch per row. + "error_message": ( + truncate_crash_message(result.error_message, limit=_ROW_ERROR_MESSAGE_MAX_CHARS) + if result.error_message + else None + ), + "error_category": (result.error_details or {}).get("error_category"), + "expected_commands": result.expected_commands, + "actual_commands": result.actual_commands, + "commands_efficiency": result.commands_efficiency, + "agent_config": (result.agent_config.model_dump() if result.agent_config else None), + "sdk_options": result.sdk_options, + "installed_tools": result.environment_info.get("installed_tools"), + "max_turns_exhausted": result.max_turns_exhausted, + "expected_turns_overage": list(overage) if overage is not None else None, + "total_turns": total_turns, + # "Visible turns" (tool calls + final reply) -- what the "within expected + # turns" metric compares against. Distinct from total_turns (SDK num_turns). + "visible_turns": visible_turn_count(result), + "expected_turns": expected_turns_value, + "has_final_reply": has_reply, + # None/False on the default path, so downstream analysis never confuses a + # truncated run with a full one. + "stopped_early": result.early_stop is not None, + "early_stop_reason": (result.early_stop.reason.value if result.early_stop is not None else None), + "turns_remaining_at_stop": ( + result.early_stop.turns_remaining_at_stop if result.early_stop is not None else None + ), + # The threshold in effect for this stop, so a sweep that varies it can tell + # which weighted-gate value produced a given verdict. + "gate_threshold": (result.early_stop.gate_threshold if result.early_stop is not None else None), + } + d["variant_id"] = variant_id + return d diff --git a/src/coder_eval/stats.py b/src/coder_eval/stats.py new file mode 100644 index 000000000..ac5d363fd --- /dev/null +++ b/src/coder_eval/stats.py @@ -0,0 +1,266 @@ +"""Distribution-free statistical helpers for the report renderers. + +This module must stay **dependency-free** — stdlib only, no ``coder_eval`` +import, direct or relative. That is what lets the numeric core be tested and +reasoned about in isolation, and it is asserted by a test rather than left to +convention (``tests/test_stats.py``). + +Display formatting deliberately lives elsewhere: ``fmt_mean_sd`` and ``fmt_p`` +return ``"N/A"``, ``"—"`` and ``"<0.001"``, which is presentation, not +computation, so they sit with the other report formatters. +""" + +from __future__ import annotations + +import logging +import math +import random +import statistics as _stats + + +logger = logging.getLogger(__name__) + + +def mean(values: list[float]) -> float: + return _stats.mean(values) if values else 0.0 + + +def stddev(values: list[float]) -> float: + """Sample standard deviation (Bessel-corrected). Returns 0.0 for n < 2.""" + return _stats.stdev(values) if len(values) >= 2 else 0.0 + + +def _betacf(a: float, b: float, x: float) -> float: + """Continued fraction for the regularized incomplete beta (Lentz's method).""" + max_iterations = 200 + eps = 3e-12 + fpmin = 1e-300 + + qab, qap, qam = a + b, a + 1.0, a - 1.0 + c = 1.0 + d = 1.0 - qab * x / qap + if abs(d) < fpmin: + d = fpmin + d = 1.0 / d + h = d + for m in range(1, max_iterations + 1): + m2 = 2 * m + # Even step of the recurrence. + aa = m * (b - m) * x / ((qam + m2) * (a + m2)) + d = 1.0 + aa * d + if abs(d) < fpmin: + d = fpmin + c = 1.0 + aa / c + if abs(c) < fpmin: + c = fpmin + d = 1.0 / d + h *= d * c + # Odd step. + aa = -(a + m) * (qab + m) * x / ((a + m2) * (qap + m2)) + d = 1.0 + aa * d + if abs(d) < fpmin: + d = fpmin + c = 1.0 + aa / c + if abs(c) < fpmin: + c = fpmin + d = 1.0 / d + delta = d * c + h *= delta + if abs(delta - 1.0) < eps: + return h + logger.warning("Incomplete beta continued fraction did not converge for a=%r, b=%r, x=%r", a, b, x) + return h + + +def regularized_incomplete_beta(a: float, b: float, x: float) -> float: + """Regularized incomplete beta function I_x(a, b), for a, b > 0 and x in [0, 1]. + + Raises ValueError outside that domain — returning NaN would let a bad input + render as a real-looking statistic downstream. + """ + if not (math.isfinite(a) and math.isfinite(b) and math.isfinite(x)): + raise ValueError(f"a, b and x must be finite, got a={a!r}, b={b!r}, x={x!r}") + if a <= 0.0 or b <= 0.0: + raise ValueError(f"a and b must be positive, got a={a!r}, b={b!r}") + if x <= 0.0: + return 0.0 + if x >= 1.0: + return 1.0 + ln_front = math.lgamma(a + b) - math.lgamma(a) - math.lgamma(b) + a * math.log(x) + b * math.log1p(-x) + front = math.exp(ln_front) + # Use the continued fraction directly where it converges fast, else via symmetry. + if x < (a + 1.0) / (a + b + 2.0): + return front * _betacf(a, b, x) / a + return 1.0 - front * _betacf(b, a, 1.0 - x) / b + + +def student_t_two_tailed_p(t_stat: float, df: float) -> float: + """Exact two-tailed p-value for Student's t: P(|T| >= |t|) = I_x(df/2, 1/2), x = df/(df + t^2). + + Non-finite inputs fail closed to 1.0 — garbage must never read as significant. + """ + if not math.isfinite(t_stat) or not math.isfinite(df) or df <= 0: + return 1.0 + x = df / (df + t_stat * t_stat) + return regularized_incomplete_beta(df / 2.0, 0.5, x) + + +def welch_t_test(a: list[float], b: list[float]) -> float | None: + """Two-tailed p-value from Welch's unequal-variances t-test (exact t distribution). + + Degrees of freedom via Welch-Satterthwaite; the t CDF is evaluated exactly + through the regularized incomplete beta (stdlib only, no scipy). Returns + None if either group has fewer than 2 observations, or holds a non-finite + value (rendered as "—" rather than a fabricated p-value). + """ + n_a, n_b = len(a), len(b) + if n_a < 2 or n_b < 2: + return None + if not all(math.isfinite(v) for v in (*a, *b)): + return None + + mean_a, mean_b = _stats.mean(a), _stats.mean(b) + var_a = _stats.variance(a) + var_b = _stats.variance(b) + + se_sq = var_a / n_a + var_b / n_b + if se_sq == 0: + # Zero variance in both groups: identical constants (p=1) or a + # deterministic difference (p=0). + return 1.0 if mean_a == mean_b else 0.0 + + t_stat = abs(mean_a - mean_b) / math.sqrt(se_sq) + df = se_sq**2 / ((var_a / n_a) ** 2 / (n_a - 1) + (var_b / n_b) ** 2 / (n_b - 1)) + return student_t_two_tailed_p(t_stat, df) + + +def bootstrap_mean_ci( + values: list[float], + n_resamples: int = 1000, + confidence: float = 0.95, + seed: int = 0, +) -> tuple[float, float, float]: + """Percentile-bootstrap confidence interval for the mean. + + Returns (mean, ci_low, ci_high). When ``len(values) < 2``, returns + (values[0], values[0], values[0]) or (0, 0, 0) for empty input. + Uses ``random.Random(seed)`` for determinism. + + Raises ValueError for a ``confidence`` outside (0, 1) or a non-positive + ``n_resamples`` — clamping those would quietly return an interval of the + wrong width, which is worse than refusing. + """ + if not 0.0 < confidence < 1.0: + raise ValueError(f"confidence must be in (0, 1), got {confidence!r}") + if n_resamples < 1: + raise ValueError(f"n_resamples must be >= 1, got {n_resamples!r}") + if not values: + return (0.0, 0.0, 0.0) + m = sum(values) / len(values) + if len(values) < 2: + return (m, m, m) + rng = random.Random(seed) + n = len(values) + resampled_means = sorted(sum(rng.choice(values) for _ in range(n)) / n for _ in range(n_resamples)) + alpha = (1.0 - confidence) / 2.0 + lo = resampled_means[int(alpha * n_resamples)] + hi = resampled_means[int((1.0 - alpha) * n_resamples) - 1] + return (m, lo, hi) + + +def wilson_interval(successes: int, n: int, confidence: float = 0.95) -> tuple[float, float]: + """Wilson score interval for a binomial proportion. Returns (low, high). + + More reliable than the normal approximation at small N and near 0/1. + """ + if n <= 0: + return (0.0, 0.0) + z = _stats.NormalDist().inv_cdf((1.0 + confidence) / 2.0) + p_hat = successes / n + denom = 1.0 + z * z / n + center = (p_hat + z * z / (2.0 * n)) / denom + half = (z * math.sqrt(p_hat * (1.0 - p_hat) / n + z * z / (4.0 * n * n))) / denom + return (max(0.0, center - half), min(1.0, center + half)) + + +def cohens_d(a: list[float], b: list[float]) -> float | None: + """Paired Cohen's d = mean(a_i - b_i) / stddev(a_i - b_i).""" + if len(a) != len(b) or len(a) < 2: + return None + diffs = [ai - bi for ai, bi in zip(a, b, strict=True)] + s = stddev(diffs) + return (sum(diffs) / len(diffs)) / s if s > 0 else None + + +def student_t_critical(confidence: float, df: float) -> float: + """Two-tailed critical value t* with P(|T| >= t*) = 1 - confidence. + + Inverts :func:`student_t_two_tailed_p` by bisection — that p is continuous and + strictly decreasing in |t|, so a plain bracket-and-halve is exact to ~1e-12 and + needs no separate quantile expansion. + """ + if not 0.0 < confidence < 1.0: + raise ValueError(f"confidence must be in (0, 1), got {confidence!r}") + if df <= 0 or not math.isfinite(df): + return math.inf + alpha = 1.0 - confidence + lo, hi = 0.0, 1.0 + while student_t_two_tailed_p(hi, df) > alpha: + lo = hi + hi *= 2.0 + if hi > 1e12: + # Only reachable for a confidence so close to 1 that t* overflows the + # bracket. Warn rather than return a silently wrong-width interval. + logger.warning( + "student_t_critical failed to bracket t* for confidence=%r, df=%r; returning a degraded upper bound", + confidence, + df, + ) + return hi + for _ in range(200): + mid = (lo + hi) / 2.0 + if mid in (lo, hi): + break + if student_t_two_tailed_p(mid, df) > alpha: + lo = mid + else: + hi = mid + return (lo + hi) / 2.0 + + +def paired_t_ci(a: list[float], b: list[float], confidence: float = 0.95) -> tuple[float, float, float] | None: + """Student-t confidence interval for mean(a_i - b_i): mean ± t* · sd/√n. + + Returns (mean_diff, ci_low, ci_high), or None if lengths differ, n < 2, or any + value is non-finite. Shares its distribution with :func:`paired_t_test`, so the + interval and the p-value always agree about whether 0 is excluded. + """ + if len(a) != len(b) or len(a) < 2: + return None + if not all(math.isfinite(v) for v in (*a, *b)): + return None + diffs = [ai - bi for ai, bi in zip(a, b, strict=True)] + n = len(diffs) + mean_diff = sum(diffs) / n + half_width = student_t_critical(confidence, n - 1) * stddev(diffs) / math.sqrt(n) + return (mean_diff, mean_diff - half_width, mean_diff + half_width) + + +def paired_t_test(a: list[float], b: list[float]) -> float | None: + """Two-tailed p-value from a paired t-test on (a_i - b_i), exact t distribution. + + Equivalent to a one-sample t-test of the differences against 0, df = n - 1. + Returns None if lengths differ, n < 2, or any value is non-finite. + """ + if len(a) != len(b) or len(a) < 2: + return None + if not all(math.isfinite(v) for v in (*a, *b)): + return None + diffs = [ai - bi for ai, bi in zip(a, b, strict=True)] + sd = stddev(diffs) + mean_diff = sum(diffs) / len(diffs) + if sd == 0: + # All diffs identical: no difference (p=1) or a deterministic shift (p=0). + return 1.0 if mean_diff == 0 else 0.0 + t_stat = abs(mean_diff) / (sd / math.sqrt(len(diffs))) + return student_t_two_tailed_p(t_stat, len(diffs) - 1) diff --git a/src/coder_eval/streaming/collector.py b/src/coder_eval/streaming/collector.py index 3e516d293..61b033006 100644 --- a/src/coder_eval/streaming/collector.py +++ b/src/coder_eval/streaming/collector.py @@ -85,7 +85,7 @@ def on_event(self, event: StreamEvent) -> None: def visible_turn_count(self) -> int: """Visible timeline entries observed so far — one per resolved tool call. - The live, in-stream counterpart of ``reports_stats.visible_turn_count``, + The live, in-stream counterpart of ``result_metrics.visible_turn_count``, which counts the very same list once the turn is a finished ``TurnRecord`` (minus its trailing final-reply entry, which cannot exist while the turn is still running). diff --git a/src/coder_eval/timing.py b/src/coder_eval/timing.py index 71eded408..25c7c77fb 100644 --- a/src/coder_eval/timing.py +++ b/src/coder_eval/timing.py @@ -201,7 +201,7 @@ def main_thread_tool_spans( The span set the generation subtraction, the head and the tail are all measured against, so they cannot disagree about which calls exist. Shared - with ``reports_stats.turn_time_buckets``; + with ``result_metrics.turn_time_buckets``; ``tests/test_timing_close_window.py::TestTheThreeToolUnionsAgree`` pins this, that, and ``scripts/timing/decompose_run.py`` together. diff --git a/tests/lint/pricing_mirror.py b/tests/lint/pricing_mirror.py new file mode 100644 index 000000000..430799b77 --- /dev/null +++ b/tests/lint/pricing_mirror.py @@ -0,0 +1,165 @@ +"""CE065 — the evalboard's rate table is generated from ``coder_eval.pricing``. + +``evalboard/lib/pricing.ts`` used to carry a hand-copied mirror of the Python +rate card. Keeping a hand-copy honest needed five layers of bookkeeping: a +regex parser that re-read ``pricing.py`` at test time, a meta-guard against that +regex silently narrowing, a ``DELIBERATELY_UNMIRRORED`` exemption set, a +staleness guard for the exemption set, and a comment begging the next reader to +keep the set honest. It still shipped a real bug — ``claude-sonnet-5``, +``gpt-5.6-sol``, ``gpt-5.6-terra`` and ``gpt-5.6-luna`` sat in the exemption set +under "the evalboard never runs them" while appearing tens of thousands of times +in the run corpus, so every one of those runs rendered "—" for cost with nothing +failing. + +If a *test* can read the table, a *generator* can emit it. So the table is no +longer copied: ``render_pricing()`` renders +``evalboard/lib/pricing.generated.ts`` from ``pricing.builtin_rates()``, ``make +pricing-mirror`` calls ``write()``, and CE065 (``check()``) re-renders and diffs +against disk. There is deliberately **no ``--check`` mode and no arg parser** — +CE065 *is* the checker, the same rule ``plugin_reference.py`` states. + +The exemption set encoded TWO different things, and they survive differently. +That three OpenRouter models must stay unpriced so ``runs.ts``'s apportionment of +the provider's real bill still fires is a property of the RATE, so it is now data +on the rate itself (``ModelPricing.per_request_billing``), beside the rate it +qualifies; nothing has to remember it. That four heavy frontier variants are not +priced on the frontend is a property of the FRONTEND, not of the rate, so it +stays here as ``DELIBERATELY_UNMIRRORED`` — an explicit list with the same +stale-membership guard the deleted test carried, because an exemption nobody +re-reads is what shipped the bug above. + +Like CE028 and CE033 this is not a ``BaseRule`` in the AST runner: it reasons +over generated text rather than one Python AST, so it is wired as a +``@pytest.mark.lint`` class in ``tests/test_custom_lint.py``. +""" + +from __future__ import annotations + +import json +import math +from collections.abc import Mapping +from pathlib import Path + +from coder_eval.pricing import ModelPricing, builtin_rates +from tests.lint.generated import diff_all, write_all + + +_MIRROR_REL = "evalboard/lib/pricing.generated.ts" +_GENERATED_HEADER = "// generated by `make pricing-mirror` — do not edit" + +# Priced in Python, deliberately NOT priced on the frontend: heavy frontier variants +# no harness runs, so a rate here buys nothing. Unlike ``per_request_billing`` this is +# not a fact about the rate — the rate is correct, the evalboard just has no use for +# it — which is why it lives beside the generator rather than on ``ModelPricing``. +# +# KEEP THIS SET HONEST. Membership silences the mirror for one id indefinitely, so a +# stale entry hides a live bug rather than a non-issue: `claude-sonnet-5`, +# `gpt-5.6-sol`, `gpt-5.6-terra` and `gpt-5.6-luna` sat in the predecessor of this set +# under "the evalboard never runs them" while appearing tens of thousands of times in +# the run corpus, so every one of those runs rendered "—" for cost with nothing +# failing. Before adding an id, grep the corpus for it — absence from run data is the +# ONLY justification, and it expires the moment a harness adopts the model. +# ``_assert_exemptions_are_live`` fails the build once an id leaves ``pricing.py``. +DELIBERATELY_UNMIRRORED = frozenset( + { + "gpt-5.4-mini", + "gpt-5.4-nano", + "gpt-5.4-pro", + "gpt-5.5-pro", + } +) + + +def _number(key: str, field: str, value: float) -> str: + """A TS numeric literal for one rate, full precision preserved. + + ``repr`` round-trips every float exactly, so 0.085898 survives verbatim. It + also emits ``inf``/``nan`` for non-finite values, which are not valid TS + numeric literals — those fail here rather than producing a file that only + breaks once ``tsc`` runs. + """ + if not math.isfinite(value): + raise ValueError(f"non-finite rate for {key!r}: {field}={value!r} cannot be rendered as TypeScript") + return repr(float(value)) + + +def _assert_exemptions_are_live(rates: Mapping[str, ModelPricing]) -> None: + """Every ``DELIBERATELY_UNMIRRORED`` id must still be priced in ``pricing.py``. + + An exemption silences the mirror for one id indefinitely. Once the id leaves + the rate card the entry silences nothing and only survives to be copied, so + making that a build failure is what forces the set to be re-read rather than + appended to. Carried over from the deleted parity test, which had it right. + """ + stale = sorted(DELIBERATELY_UNMIRRORED - rates.keys()) + if stale: + raise ValueError( + f"DELIBERATELY_UNMIRRORED names ids no longer priced in pricing.py: {', '.join(stale)} — drop them" + ) + + +def render_pricing(rates: Mapping[str, ModelPricing] | None = None) -> str: + """Render the generated TS rate table from a Python rate card. + + Two exemption axes, kept separate because they are different claims. A + ``per_request_billing`` rate must not be mirrored (a static estimate would + replace the captured actual per-call cost); a ``DELIBERATELY_UNMIRRORED`` id + simply is not worth pricing on the frontend. + """ + rates = builtin_rates() if rates is None else rates + priced = {k: v for k, v in sorted(rates.items()) if not v.per_request_billing and k not in DELIBERATELY_UNMIRRORED} + routed = sorted(k for k, v in rates.items() if v.per_request_billing) + unmirrored = sorted(DELIBERATELY_UNMIRRORED & rates.keys()) + + lines = [ + _GENERATED_HEADER, + "// Source: src/coder_eval/pricing.py — regenerate with `make pricing-mirror`.", + "", + 'import type { Pricing } from "./pricing";', + "", + ] + if routed: + lines += [ + "// Omitted (per_request_billing): the provider bills per request, so a static", + "// rate here would replace the captured ACTUAL per-call cost with an estimate.", + f"// {', '.join(routed)}", + ] + if unmirrored: + lines += [ + "// Omitted (DELIBERATELY_UNMIRRORED in tests/lint/pricing_mirror.py): priced in", + "// Python for the max_usd pre-flight, but no harness runs them on this board.", + f"// {', '.join(unmirrored)}", + ] + lines.append("export const PRICING: Record = {") + for key, rate in priced.items(): + lines.append( + f" {json.dumps(key)}: {{ " + f"inputPerMTok: {_number(key, 'input_per_mtok', rate.input_per_mtok)}, " + f"outputPerMTok: {_number(key, 'output_per_mtok', rate.output_per_mtok)}, " + f"cacheWritePerMTok: {_number(key, 'cache_write_per_mtok', rate.cache_write_per_mtok)}, " + f"cacheReadPerMTok: {_number(key, 'cache_read_per_mtok', rate.cache_read_per_mtok)} }}," + ) + lines.append("};") + return "\n".join(lines) + "\n" + + +def _rendered_files(repo_root: Path) -> dict[Path, str]: + """The full intended content of each generated file, keyed by path.""" + _assert_exemptions_are_live(builtin_rates()) + return {repo_root / _MIRROR_REL: render_pricing()} + + +def write(repo_root: Path) -> list[Path]: + """Regenerate the evalboard's rate table in place. Returns the target paths.""" + return write_all(_rendered_files(repo_root)) + + +def check(repo_root: Path) -> dict[str, str]: + """Unified diff per file whose generated content differs from disk (empty = clean).""" + return diff_all(_rendered_files(repo_root)) + + +if __name__ == "__main__": + root = Path(__file__).resolve().parents[2] + for p in write(root): + print(f"wrote {p}") diff --git a/tests/lint/rules/_layers.py b/tests/lint/rules/_layers.py new file mode 100644 index 000000000..034336775 --- /dev/null +++ b/tests/lint/rules/_layers.py @@ -0,0 +1,142 @@ +"""The package-layer predicates, declared once and shared by CE004 and CE066. + +Both rules ask where a file sits in ``src/coder_eval/``, and a second copy of +the answer is how a package added to one regex silently escapes the other. So +the package anchor and the ``cli/`` boundary are each spelled once, here. +``_model_ctor.py`` is the in-tree precedent for a ``_``-prefixed shared rule +helper. + +The two rules do NOT share an exemption set, because they do not ask the same +question. CE004 bans ``cli`` imports from everything that must run without the +CLI, which is the whole package except ``cli/`` itself. CE066 bans reaching into +the reports layer, which ``reports/`` may obviously do to itself, so its "core" +also excludes ``reports/``. When CE004 borrowed CE066's predicate wholesale it +inherited the ``reports/`` exemption, and a ``cli`` import added inside the +reports package — which the orchestrator imports mid-run, closing a +cli -> orchestration -> reports -> cli cycle — would have passed silently. + +Both scopes are ALLOWLISTS of what is exempt, so a new subpackage is in scope by +default rather than exempt until someone notices. The denylist form is what +leaves holes, twice over. An earlier draft named only +``orchestrator.py`` as the top-level core module, which exempted +``result_metrics.py`` — the very module CE066's fix message tells a violator to +move their metric into — along with ``run_record.py``, ``stats.py`` and +``timing.py``. Its successor listed ten core directories and ``isolation/`` was +not one of them, so ``isolation/docker_runner.py`` — the ``driver: docker`` +evaluation path, which imports ``models``, ``orchestration`` and ``streaming`` — +could import anything with both rules silent. + +A relative import is RESOLVED against the importing file rather than pattern- +matched: ``from .reports import x`` means ``coder_eval.reports`` in a top-level +module and ``coder_eval.orchestration.reports`` inside ``orchestration/``, so the +dots have to be counted against the file's own package. See ``_absolute_module``. + +The package regex is anchored on ``src/`` because the unanchored form made a +repo-root file core: this project's own checkout directory is named +``coder_eval``, so ``…/coder_eval/conftest.py`` matched the package. + +Blind spot: anchoring narrows that trap without closing it. A clone whose parent +directory is literally named ``src`` — ``~/src/coder_eval/conftest.py`` — still +matches, and so now does that clone's ``tests/`` tree. No path substring can +separate the package from a checkout laid out like it; closing it properly means +relativising every rule's path against the repo root. It is unreachable today: +CE004 and CE066 are only ever handed paths under the runner's ``SRC``, never the +repo root, and ``_ALSO_SCAN_TESTS`` is ``{"CE048"}``, which uses neither +predicates. ``TestCoreLayerMembership`` pins the residual so nobody reads the +anchoring as a complete fix. +""" + +import ast +import re + + +_PKG = re.compile(r"(?:^|[/\\])src[/\\]coder_eval[/\\]") +_CLI = re.compile(_PKG.pattern + r"cli[/\\]") +_REPORTS = re.compile(_PKG.pattern + r"reports[/\\]") + + +def is_package_path(filepath: str) -> bool: + """Whether ``filepath`` is anywhere under ``src/coder_eval/``.""" + return bool(_PKG.search(filepath)) + + +def is_cli_path(filepath: str) -> bool: + """Whether ``filepath`` is inside the ``cli/`` package.""" + return bool(_CLI.search(filepath)) + + +def is_core_path(filepath: str) -> bool: + """Whether ``filepath`` is in CE066's core: the package minus ``cli/`` and ``reports/``.""" + return is_package_path(filepath) and not is_cli_path(filepath) and not _REPORTS.search(filepath) + + +def _containing_package(filepath: str) -> list[str] | None: + """The dotted parts of the package a module lives in, rooted at ``coder_eval``. + + ``reports/html.py`` and ``reports/__init__.py`` both answer + ``["coder_eval", "reports"]`` — Python resolves a package's ``__init__`` against + the package itself, not its parent, so dropping the filename is right for both. + """ + match = _PKG.search(filepath) + if not match: + return None + parts = [p for p in filepath[match.end() :].replace("\\", "/").split("/") if p] + return ["coder_eval", *parts[:-1]] + + +def _absolute_module(node: ast.ImportFrom, filepath: str) -> str | None: + """The fully-qualified module a ``from … import …`` names, or None if it escapes. + + A relative import only means ``coder_eval.`` at ONE depth, and which + depth depends on where the importing file sits. An earlier form compared + ``node.module`` against the bare package name whenever ``node.level`` was + non-zero, which reads ``from .reports import x`` inside ``orchestration/`` — + i.e. ``coder_eval.orchestration.reports`` — as the reports layer. Nothing in + the tree is nested that way today, so it was a latent false positive rather + than a live one; resolving the dots against the file removes the class. + """ + if not node.level: + return node.module + package = _containing_package(filepath) + if package is None: + return None + base = package[: len(package) - (node.level - 1)] + if not base: + return None + return ".".join([*base, node.module]) if node.module else ".".join(base) + + +def imports_package(node: ast.ImportFrom, package: str, filepath: str) -> bool: + """Whether a ``from … import …`` names ``coder_eval.``, EITHER spelling. + + This exists because matching ``node.module`` alone is a trap both layering + rules fell into. A relative import keeps its dots in ``node.level`` and leaves + the rest in ``node.module``, so ``from ..cli import x`` arrives as + ``level=2, module="cli"`` — and the relative form is this codebase's dominant + idiom, so a rule that checks only the absolute path fires on almost nothing + while its tests pass. CE066 shipped that way for one review cycle; CE004 had + carried it since it was written. + + ``from . import cli`` and ``from coder_eval import cli`` are NOT matched here — + they bind the package itself rather than a name out of it, so each rule reports + them through ``is_bare_package_import`` as its own wholesale case. + """ + module = _absolute_module(node, filepath) + if module is None: + return False + full = f"coder_eval.{package}" + return module == full or module.startswith(f"{full}.") + + +def is_bare_package_import(node: ast.ImportFrom, package: str, filepath: str) -> bool: + """Whether this binds the package itself rather than a name out of it. + + Three spellings do that: ``from . import reports``, ``from .. import reports`` + and ``from coder_eval import reports``. The last is the one both rules missed + longest — it is neither relative nor a dotted path, so `node.module` is the + bare ``"coder_eval"`` and the package arrives as an alias. + + Worth catching because the binding is the dangerous half: once `reports` is a + local name, every attribute read through it is invisible to an import check. + """ + return _absolute_module(node, filepath) == "coder_eval" and any(a.name == package for a in node.names) diff --git a/tests/lint/rules/ce053_run_record_filename_literal.py b/tests/lint/rules/ce053_run_record_filename_literal.py index 93f9b8dcb..6727f592f 100644 --- a/tests/lint/rules/ce053_run_record_filename_literal.py +++ b/tests/lint/rules/ce053_run_record_filename_literal.py @@ -8,8 +8,8 @@ The constant shipped with that rationale and the twelve pre-existing literals were not converted, so it created exactly the second source of truth it argues against and delivered zero rename safety: the new modules used the constant, and -``orchestrator.py``, ``batch.py``, ``docker_runner.py``, ``reports.py``, -``reports_junit.py``, ``reports_stats.py`` and ``report_command.py`` kept the +``orchestrator.py``, ``batch.py``, ``docker_runner.py``, ``reports/markdown.py``, +``reports/junit.py``, ``reports/helpers.py`` and ``report_command.py`` kept the string — the three ``rglob("task.json")`` calls the comment specifically cites among them. diff --git a/tests/lint/rules/ce066_no_report_imports_in_core.py b/tests/lint/rules/ce066_no_report_imports_in_core.py new file mode 100644 index 000000000..e740cb986 --- /dev/null +++ b/tests/lint/rules/ce066_no_report_imports_in_core.py @@ -0,0 +1,106 @@ +"""CE066: core may import only the reports package's public WRITERS. + +The invariant is not "core must not import reports" — core legitimately *writes* +reports: ``orchestrator.py`` writes the per-task HTML and ``orchestration/batch.py`` +drives ``ReportGenerator``. What must not happen is core reaching into the reports +layer for a **metric, a statistic, a serializer or a formatter**, because that is +how a number the evaluation loop needs comes to live in a rendering module. + +Before the split that was the actual shape of the code: the orchestrator imported +``turn_time_buckets`` and ``visible_turn_count`` from ``reports_stats``, and +``orchestration/batch.py`` imported the run.json row serializer from +``reports_experiment``. Those names now live in ``result_metrics.py``, ``stats.py`` +and ``run_record.py``, and this rule is what stops the next one drifting back. + +An ALLOWLIST, not a denylist — the CE018 rationale. A newly added report helper is +banned from core by default rather than after someone notices. The list is purely +writers; ``eval_result_to_task_dict`` is deliberately absent, because carrying a +serializer on it would be the rule documenting a wart instead of the wart being +removed. + +Both the ABSOLUTE and the RELATIVE spelling are checked. That is not a detail: +the relative form is the local idiom — both surviving edges in the tree are +``from .reports import write_task_html`` (orchestrator.py) and ``from ..reports +import ReportGenerator`` (orchestration/batch.py) — and an earlier draft of this +rule matched only ``node.module``, which for a relative import holds +``"reports"`` with the dots in ``node.level``. It therefore fired on nothing the +codebase actually writes, and its own tests passed because they used the +absolute form. An unrun assertion is documentation, not enforcement. + +**Blind spot, stated deliberately:** the rule checks the imported NAME, not what +is done with it. ``from coder_eval.reports import ReportGenerator`` followed by +reaching through the class for a private helper is invisible here. That is the +cheap version, consistent with CE004's own "catches the one mistake we have +actually seen" note. +""" + +import ast + +from tests.lint.rules._layers import imports_package, is_bare_package_import, is_core_path +from tests.lint.rules.base import BaseRule + + +# The reports package's public writer entry points — the only names core may import. +ALLOWED_WRITERS = frozenset( + { + "ExperimentReportGenerator", + "ReportGenerator", + "generate_junit_xml", + "write_experiment_html", + "write_junit_xml", + "write_suite_rollups", + "write_task_html", + "write_variant_html", + } +) + +_FIX = ( + "move the metric to result_metrics.py or the statistic to stats.py — " + "core may import only the reports package's public writers" +) + + +class NoReportImportsInCore(BaseRule): + id = "CE066" + + def __init__(self, filepath: str) -> None: + super().__init__(filepath) + self._in_core = is_core_path(filepath) + + def visit_ImportFrom(self, node: ast.ImportFrom) -> None: + if not self._in_core: + self.generic_visit(node) + return + # `from . import reports`, `from .. import reports`, `from coder_eval import + # reports`: the package arrives as an alias, so there is no imported NAME to + # check and every attribute read through it is invisible. + if is_bare_package_import(node, "reports", self.filepath): + for alias in node.names: + if alias.name == "reports": + self.violation( + node, + f"architectural violation: '{alias.name}' (reports layer) imported wholesale " + f"into core — import the specific writer instead, or {_FIX}", + ) + elif imports_package(node, "reports", self.filepath): + for alias in node.names: + if alias.name not in ALLOWED_WRITERS: + self.violation( + node, + f"architectural violation: '{alias.name}' imported from " + f"'{'.' * node.level}{node.module}' (reports layer) into core — {_FIX}", + ) + self.generic_visit(node) + + def visit_Import(self, node: ast.Import) -> None: + # `import coder_eval.reports` binds the whole module, so there is no name + # to check and every attribute access through it is invisible. + if self._in_core: + for alias in node.names: + if alias.name == "coder_eval.reports" or alias.name.startswith("coder_eval.reports."): + self.violation( + node, + f"architectural violation: '{alias.name}' (reports layer) imported wholesale " + f"into core — import the specific writer instead, or {_FIX}", + ) + self.generic_visit(node) diff --git a/tests/lint/rules/no_cli_imports_in_core.py b/tests/lint/rules/no_cli_imports_in_core.py index f09f4018a..b4f7ba1d2 100644 --- a/tests/lint/rules/no_cli_imports_in_core.py +++ b/tests/lint/rules/no_cli_imports_in_core.py @@ -1,15 +1,26 @@ """CE004: core layers must not import from coder_eval.cli. -The "core" layer comprises every package that should be usable without the -CLI: criteria/, evaluation/, models/, simulation/, scoring/, streaming/, -errors/, orchestration/, agents/, harbor/. Importing from coder_eval.cli -creates an upward dependency that breaks testability in isolation. - -``harbor/`` joined this list for the same reason ``orchestration/`` is on it: +The rule's scope is everything under src/coder_eval/ except the cli/ package +itself. Importing from coder_eval.cli creates an upward dependency that breaks +testability in isolation. The package anchor and the cli/ boundary live in +``_layers`` so CE004 and CE066 cannot drift apart about where either is; +re-enumerating the packages here is how that list rots. + +``reports/`` is in scope, unlike under CE066. The reports package runs without +the CLI — the orchestrator writes a task report mid-run — so a ``cli`` import +there closes a cli -> orchestration -> reports -> cli cycle. CE004 once borrowed +CE066's core predicate whole and inherited its ``reports/`` exemption; nothing +had imported ``cli`` from there yet, so the hole was latent rather than live. + +``harbor/`` is in scope for the same reason ``orchestration/`` is: its reward writer wants to raise a plain exception (``RewardWriteSkippedError``, or the re-exported ``RegradeError``) and let the CLI wrap it into an exit code — exactly the ``orchestration/regrade.py`` -> ``evaluate`` shape. +Both the absolute and the RELATIVE spelling are checked — see +``_layers.imports_package`` for why that distinction is load-bearing rather than +pedantic. + Note: this is a single, narrow rule (no upward imports into cli). For a fully layered import graph (no upward imports between any layers), evaluate import-linter / grimp — purpose-built for that. CE004 is the cheap version @@ -17,36 +28,34 @@ """ import ast -import re +from tests.lint.rules._layers import imports_package, is_bare_package_import, is_cli_path, is_package_path from tests.lint.rules.base import BaseRule -_CORE_DIRS = re.compile( - r"[/\\](criteria|evaluation|models|simulation|scoring|streaming|errors|orchestration|agents|harbor)[/\\]" -) -_BANNED = re.compile(r"^coder_eval\.cli") - - class NoCliImportsInCore(BaseRule): id = "CE004" def __init__(self, filepath: str) -> None: super().__init__(filepath) - self._in_core = bool(_CORE_DIRS.search(filepath)) + self._in_scope = is_package_path(filepath) and not is_cli_path(filepath) def visit_ImportFrom(self, node: ast.ImportFrom) -> None: - if self._in_core and node.module and _BANNED.match(node.module): + # Both spellings: `from coder_eval.cli import x` AND `from ..cli import x`. + if self._in_scope and ( + imports_package(node, "cli", self.filepath) or is_bare_package_import(node, "cli", self.filepath) + ): + named = f"{'.' * node.level}{node.module or 'cli'}" self.violation( node, - f"architectural violation: '{node.module}' (cli layer) imported from core layer", + f"architectural violation: '{named}' (cli layer) imported from core layer", ) self.generic_visit(node) def visit_Import(self, node: ast.Import) -> None: - if self._in_core: + if self._in_scope: for alias in node.names: - if _BANNED.match(alias.name): + if alias.name == "coder_eval.cli" or alias.name.startswith("coder_eval.cli."): self.violation( node, f"architectural violation: '{alias.name}' (cli layer) imported from core layer", diff --git a/tests/lint/runner.py b/tests/lint/runner.py index 1ea349a1c..81ddadc6d 100644 --- a/tests/lint/runner.py +++ b/tests/lint/runner.py @@ -41,6 +41,7 @@ from tests.lint.rules.ce061_window_via_close_window import WindowViaCloseWindow from tests.lint.rules.ce063_no_busy_ms_in_agents import NoBusyMsInAgents from tests.lint.rules.ce064_turn_bracket_on_the_clock import TurnBracketOnTheClock +from tests.lint.rules.ce066_no_report_imports_in_core import NoReportImportsInCore from tests.lint.rules.no_agent_timing_access import NoAgentTimingAccess from tests.lint.rules.no_blocking_io_in_async import NoBlockingIoInAsync from tests.lint.rules.no_cli_imports_in_core import NoCliImportsInCore @@ -61,7 +62,16 @@ # to 063. It was claimed during the turn-timing work and then folded into CE063 # rather than shipped. An id is a permanent documentation anchor: a suppression # comment carrying 062 in an older branch, review or commit message must never -# start meaning something new. Claim 065 next. +# start meaning something new. +# +# Claim 068 next. NOTE 065 IS TAKEN and is not in ALL_RULES: doc-surface and +# whole-tree rules are `@pytest.mark.lint` classes in tests/test_custom_lint.py +# rather than BaseRules, so the `_rule_ids` uniqueness assert below cannot see +# them. Enumerating them here is how this note fell behind CE044, so grep +# instead: `grep -E '^class Test(CE[0-9]{3})' tests/test_custom_lint.py`. Spell +# it `[0-9]`, not `\d` — GNU and BSD `grep -E` read `\d` as a literal `d` and +# report zero hits, which reads as "no ids taken". The whole id space is +# unioned in one place by TestRuffExternalCoversEveryRule._known(). type RuleClass = type[BaseRule] ALL_RULES: list[RuleClass] = [ @@ -110,6 +120,7 @@ WindowViaCloseWindow, NoBusyMsInAgents, TurnBracketOnTheClock, + NoReportImportsInCore, ] # Anti-shadow invariant (mirrors AgentRegistry / register_pricing): every CE rule diff --git a/tests/test_antigravity_agent.py b/tests/test_antigravity_agent.py index f1b32272d..117c8f892 100644 --- a/tests/test_antigravity_agent.py +++ b/tests/test_antigravity_agent.py @@ -1372,7 +1372,7 @@ async def __aexit__(self, *exc): # # max_turns was accepted and never read on this backend, so a task capping turns ran # uncapped here while the same file capped on Claude Code. The cap counts VISIBLE -# turns (tool calls — reports_stats.visible_turn_count's unit), enforced on the same +# turns (tool calls — result_metrics.visible_turn_count's unit), enforced on the same # step-loop boundary as the cooperative stop. diff --git a/tests/test_classification_match.py b/tests/test_classification_match.py index fa205c890..f3efced48 100644 --- a/tests/test_classification_match.py +++ b/tests/test_classification_match.py @@ -20,7 +20,7 @@ TaskDefinition, TaskResult, ) -from coder_eval.reports import _compute_suite_rollup, _render_suite_markdown +from coder_eval.reports.markdown import _compute_suite_rollup, _render_suite_markdown class _FakeSandbox: @@ -474,7 +474,7 @@ def test_threshold_on_unknown_metric_fails_for_any_criterion(self, tmp_path: Pat def test_missing_aggregator_explicit_none(self, tmp_path: Path) -> None: # An override that explicitly returns None still hits the # _build_missing_aggregator path — rare but possible. - from coder_eval.reports import _build_missing_aggregator + from coder_eval.reports.markdown import _build_missing_aggregator fallback = _build_missing_aggregator("custom", {"accuracy": 0.5}) assert fallback.error is not None diff --git a/tests/test_cli_empty_glob.py b/tests/test_cli_empty_glob.py index 5f6f18336..e8dc4885e 100644 --- a/tests/test_cli_empty_glob.py +++ b/tests/test_cli_empty_glob.py @@ -135,7 +135,7 @@ def test_cli_accepts_explicit_file_paths(tmp_path): patch("coder_eval.orchestration.batch.run_batch", return_value=(mock_summary, [])) as mock_batch, patch("coder_eval.cli.console.console.print"), patch("coder_eval.logging_config.aggregate_task_logs"), - patch("coder_eval.reports_experiment.ExperimentReportGenerator.write_reports"), + patch("coder_eval.reports.ExperimentReportGenerator.write_reports"), ): # Should not raise error - file exists asyncio.run( @@ -200,7 +200,7 @@ def test_cli_expands_valid_glob_patterns(tmp_path): patch("coder_eval.orchestration.batch.run_batch", return_value=(mock_summary, [])) as mock_batch, patch("coder_eval.cli.console.console.print"), patch("coder_eval.logging_config.aggregate_task_logs"), - patch("coder_eval.reports_experiment.ExperimentReportGenerator.write_reports"), + patch("coder_eval.reports.ExperimentReportGenerator.write_reports"), ): asyncio.run( _run_all_tasks( diff --git a/tests/test_codex_agent.py b/tests/test_codex_agent.py index 41076cb63..5f9654f05 100644 --- a/tests/test_codex_agent.py +++ b/tests/test_codex_agent.py @@ -2033,7 +2033,7 @@ class TestMaxTurnsVisibleTurnCap: Codex delivers one SDK turn per ``communicate()``, so a native turn counter would cap at 1 and mean nothing; the cap therefore counts VISIBLE turns (completed tool - calls — the unit ``reports_stats.visible_turn_count`` sums) and is enforced on the + calls — the unit ``result_metrics.visible_turn_count`` sums) and is enforced on the same pump boundary as the cooperative stop. """ diff --git a/tests/test_cost_accounting_paths.py b/tests/test_cost_accounting_paths.py index e81c227b1..a5d1c7c49 100644 --- a/tests/test_cost_accounting_paths.py +++ b/tests/test_cost_accounting_paths.py @@ -26,7 +26,7 @@ TurnRecord, ) from coder_eval.orchestration.batch import check_pricing_coverage -from coder_eval.reports_experiment import eval_result_to_task_dict +from coder_eval.run_record import eval_result_to_task_dict def _resolved(model: str | None, tmp_path: Path, criteria: list[dict[str, Any]] | None = None) -> ResolvedTask: diff --git a/tests/test_custom_lint.py b/tests/test_custom_lint.py index 3ae0223be..4c1429104 100644 --- a/tests/test_custom_lint.py +++ b/tests/test_custom_lint.py @@ -17,6 +17,7 @@ import pytest +from tests.lint.rules._layers import is_core_path from tests.lint.runner import ALL_RULES, check_paths @@ -131,7 +132,7 @@ def _run(src: str, *, in_agents: bool = True): from tests.lint.rules.ce043_no_command_output_truncation import NoCommandOutputTruncation - path = "src/coder_eval/agents/codex_agent.py" if in_agents else "src/coder_eval/reports_html.py" + path = "src/coder_eval/agents/codex_agent.py" if in_agents else "src/coder_eval/reports/html.py" return NoCommandOutputTruncation(path).check(ast.parse(src)) @pytest.mark.parametrize( @@ -1567,8 +1568,6 @@ def test_activation_template_makes_the_skill_reachable(self): ) def test_activation_rows_have_both_polarities(self): - import json - rows = [ json.loads(line) for line in (self.TEMPLATES / "activation-rows.jsonl").read_text(encoding="utf-8").splitlines() @@ -2095,6 +2094,355 @@ def test_task_rubric_is_bundled_and_read_by_its_readers(self): ) +# The `driver: docker` evaluation path, and the reason the core-layer predicate +# became an allowlist: the directory-list form it replaced named ten packages and +# `isolation` was not one, so the driver that runs the whole evaluation loop in a +# container was invisible to BOTH layering rules. Shared, so the two pins below +# cannot drift to different paths. +CORE_ISOLATION = "/repo/src/coder_eval/isolation/docker_runner.py" + + +@pytest.mark.lint +class TestCE004CatchesBothImportSpellings: + """CE004 must fire on the RELATIVE form, not only `coder_eval.cli`. + + It had checked `node.module` alone since it was written, so `from ..cli + import x` — the codebase's dominant idiom — passed silently. Found while + fixing the identical bug in CE066; the shared `_layers.imports_package` + helper is what stops the two drifting again. + """ + + @staticmethod + def _violations(source: str, filepath: str) -> list: + import ast + + from tests.lint.rules.no_cli_imports_in_core import NoCliImportsInCore + + return list(NoCliImportsInCore(filepath).check(ast.parse(source))) + + CORE = "/repo/src/coder_eval/orchestration/batch.py" + + @pytest.mark.parametrize( + "source", + [ + "from ..cli import run_command", + "from ..cli.run_command import run_pipeline", + "from .. import cli", + "from coder_eval.cli import run_command", + "import coder_eval.cli", + ], + ) + def test_every_spelling_of_a_cli_import_violates(self, source): + assert self._violations(source, self.CORE), f"CE004 missed: {source}" + + @pytest.mark.parametrize("source", ["from ..models import TurnRecord", "from ..client import X"]) + def test_a_non_cli_import_does_not_violate(self, source): + """`..client` must not prefix-match `cli` — the old `^coder_eval\\.cli` + regex would have matched `coder_eval.client` too.""" + assert not self._violations(source, self.CORE) + + def test_a_non_core_file_is_exempt(self): + assert not self._violations("from ..cli import run_command", "/repo/src/coder_eval/cli/report_command.py") + + def test_the_docker_driver_is_core(self): + assert self._violations("from ..cli import run_command", CORE_ISOLATION) + + def test_the_reports_package_is_in_scope(self): + """CE004's only exemption is `cli/`. The reports package runs without the + CLI — the orchestrator writes a task report mid-run — so a `cli` import + there closes a cli -> orchestration -> reports -> cli cycle. It was exempt + only because CE004 borrowed CE066's core predicate.""" + assert self._violations("from ..cli import run_command", "/repo/src/coder_eval/reports/markdown.py") + + +@pytest.mark.lint +class TestCE066NoReportImportsInCore: + """CE066 — core may import only the reports package's public writers. + + Before the split the orchestrator imported `turn_time_buckets` and + `visible_turn_count` from `reports_stats`, and `orchestration/batch.py` + imported the run.json row serializer from `reports_experiment`. Those names + moved to `result_metrics` / `stats` / `run_record`; this rule is what stops + the next one drifting back. + """ + + @staticmethod + def _violations(source: str, filepath: str) -> list: + import ast + + from tests.lint.rules.ce066_no_report_imports_in_core import NoReportImportsInCore + + return list(NoReportImportsInCore(filepath).check(ast.parse(source))) + + CORE = "/repo/src/coder_eval/orchestration/batch.py" + ORCHESTRATOR = "/repo/src/coder_eval/orchestrator.py" + NON_CORE = "/repo/src/coder_eval/cli/report_command.py" + + def test_a_non_writer_imported_into_core_violates(self): + found = self._violations("from coder_eval.reports import format_score", self.CORE) + assert len(found) == 1 + assert "format_score" in found[0].message + # The message must say what to do, not just that it is wrong. + assert "result_metrics" in found[0].message and "stats.py" in found[0].message + + def test_a_writer_imported_into_core_does_not_violate(self): + assert not self._violations("from coder_eval.reports import write_task_html", self.CORE) + + def test_top_level_orchestrator_is_core_even_though_it_is_in_no_package(self): + """CE004's directory regex cannot see this file, and it held 3 of the 5 + edges the rule exists to prevent — so it is the rule's main target.""" + assert self._violations("from coder_eval.reports import turn_time_buckets", self.ORCHESTRATOR) + assert not self._violations("from coder_eval.reports import write_task_html", self.ORCHESTRATOR) + + def test_a_submodule_import_is_checked_too(self): + assert self._violations("from coder_eval.reports.helpers import fmt_p", self.CORE) + + @pytest.mark.parametrize( + "source", + [ + "from ..reports import format_score", + "from ..reports.helpers import VariantSeries", + "from .. import reports", + ], + ) + def test_the_relative_spelling_is_caught(self, source): + """The relative form is the LOCAL IDIOM — both surviving edges use it. + + A relative import keeps its dots in `node.level` and leaves + `node.module == "reports"`, so a rule matching only the absolute + `coder_eval.reports` fires on nothing this codebase actually writes. + """ + assert self._violations(source, self.CORE), f"CE066 missed the relative form: {source}" + + def test_a_relative_writer_import_is_still_allowed(self): + """orchestrator.py's real `from .reports import write_task_html`.""" + assert not self._violations("from .reports import write_task_html", self.ORCHESTRATOR) + + @pytest.mark.parametrize( + ("source", "filepath"), + [ + # `.reports` from a sub-package is `coder_eval..reports`, and + # `...reports` from a sub-package escapes `coder_eval` entirely. + ("from .reports import format_score", "/repo/src/coder_eval/orchestration/batch.py"), + ("from ...reports import format_score", "/repo/src/coder_eval/orchestration/batch.py"), + ], + ) + def test_a_relative_import_is_resolved_against_the_importing_file(self, source, filepath): + """The dots are counted, not merely noticed. + + Matching `node.module == "reports"` for any non-zero level reads a nested + `coder_eval.orchestration.reports` — or a sibling of `coder_eval` — as the + reports layer. Latent (nothing in the tree is nested that way), which is + exactly why it needs a pin rather than a comment. + """ + assert not self._violations(source, filepath) + + def test_a_top_level_core_module_other_than_the_orchestrator_is_core(self): + """`result_metrics.py` is where the violation message tells you to move a + metric TO — exempting it would be a hole in the middle of the rule.""" + assert self._violations("from .reports.helpers import fmt_p", "/repo/src/coder_eval/result_metrics.py") + assert self._violations("from .reports import format_score", "/repo/src/coder_eval/run_record.py") + + def test_wholesale_module_import_into_core_violates(self): + """No name to check, so every attribute access through it is invisible.""" + assert self._violations("import coder_eval.reports", self.CORE) + + def test_a_non_core_file_is_exempt(self): + assert not self._violations("from coder_eval.reports import format_score", self.NON_CORE) + + def test_the_docker_driver_is_core(self): + assert self._violations("from ..reports import format_score", CORE_ISOLATION) + assert not self._violations("from ..reports import write_task_html", CORE_ISOLATION) + + def test_the_reports_package_itself_stays_exempt(self): + """Pins that narrowing CE004's scope did not widen this rule's: a report + module reaching a sibling's non-writer is the package's own business.""" + assert not self._violations("from ..reports.helpers import fmt_p", "/repo/src/coder_eval/reports/markdown.py") + + def test_every_allowlisted_name_resolves_in_the_package(self): + """Staleness guard: a renamed writer must not leave a dead entry silencing + the rule. This is the pattern the deleted pricing test used correctly. + """ + import coder_eval.reports as pkg + from tests.lint.rules.ce066_no_report_imports_in_core import ALLOWED_WRITERS + + missing = sorted(n for n in ALLOWED_WRITERS if not hasattr(pkg, n)) + assert not missing, f"CE066 allowlists names that no longer exist in coder_eval.reports: {missing}" + + +@pytest.mark.lint +class TestCoreLayerMembership: + """`_layers` is the single definition of where a file sits, for CE004 and CE066. + + Pinned against the real filesystem because the core predicate's two previous + forms were denylists that each left a hole: the first exempted every top-level + module but `orchestrator.py`, the second named ten directories and missed + `isolation/`. The allowlist form has no per-package list to keep honest — only + each rule's exemption set, which is what this class pins: `{cli, reports}` for + CE066's core, `{cli}` for CE004's scope. + + Deliberately NOT named `TestCE\\d{3}`: that prefix is this file's convention + for a class guarding one numbered rule, and this class guards the predicates + two rules share. Taking a CE number would claim an id that indexes no rule. + """ + + NON_CORE = frozenset({"cli", "reports"}) + PKG = SRC / "coder_eval" + + def test_exactly_two_packages_are_non_core(self): + dirs = [d for d in self.PKG.iterdir() if d.is_dir() and d.name != "__pycache__"] + found = {d.name for d in dirs if not is_core_path(str(d / "x.py"))} + assert found == self.NON_CORE, f"the non-core set moved: {sorted(found)}" + + def test_every_module_is_classified_by_its_top_level_package(self): + misclassified = [ + str(py.relative_to(self.PKG)) + for py in self.PKG.rglob("*.py") + if is_core_path(str(py)) is not (py.relative_to(self.PKG).parts[0] not in self.NON_CORE) + ] + assert not misclassified, f"is_core_path disagrees with the package layout for: {misclassified}" + + def test_ce004_scope_is_every_module_outside_cli(self): + """CE004 exempts only `cli/`. It once inherited CE066's `reports/` + exemption by borrowing the core predicate whole. + + Runs the RULE at every real module path rather than recomputing its scope + from the helpers, so it fails if the rule stops using them. The probe uses + the ABSOLUTE spelling deliberately: a relative one resolves against the + importing file, so `..cli` names the cli layer only from inside a + sub-package and would test depth here instead of scope.""" + import ast + + from tests.lint.rules.no_cli_imports_in_core import NoCliImportsInCore + + cli_import = ast.parse("from coder_eval.cli import run_command") + misscoped = [ + str(py.relative_to(self.PKG)) + for py in self.PKG.rglob("*.py") + if bool(list(NoCliImportsInCore(str(py)).check(cli_import))) is (py.relative_to(self.PKG).parts[0] == "cli") + ] + assert not misscoped, f"CE004's scope disagrees with the package layout for: {misscoped}" + + @pytest.mark.parametrize( + "path", + [ + # A `src` component that is NOT the package's parent, and none at all. + "/home/dev/src/exp/coder_eval/conftest.py", + "/home/dev/projects/coder_eval/conftest.py", + ], + ) + def test_a_repo_root_file_is_not_core(self, path): + """The checkout directory is itself named `coder_eval`, so the unanchored + regex made every repo-root module core. Anchoring on `src/` is the fix.""" + assert not is_core_path(path) + + def test_the_src_named_parent_residual_is_unreachable_not_fixed(self): + """Blind spot, pinned so the anchoring is not mistaken for a complete fix. + + A clone at `~/src/coder_eval` collides with the anchor itself, and no path + substring can separate it from the package. Unreachable: both rules are + only ever handed paths under the runner's `SRC`. + """ + assert is_core_path("/Users/x/src/coder_eval/conftest.py") + assert is_core_path("/Users/x/src/coder_eval/tests/test_a.py") + + def test_neither_consumer_is_ever_handed_a_path_outside_src(self): + """The residual above is harmless only because both rules scan `SRC` alone. + + Adding CE004 or CE066 to `_ALSO_SCAN_TESTS` would hand them the whole + `tests/` tree, and on a clone at `~/src/coder_eval` — an ordinary layout — + that tree matches `_PKG`. The reachability argument lives in + `_layers.py`'s docstring as prose; this is the line that enforces it. + """ + assert {"CE004", "CE066"}.isdisjoint(_ALSO_SCAN_TESTS) + + @pytest.mark.parametrize( + ("relative", "expected"), + [ + ("src/coder_eval/orchestrator.py", True), + ("src/coder_eval/reports/markdown.py", False), + # Only the PACKAGES are non-core: each layer pattern requires a + # trailing separator, so a top-level module whose name merely starts with + # `reports` or `cli` stays core. Nothing in the tree has that shape + # today, so this is the only thing pinning the boundary. + ("src/coder_eval/reports_legacy.py", True), + ("src/coder_eval/cli_helpers.py", True), + ], + ) + def test_the_relative_and_absolute_spelling_agree(self, relative, expected): + assert is_core_path(relative) is expected + assert is_core_path(f"/repo/{relative}") is expected + + +@pytest.mark.lint +class TestCE065PricingMirrorParity: + """CE065 — the evalboard's rate table is generated from coder_eval.pricing. + + lib/pricing.ts used to hand-copy the Python rate card, guarded by a regex + parser, a meta-guard on that regex, an exemption set and a staleness guard + for the exemption set — five layers that still let four heavily-used models + render "—" for cost. The table is now generated; `make pricing-mirror` + writes it and this class diffs it. Reasons over generated text rather than + one Python AST, so it lives here rather than in the AST runner. + + Generating the table removed the regex layers, not the two exemptions + themselves — a generator that quietly PRICES a model the hand-copy skipped + has changed behaviour under cover of a refactor. Both axes are asserted + below, from their declared sources. + """ + + REPO_ROOT = Path(__file__).parent.parent + + def test_generated_mirror_matches_disk(self): + from tests.lint.pricing_mirror import check + + findings = check(self.REPO_ROOT) + assert not findings, ( + "\nThe evalboard's rate table drifted from src/coder_eval/pricing.py — run " + "`make pricing-mirror` to regenerate:\n\n" + + "\n\n".join(f"{path}:\n{diff}" for path, diff in sorted(findings.items())) + ) + + def test_every_statically_priced_model_is_mirrored(self): + """Table-driven, so a new rate in pricing.py needs zero edits here. + + Two exclusions, each read from where it is declared rather than repeated: + `per_request_billing` on the rate, and `DELIBERATELY_UNMIRRORED` beside the + generator. What the old hand-copy got wrong was not HAVING an exemption set + but letting it go stale unnoticed, which `_assert_exemptions_are_live` now + fails the build on. + """ + from coder_eval.pricing import builtin_rates + from tests.lint.pricing_mirror import DELIBERATELY_UNMIRRORED, render_pricing + + rendered = render_pricing() + for key, rate in builtin_rates().items(): + # Build the needle the way the renderer builds the row, so a key + # needing escaping is not reported as spuriously missing. + needle = json.dumps(key) + ": {" + if rate.per_request_billing: + assert needle not in rendered, ( + f"{key} bills per request — statically pricing it on the frontend replaces " + "the captured actual per-call cost with an estimate" + ) + elif key in DELIBERATELY_UNMIRRORED: + assert needle not in rendered, ( + f"{key} is exempt from the mirror — pricing it here widens the frontend " + "table past what the hand-copy it replaced priced" + ) + else: + assert needle in rendered, f"{key} is priced in pricing.py but missing from the mirror" + + def test_the_exemption_set_is_not_stale(self): + """The guard the deleted parity test carried. An id that has left + `pricing.py` silences nothing and only survives to be copied, so its + membership must be a build failure rather than a comment.""" + from coder_eval.pricing import builtin_rates + from tests.lint.pricing_mirror import _assert_exemptions_are_live + + _assert_exemptions_are_live(builtin_rates()) + + @pytest.mark.lint class TestCE033PluginReferenceParity: """CE033 — the plugin's bundled criteria reference is generated from the models. @@ -4111,6 +4459,7 @@ def test_the_rule_is_now_exemption_free(self): assert suppressed == set() +@pytest.mark.lint class TestRuffExternalCoversEveryRule: """Every CE rule's documented `# noqa` must be accepted by ruff. @@ -4120,22 +4469,48 @@ class TestRuffExternalCoversEveryRule: advertise `# noqa: CE054` / `# noqa: CE048` as the supported escape hatch. So the first person to use the documented exemption got a red `make check` instead, for doing exactly what the rule told them to. + + The list then drifted a SECOND time, and this class is why it drifted + quietly: it read `ALL_RULES` alone, so it could not see a rule that is a + `@pytest.mark.lint` class here rather than a `BaseRule`. CE044 and CE065 are + both such rules, both were missing, and only CE065 was noticed — by a human + reading a diff. `_known()` now unions both registries. + + Both directions are asserted. A declared id for a deleted rule is the + exemption-set rot that the generated pricing table exists to remove. + + Blind spot: the `@pytest.mark.lint` half of `_known()` discovers ids by the + `class TestCE\\d{3}` naming convention, which every such class follows today + but nothing enforces. A class named otherwise is invisible here, and its id + can go undeclared exactly as CE044 did. + + Nothing is red today for want of these two entries — no `# noqa: CE044` or + `# noqa: CE065` exists in the tree — so this is pre-emptive rather than the + fix for a broken build. """ @staticmethod def _external() -> set[str]: import tomllib - from pathlib import Path - data = tomllib.loads(Path("pyproject.toml").read_text(encoding="utf-8")) + data = tomllib.loads((SRC.parent / "pyproject.toml").read_text(encoding="utf-8")) return set(data["tool"]["ruff"]["lint"]["external"]) - def test_every_registered_rule_is_listed(self): - from tests.lint.runner import ALL_RULES + @staticmethod + def _known() -> set[str]: + own_source = Path(__file__).read_text(encoding="utf-8") + return {r.id for r in ALL_RULES} | set(re.findall(r"^class Test(CE\d{3})", own_source, re.M)) - missing = sorted({r.id for r in ALL_RULES} - self._external()) + def test_every_registered_rule_is_listed(self): + missing = sorted(self._known() - self._external()) assert not missing, f"add to [tool.ruff.lint] external in pyproject.toml: {missing}" + def test_no_dead_entry_survives(self): + """A declared id for a rule that no longer exists silences RUF102 for a + code nothing defines — the exemption-set rot the pricing mirror removed.""" + dead = sorted(self._external() - self._known()) + assert not dead, f"pyproject.toml [tool.ruff.lint] external declares ids with no such rule: {dead}" + def test_every_listed_id_is_well_formed(self): """Cheap guard against a typo silently widening the allowlist.""" bad = sorted(i for i in self._external() if not re.fullmatch(r"CE\d{3}", i)) @@ -4198,7 +4573,7 @@ class TestCE053NoRunRecordFilenameLiteral: """CE053 flags a `task.json` literal outside path_utils.""" @staticmethod - def _run(src: str, filepath: str = "src/coder_eval/reports.py"): + def _run(src: str, filepath: str = "src/coder_eval/reports/markdown.py"): import ast from tests.lint.rules.ce053_run_record_filename_literal import NoRunRecordFilenameLiteral diff --git a/tests/test_docker_runner_container_death.py b/tests/test_docker_runner_container_death.py index ab52cb400..0a76ca404 100644 --- a/tests/test_docker_runner_container_death.py +++ b/tests/test_docker_runner_container_death.py @@ -62,7 +62,7 @@ def test_written_when_task_json_missing(self, run_dir): asyncio.run(runner._write_synthetic_task_json(target, error)) # Round-trips through the SAME parse every downstream consumer uses - # (batch.py / reports.py / reports_stats.py) — not just a raw key check — + # (batch.py / reports/markdown.py / result_metrics.py) — not just a raw key check — # so a schema change that broke validation on the synthetic record fails here. parsed = EvaluationResult.model_validate_json(target.read_text(encoding="utf-8")) assert parsed.final_status == FinalStatus.ERROR diff --git a/tests/test_durations.py b/tests/test_durations.py new file mode 100644 index 000000000..b762cc84e --- /dev/null +++ b/tests/test_durations.py @@ -0,0 +1,35 @@ +"""Unit tests for coder_eval.durations.format_ms. + +`format_ms` moved out of `formatting.py` so the reports package does not inherit +a `claude_agent_sdk` dependency for a 14-line formatter. The `None`-vs-`0.0` +boundary is the CE058 contract — an unmeasured duration renders as a dash, a +measured zero as `0ms` — and is asserted explicitly here because throwing that +distinction away at the last step is exactly how one surface comes to print +`0ms` where another prints a dash. +""" + +import pytest + +from coder_eval.durations import format_ms + + +@pytest.mark.parametrize( + ("ms", "expected"), + [ + (None, "—"), + (0.0, "0ms"), + (1.0, "1ms"), + (999.4, "999ms"), + (999.6, "1000ms"), + (1000.0, "1.00s"), + (1500.0, "1.50s"), + (61_000.0, "61.00s"), + ], +) +def test_format_ms(ms, expected): + assert format_ms(ms) == expected + + +def test_unmeasured_and_measured_zero_are_distinguishable(): + """The whole point of the CE058 contract, in one assertion.""" + assert format_ms(None) != format_ms(0.0) diff --git a/tests/test_early_stop.py b/tests/test_early_stop.py index ff57c63d4..791098bd3 100644 --- a/tests/test_early_stop.py +++ b/tests/test_early_stop.py @@ -79,8 +79,8 @@ from coder_eval.orchestration.experiment import load_experiment, resolve_all_tasks from coder_eval.orchestrator import Orchestrator, build_task_event from coder_eval.reports import ReportGenerator -from coder_eval.reports_experiment import eval_result_to_task_dict -from coder_eval.reports_html import _render_criteria, _render_header +from coder_eval.reports.html import _render_criteria, _render_header +from coder_eval.run_record import eval_result_to_task_dict from coder_eval.streaming.events import ( AgentEndEvent, AgentEndStatus, diff --git a/tests/test_experiment_reports.py b/tests/test_experiment_reports.py index 71f7d139b..a6e72271b 100644 --- a/tests/test_experiment_reports.py +++ b/tests/test_experiment_reports.py @@ -14,8 +14,7 @@ VariantAggregate, VariantResult, ) -from coder_eval.reports_experiment import ExperimentReportGenerator -from coder_eval.reports_stats import describe_prompt_config +from coder_eval.reports import ExperimentReportGenerator, describe_prompt_config from tests._fixtures.report_snapshots import assert_matches_snapshot @@ -728,7 +727,7 @@ class TestStatisticalHelpers: def test_welch_t_test_identical_groups(self): """Identical groups should produce p-value of 1.0.""" - from coder_eval.reports_stats import welch_t_test + from coder_eval.stats import welch_t_test p = welch_t_test([1.0, 2.0, 3.0], [1.0, 2.0, 3.0]) assert p is not None @@ -736,7 +735,7 @@ def test_welch_t_test_identical_groups(self): def test_welch_t_test_different_groups(self): """Very different groups should produce low p-value.""" - from coder_eval.reports_stats import welch_t_test + from coder_eval.stats import welch_t_test p = welch_t_test([1.0, 1.1, 0.9, 1.0, 1.05], [5.0, 5.1, 4.9, 5.0, 5.05]) assert p is not None @@ -744,13 +743,13 @@ def test_welch_t_test_different_groups(self): def test_welch_t_test_insufficient_data(self): """Single observation per group should return None.""" - from coder_eval.reports_stats import welch_t_test + from coder_eval.stats import welch_t_test assert welch_t_test([1.0], [2.0]) is None def test_welch_t_test_exact_reference_value(self): """Exact Student-t p-value, cross-checked against scipy ttest_ind(equal_var=False).""" - from coder_eval.reports_stats import welch_t_test + from coder_eval.stats import welch_t_test # Equal variances 2.5, n=5 each ⇒ t=1.0, Welch-Satterthwaite df=8. p = welch_t_test([1.0, 2.0, 3.0, 4.0, 5.0], [2.0, 3.0, 4.0, 5.0, 6.0]) @@ -759,14 +758,14 @@ def test_welch_t_test_exact_reference_value(self): def test_welch_t_test_zero_variance_different_means(self): """Zero variance in both groups: deterministic difference ⇒ 0.0, identical ⇒ 1.0.""" - from coder_eval.reports_stats import welch_t_test + from coder_eval.stats import welch_t_test assert welch_t_test([1.0, 1.0], [2.0, 2.0]) == 0.0 assert welch_t_test([1.0, 1.0], [1.0, 1.0]) == 1.0 def test_student_t_two_tailed_p_small_df_not_normal(self): """Small df must use t tails, not normal ones — the regression sensor for the old bug.""" - from coder_eval.reports_stats import student_t_two_tailed_p + from coder_eval.stats import student_t_two_tailed_p p = student_t_two_tailed_p(2.5, 4.0) assert abs(p - 0.06677) < 5e-4 @@ -775,7 +774,7 @@ def test_student_t_two_tailed_p_small_df_not_normal(self): def test_student_t_two_tailed_p_critical_values(self): """Round-trip the standard t-table critical values.""" - from coder_eval.reports_stats import student_t_two_tailed_p + from coder_eval.stats import student_t_two_tailed_p assert abs(student_t_two_tailed_p(2.776, 4.0) - 0.05) < 1e-3 assert abs(student_t_two_tailed_p(2.228, 10.0) - 0.05) < 1e-3 @@ -787,14 +786,14 @@ def test_student_t_two_tailed_p_large_df_matches_normal(self): """At huge df the t distribution converges to the normal.""" import statistics - from coder_eval.reports_stats import student_t_two_tailed_p + from coder_eval.stats import student_t_two_tailed_p normal_p = 2.0 * statistics.NormalDist().cdf(-1.96) assert abs(student_t_two_tailed_p(1.96, 1e6) - normal_p) < 1e-5 def test_regularized_incomplete_beta_closed_forms(self): """I_x(a,b) against the closed forms it has for b=1, a=1, and the symmetric point.""" - from coder_eval.reports_stats import regularized_incomplete_beta + from coder_eval.stats import regularized_incomplete_beta assert abs(regularized_incomplete_beta(2.0, 1.0, 0.3) - 0.3**2) < 1e-10 assert abs(regularized_incomplete_beta(1.0, 3.0, 0.4) - (1.0 - 0.6**3)) < 1e-10 @@ -804,7 +803,7 @@ def test_regularized_incomplete_beta_closed_forms(self): def test_paired_t_test_exact_reference_value(self): """Exact paired-t p-value, cross-checked against scipy ttest_rel.""" - from coder_eval.reports_stats import paired_t_test, welch_t_test + from coder_eval.stats import paired_t_test, welch_t_test a = [0.9, 0.5, 0.8, 0.7, 0.95] b = [0.7, 0.55, 0.6, 0.72, 0.8] @@ -819,21 +818,22 @@ def test_paired_t_test_exact_reference_value(self): def test_paired_t_test_constant_shift_and_identical(self): """Zero-sd differences: deterministic shift ⇒ 0.0, identical lists ⇒ 1.0.""" - from coder_eval.reports_stats import paired_t_test + from coder_eval.stats import paired_t_test assert paired_t_test([1.0, 2.0, 3.0, 4.0, 5.0], [2.0, 3.0, 4.0, 5.0, 6.0]) == 0.0 assert paired_t_test([1.0, 2.0, 3.0], [1.0, 2.0, 3.0]) == 1.0 def test_paired_t_test_invalid_input(self): """Length mismatch or fewer than 2 pairs returns None.""" - from coder_eval.reports_stats import paired_t_test + from coder_eval.stats import paired_t_test assert paired_t_test([1.0, 2.0], [1.0]) is None assert paired_t_test([1.0], [2.0]) is None def test_non_finite_inputs_never_report_significance(self): """NaN/inf must not produce a fabricated p-value — fail closed, never significant.""" - from coder_eval.reports_stats import fmt_p, paired_t_test, student_t_two_tailed_p, welch_t_test + from coder_eval.reports.html import fmt_p + from coder_eval.stats import paired_t_test, student_t_two_tailed_p, welch_t_test nan, inf = float("nan"), float("inf") assert student_t_two_tailed_p(nan, 4.0) == 1.0 @@ -846,7 +846,7 @@ def test_non_finite_inputs_never_report_significance(self): def test_mean_and_stddev(self): """Basic mean and stddev calculations.""" - from coder_eval.reports_stats import mean, stddev + from coder_eval.stats import mean, stddev assert mean([1.0, 2.0, 3.0]) == 2.0 assert abs(stddev([1.0, 2.0, 3.0]) - 1.0) < 1e-10 @@ -854,7 +854,7 @@ def test_mean_and_stddev(self): def test_fmt_mean_sd(self): """Format mean ± stddev string.""" - from coder_eval.reports_stats import fmt_mean_sd + from coder_eval.reports.html import fmt_mean_sd result = fmt_mean_sd([1.0, 2.0, 3.0]) assert "2.000" in result @@ -1259,14 +1259,14 @@ def _result_with_replicates() -> ExperimentResult: def test_duration_is_per_run_not_replicate_inflated(self): """duration_seconds is summed across replicates, so it must be divided out.""" - from coder_eval.reports_stats import collect_variant_series + from coder_eval.reports import collect_variant_series series = collect_variant_series(self._result_with_replicates()) assert series["a"].durations == [30.0] def test_html_and_markdown_report_the_same_duration(self): """Regression: the HTML copy of this collector used the raw summed duration.""" - from coder_eval.reports_html import _experiment_aggregate_metrics + from coder_eval.reports.html import _experiment_aggregate_metrics result = self._result_with_replicates() md = ExperimentReportGenerator.generate_experiment_report(result) @@ -1277,7 +1277,7 @@ def test_html_and_markdown_report_the_same_duration(self): def test_result_for_unknown_variant_is_ignored(self): """A task result naming a variant outside variant_ids must not raise.""" - from coder_eval.reports_stats import collect_variant_series + from coder_eval.reports import collect_variant_series result = self._result_with_replicates() result.task_summaries[0].variant_results.append( @@ -1427,7 +1427,7 @@ def test_paired_comparison_keeps_tasks_with_unequal_replicate_counts(self): def test_html_renders_the_same_paired_numbers_as_markdown(self): """Both reporters render one shared computation, so they cannot disagree.""" - from coder_eval.reports_html import _experiment_paired_comparison + from coder_eval.reports.html import _experiment_paired_comparison result = self._make_result( ["a", "b"], @@ -1442,7 +1442,7 @@ def test_html_renders_the_same_paired_numbers_as_markdown(self): assert "**Paired mean diff (a - b)**: +0.117 [95% CI -0.242, +0.475], Cohen's d = 0.81, p = 0.296" in md def test_html_paired_section_absent_for_three_variants(self): - from coder_eval.reports_html import _experiment_paired_comparison + from coder_eval.reports.html import _experiment_paired_comparison result = self._make_result( ["a", "b", "c"], @@ -1479,7 +1479,7 @@ def test_paired_comparison_ignores_tasks_with_no_scores(self): class TestExperimentReportSnapshots: """Byte-identical characterization snapshots for generate_experiment_report — the safety net for its decomposition. Output is deterministic: bootstrap_mean_ci in - reports_stats uses a fixed default seed, so no scrubbing is needed (do not pass a + coder_eval.stats uses a fixed default seed, so no scrubbing is needed (do not pass a varying seed); the paired section is closed-form Student-t with no randomness.""" def test_experiment_report_snapshot_2variant(self): @@ -1716,7 +1716,7 @@ class TestUngradedRenderingInMarkdown: Five ungraded edits landed on `reports_html.py` with tests and the identical five on `reports_experiment.py` with none, so the two renderers could drift apart while the suite stayed green. These also cover the four new public - `reports_stats` helpers, which no test called at all — including + `coder_eval.stats` helpers, which no test called at all — including `is_env_table_key`, a behaviour change (three key families newly hidden from BOTH Environment tables) that shipped unasserted in either direction. """ @@ -1764,7 +1764,7 @@ def _ungraded_result(): ) def test_the_report_names_the_fourth_bucket_and_publishes_no_zero(self): - from coder_eval.reports_experiment import ExperimentReportGenerator + from coder_eval.reports import ExperimentReportGenerator md = ExperimentReportGenerator.generate_experiment_report(self._ungraded_result()) @@ -1779,7 +1779,7 @@ def test_duration_and_tokens_survive_an_ungraded_row(self): """Only the SCORE is missing. Dropping the row whole made an all-ungraded experiment render `Avg Duration | N/A` with Tokens absent entirely — the bug `collect_variant_series`' own comment describes.""" - from coder_eval.reports_stats import collect_variant_series + from coder_eval.reports import collect_variant_series series = collect_variant_series(self._ungraded_result())["a"] @@ -1788,7 +1788,7 @@ def test_duration_and_tokens_survive_an_ungraded_row(self): assert series.tokens == [900.0] def test_format_score_distinguishes_unmeasured_from_zero(self): - from coder_eval.reports_stats import UNGRADED_SCORE_TEXT, format_score + from coder_eval.reports import UNGRADED_SCORE_TEXT, format_score assert format_score(None) == UNGRADED_SCORE_TEXT assert format_score(0.0) == "0.000" @@ -1798,12 +1798,12 @@ def test_format_score_distinguishes_unmeasured_from_zero(self): "key", ["installed_tools", "command_base_path", "reference_digest", "graded_by_api_routing"] ) def test_env_table_hides_the_noise_keys(self, key): - from coder_eval.reports_stats import is_env_table_key + from coder_eval.reports import is_env_table_key assert not is_env_table_key(key) def test_env_table_keeps_ordinary_keys(self): - from coder_eval.reports_stats import is_env_table_key + from coder_eval.reports import is_env_table_key assert is_env_table_key("coder_eval") assert is_env_table_key("api_routing") diff --git a/tests/test_lint_runner.py b/tests/test_lint_runner.py index 87129db84..b2cfc0415 100644 --- a/tests/test_lint_runner.py +++ b/tests/test_lint_runner.py @@ -159,7 +159,7 @@ def test_ce003_skips_files_outside_criteria(write_py): def test_ce004_flags_cli_import_in_core(write_py, tmp_path): - sub = tmp_path / "coder_eval" / "evaluation" + sub = tmp_path / "src" / "coder_eval" / "evaluation" sub.mkdir(parents=True) path = sub / "thing.py" path.write_text("from coder_eval.cli.utils import something\n", encoding="utf-8") @@ -169,7 +169,7 @@ def test_ce004_flags_cli_import_in_core(write_py, tmp_path): def test_ce004_flags_bare_cli_import_in_core(write_py, tmp_path): - sub = tmp_path / "coder_eval" / "criteria" + sub = tmp_path / "src" / "coder_eval" / "criteria" sub.mkdir(parents=True) path = sub / "thing.py" path.write_text("import coder_eval.cli\n", encoding="utf-8") @@ -180,7 +180,7 @@ def test_ce004_flags_bare_cli_import_in_core(write_py, tmp_path): def test_ce004_flags_in_models_layer(write_py, tmp_path): """models/ is part of the core layer in the expanded rule.""" - sub = tmp_path / "coder_eval" / "models" + sub = tmp_path / "src" / "coder_eval" / "models" sub.mkdir(parents=True) path = sub / "thing.py" path.write_text("from coder_eval.cli import app\n", encoding="utf-8") @@ -189,7 +189,7 @@ def test_ce004_flags_in_models_layer(write_py, tmp_path): def test_ce004_allows_cli_import_in_cli(write_py, tmp_path): - sub = tmp_path / "coder_eval" / "cli" + sub = tmp_path / "src" / "coder_eval" / "cli" sub.mkdir(parents=True) path = sub / "thing.py" path.write_text("from coder_eval.cli.utils import something\n", encoding="utf-8") @@ -484,7 +484,7 @@ def test_ce009_allows_subclass_inheriting_forbid_from_same_file(tmp_path: Path) assert violations == [] -def test_ce008_skips_files_outside_scope(tmp_path: Path) -> None: +def test_ce009_skips_files_outside_scope(tmp_path: Path) -> None: """Files outside tasks.py / criteria.py are not flagged (results.py uses extra='allow').""" from tests.lint.rules.yaml_models_forbid_extras import YamlModelsForbidExtras diff --git a/tests/test_orchestrator.py b/tests/test_orchestrator.py index b894ac6f5..00a4c79f9 100644 --- a/tests/test_orchestrator.py +++ b/tests/test_orchestrator.py @@ -2276,7 +2276,7 @@ def test_finalize_result_logs_summary_on_success(tmp_path, caplog): with ( caplog.at_level(_logging.INFO, logger="coder_eval.orchestrator"), - patch("coder_eval.reports_html.write_task_html", return_value=None), + patch("coder_eval.reports.write_task_html", return_value=None), ): orch._finalize_result(start_time=time.time() - 1.5) @@ -2301,7 +2301,7 @@ def test_finalize_result_logs_summary_on_timeout(tmp_path, caplog): with ( caplog.at_level(_logging.INFO, logger="coder_eval.orchestrator"), - patch("coder_eval.reports_html.write_task_html", return_value=None), + patch("coder_eval.reports.write_task_html", return_value=None), ): orch._finalize_result(start_time=time.time()) @@ -2324,7 +2324,7 @@ def test_finalize_result_logs_zero_score_when_no_criteria(tmp_path, caplog): orch = _bootstrap_finalize_orchestrator(tmp_path, final_status=FinalStatus.ERROR, iterations=0) with ( caplog.at_level(_logging.INFO, logger="coder_eval.orchestrator"), - patch("coder_eval.reports_html.write_task_html", return_value=None), + patch("coder_eval.reports.write_task_html", return_value=None), ): orch._finalize_result(start_time=time.time()) diff --git a/tests/test_orchestrator_error_log_tail.py b/tests/test_orchestrator_error_log_tail.py index 1c016c4fb..b1c18f913 100644 --- a/tests/test_orchestrator_error_log_tail.py +++ b/tests/test_orchestrator_error_log_tail.py @@ -33,7 +33,7 @@ def _build_orchestrator(tmp_path: Path) -> Orchestrator: def _patch_finalize_persistence(): """Skip the on-disk persistence side-effects of _finalize_result.""" - return patch("coder_eval.reports_html.write_task_html", return_value=None) + return patch("coder_eval.reports.write_task_html", return_value=None) @pytest.mark.asyncio diff --git a/tests/test_orchestrator_telemetry.py b/tests/test_orchestrator_telemetry.py index 9d6699d4a..623adcf72 100644 --- a/tests/test_orchestrator_telemetry.py +++ b/tests/test_orchestrator_telemetry.py @@ -61,7 +61,7 @@ def _bootstrap(tmp_path, *, final_status, duration=None, score=None, iterations= def _finalize_and_capture(orch): with ( patch("coder_eval.telemetry.track_event") as mock_track, - patch("coder_eval.reports_html.write_task_html", return_value=None), + patch("coder_eval.reports.write_task_html", return_value=None), ): orch._finalize_result(start_time=time.time() - 1.0) return mock_track @@ -340,4 +340,4 @@ def test_it_does_not_sum_anything_itself(self): source = inspect.getsource(build_task_event) assert "turn_time_buckets(result)" in source - assert "harness_startup_ms" not in source, "the summation belongs to reports_stats" + assert "harness_startup_ms" not in source, "the summation belongs to result_metrics" diff --git a/tests/test_pricing_mirror.py b/tests/test_pricing_mirror.py new file mode 100644 index 000000000..c601d0c5c --- /dev/null +++ b/tests/test_pricing_mirror.py @@ -0,0 +1,127 @@ +"""Unit tests for the generated evalboard rate table (CE065's renderer). + +These cover `render_pricing` against small explicit rate cards rather than the +live table — the live table's contents are CE065's job, and asserting against +its size here would make every reprice a two-file edit, which is the coupling +the generator exists to remove. +""" + +import math +import re + +import pytest + +from coder_eval.pricing import ModelPricing, builtin_rates +from tests.lint.pricing_mirror import ( + DELIBERATELY_UNMIRRORED, + _assert_exemptions_are_live, + render_pricing, +) + + +ORDINARY = ModelPricing(1.0, 2.0, 3.0, 4.0) +PER_REQUEST = ModelPricing(5.0, 6.0, 7.0, 8.0, per_request_billing=True) + + +def test_each_ts_field_carries_its_own_python_rate(): + """ORDINARY's four rates are distinct, so a swapped pair fails here rather + than only in the evalboard's vitest job.""" + rendered = render_pricing({"plain-model": ORDINARY}) + assert ("inputPerMTok: 1.0, outputPerMTok: 2.0, cacheWritePerMTok: 3.0, cacheReadPerMTok: 4.0") in rendered + + +def test_per_request_rows_are_omitted_and_ordinary_ones_kept(): + rendered = render_pricing({"vendor/routed": PER_REQUEST, "plain-model": ORDINARY}) + assert '"plain-model": {' in rendered + assert '"vendor/routed": {' not in rendered + # The reader of the generated file is told why the row is absent. + assert "per_request_billing" in rendered + assert "vendor/routed" in rendered + + +def test_output_is_deterministic_and_sorted(): + rates = {"zeta": ORDINARY, "alpha": ORDINARY, "mid": ORDINARY} + first = render_pricing(rates) + assert first == render_pricing(rates) + keys = [line.split('"')[1] for line in first.splitlines() if line.startswith(' "')] + assert keys == sorted(keys) == ["alpha", "mid", "zeta"] + + +@pytest.mark.parametrize("bad", [math.inf, -math.inf, math.nan]) +def test_non_finite_rate_raises_naming_the_key(bad): + with pytest.raises(ValueError, match="broken-model"): + render_pricing({"broken-model": ModelPricing(bad, 2.0, 3.0, 4.0)}) + + +def test_high_precision_rate_survives_verbatim(): + rendered = render_pricing({"precise": ModelPricing(1.030776, 2.061552, 1.030776, 0.085898)}) + assert "0.085898" in rendered + assert "1.030776" in rendered + + +def test_all_zero_free_model_is_emitted_not_skipped(): + """The skip predicate is `per_request_billing`, never falsiness — a free + model is a real rate and must price as $0, not render as unpriced.""" + rendered = render_pricing({"free-model": ModelPricing(0.0, 0.0, 0.0, 0.0)}) + assert '"free-model": {' in rendered + + +def test_key_needing_escaping_is_emitted_as_valid_json(): + rendered = render_pricing({'weird"key\\x': ORDINARY}) + assert r'"weird\"key\\x": {' in rendered + + +def test_generated_header_marks_the_file_as_generated(): + rendered = render_pricing({"plain-model": ORDINARY}) + assert rendered.startswith("// generated by `make pricing-mirror` — do not edit") + assert rendered.endswith("};\n") + assert 'import type { Pricing } from "./pricing";' in rendered + + +def test_builtin_rates_is_read_only_and_populated(): + rates = builtin_rates() + assert "claude-sonnet-5" in rates + with pytest.raises(TypeError): + rates["claude-sonnet-5"] = ORDINARY # type: ignore[index] + + +class TestTheSecondExemptionAxis: + """`DELIBERATELY_UNMIRRORED` — priced in Python, not priced on the frontend. + + Separate from `per_request_billing` because the two make different claims: a + routed rate MUST NOT be mirrored (an estimate would displace a real captured + figure), whereas these are simply not worth pricing on the board. Folding them + into one axis is what silently widened the generated table past the hand-copy + it replaced. + """ + + def test_an_exempted_id_is_omitted_but_still_priced_in_python(self): + exempt = next(iter(DELIBERATELY_UNMIRRORED)) + assert exempt in builtin_rates() + assert f'"{exempt}": {{' not in render_pricing() + + def test_the_generated_file_names_both_axes_separately(self): + rendered = render_pricing() + assert "per_request_billing" in rendered + assert "DELIBERATELY_UNMIRRORED" in rendered + + def test_a_stale_exemption_fails_the_build(self): + """The guard the deleted parity test carried, and the reason it mattered: + an id that has left `pricing.py` silences nothing and only survives to be + copied.""" + with pytest.raises(ValueError, match=re.escape("gpt-5.4-pro")): + _assert_exemptions_are_live({"claude-sonnet-5": ORDINARY}) + + def test_a_live_exemption_set_passes(self): + _assert_exemptions_are_live(builtin_rates()) + + def test_the_generated_table_reproduces_what_the_hand_copy_priced(self): + """The mirror must not silently PRICE a model the frontend deliberately did + not. Stated as the set identity rather than a literal key list, so an + ordinary reprice stays a one-file edit and only an exemption change moves it. + """ + rendered = render_pricing() + keys = {line.split('"')[1] for line in rendered.splitlines() if line.startswith(' "')} + rates = builtin_rates() + routed = {k for k, v in rates.items() if v.per_request_billing} + assert keys == rates.keys() - routed - DELIBERATELY_UNMIRRORED diff --git a/tests/test_pricing_registry.py b/tests/test_pricing_registry.py index 928e29f13..b9e0123f1 100644 --- a/tests/test_pricing_registry.py +++ b/tests/test_pricing_registry.py @@ -119,3 +119,26 @@ def test_all_zero_rate_resolves_and_blocks_shadowing(): assert calculate_cost("free-1", 1_000_000, 1_000_000) == 0.0 with pytest.raises(ValueError, match="already registered"): register_pricing({"free-1": ModelPricing(1.0, 0.0, 0.0, 0.0)}) + + +def test_four_positional_args_still_construct_with_per_request_defaulted(): + """`per_request_billing` is defaulted and last, so out-of-tree plugin rate + cards (coder_eval_uipath/pricing.py) keep working unchanged.""" + assert ModelPricing(1.0, 2.0, 3.0, 4.0).per_request_billing is False + + +def test_conflicting_per_request_billing_is_a_real_conflict(): + """Two plugins disagreeing about whether a model is per-request billed is a + price disagreement: it decides whether a frontend shows an estimate or the + provider's actual apportioned bill.""" + register_pricing({"acme-1": ModelPricing(1.0, 2.0, 3.0, 4.0)}) + with pytest.raises(ValueError, match="refusing to shadow"): + register_pricing({"acme-1": ModelPricing(1.0, 2.0, 3.0, 4.0, per_request_billing=True)}) + + +def test_per_request_billing_model_is_still_priced_in_python(): + """The flag is metadata for the frontend; Python keeps pricing the model so + the `max_usd` pre-flight still has a figure to work from.""" + assert is_priced("moonshotai/kimi-k3") + # 1M uncached input at $3/MTok. + assert calculate_cost("moonshotai/kimi-k3", 1_000_000, 0) == 3.0 diff --git a/tests/test_replicate_stats.py b/tests/test_replicate_stats.py index e0c990128..089d70c5a 100644 --- a/tests/test_replicate_stats.py +++ b/tests/test_replicate_stats.py @@ -1,8 +1,8 @@ -"""Unit tests for replicate statistics helpers in reports_stats.""" +"""Unit tests for replicate statistics helpers in coder_eval.stats.""" import pytest -from coder_eval.reports_stats import ( +from coder_eval.stats import ( bootstrap_mean_ci, cohens_d, paired_t_ci, diff --git a/tests/test_reports.py b/tests/test_reports.py index 0b8df6347..18e9cb47d 100644 --- a/tests/test_reports.py +++ b/tests/test_reports.py @@ -1179,7 +1179,7 @@ def test_aggregate_command_statistics_nested_layout(tmp_path): def test_report_generator_private_methods_used_by_experiment_reports(): - """Verify all private methods called by reports_experiment.py exist on ReportGenerator.""" + """Verify all private methods called by reports/experiment.py exist on ReportGenerator.""" required_methods = [ "_generate_generation_metrics_section", "_generate_token_usage_section", @@ -1386,9 +1386,9 @@ def test_an_ordinary_graded_run_is_untouched(self): class TestTheGenerationMetricsBuckets: """The markdown table's four bucket columns, READ off the row projection. - `reports.py` neither sums nor validates anything here: the numbers are - computed once by `reports_stats.turn_time_buckets` and carried as - task-level keys by `reports_experiment.eval_result_to_task_dict`. The rows + `reports/markdown.py` neither sums nor validates anything here: the numbers are + computed once by `result_metrics.turn_time_buckets` and carried as + task-level keys by `run_record.eval_result_to_task_dict`. The rows below are that projection's shape, not a `TurnRecord`. """ @@ -1476,3 +1476,62 @@ def test_an_unmeasured_average_is_omitted(self): def test_an_ordinary_average_is_unchanged(self): lines = self._section(150.0, total=450.0) assert any("**Average Command Time**: 150.0ms" in line for line in lines) + + +class TestSlowestCommandsTruncation: + """The markdown renderer truncates `parameters` at SLOW_PARAMS_PREVIEW_CHARS. + + Asserted against the CONSTANT, not the literal 50 it used to hardcode, so the + test still pins the behaviour if the constant moves. reports/html.py already + read the constant; reports/markdown.py — the module that DEFINES it — did not. + """ + + @staticmethod + def _rows(param_len: int) -> list[str]: + from coder_eval.models import CommandStatistics, SlowestCommandInfo + from coder_eval.reports import ReportGenerator + + # `parameters` renders via str(dict), so pad the VALUE until the rendered + # string reaches the wanted length rather than guessing the dict overhead. + overhead = len(str({"cmd": ""})) + stats = CommandStatistics( + total_commands=1, + successful_commands=1, + slowest_commands=[ + SlowestCommandInfo(tool="Bash", duration_ms=1234.0, parameters={"cmd": "x" * (param_len - overhead)}) + ], + ) + return ReportGenerator._generate_command_statistics_section(stats) + + def test_longer_than_the_cap_is_truncated_with_an_ellipsis(self): + from coder_eval.reports.markdown import SLOW_PARAMS_PREVIEW_CHARS + + row = next(line for line in self._rows(SLOW_PARAMS_PREVIEW_CHARS + 40) if line.startswith("| Bash |")) + assert "..." in row + params_cell = row.split("|")[3].strip() + assert len(params_cell) == SLOW_PARAMS_PREVIEW_CHARS + len("...") + + def test_exactly_the_cap_is_not_truncated(self): + from coder_eval.reports.markdown import SLOW_PARAMS_PREVIEW_CHARS + + row = next(line for line in self._rows(SLOW_PARAMS_PREVIEW_CHARS) if line.startswith("| Bash |")) + assert "..." not in row + assert len(row.split("|")[3].strip()) == SLOW_PARAMS_PREVIEW_CHARS + + +class TestReportsDoesNotImportCriteria: + """`reports/markdown.py`'s `criteria` import must stay function-local. + + `coder_eval/criteria/__init__.py` runs pkgutil auto-discovery with registry + side effects; hoisting it would put full criterion discovery on the import + path of every `import coder_eval.reports`. Phase 2 hoisted 18 other locals + and deliberately left this one — this test is what keeps that decision true. + """ + + def test_importing_reports_does_not_pull_in_criteria(self): + import subprocess + import sys + + code = "import coder_eval.reports, sys; print('coder_eval.criteria' in sys.modules)" + out = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True, check=True) + assert out.stdout.strip() == "False", "coder_eval.reports must not import coder_eval.criteria at module level" diff --git a/tests/test_reports_experiment.py b/tests/test_reports_experiment.py deleted file mode 100644 index 02ff1e791..000000000 --- a/tests/test_reports_experiment.py +++ /dev/null @@ -1,147 +0,0 @@ -"""Unit tests for coder_eval.reports_experiment.eval_result_to_task_dict.""" - -from datetime import datetime - -from coder_eval.models import ( - AgentKind, - CommandTelemetry, - EvaluationResult, - FinalStatus, - ResultSummary, - TaskConfigRecord, - TurnRecord, -) -from coder_eval.reports_experiment import eval_result_to_task_dict - - -def _make_result( - *, - resolved: dict | None = None, - turns: list[TurnRecord] | None = None, - task_config: bool = True, -) -> EvaluationResult: - cfg: TaskConfigRecord | None = None - if task_config: - cfg = TaskConfigRecord(resolved=resolved or {}, source_yaml="") - return EvaluationResult( - task_id="t", - task_description="d", - variant_id="v", - agent_type=AgentKind.CLAUDE_CODE, - started_at=datetime.now(), - final_status=FinalStatus.SUCCESS, - iteration_count=0, - task_config=cfg, - turns=turns or [], - ) - - -def _turn(n: int | None) -> TurnRecord: - return TurnRecord(iteration=1, user_input="p", agent_output="a", num_turns=n) - - -def _visible_turn(commands: int = 0, reply: str | None = None) -> TurnRecord: - """TurnRecord with `commands` tool calls and an optional final reply.""" - return TurnRecord( - iteration=1, - user_input="p", - agent_output="a", - commands=[ - CommandTelemetry(tool_name="Bash", tool_id=f"t{i}", timestamp=datetime.now()) for i in range(commands) - ], - result_summary=(ResultSummary(is_error=False, subtype="success", result=reply) if reply is not None else None), - ) - - -class TestVisibleTurns: - def test_counts_tool_calls_plus_final_reply(self): - # 5 tool calls + a final reply = 6 visible turns. - result = _make_result(turns=[_visible_turn(commands=5, reply="done")]) - d = eval_result_to_task_dict(result) - assert d["visible_turns"] == 6 - - def test_no_reply_omits_plus_one(self): - result = _make_result(turns=[_visible_turn(commands=4), _visible_turn(commands=5)]) - d = eval_result_to_task_dict(result) - assert d["visible_turns"] == 9 - - def test_empty_turns(self): - result = _make_result(turns=[]) - d = eval_result_to_task_dict(result) - assert d["visible_turns"] == 0 - - -class TestReplicateIndex: - def test_emits_replicate_index_when_supplied(self): - # Repeated runs share a task_id; the replicate index is what distinguishes - # the rows so downstream consumers (evalboard) don't collapse them to one. - result = _make_result(turns=[]) - assert eval_result_to_task_dict(result, replicate_index=2)["replicate_index"] == 2 - - def test_replicate_index_defaults_to_none(self): - result = _make_result(turns=[]) - assert eval_result_to_task_dict(result)["replicate_index"] is None - - -class TestTotalTurns: - def test_emits_total_turns(self): - result = _make_result(turns=[_turn(2), _turn(3), _turn(4)]) - d = eval_result_to_task_dict(result) - assert d["total_turns"] == 9 - - def test_handles_none(self): - result = _make_result(turns=[_turn(None), _turn(3), _turn(None)]) - d = eval_result_to_task_dict(result) - assert d["total_turns"] == 3 - - def test_empty_turns(self): - result = _make_result(turns=[]) - d = eval_result_to_task_dict(result) - assert d["total_turns"] == 0 - - -class TestExpectedTurnsKey: - def test_emits_when_configured(self): - result = _make_result( - resolved={"run_limits": {"expected_turns": 12}}, - turns=[_turn(5)], - ) - d = eval_result_to_task_dict(result) - assert d["expected_turns"] == 12 - - def test_none_when_unset(self): - result = _make_result( - resolved={"run_limits": {"max_turns": 10}}, - turns=[_turn(5)], - ) - d = eval_result_to_task_dict(result) - assert d["expected_turns"] is None - - def test_none_when_task_config_none(self): - result = _make_result(task_config=False, turns=[_turn(5)]) - d = eval_result_to_task_dict(result) - assert d["expected_turns"] is None - - def test_none_when_invalid_type(self): - result = _make_result( - resolved={"run_limits": {"expected_turns": "ten"}}, - turns=[_turn(5)], - ) - d = eval_result_to_task_dict(result) - assert d["expected_turns"] is None - - def test_none_when_zero(self): - result = _make_result( - resolved={"run_limits": {"expected_turns": 0}}, - turns=[_turn(5)], - ) - d = eval_result_to_task_dict(result) - assert d["expected_turns"] is None - - def test_none_when_run_limits_not_dict(self): - result = _make_result( - resolved={"run_limits": "not-a-dict"}, - turns=[_turn(5)], - ) - d = eval_result_to_task_dict(result) - assert d["expected_turns"] is None diff --git a/tests/test_reports_html.py b/tests/test_reports_html.py index 161a1d376..7afcd31d0 100644 --- a/tests/test_reports_html.py +++ b/tests/test_reports_html.py @@ -1,4 +1,4 @@ -"""Tests for HTML report generation (coder_eval.reports_html).""" +"""Tests for HTML report generation (coder_eval.reports.html).""" from __future__ import annotations @@ -28,14 +28,14 @@ VariantResult, parse_agent_config, ) -from coder_eval.reports_html import ( +from coder_eval.reports import ( HTMLReportGenerator, - _status_badge, safe_write, write_experiment_html, write_task_html, write_variant_html, ) +from coder_eval.reports.html import _status_badge # "ungraded" is the one category whose badge is legitimately neutral: the row @@ -776,7 +776,7 @@ def test_write_experiment_html_uses_safe_write(tmp_path: Path): def test_write_task_html_returns_none_on_render_failure(tmp_path: Path, monkeypatch): """When the renderer blows up, write_task_html returns None rather than raising — so the orchestrator-side emission path cannot mask the run outcome.""" - from coder_eval import reports_html + from coder_eval.reports import html as reports_html def _boom(result): raise RuntimeError("render failed") @@ -1343,10 +1343,11 @@ def test_no_surface_publishes_a_fabricated_zero(self): class TestGenerationMetricsBuckets: """The offline report carries the same four buckets as the evalboard. - `reports_html` is described in CLAUDE.md as the evalboard's static twin, and + `reports/html.py` is a self-contained offline snapshot of a run — no parity + guarantee against the evalboard, and it rendered only Total Latency / Turns / Avg Turn Latency — so anyone reading the artifact rather than the dashboard got none of the wall-clock - accounting. The arithmetic lives in `reports_stats.turn_time_buckets`; this + accounting. The arithmetic lives in `result_metrics.turn_time_buckets`; this asserts the rendering AND, through it, that arithmetic. """ @@ -1429,7 +1430,7 @@ def _turn( ) def test_each_bucket_is_summed_across_turns(self): - from coder_eval.reports_stats import turn_time_buckets + from coder_eval.result_metrics import turn_time_buckets result = _make_result( iterations=[ @@ -1469,7 +1470,7 @@ def test_an_unmeasured_bucket_renders_a_dash_not_zero(self): CE058 enforces in `src/`, and the reason the evalboard's `sumMeasured` returns null. """ - from coder_eval.reports_stats import turn_time_buckets + from coder_eval.result_metrics import turn_time_buckets result = _make_result(iterations=[self._turn(startup=None, teardown=None, generations=[(500, 1500, 800.0)])]) buckets = turn_time_buckets(result) @@ -1486,7 +1487,7 @@ def test_a_measured_zero_still_renders_as_zero(self): Asserted through the RENDERER, not just the arithmetic — the dash is a rendering decision, so its counterexample has to be one too. """ - from coder_eval.reports_stats import turn_time_buckets + from coder_eval.result_metrics import turn_time_buckets result = _make_result(iterations=[self._turn(startup=0.0, teardown=0.0, generations=[(500, 1500, 800.0)])]) assert turn_time_buckets(result).startup_ms == 0.0 @@ -1502,7 +1503,7 @@ def test_an_unmeasured_bucket_still_counts_as_zero_in_the_residual(self): missing time surfaces in Unaccounted rather than vanishing. That is the rule `scripts/timing/decompose_run.py::_turn_buckets` already applies. """ - from coder_eval.reports_stats import turn_time_buckets + from coder_eval.result_metrics import turn_time_buckets result = _make_result(iterations=[self._turn(startup=None, teardown=None, generations=[(500, 1500, 800.0)])]) # 90s task, 800ms of generation, nothing else measured. @@ -1517,7 +1518,7 @@ def test_a_negative_residual_is_rendered_signed_not_clamped(self): past the task's own wall clock, which is what overlapping looks like in the buckets. """ - from coder_eval.reports_stats import turn_time_buckets + from coder_eval.result_metrics import turn_time_buckets result = _make_result( iterations=[self._turn(startup=0.0, teardown=0.0, generations=[(0, 60_000, 60_000.0)], tools=(0, 60_000))] @@ -1536,7 +1537,7 @@ def test_sub_agent_generations_and_their_tools_are_excluded(self): The spawning Agent call's own interval already spans the child's run, so counting either books it twice. """ - from coder_eval.reports_stats import turn_time_buckets + from coder_eval.result_metrics import turn_time_buckets result = _make_result( iterations=[ @@ -1561,7 +1562,7 @@ def test_a_stored_tool_union_renders_the_same_grid_as_a_derived_one(self): path — which every run recorded from now on takes — reaches the same cell. """ - from coder_eval.reports_stats import turn_time_buckets + from coder_eval.result_metrics import turn_time_buckets kwargs = {"startup": 500.0, "teardown": 100.0, "generations": [(500, 1500, 800.0)], "tools": (600, 800)} legacy = _make_result(iterations=[self._turn(**kwargs)]) @@ -1579,7 +1580,7 @@ def test_a_stored_measured_zero_is_not_re_derived(self): so reading the stored value with truthiness instead of `is not None` would silently replace a measurement with a re-derivation. """ - from coder_eval.reports_stats import turn_time_buckets + from coder_eval.result_metrics import turn_time_buckets result = _make_result( iterations=[ @@ -1607,7 +1608,7 @@ def test_a_legacy_record_missing_the_field_entirely_still_validates(self): restored = TurnRecord.model_validate(raw) assert restored.tool_union_ms is None - from coder_eval.reports_stats import turn_time_buckets + from coder_eval.result_metrics import turn_time_buckets assert turn_time_buckets(_make_result(iterations=[restored])).tool_ms == pytest.approx(200.0) @@ -1624,7 +1625,7 @@ def test_a_turn_that_recorded_no_tool_span_has_no_tool_total(self): from one that ran none, so the presence of a SPAN decides — the same None-vs-0 distinction CE058 enforces in `src/`. """ - from coder_eval.reports_stats import turn_time_buckets + from coder_eval.result_metrics import turn_time_buckets result = _make_result(iterations=[self._turn(startup=500.0, teardown=100.0, generations=[(500, 1500, 800.0)])]) assert turn_time_buckets(result).tool_ms is None @@ -1637,7 +1638,7 @@ def test_a_run_with_no_duration_has_no_residual(self): be on the value. The evalboard keeps its own residual null for exactly this case. """ - from coder_eval.reports_stats import turn_time_buckets + from coder_eval.result_metrics import turn_time_buckets result = _make_result(iterations=[self._turn(startup=500.0, teardown=100.0, generations=[(500, 1500, 800.0)])]) result.duration_seconds = 0.0 @@ -1645,9 +1646,113 @@ def test_a_run_with_no_duration_has_no_residual(self): assert self._stat(HTMLReportGenerator().generate_task_html(result), "Unaccounted") == "—" def test_a_run_with_no_turns_does_not_raise(self): - from coder_eval.reports_stats import turn_time_buckets + from coder_eval.result_metrics import turn_time_buckets buckets = turn_time_buckets(_make_result(iterations=[])) assert buckets.generation_ms is None assert buckets.tool_ms is None HTMLReportGenerator().generate_task_html(_make_result(iterations=[])) + + +class TestVariantTokenUsageTotal: + """The variant Token Usage card's Total is TokenUsage.total_tokens, summed. + + It used to re-derive the formula inline (input + output + cacheWrite + + cacheRead) — a fourth home for arithmetic the model already owns. Cache + buckets are non-zero here, which is the case where a wrong formula diverges. + """ + + @staticmethod + def _results() -> list: + from datetime import datetime + + from coder_eval.models import AgentKind, EvaluationResult, FinalStatus, TokenUsage + + usages = [ + TokenUsage( + uncached_input_tokens=100, + output_tokens=200, + cache_creation_input_tokens=300, + cache_read_input_tokens=400, + ), + TokenUsage( + uncached_input_tokens=7, + output_tokens=11, + cache_creation_input_tokens=13, + cache_read_input_tokens=17, + ), + ] + return [ + EvaluationResult( + task_id=f"t{i}", + task_description="d", + agent_type=AgentKind.NONE, + started_at=datetime(2026, 1, 1), + final_status=FinalStatus.SUCCESS, + iteration_count=1, + total_token_usage=u, + ) + for i, u in enumerate(usages) + ] + + def test_total_equals_the_sum_of_total_tokens(self): + from coder_eval.reports.html import _render_variant_token_usage + + results = self._results() + expected = sum(r.total_token_usage.total_tokens for r in results) + # 100+200+300+400 + 7+11+13+17 — every bucket contributes. + assert expected == 1048 + assert f'
{expected:,}
' in _render_variant_token_usage(results) + + def test_no_usages_renders_nothing(self): + from coder_eval.reports.html import _render_variant_token_usage + + assert _render_variant_token_usage([]) == "" + + +class TestSlowestCommandsTruncationHtml: + """The HTML renderer truncates `parameters` at SLOW_PARAMS_PREVIEW_CHARS. + + The markdown twin got both a truncation and a boundary test when the + constant was introduced; the HTML side got neither, so the `+= "..."` + branch was the one uncovered line in the renderer. + """ + + @staticmethod + def _params_cell(param_len: int) -> str: + import re + from html import unescape + + from coder_eval.reports.html import _render_command_stats + + # `parameters` renders via str(dict), so pad the VALUE until the rendered + # string reaches the wanted length rather than guessing the dict overhead. + overhead = len(str({"cmd": ""})) + stats = CommandStatistics( + total_commands=1, + successful_commands=1, + slowest_commands=[ + SlowestCommandInfo(tool="Bash", duration_ms=1234.0, parameters={"cmd": "x" * (param_len - overhead)}) + ], + ) + cell = re.search(r"(.*?)", _render_command_stats(stats)) + assert cell is not None, "the slowest-commands row did not render" + # `str(dict)` emits single quotes, which _esc renders as ' — so the + # raw cell is longer than the preview. Measure the unescaped text. + return unescape(cell.group(1)) + + def test_longer_than_the_cap_is_truncated_with_an_ellipsis(self): + from coder_eval.reports.markdown import SLOW_PARAMS_PREVIEW_CHARS + + cell = self._params_cell(SLOW_PARAMS_PREVIEW_CHARS + 40) + assert cell.endswith("...") + assert len(cell) == SLOW_PARAMS_PREVIEW_CHARS + len("...") + + def test_exactly_the_cap_is_not_truncated(self): + """The predicate is `>`, so the boundary must NOT gain an ellipsis — + this is the case that catches a `>=` typo.""" + from coder_eval.reports.markdown import SLOW_PARAMS_PREVIEW_CHARS + + cell = self._params_cell(SLOW_PARAMS_PREVIEW_CHARS) + assert "..." not in cell + assert len(cell) == SLOW_PARAMS_PREVIEW_CHARS diff --git a/tests/test_reports_junit.py b/tests/test_reports_junit.py index 0785f1cbf..3d47ce425 100644 --- a/tests/test_reports_junit.py +++ b/tests/test_reports_junit.py @@ -1,4 +1,4 @@ -"""Unit tests for the disk-driven JUnit XML writer (``reports_junit``). +"""Unit tests for the disk-driven JUnit XML writer (``reports/junit.py``). All tests are hermetic: the run directory is built by hand under ``tmp_path`` (no agents, no API). Test-side XML parsing uses ``defusedxml`` as @@ -17,7 +17,7 @@ from defusedxml.ElementTree import fromstring from coder_eval.models import SuiteRollup, ThresholdCheck -from coder_eval.reports_junit import generate_junit_xml, write_junit_xml +from coder_eval.reports import generate_junit_xml, write_junit_xml def _props(case: Any) -> dict[str, str]: @@ -672,7 +672,7 @@ def test_nested_task_id_with_dotdot_still_cannot_escape(write_run_json: Callable def test_informational_criterion_not_rendered_as_failure(write_run_json: Callable[..., Path], tmp_path: Path) -> None: """A non-gating (informational) criterion below its threshold must render as [INFO], never [FAIL] — it is excluded from the score/gate, so it cannot be - the failure cause (mirrors reports.py's `if not cr.gating`).""" + the failure cause (mirrors reports/markdown.py's `if not cr.gating`).""" run_dir = tmp_path / "run" rows = [_row("t_fail", "FAILURE", variant_id="v1", replicate_index=0)] write_run_json(run_dir, rows) @@ -724,7 +724,7 @@ def test_parity_real_producer_output_through_writer(write_run_json: Callable[... Every other test builds rows via the synthetic ``_row`` helper, which hand-copies the keys the writer reads. This one runs the actual producer - (``reports_experiment.eval_result_to_task_dict``, the batch.py path) so a + (``run_record.eval_result_to_task_dict``, the batch.py path) so a producer-side rename of ``status`` / ``task_path`` / ``total_cost_usd`` / ``model_used`` / ``total_tokens`` / ``visible_turns`` / ``weighted_score`` (RunSummary.task_results is an untyped ``list[dict[str, Any]]``) can no longer @@ -734,7 +734,7 @@ def test_parity_real_producer_output_through_writer(write_run_json: Callable[... from datetime import datetime from coder_eval.models import AgentKind, EvaluationResult, FinalStatus, TokenUsage - from coder_eval.reports_experiment import eval_result_to_task_dict + from coder_eval.run_record import eval_result_to_task_dict result = EvaluationResult( task_id="mytask", diff --git a/tests/test_reports_package.py b/tests/test_reports_package.py new file mode 100644 index 000000000..7f6b43e80 --- /dev/null +++ b/tests/test_reports_package.py @@ -0,0 +1,83 @@ +"""Layering and packaging invariants for the `coder_eval.reports` package. + +These are the things the Phase 4 split is *for*, and each is otherwise +unobservable: the package's public surface, and the two dependencies it must +not have. +""" + +import subprocess +import sys + +import pytest + +import coder_eval.reports as pkg + + +class TestPublicSurface: + def test_every_name_in_all_is_bound(self): + """An `__all__` entry with no matching import makes `from ... import *` + raise. (A submodule RENAME fails earlier, at this module's own import.)""" + missing = sorted(n for n in pkg.__all__ if not hasattr(pkg, n)) + assert not missing, f"declared in __all__ but not bound on the package: {missing}" + + def test_every_writer_ce066_allowlists_is_public_here(self): + """The two lists are one contract seen from opposite sides: CE066 permits + core to import exactly these, so each must be part of the package's + declared surface, not merely reachable through a submodule.""" + from tests.lint.rules.ce066_no_report_imports_in_core import ALLOWED_WRITERS + + assert set(pkg.__all__) >= ALLOWED_WRITERS, sorted(ALLOWED_WRITERS - set(pkg.__all__)) + + def test_an_internal_rendering_constant_is_not_reachable(self): + """SLOW_PARAMS_PREVIEW_CHARS is a private layout decision; `.html` imports + it from `.markdown` directly. Asserted with hasattr rather than against + `__all__`, so a stray re-export or star-import is caught too.""" + assert not hasattr(pkg, "SLOW_PARAMS_PREVIEW_CHARS") + + +class TestTheLeafHasNoUnwantedDependencies: + """Assertions run in a FRESH interpreter: this process has already imported + half the package, so `sys.modules` here would prove nothing. + """ + + @staticmethod + def _imports(imported: str, module: str) -> bool: + code = f"import {imported}, sys; print({module!r} in sys.modules)" + out = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True, check=True) + return out.stdout.strip() == "True" + + def test_durations_is_free_of_the_agent_sdk(self): + """This is what moving `format_ms` out of `formatting.py` buys. + + NOT the same as `coder_eval.reports` being SDK-free — it cannot be: + `coder_eval.models.agent_config` imports `ClaudeAgentOptions`, and every + report module needs `models`. What the split achieves is that the + FORMATTER is usable without the SDK, and that the reports package no + longer reaches through the SDK-shaped `formatting` module for it. + """ + assert not self._imports("coder_eval.durations", "claude_agent_sdk") + + def test_reports_does_not_import_the_sdk_shaped_formatting_module(self): + assert not self._imports("coder_eval.reports", "coder_eval.formatting") + + def test_reports_does_not_import_criteria(self): + """`markdown.py`'s criteria import must stay function-local — it runs + pkgutil auto-discovery with registry side effects.""" + assert not self._imports("coder_eval.reports", "coder_eval.criteria") + + +@pytest.mark.parametrize("submodule", ["markdown", "html", "experiment", "junit", "helpers"]) +def test_no_submodule_imports_the_package_by_name(submodule): + """`from . import X` / `from coder_eval.reports import X` inside a submodule + re-enters __init__ mid-initialization. Every intra-package import must name + a sibling module directly.""" + import ast + from pathlib import Path + + path = Path(__file__).parent.parent / "src" / "coder_eval" / "reports" / f"{submodule}.py" + tree = ast.parse(path.read_text(encoding="utf-8")) + for node in ast.walk(tree): + if isinstance(node, ast.ImportFrom): + # `from . import X` is level=1 with module None; `from coder_eval.reports import X` is absolute. + assert not (node.level == 1 and node.module is None), f"{submodule}.py re-enters the package __init__" + assert node.module != "coder_eval.reports", f"{submodule}.py re-enters the package __init__" diff --git a/tests/test_reports_stats.py b/tests/test_reports_stats.py deleted file mode 100644 index 825b31f4d..000000000 --- a/tests/test_reports_stats.py +++ /dev/null @@ -1,138 +0,0 @@ -"""Unit tests for shared report helpers in coder_eval.reports_stats.""" - -from datetime import datetime - -from coder_eval.analysis import calculate_command_statistics -from coder_eval.models import ( - AgentKind, - CommandTelemetry, - EvaluationResult, - FinalStatus, - ResultSummary, - TaskConfigRecord, - TurnRecord, -) -from coder_eval.reports_stats import expected_turns_overage, has_final_reply, visible_turn_count - - -def _make_result( - *, - resolved: dict | None = None, - turns: list[TurnRecord] | None = None, - task_config: bool = True, -) -> EvaluationResult: - cfg: TaskConfigRecord | None = None - if task_config: - cfg = TaskConfigRecord(resolved=resolved or {}, source_yaml="") - return EvaluationResult( - task_id="t", - task_description="d", - variant_id="v", - agent_type=AgentKind.CLAUDE_CODE, - started_at=datetime.now(), - final_status=FinalStatus.SUCCESS, - iteration_count=0, - task_config=cfg, - turns=turns or [], - ) - - -def _cmd(idx: int) -> CommandTelemetry: - return CommandTelemetry( - tool_name="Bash", - tool_id=f"t{idx}", - timestamp=datetime.now(), - ) - - -def _turn(commands: int = 0, reply: str | None = None) -> TurnRecord: - """Build a TurnRecord with `commands` tool calls and an optional final reply.""" - return TurnRecord( - iteration=1, - user_input="p", - agent_output="a", - commands=[_cmd(i) for i in range(commands)], - result_summary=(ResultSummary(is_error=False, subtype="success", result=reply) if reply is not None else None), - ) - - -class TestExpectedTurnsOverage: - def test_strict_greater_than(self): - # 5 tools + reply = 6 visible turns. Budget 6 → no overage (equal). - result = _make_result( - resolved={"run_limits": {"expected_turns": 6}}, - turns=[_turn(commands=5, reply="done")], - ) - assert expected_turns_overage(result) is None - - # 5 tools + reply = 6 visible turns. Budget 5 → overage (6 > 5). - result = _make_result( - resolved={"run_limits": {"expected_turns": 5}}, - turns=[_turn(commands=5, reply="done")], - ) - assert expected_turns_overage(result) == (6, 5) - - def test_missing_reply_skipped(self): - # Tools across multiple iterations sum correctly; absent reply - # contributes nothing (no +1). - result = _make_result( - resolved={"run_limits": {"expected_turns": 5}}, - turns=[_turn(commands=4), _turn(commands=5)], - ) - assert expected_turns_overage(result) == (9, 5) - - def test_task_config_none(self): - result = _make_result(task_config=False, turns=[_turn(commands=10)]) - assert expected_turns_overage(result) is None - - def test_run_limits_missing(self): - result = _make_result(resolved={}, turns=[_turn(commands=10)]) - assert expected_turns_overage(result) is None - - def test_expected_turns_unset(self): - result = _make_result(resolved={"run_limits": {"max_turns": 10}}, turns=[_turn(commands=20)]) - assert expected_turns_overage(result) is None - - def test_invalid_expected_type(self): - result = _make_result(resolved={"run_limits": {"expected_turns": "ten"}}, turns=[_turn(commands=20)]) - assert expected_turns_overage(result) is None - - def test_expected_turns_zero_treated_as_invalid(self): - # Defensive: the model enforces ge=1, but a hand-rolled task.json could - # still inject 0 — the helper must treat it as a disabled check. - result = _make_result(resolved={"run_limits": {"expected_turns": 0}}, turns=[_turn(commands=10)]) - assert expected_turns_overage(result) is None - - def test_run_limits_not_a_dict(self): - result = _make_result(resolved={"run_limits": "not-a-dict"}, turns=[_turn(commands=10)]) - assert expected_turns_overage(result) is None - - def test_empty_turns(self): - result = _make_result(resolved={"run_limits": {"expected_turns": 1}}, turns=[]) - assert expected_turns_overage(result) is None - - -class TestTurnDefinitionMatchesDoc: - """Pin the turn definition: - - visible_turn_count == command_stats.total_commands + (1 if final reply) - - The evalboard "Turns" cell, ``displayedTurns``/``actual_commands``, and the - proposed historical reconstruction all read ``command_stats.total_commands`` - (i.e. the tool-call part of the persisted ``visible_turns`` field). If - someone later changes ``calculate_command_statistics`` to filter commands, - that count would silently diverge from ``visible_turn_count`` and these - cells would drift. This test fails first if that ever happens. - """ - - def test_mixed_tools_with_final_reply(self): - # 2 + 3 tool calls across two iterations, plus a final reply. - result = _make_result(turns=[_turn(commands=2), _turn(commands=3, reply="done")]) - stats = calculate_command_statistics(result.iterations) - assert visible_turn_count(result) == stats.total_commands + (1 if has_final_reply(result) else 0) - - def test_tools_without_final_reply(self): - # Crashed before producing a reply: the +1 must be omitted on both sides. - result = _make_result(turns=[_turn(commands=4)]) - stats = calculate_command_statistics(result.iterations) - assert visible_turn_count(result) == stats.total_commands + (1 if has_final_reply(result) else 0) diff --git a/tests/test_result_metrics.py b/tests/test_result_metrics.py new file mode 100644 index 000000000..a818ae048 --- /dev/null +++ b/tests/test_result_metrics.py @@ -0,0 +1,219 @@ +"""Unit tests for coder_eval.result_metrics — EvaluationResult-derived metrics. + +These are consumed by the ORCHESTRATOR during a run as well as by the reporters, +which is why they are not in a ``reports*`` module. The `None`-vs-`0.0` contract +(CE058) is the load-bearing behaviour: an unmeasured bucket must survive as +`None` so it renders as a dash rather than claiming a measurement nobody took. +""" + +from datetime import datetime + +from coder_eval.analysis import calculate_command_statistics +from coder_eval.models import ( + AgentKind, + AssistantMessage, + CommandTelemetry, + EvaluationResult, + FinalStatus, + ResultSummary, + TaskConfigRecord, + TurnRecord, +) +from coder_eval.result_metrics import ( + TurnTimeBuckets, + expected_turns_overage, + has_final_reply, + turn_time_buckets, + visible_turn_count, +) + + +def _result(turns: list[TurnRecord], duration: float = 0.0) -> EvaluationResult: + return EvaluationResult( + task_id="t", + task_description="d", + agent_type=AgentKind.CLAUDE_CODE, + started_at=datetime(2026, 1, 1), + final_status=FinalStatus.SUCCESS, + iteration_count=len(turns), + iterations=turns, + duration_seconds=duration, + ) + + +def _turn(**kw) -> TurnRecord: + return TurnRecord(**({"iteration": 1, "user_input": "u", "agent_output": "a"} | kw)) + + +def _generation(ms: float) -> AssistantMessage: + at = datetime(2026, 1, 1) + return AssistantMessage(content="x", started_at=at, completed_at=at, generation_duration_ms=ms) + + +class TestTurnTimeBuckets: + def test_every_measured_bucket_is_summed_across_turns(self): + turns = [ + _turn(harness_startup_ms=10.0, harness_teardown_ms=5.0, messages=[_generation(20.0)]), + _turn(harness_startup_ms=1.0, harness_teardown_ms=2.0, messages=[_generation(3.0)]), + ] + buckets = turn_time_buckets(_result(turns, duration=1.0)) + assert buckets.startup_ms == 11.0 + assert buckets.teardown_ms == 7.0 + assert buckets.generation_ms == 23.0 + assert buckets.unaccounted_ms == 1000.0 - 11.0 - 23.0 - 7.0 + + def test_tool_time_is_summed_from_the_stored_union(self): + """`tool_union_ms` is the collector's own span set, so reading it is how + this surface and the collector agree rather than merely coincide.""" + turns = [_turn(tool_union_ms=40.0), _turn(tool_union_ms=2.0)] + assert turn_time_buckets(_result(turns, duration=1.0)).tool_ms == 42.0 + + def test_a_stored_zero_tool_union_is_a_measurement_not_a_miss(self): + """Spans were recorded and occupied no measurable time — that is `0ms`, + not a dash, and must not fall through to re-derivation.""" + assert turn_time_buckets(_result([_turn(tool_union_ms=0.0)])).tool_ms == 0.0 + + def test_an_unmeasured_bucket_stays_none_and_is_not_coerced_to_zero(self): + """The CE058 contract — `0.0` would claim a measurement nobody took.""" + buckets = turn_time_buckets(_result([_turn(messages=[_generation(20.0)])])) + assert buckets.startup_ms is None + assert buckets.teardown_ms is None + assert buckets.tool_ms is None + assert buckets.generation_ms == 20.0 + + def test_a_measured_zero_stays_zero(self): + """Measured 0.0 differs from unmeasured — it renders as `0ms`, not a dash.""" + assert turn_time_buckets(_result([_turn(harness_startup_ms=0.0)])).startup_ms == 0.0 + + def test_an_untimed_run_has_no_residual_rather_than_a_negative_one(self): + """duration_seconds == 0.0 means never timed; subtracting real buckets + from it would render a fabricated negative residual.""" + assert turn_time_buckets(_result([_turn(harness_startup_ms=10.0)])).unaccounted_ms is None + + def test_empty_iterations_measures_nothing(self): + assert turn_time_buckets(_result([])) == TurnTimeBuckets(None, None, None, None, None) + + +def _make_result( + *, + resolved: dict | None = None, + turns: list[TurnRecord] | None = None, + task_config: bool = True, +) -> EvaluationResult: + cfg: TaskConfigRecord | None = None + if task_config: + cfg = TaskConfigRecord(resolved=resolved or {}, source_yaml="") + return EvaluationResult( + task_id="t", + task_description="d", + variant_id="v", + agent_type=AgentKind.CLAUDE_CODE, + started_at=datetime.now(), + final_status=FinalStatus.SUCCESS, + iteration_count=0, + task_config=cfg, + turns=turns or [], + ) + + +def _cmd(idx: int) -> CommandTelemetry: + return CommandTelemetry( + tool_name="Bash", + tool_id=f"t{idx}", + timestamp=datetime.now(), + ) + + +def _turn_with_commands(commands: int = 0, reply: str | None = None) -> TurnRecord: + """Build a TurnRecord with `commands` tool calls and an optional final reply.""" + return TurnRecord( + iteration=1, + user_input="p", + agent_output="a", + commands=[_cmd(i) for i in range(commands)], + result_summary=(ResultSummary(is_error=False, subtype="success", result=reply) if reply is not None else None), + ) + + +class TestExpectedTurnsOverage: + def test_strict_greater_than(self): + # 5 tools + reply = 6 visible turns. Budget 6 → no overage (equal). + result = _make_result( + resolved={"run_limits": {"expected_turns": 6}}, + turns=[_turn_with_commands(commands=5, reply="done")], + ) + assert expected_turns_overage(result) is None + + # 5 tools + reply = 6 visible turns. Budget 5 → overage (6 > 5). + result = _make_result( + resolved={"run_limits": {"expected_turns": 5}}, + turns=[_turn_with_commands(commands=5, reply="done")], + ) + assert expected_turns_overage(result) == (6, 5) + + def test_missing_reply_skipped(self): + # Tools across multiple iterations sum correctly; absent reply + # contributes nothing (no +1). + result = _make_result( + resolved={"run_limits": {"expected_turns": 5}}, + turns=[_turn_with_commands(commands=4), _turn_with_commands(commands=5)], + ) + assert expected_turns_overage(result) == (9, 5) + + def test_task_config_none(self): + result = _make_result(task_config=False, turns=[_turn_with_commands(commands=10)]) + assert expected_turns_overage(result) is None + + def test_run_limits_missing(self): + result = _make_result(resolved={}, turns=[_turn_with_commands(commands=10)]) + assert expected_turns_overage(result) is None + + def test_expected_turns_unset(self): + result = _make_result(resolved={"run_limits": {"max_turns": 10}}, turns=[_turn_with_commands(commands=20)]) + assert expected_turns_overage(result) is None + + def test_invalid_expected_type(self): + result = _make_result( + resolved={"run_limits": {"expected_turns": "ten"}}, turns=[_turn_with_commands(commands=20)] + ) + assert expected_turns_overage(result) is None + + def test_expected_turns_zero_treated_as_invalid(self): + # Defensive: the model enforces ge=1, but a hand-rolled task.json could + # still inject 0 — the helper must treat it as a disabled check. + result = _make_result(resolved={"run_limits": {"expected_turns": 0}}, turns=[_turn_with_commands(commands=10)]) + assert expected_turns_overage(result) is None + + def test_run_limits_not_a_dict(self): + result = _make_result(resolved={"run_limits": "not-a-dict"}, turns=[_turn_with_commands(commands=10)]) + assert expected_turns_overage(result) is None + + def test_empty_turns(self): + result = _make_result(resolved={"run_limits": {"expected_turns": 1}}, turns=[]) + assert expected_turns_overage(result) is None + + +class TestTurnDefinitionMatchesDoc: + """Pin the turn definition: + + visible_turn_count == command_stats.total_commands + (1 if final reply) + + The evalboard "Turns" cell, ``displayedTurns``/``actual_commands``, and the + proposed historical reconstruction all read ``command_stats.total_commands`` + (i.e. the tool-call part of the persisted ``visible_turns`` field). If + someone later changes ``calculate_command_statistics`` to filter commands, + that count would silently diverge from ``visible_turn_count`` and these + cells would drift. This test fails first if that ever happens. + """ + + def test_mixed_tools_with_final_reply(self): + # 2 + 3 tool calls across two iterations, plus a final reply. + result = _make_result(turns=[_turn_with_commands(commands=2), _turn_with_commands(commands=3, reply="done")]) + stats = calculate_command_statistics(result.iterations) + assert visible_turn_count(result) == stats.total_commands + (1 if has_final_reply(result) else 0) + + def test_tools_without_final_reply(self): + # Crashed before producing a reply: the +1 must be omitted on both sides. + result = _make_result(turns=[_turn_with_commands(commands=4)]) + stats = calculate_command_statistics(result.iterations) + assert visible_turn_count(result) == stats.total_commands + (1 if has_final_reply(result) else 0) diff --git a/tests/test_run_limits_orchestrator.py b/tests/test_run_limits_orchestrator.py index 7ef8a9360..86760d485 100644 --- a/tests/test_run_limits_orchestrator.py +++ b/tests/test_run_limits_orchestrator.py @@ -328,7 +328,7 @@ def _invoke_finalize(orch: Orchestrator) -> None: # The report_path lives under tmp_path so the write_text call lands # in a real (test-scoped) file and we don't need to mock pathlib. with ( - _patch("coder_eval.reports_html.write_task_html", return_value=None), + _patch("coder_eval.reports.write_task_html", return_value=None), _patch("coder_eval.evaluation.judge_persistence.spill_judge_transcripts", return_value=None), ): orch._finalize_result(_time.time()) diff --git a/tests/test_run_record.py b/tests/test_run_record.py new file mode 100644 index 000000000..c2d0f82c3 --- /dev/null +++ b/tests/test_run_record.py @@ -0,0 +1,273 @@ +"""Characterization + layering tests for the run.json row serializer. + +``eval_result_to_task_dict`` moved out of the experiment reporter into +``coder_eval.run_record`` — its *home* changed, its *body* did not. The snapshot +below was captured from the pre-move function, so any difference is a mistake in +the move rather than an intended change. It also pins the run.json key set, which +the evalboard and every archived run depend on. +""" + +import ast +from datetime import datetime +from pathlib import Path + +from coder_eval.models import ( + AgentKind, + CommandTelemetry, + EvaluationResult, + FinalStatus, + ResultSummary, + TaskConfigRecord, + TokenUsage, + TurnRecord, +) +from coder_eval.run_record import eval_result_to_task_dict + + +REPO_ROOT = Path(__file__).parent.parent + + +EXPECTED_ROW = { + "actual_commands": None, + "agent_config": None, + "agent_cost_usd": 0.5, + "cache_creation_input_tokens": 300, + "cache_read_input_tokens": 400, + "commands_efficiency": None, + "cost_complete": True, + "duration": 3.0, + "early_stop_reason": None, + "error_category": None, + "error_message": None, + "expected_commands": None, + "expected_turns": None, + "expected_turns_overage": None, + "gate_threshold": None, + "generation_ms": None, + "has_final_reply": False, + "input_tokens": 100, + "installed_tools": None, + "iteration_count": 1, + "iterations": [ + { + "assistant_turn_count": 0, + "command_count": 0, + "crash_reason": None, + "crashed": False, + "duration_seconds": 2.5, + "iteration": 1, + } + ], + "judge_cost_usd": None, + "max_turns_exhausted": False, + "model_used": "claude-haiku-4-5", + "output_tokens": 200, + "reference_similarity": None, + "replicate_index": 2, + "sdk_options": None, + "simulator_cost_usd": None, + "startup_ms": None, + "status": "SUCCESS", + "stopped_early": False, + "tags": [], + "task_id": "char-task", + "task_path": None, + "teardown_ms": None, + "tool_ms": None, + "total_cost_usd": 0.5, + "total_tokens": 1000, + "total_turns": 0, + "turns_remaining_at_stop": None, + "variant_id": None, + "visible_turns": 0, + "weighted_score": 0.75, +} + + +def _result() -> EvaluationResult: + usage = TokenUsage( + uncached_input_tokens=100, + output_tokens=200, + cache_creation_input_tokens=300, + cache_read_input_tokens=400, + total_cost_usd=0.5, + ) + turn = TurnRecord(iteration=1, user_input="u", agent_output="a", token_usage=usage, duration_seconds=2.5) + return EvaluationResult( + task_id="char-task", + task_description="characterization", + agent_type=AgentKind.CLAUDE_CODE, + started_at=datetime(2026, 1, 2, 3, 4, 5), + final_status=FinalStatus.SUCCESS, + iteration_count=1, + iterations=[turn], + weighted_score=0.75, + model_used="claude-haiku-4-5", + duration_seconds=3.0, + total_token_usage=usage, + ) + + +class TestSerializerIsUnchangedByTheMove: + def test_row_matches_the_pre_move_snapshot(self): + assert eval_result_to_task_dict(_result(), replicate_index=2) == EXPECTED_ROW + + def test_the_run_json_key_set_is_unchanged(self): + """Keys are the contract the evalboard and archived runs read.""" + assert set(eval_result_to_task_dict(_result()).keys()) == set(EXPECTED_ROW) + + def test_input_tokens_carries_the_uncached_slice_not_the_derived_total(self): + """Same word, two quantities — the key is NOT TokenUsage.input_tokens.""" + row = eval_result_to_task_dict(_result()) + assert row["input_tokens"] == 100 + assert _result().total_token_usage.input_tokens == 800 + + +class TestRunRecordIsNotInTheReportsLayer: + """The whole point of the move — otherwise unobservable. + + Carrying the run.json serializer in the reports layer was the only reason + ``orchestration/batch.py`` imported from ``reports*`` at all. + """ + + def test_module_imports_nothing_from_the_reports_layer(self): + source = (REPO_ROOT / "src" / "coder_eval" / "run_record.py").read_text(encoding="utf-8") + offenders = [ + node.module + for node in ast.walk(ast.parse(source)) + if isinstance(node, ast.ImportFrom) and node.module and "reports" in node.module + ] + assert not offenders, f"run_record must not import the reports layer, found: {offenders}" + + +def _make_result( + *, + resolved: dict | None = None, + turns: list[TurnRecord] | None = None, + task_config: bool = True, +) -> EvaluationResult: + cfg: TaskConfigRecord | None = None + if task_config: + cfg = TaskConfigRecord(resolved=resolved or {}, source_yaml="") + return EvaluationResult( + task_id="t", + task_description="d", + variant_id="v", + agent_type=AgentKind.CLAUDE_CODE, + started_at=datetime.now(), + final_status=FinalStatus.SUCCESS, + iteration_count=0, + task_config=cfg, + turns=turns or [], + ) + + +def _turn_with_expected(n: int | None) -> TurnRecord: + return TurnRecord(iteration=1, user_input="p", agent_output="a", num_turns=n) + + +def _visible_turn_with_expected(commands: int = 0, reply: str | None = None) -> TurnRecord: + """TurnRecord with `commands` tool calls and an optional final reply.""" + return TurnRecord( + iteration=1, + user_input="p", + agent_output="a", + commands=[ + CommandTelemetry(tool_name="Bash", tool_id=f"t{i}", timestamp=datetime.now()) for i in range(commands) + ], + result_summary=(ResultSummary(is_error=False, subtype="success", result=reply) if reply is not None else None), + ) + + +class TestVisibleTurns: + def test_counts_tool_calls_plus_final_reply(self): + # 5 tool calls + a final reply = 6 visible turns. + result = _make_result(turns=[_visible_turn_with_expected(commands=5, reply="done")]) + d = eval_result_to_task_dict(result) + assert d["visible_turns"] == 6 + + def test_no_reply_omits_plus_one(self): + result = _make_result(turns=[_visible_turn_with_expected(commands=4), _visible_turn_with_expected(commands=5)]) + d = eval_result_to_task_dict(result) + assert d["visible_turns"] == 9 + + def test_empty_turns(self): + result = _make_result(turns=[]) + d = eval_result_to_task_dict(result) + assert d["visible_turns"] == 0 + + +class TestReplicateIndex: + def test_emits_replicate_index_when_supplied(self): + # Repeated runs share a task_id; the replicate index is what distinguishes + # the rows so downstream consumers (evalboard) don't collapse them to one. + result = _make_result(turns=[]) + assert eval_result_to_task_dict(result, replicate_index=2)["replicate_index"] == 2 + + def test_replicate_index_defaults_to_none(self): + result = _make_result(turns=[]) + assert eval_result_to_task_dict(result)["replicate_index"] is None + + +class TestTotalTurns: + def test_emits_total_turns(self): + result = _make_result(turns=[_turn_with_expected(2), _turn_with_expected(3), _turn_with_expected(4)]) + d = eval_result_to_task_dict(result) + assert d["total_turns"] == 9 + + def test_handles_none(self): + result = _make_result(turns=[_turn_with_expected(None), _turn_with_expected(3), _turn_with_expected(None)]) + d = eval_result_to_task_dict(result) + assert d["total_turns"] == 3 + + def test_empty_turns(self): + result = _make_result(turns=[]) + d = eval_result_to_task_dict(result) + assert d["total_turns"] == 0 + + +class TestExpectedTurnsKey: + def test_emits_when_configured(self): + result = _make_result( + resolved={"run_limits": {"expected_turns": 12}}, + turns=[_turn_with_expected(5)], + ) + d = eval_result_to_task_dict(result) + assert d["expected_turns"] == 12 + + def test_none_when_unset(self): + result = _make_result( + resolved={"run_limits": {"max_turns": 10}}, + turns=[_turn_with_expected(5)], + ) + d = eval_result_to_task_dict(result) + assert d["expected_turns"] is None + + def test_none_when_task_config_none(self): + result = _make_result(task_config=False, turns=[_turn_with_expected(5)]) + d = eval_result_to_task_dict(result) + assert d["expected_turns"] is None + + def test_none_when_invalid_type(self): + result = _make_result( + resolved={"run_limits": {"expected_turns": "ten"}}, + turns=[_turn_with_expected(5)], + ) + d = eval_result_to_task_dict(result) + assert d["expected_turns"] is None + + def test_none_when_zero(self): + result = _make_result( + resolved={"run_limits": {"expected_turns": 0}}, + turns=[_turn_with_expected(5)], + ) + d = eval_result_to_task_dict(result) + assert d["expected_turns"] is None + + def test_none_when_run_limits_not_dict(self): + result = _make_result( + resolved={"run_limits": "not-a-dict"}, + turns=[_turn_with_expected(5)], + ) + d = eval_result_to_task_dict(result) + assert d["expected_turns"] is None diff --git a/tests/test_simulation_integration.py b/tests/test_simulation_integration.py index eeed9962b..36a209127 100644 --- a/tests/test_simulation_integration.py +++ b/tests/test_simulation_integration.py @@ -331,7 +331,7 @@ async def test_standalone_turn_records_the_simulator_call_duration(tmp_path, mon ``TurnRecord.duration_seconds`` defaults to 0.0 and no caller passed it, so this turn reported 0s for a simulator call that really took seconds — and - ``reports_html`` divides by the turn count, halving ``avg_turn`` for every + ``reports/html.py`` divides by the turn count, halving ``avg_turn`` for every simulation task. """ _install_fake_agent(monkeypatch, scenario="success") diff --git a/tests/test_stats.py b/tests/test_stats.py new file mode 100644 index 000000000..48a1de4f5 --- /dev/null +++ b/tests/test_stats.py @@ -0,0 +1,49 @@ +"""Unit tests for coder_eval.stats — the dependency-free numeric core. + +This module imports **only** from ``coder_eval.stats``. That is deliberate and +is itself asserted below: the whole reason the statistics live in their own +module is that they can be reasoned about without dragging in models, timing or +the report layer. +""" + +import ast +from pathlib import Path + +from coder_eval.stats import mean, stddev + + +REPO_ROOT = Path(__file__).parent.parent + + +class TestStatsIsDependencyFree: + """`stats.py` must import nothing from coder_eval, directly or relatively. + + Cheaper as a unit test than a lint rule: it guards exactly one file. If it + ever fails, the numeric core has grown a dependency and stopped being the + thing that can be tested in isolation. + """ + + def test_no_coder_eval_import_of_any_form(self): + source = (REPO_ROOT / "src" / "coder_eval" / "stats.py").read_text(encoding="utf-8") + offenders: list[str] = [] + for node in ast.walk(ast.parse(source)): + if isinstance(node, ast.ImportFrom): + # level > 0 is a relative import (`from .x import y`). + if node.level > 0 or (node.module or "").startswith("coder_eval"): + offenders.append(f"from {'.' * node.level}{node.module or ''}") + elif isinstance(node, ast.Import): + offenders += [a.name for a in node.names if a.name.startswith("coder_eval")] + assert not offenders, f"stats.py must stay dependency-free, found: {offenders}" + + +class TestDependencyFreeBehaviourIsReal: + """A smoke check that the extracted module actually computes. + + Deliberately thin: `test_replicate_stats.py` and `test_experiment_reports.py` + own the numeric coverage, and duplicating it here would mean two places to + update per formula. What this file exists for is the invariant above. + """ + + def test_the_module_computes_without_any_coder_eval_import(self): + assert mean([1.0, 2.0, 3.0]) == 2.0 + assert stddev([1.0, 2.0, 3.0]) == 1.0 diff --git a/tests/test_reports_stats_nonfinite.py b/tests/test_stats_nonfinite.py similarity index 88% rename from tests/test_reports_stats_nonfinite.py rename to tests/test_stats_nonfinite.py index fe9f8ce6e..c7658e12b 100644 --- a/tests/test_reports_stats_nonfinite.py +++ b/tests/test_stats_nonfinite.py @@ -1,4 +1,4 @@ -"""Harness: no public numeric helper in reports_stats may launder NaN/inf into a report. +"""Harness: no public numeric helper in coder_eval.stats may launder NaN/inf into a report. This enumerates the module rather than listing functions by hand, so a helper added later is covered automatically — the failure mode this guards (a new statistic that @@ -15,7 +15,7 @@ import pytest -from coder_eval import reports_stats +from coder_eval import stats # Thin wrappers over the stdlib that intentionally propagate whatever they are given; @@ -27,11 +27,11 @@ def _numeric_helpers(): - """Public reports_stats functions whose parameters are all floats / float lists.""" - for name, fn in vars(reports_stats).items(): + """Public coder_eval.stats functions whose parameters are all floats / float lists.""" + for name, fn in vars(stats).items(): if name.startswith("_") or name in _PASSTHROUGH_HELPERS or not inspect.isfunction(fn): continue - if fn.__module__ != reports_stats.__name__: + if fn.__module__ != stats.__name__: continue hints = inspect.get_annotations(fn, eval_str=True) params = inspect.signature(fn).parameters diff --git a/tests/test_suite_rollup.py b/tests/test_suite_rollup.py index 7adb9c7b8..9151b3b1c 100644 --- a/tests/test_suite_rollup.py +++ b/tests/test_suite_rollup.py @@ -20,12 +20,12 @@ SuiteRollup, TaskResult, ) -from coder_eval.reports import ( +from coder_eval.reports import write_suite_rollups +from coder_eval.reports.markdown import ( _attach_row_accounting, _compute_suite_rollup, _render_criterion_aggregate, _render_suite_markdown, - write_suite_rollups, ) @@ -308,7 +308,7 @@ def test_skips_non_dataset_tasks(self, tmp_path: Path) -> None: def test_failed_samples_capped(self, tmp_path: Path) -> None: # Generate more failed rows than the cap to confirm truncation. - from coder_eval.reports import _FAILED_SAMPLE_LIMIT + from coder_eval.reports.markdown import _FAILED_SAMPLE_LIMIT rows = [ _make_row( diff --git a/tests/test_teardown_interrupt.py b/tests/test_teardown_interrupt.py index 4f28882e3..ff5920198 100644 --- a/tests/test_teardown_interrupt.py +++ b/tests/test_teardown_interrupt.py @@ -42,7 +42,7 @@ def _build_orchestrator(tmp_path: Path) -> Orchestrator: def _patch_finalize_persistence(): """Skip the on-disk persistence side-effects of _finalize_result.""" - return patch("coder_eval.reports_html.write_task_html", return_value=None) + return patch("coder_eval.reports.write_task_html", return_value=None) @pytest.mark.asyncio diff --git a/tests/test_threshold_enforcement.py b/tests/test_threshold_enforcement.py index 4a31a7389..ac5d6b1c7 100644 --- a/tests/test_threshold_enforcement.py +++ b/tests/test_threshold_enforcement.py @@ -200,7 +200,7 @@ def test_criterion_result_defaults_to_gating(self): def test_html_report_labels_informational_and_excludes_it_from_the_count(self): """The HTML header counts gating criteria only, and the row says why.""" - from coder_eval.reports_html import _render_criteria + from coder_eval.reports.html import _render_criteria html = _render_criteria( [ diff --git a/tests/test_ungraded_reporting.py b/tests/test_ungraded_reporting.py index a29627040..93450ca9c 100644 --- a/tests/test_ungraded_reporting.py +++ b/tests/test_ungraded_reporting.py @@ -145,7 +145,7 @@ def test_ungraded_loses_to_every_real_outcome_when_picking_the_worst_status() -> def test_markdown_pass_rate_uses_the_graded_denominator() -> None: - from coder_eval.reports import _pass_rate_lines + from coder_eval.reports.markdown import _pass_rate_lines text = "\n".join(_pass_rate_lines(_summary())) @@ -154,7 +154,7 @@ def test_markdown_pass_rate_uses_the_graded_denominator() -> None: def test_markdown_reports_no_rate_at_all_for_a_fully_ungraded_run() -> None: - from coder_eval.reports import _pass_rate_lines + from coder_eval.reports.markdown import _pass_rate_lines text = "\n".join(_pass_rate_lines(_summary(tasks_succeeded=0, tasks_not_graded=2, tasks_measured=0))) @@ -165,7 +165,7 @@ def test_markdown_reports_no_rate_at_all_for_a_fully_ungraded_run() -> None: def test_markdown_reports_no_rate_change_for_an_ordinary_graded_run() -> None: """The regression guard: adding the fourth bucket must not alter any surface of a run that has none.""" - from coder_eval.reports import _pass_rate_lines + from coder_eval.reports.markdown import _pass_rate_lines text = "\n".join(_pass_rate_lines(_summary(tasks_run=2, tasks_succeeded=1, tasks_failed=1, tasks_not_graded=0))) @@ -184,7 +184,7 @@ def _junit_for(status: FinalStatus, tmp_path: Path) -> Any: # defusedxml on the test side, matching tests/test_reports_junit.py. from defusedxml.ElementTree import parse as parse_xml - from coder_eval.reports_junit import write_junit_xml + from coder_eval.reports import write_junit_xml run_dir = tmp_path / "run" task_dir = run_dir / "default" / "t" / "00"