Skip to content

Commit 396c22c

Browse files
uipreligaclaude
andauthored
refactor(reports): generate the pricing mirror and split the reports layer (#176)
* feat(pricing): 1/5 — generate the evalboard rate table from pricing.py The frontend's rate table was a hand-copy of `_PRICING`, kept honest by 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 asking 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 ~32k / ~2k / ~17k / ~2k times in the run corpus, so those runs rendered "—" for cost with nothing failing. If a test can read the table, a generator can emit it. `make pricing-mirror` now renders evalboard/lib/pricing.generated.ts from pricing.builtin_rates(), and CE065 re-renders and diffs it. All five bookkeeping layers delete. The one thing the exemption set encoded that was NOT bookkeeping — that three OpenRouter models must stay unpriced on the frontend so runs.ts apportions the provider's real per-call bill instead of a static estimate — becomes data on the rate itself: ModelPricing.per_request_billing. The generator skips those rows, so nothing has to remember them. The field is defaulted and last, so 4-positional construction (including the out-of-tree coder_eval_uipath rate card) is unaffected; it participates in register_pricing's anti-shadow comparison, which is correct. Generating the table deliberately ADDS gpt-5.4-mini, gpt-5.4-nano, gpt-5.4-pro and gpt-5.5-pro to the frontend — the four exemption entries that were pure drift. Also corrects every surface that told a reader to hand-edit pricing.ts or named the deleted parity test: the Makefile and CI-job comments, litellm/README.md's "register in both tables" step, CLAUDE.md, and two stale evalboard consumer comments. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SGRvWucHgYgpcGhmyHrdBg * refactor(reports): 2/5 — two DRY fixes and hoist 18 function-local imports SLOW_PARAMS_PREVIEW_CHARS was defined in reports.py and used by reports_html.py, while the module that DEFINES it truncated with the literal 50 twice. The variant Token Usage card re-derived TokenUsage.total_tokens inline — a fourth home for arithmetic the model already owns. Both now have exactly one definition; neither changes a rendered byte at today's values, which is the point. Of the 19 function-local imports across the three report modules, 15 had their module edge at top level already, so hoisting them is adding a name to an existing line. Three add a genuinely new top-level edge (analysis in reports.py and reports_html.py, reports_html in reports_experiment.py — verified acyclic). ONE stays deferred: reports.py's `from .criteria import ...`. Importing coder_eval.criteria runs pkgutil auto-discovery with registry side effects, which hoisting would put on the path of every `import coder_eval.reports`. It now carries a comment saying so, and a subprocess test asserts coder_eval.criteria stays out of sys.modules — the decision is pinned rather than remembered. Also documents the run.json `input_tokens` seam at its writer: the key carries uncached_input_tokens, NOT the derived TokenUsage.input_tokens total, and evalboard/lib/runs.ts depends on that reading. Same word, two quantities — the name is fixed by the run.json contract and cannot change without breaking archived runs. No golden value is re-baselined: the test diff is additions only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SGRvWucHgYgpcGhmyHrdBg * refactor(reports): 3/5 — split reports_stats.py into stats, result_metrics and helpers reports_stats.py had become three unrelated modules sharing a file: a distribution-free numeric core, a set of EvaluationResult metrics the ORCHESTRATOR consumes mid-run, and the report renderers' own helpers. The orchestrator importing a "reports" module for a number it needs during a run is the layering wart; the file was the reason it had to. stats.py pure statistics. Imports NOTHING from coder_eval — asserted by a test, not by convention, because that is the whole reason the numeric core can be reasoned about in isolation. result_metrics.py turn_time_buckets, visible_turn_count, has_final_reply, expected_turns_overage. Consumed by the orchestrator during a run as well as by the reporters, so it is not a report. Deliberately NOT folded into timing.py, which has no EvaluationResult dependency and is imported by every agent. reports_stats.py what is left: report-shaped helpers over variant and experiment results, plus the display formatters (fmt_mean_sd, fmt_p) — presentation, not computation. Its cycle rationale is restated rather than deleted: it is LIVE, not historical. experiment -> html -> helpers, so folding the helpers into the experiment reporter would close a cycle. Also moves eval_result_to_task_dict to run_record.py. It writes one run.json row — a run-record serializer, not a report — and its placement was the only reason orchestration/batch.py reached into the reports layer at all. Moving it is what lets Phase 4's CE066 allowlist be purely writers instead of carrying a serializer as a permanent exception. Every one of the 21 moved definitions is byte-identical to its pre-move form, verified by AST comparison. A characterization test pins the full run.json row against a snapshot captured before the move; the non-finite sensor is retargeted to coder_eval.stats and still guards the same 7 functions. Two test files that held only tests for moved names are merged into the files named for those names, rather than left behind as misnamed orphans. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SGRvWucHgYgpcGhmyHrdBg * refactor(reports): 4/5 — reports/ package, durations.py, and CE066 The five reports_*.py modules become a package (markdown/html/experiment/junit/ helpers) whose __init__ re-exports the public writer surface. Private names stay private: the 12 that tests reach for are imported from their submodule, so the package's API is not a function of its test suite. `format_ms` moves to durations.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. NOTE this does NOT make the package SDK-free — models/agent_config.py imports ClaudeAgentOptions and every report module needs models. The docstring and tests say what is actually true rather than what the plan hoped. CE066 pins the layering the split establishes: core may import only the package's public WRITERS. An allowlist, not a denylist, so a new report helper is banned from core by default. It checks BOTH the absolute and the relative spelling — the 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 tests passed because they used the absolute form. An unrun assertion is documentation, not enforcement. The core-layer predicate moves to a shared _layers.py that CE004 and CE066 both read, so a package added to one cannot escape the other. It is stated as "every module directly under src/coder_eval/ is core": naming only orchestrator.py left result_metrics.py exempt — the module CE066's own fix message tells you to move your metric into. Also fixes a latent packaging break: .gitignore's bare `reports/` (meant for run output) matched src/coder_eval/reports/, leaving __init__.py untracked and building a wheel with zero files under coder_eval/reports/. Anchored to /reports/. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: 5/5 — retarget every reports_* reference and record the rationale The tree, the review-command module lists and a handful of prose references still named modules that no longer exist. CLAUDE.md's `reports_html.py` line also called it "the evalboard's static twin" — a parity promise nobody was keeping: it has ~40 private renderers and nothing tests it against the 31k-line evalboard. That line goes with the entry. Adds `## The reports package` to .claude/architecture-notes.md, which is where CLAUDE.md's preamble sends a reader for rationale. It records the CE066 layering, why stats.py / result_metrics.py / run_record.py each sit outside the package, the relative-import trap CE066's first draft fell into, and — the part a future reader would otherwise re-litigate — the DECISION NOT to build a shared section-data layer, with the measurement behind it: only 1 of the 4 "duplicated" section pairs shares an input shape, and the differences in the rest are per-surface presentation, not drift. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(lint): CE004 never fired on the relative import spelling CE004 matched `node.module` against `^coder_eval\.cli`, but a relative import keeps its dots in `node.level` and leaves the rest in `node.module` — so `from ..cli import run_command` arrives as `level=2, module="cli"` and matched nothing. The relative form is this codebase's dominant idiom, so the rule has been guarding roughly nothing since it was written, with its tests green because they used the absolute spelling. Found while fixing the identical bug in CE066 during the reports split. Two rules independently falling into the same trap is the definition of a shared helper, so the matching moves to `_layers.imports_package` next to the core-layer predicate both rules already share — the same reasoning that put `is_core_path` there. The helper also fixes a narrower bug the regex had: `^coder_eval\.cli` prefix- matched `coder_eval.client`. Matching is now on a package boundary. Adds the regression tests both rules were missing: every spelling of a banned import, plus the prefix-bleed case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: record two deferred harness candidates from the reports consolidation Both are real gaps with a known shape but neither is ~30 minutes of work: a hand-edit guard for generated surfaces needs a checksum gate rather than a diff, and a prose-path resolver has the same tree-parsing problem the plan already measured and declined for CLAUDE.md alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: code review fixes for the reports consolidation Three findings from the cross-phase review, all whole-diff-only: CE004 and CE066 both missed a THIRD import spelling. `imports_package` handled the absolute and relative forms, and `is_bare_package_import` handled `from . import reports` — but `from coder_eval import reports` is level=0 with module="coder_eval", which matched neither. It binds the package, so every attribute read through it was invisible to both rules. That spelling is a real in-tree idiom (`from coder_eval import __version__`), which the tests now pin as the negative case alongside the positives. reports/__init__ published nine names nothing imports. The export list was measured before packaging as "every name imported from a reports module", which at that point included the five modules importing EACH OTHER; those became intra-package `from .markdown import …` and need no re-export. 25 -> 16, and the docstring no longer claims more than it delivers. Stale cross-repo pointers the phase greps missed by stopping at the src/ boundary: two evalboard tests still cite the deleted pricing-parity test as the authority on rates (it is CE065 now), runs.ts and variants.ts still name reports_experiment.py / reports_junit.py, and a workflow comment does too. The runs.ts one matters most — it is the far half of the `input_tokens` seam Phase 2 deliberately documented at its writer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(lint): 1/4 — is_core_path covers the whole core layer `_layers.is_core_path` was a denylist of ten directory names plus a top-level-module regex, so `isolation/` — the `driver: docker` evaluation path — was invisible to BOTH CE004 and CE066. Its unanchored `[/\\]coder_eval[/\\]` also classified a repo-root file as core, because this project's own checkout directory is named `coder_eval`. Replace it with the anchored allowlist its own docstring already described: everything under `src/coder_eval/` is core except the `cli/` and `reports/` packages, so a new subpackage is core by default rather than exempt until someone notices. Uses the established `(?:^|[/\\])src[/\\]coder_eval[/\\]` spelling rather than a new variant — the defect being fixed was a regex that disagreed with its siblings. Core-set delta: +isolation/__init__.py, +isolation/docker_runner.py, +resources/__init__.py; zero removals; zero new CE004/CE066 violations. Four CE004 fixtures in tests/test_lint_runner.py built paths without a `src/` segment; three stop firing under the anchor and the fourth passes vacuously, so all four are re-anchored. `make lint` does not run that file. `TestCoreLayerMembership` pins the non-core set against the real filesystem and pins the residual `~/src/coder_eval` collision as unreachable rather than asserting it away. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(lint): 2/4 — declare CE044 and CE065 to ruff, and pin the id space `[tool.ruff.lint] external` is what stops ruff reporting RUF102 for a `# noqa: CE0xx` it does not own. It was two ids short: CE065, and CE044 — which the review missed and only prototyping the sensor found. Both are `@pytest.mark.lint` classes rather than BaseRules, and that is exactly why they slipped: `TestRuffExternalCoversEveryRule` already guarded this list, but derived the known ids from `ALL_RULES` alone, so it was blind to half the rule space it was meant to cover. Extend that class rather than adding a second one beside it: `_known()` now unions `ALL_RULES` with the `class TestCE\d{3}` ids scraped from this file, and `test_no_dead_entry_survives` asserts the other direction, so a declared id for a deleted rule fails too. Both messages name pyproject.toml and the offending ids. `tests/lint/runner.py`'s id-claiming note carried the same hand-maintained enumeration and had fallen behind CE044 identically; it now points at the grep instead of listing the ids. Nothing was red for want of these two entries — no `# noqa: CE044` or `# noqa: CE065` exists in the tree — so the fix is pre-emptive. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(reports): 3/4 — cover the HTML slowest-commands truncation branch `reports/html.py:825` — the `params_preview += "..."` arm — was the one uncovered line in the renderer. Its markdown twin got both a truncation and a boundary test when SLOW_PARAMS_PREVIEW_CHARS was introduced; the HTML side got neither. Adds the twin pair. Both read the constant rather than the literal 50, so they survive a change to it, and both were mutation-checked: flipping `>` to `>=` fails the boundary case, deleting the ellipsis arm fails the truncation case. `str(dict)` emits single quotes that `_esc` renders as `&#x27;`, so the raw cell is longer than the preview. The assertion unescapes before measuring; the renderer's escaping is untouched. No source change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: 4/4 — retarget prose that names deleted report modules The consolidation's own greps could not reach these: nine present-tense references in tests/ naming `reports.py`, `reports_html.py`, `reports_experiment.py` or `reports_junit.py` — modules that no longer exist — plus CE053's fixture default, whose `: str = ` spacing hid it from a `filepath="src/…"` grep. Classified rather than sed'd: eight further sites keep their wording because they date a past incident in past tense, name a test file that still exists, or are the deliberate `reports_html` local alias. All eight verified unedited. Also deletes CLAUDE.md's `optimize/` tree entry. The directory is absent from HEAD and from main; it lives only on the unmerged `feat/plugin-optimize-skill` branch, so the tree as documented did not match the tree as shipped. No source file changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: code review fixes for the reports-consolidation review fixes Two Medium findings, each flagged independently by a reviewer: - `_layers.py` spelled the 30-char package prefix twice, in the module whose own docstring argues a second copy of a definition is how the two drift. `_NON_CORE` now derives from `_PKG.pattern`, so widening one cannot leave the other behind — a divergence no test could have seen, because both assertions only ever feed src-layout paths. Compiled pattern unchanged. - The trailing `[/\\]` in `(cli|reports)[/\\]` is what keeps a top-level module whose name merely STARTS with `reports` or `cli` core. Nothing pinned it: deleting the separator left all 738 tests green. Pins added for `reports_legacy.py` and `cli_helpers.py`; both fail under that mutation. Also, from the same reviews: - Pin the reachability argument that made the `~/src/coder_eval` residual safe. It lived only as docstring prose; adding CE004 or CE066 to `_ALSO_SCAN_TESTS` would hand them a whole `tests/` tree that matches `_PKG` on an ordinary clone layout. Now one assertion. - `tests/lint/runner.py`'s new grep told the next author to run `^class Test(CE\d{3})`. GNU and BSD `grep -E` read `\d` as a literal `d` and report zero hits — "no ids taken", the exact miss the note exists to prevent. Respelled `[0-9]{3}`. - CLAUDE.md's tree audit ran one way only. Deleting the phantom `optimize/` entry was right, but `errors/` and `plugins.py` exist and were absent — and `plugins.py` is the SPI the "Adding a New Agent" section points at. - Record `ce048`'s near-variant of the shared path regex in the candidates entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(lint): CE067 — CLAUDE.md's tree must name every top-level package member Harness loop, closed in-session. The directory tree is the map an assistant reads before touching this package, and it drifted in both directions across two consecutive plans while only one direction was ever audited: a phantom `optimize/` row survived for a directory that lives solely on an unmerged branch, and the sweep that removed it walked entries -> filesystem, so it could not see that `errors/` and `plugins.py` were missing — the second being the plugin SPI that "Adding a New Agent" tells you to use. Asserts both directions over the top-level rows only; nested rows stay illustrative, so a new sibling module is not a forced docs edit. Proven to fire each way: re-adding the `optimize/` row and deleting the `plugins.py` row each fail with the offending name. Writing it also found a third omission the hand audit missed — `__init__.py`, here deliberately ignored along with the build and typing markers. Claims id 067; the Phase 2 parity sensor required the pyproject entry immediately, which is the sensor doing its job. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(lint): drop a personal path and de-duplicate the isolation pin No test datum should carry a developer's home directory. The repo-root case now uses `/home/dev/src/exp/coder_eval/conftest.py`, which keeps the shape that mattered — a `src` component that is NOT the package's parent, so it exercises the anchor rather than merely the absence of `src`. `ISOLATION` and a verbatim three-line docstring were copied into both the CE004 and CE066 test classes. Hoisted to one module-level `CORE_ISOLATION` with the rationale stated once, so the two pins cannot drift to different paths. Verified the pins still bite: re-exempting `isolation/` in `_NON_CORE` fails four tests. Also renames `test_ce008_skips_files_outside_scope` to `ce009` — it sits in the CE009 block and exercises `YamlModelsForbidExtras` (CE009), while CE008's own tests cover `ReadTextExplicitEncoding`. Pre-existing mislabel, flagged in review. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(lint): drop two function-local json imports that shadow the module one `tests/test_custom_lint.py` imports `json` at module level, and two tests re-imported it inside the function body. CodeQL flagged the one this PR added (in the pricing-mirror test); the other, in the activation-rows test, predates the PR and has the same shape, so both go. Ruff has no rule for a repeated import inside a function body, which is why `make check` never saw either. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(lint): CE004 checks reports/ — stop borrowing CE066's exemption set CE004 and CE066 shared one predicate, `is_core_path`, whose exemption set is `{cli, reports}`. That set is CE066's: the reports package may reach into itself. CE004's is only `{cli}` — the reports package runs without the CLI (the orchestrator writes a task report mid-run), so a `cli` import added inside `reports/` closes a cli -> orchestration -> reports -> cli cycle, and CE004 would have stayed silent. Latent, not live: widening the scope finds 0 violations. The rules still share what must not drift — the anchored package regex and the `cli/` boundary, now `is_package_path` and `is_cli_path` — but each states its own exemptions. CE066 keeps `is_core_path`; CE004's scope is the package minus `cli/`. Tests, each mutation-checked against CE004 going back to the core predicate: - `test_the_reports_package_is_in_scope` — a `cli` import in reports/ violates. - `TestCoreLayerMembership.test_ce004_scope_is_every_module_outside_cli` runs the RULE at every real module path. Its first draft recomputed the scope from the helpers and passed under that mutation, so it now calls the rule. - `test_the_reports_package_itself_stays_exempt` pins that CE066's scope did not widen with CE004's. Closes the deferred harness candidate; architecture notes updated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(lint): resolve a relative import against the importing file CE004 and CE066 matched `node.module` against the bare package name whenever `node.level` was non-zero, so `from .reports import x` inside `orchestration/` — i.e. `coder_eval.orchestration.reports` — read as the reports layer, and `from ...reports import x` from a sub-package read as it too although it escapes `coder_eval` entirely. Latent: nothing in the tree is nested that way today. `_absolute_module` now resolves the dots against the file's own package, so both rules compare one fully-qualified name in either spelling. The CE004 scope probe moves to the absolute spelling: a relative one resolves against the importing file, so `..cli` names the cli layer only from inside a sub-package and that test would measure depth instead of scope. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: reconcile the reports split with main's docs restructure Rebase fallout from #174/#177, which replaced CLAUDE.md's fenced directory tree with `ls` plus selective bullets, split `.claude/architecture-notes.md` into `.claude/notes/`, and added the `make docs-budget` prose gate. - Drop CE067. It asserted that CLAUDE.md's tree names every top-level package member; that tree no longer exists, and the bullets that replaced it are deliberately not exhaustive, so the rule's invariant is now false by design. The surviving half — no bullet may name a path that does not exist — is recorded in .claude/harness-candidates.md rather than rebuilt here. - Move the reports-package rationale into `.claude/notes/reporting.md`, and retarget `reports_stats` / `reports_junit` in notes/ to their new homes. - Bring `result_metrics.py` and `run_record.py` under the prose budget by adopting main's trimmed wording for the comments and docstrings they inherited, including its `.claude/notes/` pointers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(pricing): carry the exemption set into the generated mirror The generator kept only `per_request_billing` as an exemption axis, so it reproduced three of the hand-copy's seven exclusions and silently priced the other four. Those four were not drift: `gpt-5.4-mini`, `gpt-5.4-nano`, `gpt-5.4-pro` and `gpt-5.5-pro` were enumerated exemptions in the parity test this branch deletes. Pricing them is a behaviour change smuggled in as a refactor — latent only because no experiment config references them today. The exemption set encoded two different claims and they now survive separately. That a routed model MUST NOT be priced is a fact about the rate, and stays on `ModelPricing.per_request_billing`. That a heavy frontier variant is not worth pricing on the board is a fact about the FRONTEND, so it is `DELIBERATELY_UNMIRRORED` beside the generator. What the hand-copy got wrong was not having an exemption set but letting it go stale unnoticed, so the deleted test's staleness guard comes back as `_assert_exemptions_are_live`: an id that has left `pricing.py` fails `make pricing-mirror` and CE065 rather than sitting there silencing nothing. The generated table now reproduces the hand-copy's 52 keys exactly — no additions, no removals. CE065 asserts both axes from their declared sources, and the evalboard's consumption guard pins the four as unpriced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 30f78f4 commit 396c22c

85 files changed

Lines changed: 3326 additions & 1627 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.claude/commands/coder-eval-code-review-full.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ in a value that doesn't match the formula.
348348
- 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.
349349
- 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).
350350
- 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?
351-
- 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)?
351+
- 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)?
352352
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.
353353

354354
3. **Check exhaustiveness when an enum / Literal / status set changes.**
@@ -401,7 +401,7 @@ in a value that doesn't match the formula.
401401
- **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.
402402
- **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?
403403
- **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.
404-
- **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"`?
404+
- **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"`?
405405
- **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.
406406
407407
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."

‎.claude/harness-candidates.md‎

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,7 @@ with the two `action.yml` items above — one considered change to the action's
450450
`verify-published-action.yml` reads `task_results[*].status` / `weighted_score` /
451451
`total_tokens`, and `action.yml`'s score gate reads `weighted_score` / `task_id`.
452452
These are string keys in shell/YAML that no test or type-checker binds to
453-
`eval_result_to_task_dict` (`reports_experiment.py`), so renaming a key there
453+
`eval_result_to_task_dict` (`run_record.py`), so renaming a key there
454454
silently turns an external gate into a no-op — a reviewer here proposed
455455
`final_status`, which does not exist in `run.json` and would have made a new
456456
assertion dead on arrival. Guard: assert the key set that non-Python consumers
@@ -859,7 +859,7 @@ re-derive from scratch.
859859
an existing form. Caught in: the turn-timing consolidation, Phase 5 review.
860860

861861
- [ ] **Pre-existing, surfaced by the turn-timing final review:
862-
`reports_stats.regularized_incomplete_beta` clamps an out-of-domain `x`
862+
`stats.regularized_incomplete_beta` clamps an out-of-domain `x`
863863
instead of raising.** Its docstring says "Raises ValueError outside that
864864
domain — returning NaN would let a bad input render as a real-looking
865865
statistic downstream", and it does raise for a non-finite `a`/`b`/`x` and for
@@ -916,3 +916,49 @@ re-derive from scratch.
916916
(AST-comparing every `ClassDef` first line against a base ref) was written
917917
and used throughout Phase 6 and is the thing to promote if that access
918918
appears. Caught in: prose mass reduction, Phase 6.
919+
920+
- [ ] A generated surface (`*.generated.*`) has no mechanical guard against being hand-edited
921+
— CE065/CE033/CE028 all catch *drift* (source changed, output not regenerated) but an edit
922+
to BOTH passes cleanly. Guarding it needs a checksum or a git-attribute gate, not a diff,
923+
so it is a different shape of sensor. — caught during the reports consolidation (CE065).
924+
- [ ] No rule resolves file paths named in PROSE (comments, docstrings, Markdown) across
925+
`src/`, `evalboard/`, `litellm/` and `.github/`. That consolidation hand-fixed ~25 stale
926+
module references across five phases, and two reviewers each found more the greps missed.
927+
The plan's Open Questions measured and declined the CLAUDE.md-only variant (its stale refs
928+
live in an ASCII tree, not backticks); a wider variant has the same parsing problem plus
929+
legitimate non-resolving refs (container paths, plugin-relative paths). Recorded because
930+
the recurrence is now the argument, not the idea. — caught during the reports consolidation.
931+
- [ ] The anchored package regex `(?:^|[/\\])src[/\\]coder_eval[/\\]` is compiled
932+
independently across the rule tree — `ce050_no_union_getattr_probe.py:101`,
933+
`ce051_no_driver_override.py:60`, `ce052_process_lethal_must_be_container_gated.py:78`,
934+
`ce053_run_record_filename_literal.py:65`, `ce054_env_info_key_round_trip.py:66`,
935+
`ce056_no_container_env_literal.py:51` and `ce058_no_timing_literal.py:116` — seven
936+
rule modules, to which `_layers.py` adds one more (its `_CLI` and `_REPORTS` derive from it), with the
937+
`agents/`-suffixed variant of the same idiom in
938+
`_model_ctor.py:28` and `ce059_generation_window_is_two_reads.py:45`, plus a near-variant
939+
in `ce037_no_dead_private_helper.py:61` and a `cli/`-suffixed one in
940+
`ce048_no_in_process_typer_command_call.py:68`. `_layers.py` is the designated shared rule-helper
941+
module, though `_model_ctor.py` is an equal peer and a generic src-path regex arguably
942+
belongs in a neutrally named helper rather than one named `_layers`. Not hoisted here
943+
because retargeting seven unrelated rules needs a per-rule verification that its scope
944+
did not shift — a second refactor inside a review-fix plan. The new copies were written
945+
in the established *spelling* deliberately: the defect being fixed was a regex that
946+
disagreed with its siblings, so a new variant would be that defect again. — caught during the reports-consolidation review fixes, Phase 1.
947+
- [x] ~~**CE004 inherits CE066's `reports/` exemption because the two rules share one
948+
predicate.**~~ **DONE.** `_layers.py` now shares the package anchor and the `cli/`
949+
boundary (`is_package_path`, `is_cli_path`) rather than one exemption set. CE066 keeps
950+
`is_core_path` (`{cli, reports}` exempt); CE004's scope is the package minus `cli/`.
951+
Widening CE004 to `reports/` found 0 violations. `test_the_reports_package_is_in_scope`
952+
and `TestCoreLayerMembership.test_ce004_scope_is_every_module_outside_cli` both fail if
953+
CE004 goes back to the core predicate; `test_the_reports_package_itself_stays_exempt`
954+
pins that CE066's scope did not widen with it. — caught in the reports-consolidation
955+
review fixes, Phase 1 quality review.
956+
957+
- [ ] **A CLAUDE.md Directory Structure bullet naming a path that no longer exists.**
958+
Shipped briefly as CE067 over the fenced `coder_eval/` tree, then removed when that
959+
tree was replaced by `ls` plus selective bullets — the exhaustive half of the rule
960+
became false by design. The surviving half is still real: the bullets name modules
961+
(`result_metrics.py`, `reports/html.py`, `models/container_paths.py`) and a rename
962+
leaves them stale with nothing failing. Needs a backtick-path extractor scoped to
963+
one section, which is the narrow case of the prose-path candidate above. — caught
964+
during the reports consolidation rebase.

‎.claude/notes/orchestration.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
`run` and `execute` share one body (`run_command.run_pipeline`) and differ solely in
4949
that flag, so there is no third code path. Three things are refused rather than
5050
degraded: `--junit-xml` (a report of verdicts, and there are none — though
51-
`reports_junit` still emits `<skipped>` for an ungraded row it encounters),
51+
`reports/junit.py` still emits `<skipped>` for an ungraded row it encounters),
5252
`--allow-host-grading` (it decides how an ungraded row is GRADED, and `execute` grades
5353
nothing), and simulation tasks (their turn-continuation logic reads criteria results,
5454
so an ungraded dialog would silently change its own stopping behavior). `stop_early:`

‎.claude/notes/reporting.md‎

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,11 +184,80 @@ malformed record — which raises while building the per-call breakdown — abor
184184
with the run untouched, matching the caller's "keeping static pricing" contract. Spend
185185
tagged with an iteration no turn has is surfaced rather than silently dropped.
186186

187+
## The reports package
188+
189+
`reports/` is a **leaf**: it may import from anywhere in `coder_eval`, and the core layers
190+
may import only its public *writer* entry points. That asymmetry is the whole point, and
191+
**CE066** enforces it. The invariant is not "core must not import reports" — core
192+
legitimately *writes* reports (`orchestrator.py` writes the per-task HTML,
193+
`orchestration/batch.py` drives `ReportGenerator`). It is that a **metric, a statistic, a
194+
serializer or a formatter** must never be reached out of the rendering layer.
195+
196+
That was the actual shape of the code before the split. `reports_stats.py` was three
197+
unrelated modules sharing a file, and the orchestrator imported `turn_time_buckets` and
198+
`visible_turn_count` from it *during a run* — a number the evaluation loop needs, living in
199+
a reporting module. The three pieces now sit where their consumers are:
200+
201+
- **`stats.py`** — distribution-free statistics, **dependency-free by contract**: stdlib
202+
only, no `coder_eval` import, direct or relative. A unit test parses its AST and asserts
203+
that, rather than leaving it to convention, because being reasonable-about-in-isolation is
204+
the only reason it is a separate module. Display formatters (`fmt_mean_sd`, `fmt_p`) stay
205+
in `reports/helpers.py`: they return `"N/A"`, `"—"` and `"<0.001"`, which is presentation.
206+
- **`result_metrics.py`** — metrics derived from a finished `EvaluationResult`, consumed by
207+
the orchestrator mid-run as well as by the reporters. Deliberately **not** folded into
208+
`timing.py`, which has no `EvaluationResult` dependency and is imported by every agent
209+
adapter; adding one would widen that surface for everyone.
210+
- **`run_record.py`** — the `run.json` task-row serializer. It is a run-record serializer,
211+
not a report, and its old home inside the experiment reporter was the *only* reason
212+
`orchestration/batch.py` reached into the reports layer at all. Moving it is what lets
213+
CE066's allowlist be purely writers; carrying a serializer on that list would be the rule
214+
documenting a wart instead of the wart being removed.
215+
216+
**CE066 checks both the absolute and the relative import spelling.** Its first draft matched
217+
only `node.module`, which for `from ..reports import X` holds `"reports"` with the dots in
218+
`node.level` — so it fired on neither of the two real edges in the tree, and its own tests
219+
passed because they used the absolute form. The layer predicates live in
220+
`tests/lint/rules/_layers.py` so CE004 and CE066 cannot drift about where the package or its
221+
`cli/` boundary is. Each rule's scope is an allowlist of what is *exempt*, so a new subpackage
222+
is in scope by default: CE066's core is *everything under `src/coder_eval/` except `cli/` and
223+
`reports/`*, and CE004's scope is *everything except `cli/`*. The two sets differ on purpose —
224+
the reports package runs without the CLI, so it must not import `cli`, but it may reach into
225+
itself. CE004 first borrowed CE066's predicate whole and so inherited the `reports/`
226+
exemption. Both denylist forms before that also leaked: naming only `orchestrator.py` left
227+
`result_metrics.py` exempt (the module CE066's own fix message points at), and its
228+
ten-directory successor never named `isolation/`, leaving the `driver: docker` evaluation path
229+
invisible to both rules.
230+
231+
**`format_ms` lives in `durations.py`, not `formatting.py`.** `formatting.py` imports
232+
`claude_agent_sdk` for the payload formatters, and the reports package should not reach
233+
through an SDK-shaped module for a 14-line duration formatter. This does *not* make the
234+
package SDK-free — `models/agent_config.py` imports `ClaudeAgentOptions` and every report
235+
module needs `models` — so the tests assert what is true: `durations.py` is SDK-free, and
236+
`reports` no longer imports `coder_eval.formatting`.
237+
238+
### Rejected: a shared section-data layer
239+
240+
The markdown and HTML reporters render four "duplicated" sections. All four pairs were read
241+
in full before deciding, and **only one shares an input shape** (command statistics, both
242+
taking `CommandStatistics`); the others take a `list[dict]` row, a `TokenUsage`, an
243+
`EvaluationResult` and a `list[EvaluationResult]` across three different scopes. The
244+
remaining differences are legitimate per-surface presentation, not drift: `:.1f%` vs `:.0f%`,
245+
and an unmeasured average **hidden** in markdown versus **dashed** in HTML — two valid
246+
renderings of the same `None`. Building the adapter would mean normalizing dict-row and
247+
live-model inputs across three scopes, touching the `run.json` contract, to remove about
248+
twenty lines. Rejected on KISS/YAGNI. The two things in those pairs that *were* real — a
249+
literal `50` beside its own `SLOW_PARAMS_PREVIEW_CHARS`, and a hand-rolled
250+
`TokenUsage.total_tokens` — were simply fixed.
251+
252+
`analysis.py` and `formatting.py` stay top-level on purpose: they are not report modules,
253+
and moving them in would give the package an SDK dependency and force CE066 to exempt the
254+
orchestrator's `analysis` import.
255+
187256
## Report rollups and the HTML twin
188257

189-
`reports_html.py` is the evalboard's STATIC TWIN: the two render the same run and must
258+
`reports/html.py` is the evalboard's STATIC TWIN: the two render the same run and must
190259
agree, so a rule implemented on one side belongs on the other. The arithmetic itself lives
191-
in `reports_stats.py` and the renderers only format it.
260+
in `result_metrics.py` and `stats.py`; the renderers only format it.
192261

193262
### An unmeasured value is never zero
194263

‎.claude/notes/timing.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ per-harness composition.
144144

145145
The span set the generation subtraction, the head and the tail are all measured against,
146146
so they cannot disagree about which calls exist. Shared with
147-
`reports_stats.turn_time_buckets`, which answers the same question about a finished
147+
`result_metrics.turn_time_buckets`, which answers the same question about a finished
148148
`TurnRecord` — a second typed copy of this rule is how two report surfaces come to publish
149149
two different tool totals for one run. (`scripts/timing/decompose_run.py` keeps its own,
150150
over raw `task.json` dicts rather than models; that is the sanctioned third reader, and

‎.github/workflows/pr-checks.yml‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -181,19 +181,21 @@ jobs:
181181
echo "📊 All checks passed: formatting, linting, types, security, tests"
182182
183183
evalboard:
184-
# The dashboard's own gate. `evalboard/` ships ~460 vitest assertions,
185-
# including the pricing drift guard that asserts lib/pricing.ts still agrees
186-
# with src/coder_eval/pricing.py — and until this job existed NOTHING ran
187-
# them: not a workflow, not a Makefile target, not a pre-commit hook. The
188-
# guard was consequently red on `main` for weeks while a 3x-wrong Opus rate
189-
# and five unpriced in-use models shipped to the board. An unrun assertion is
190-
# documentation, not enforcement.
184+
# The dashboard's own gate. `evalboard/` ships ~460 vitest assertions, and
185+
# until this job existed NOTHING ran them: not a workflow, not a Makefile
186+
# target, not a pre-commit hook. The pricing guard was consequently red on
187+
# `main` for weeks while a 3x-wrong Opus rate and five unpriced in-use models
188+
# shipped to the board. An unrun assertion is documentation, not enforcement.
189+
#
190+
# Rate-table drift is no longer this job's concern: lib/pricing.generated.ts
191+
# is GENERATED from src/coder_eval/pricing.py, and CE065 in `quality-gate`
192+
# fails a reprice that was not regenerated. What runs here is the CONSUMPTION
193+
# half (pricing-generated.test.ts) — a generated file that is missing, empty
194+
# or narrow fails the board's own build.
191195
#
192196
# Deliberately NOT path-filtered. `paths:` is workflow-scoped in GitHub
193-
# Actions, and the parity guard's whole point is that a reprice in
194-
# src/coder_eval/pricing.py — a pure-Python diff touching no evalboard file —
195-
# must trip it. A `evalboard/**`-only filter would skip exactly the change
196-
# class this job exists to catch.
197+
# Actions, and a skipped required check blocks a PR rather than passing it —
198+
# so the filter buys nothing and costs a merge-blocking pending status.
197199
name: Evalboard (Types, Tests, Build)
198200
# Fork-PR carve-out — see `quality-gate`. `pnpm install --frozen-lockfile` runs
199201
# the PR's own lockfile install scripts, same untrusted-code class.

‎.github/workflows/verify-published-action.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ jobs:
463463
# The JUnit report must describe the same run, not merely be parseable: an empty
464464
# but well-formed <testsuites/> passed the old parse-only check. Trusted,
465465
# self-generated input (our writer emits no DTDs/entities), so stdlib ET is fine.
466-
# `>=` not `==`: reports_junit.py also emits synthetic `skipped` / `suite-gates`
466+
# `>=` not `==`: reports/junit.py also emits synthetic `skipped` / `suite-gates`
467467
# testsuites, which only ever ADD cases.
468468
cases = len(list(ET.parse(os.environ["JUNIT"]).getroot().iter("testcase")))
469469
if cases < len(rows):

‎.gitignore‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ uipath.json
6161
.claude/settings.local.json
6262
c/
6363
runs/
64-
reports/
64+
# Anchored to the repo root: this is generated RUN OUTPUT, not the
65+
# `src/coder_eval/reports/` package, which a bare `reports/` also matched.
66+
/reports/
6567
/runs/**/artifacts/
6668
.DS_Store
6769
tmp/

0 commit comments

Comments
 (0)