Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions .claude/harness-candidates.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,3 +127,62 @@ Deferred lint/test guardrails surfaced during reviews. Promote to a `CExxx` rule
that environment injects the criterion. Until then CE030 stays scoped to the four
top-level models; the `command_pattern`/`exclude_pattern` contract this PR changed is
documented in the Field descriptions and TASK_DEFINITION_GUIDE regardless.

## From the evalboard Path-to-GA de-tag / mature-passes fix (4e5bbc4…dd5f7e9) — TS-side guards deferred

Context: the CExxx harness is a **Python** AST runner over `src/coder_eval/`, so none
of the invariants below are mechanizable in it. Each would need a TypeScript lint
harness (eslint config + custom rules) that `evalboard/` does not have today —
standing one up for three call sites fails the KISS/YAGNI gate. Deferring rather
than dropping; promote if a fourth TS-side invariant appears, and stand up the
harness once for all of them.

> **Update (PR #94 review round 2).** The *execution* half of this gap is closed:
> `evalboard/` is now gated by the `evalboard` job in `.github/workflows/pr-checks.yml`
> and reachable locally via `make evalboard-verify`, so the vitest suite (including
> the pricing drift guard) is enforcement rather than documentation. What remains
> deferred below is the *static-analysis* half — eslint has still not been stood up.
> The review that prompted this round names four more candidate TS rules (raw
> `status === "SUCCESS"` outside `lib/status.ts`; DOM-global shadowing in props;
> inline copies of the tag predicate; per-run tooltip copy reused on aggregate
> surfaces), which meets the "fourth invariant" promotion bar stated above —
> **stand up eslint next time `evalboard/` is touched substantively.**

- [ ] **"Every consumer of `RunOverviewTask.matureSkipped` must decide explicitly
whether a carry-forward row counts."** Four consumers now, and they deliberately
DISAGREE: `lib/trends.ts` and `app/runs/[id]/run-view.tsx` count a mature skip as
a pass; `lib/overview.ts::buildTagTaskRows` excludes it from both terms
(`/path-to-ga` is a GA-readiness page). A new consumer silently inheriting either
convention is a real hazard. Guard shape: flag a file that reads `.matureSkipped`
without a nearby comment naming its convention — weak, hence the deferral. Closed
for now by unit tests that assert the exclusion from BOTH numerator and denominator
(`lib/__tests__/overview.test.ts` → `describe("buildTagTaskRows")`).

- [x] ~~**`taskCarriesRepoTag` is the single repo-provenance tag predicate — but one
duplicate survives.**~~ **RESOLVED in PR #94 review round 2.** The predicate moved to
a dependency-free `lib/tags.ts` (structurally typed on `{skill, tags}` so
`RunOverviewTask`, `TaskResultSummary` and `TaskTrend` all satisfy it), re-exported
from `lib/overview.ts` for existing callers. Both inline copies now import it:
`app/runs/[id]/run-view.tsx` (the `"use client"` one that could not before) and
`lib/trends.ts::trendMatchesTag` (a third copy the original deferral missed).
Still worth a lint rule ("no inline `tags.includes(x) || skill === x`") to catch
future copies — folded into the eslint promotion noted above.

- [ ] **The de-tag rule fails CLOSED on a newest run that loads fine but stamps no
`tags`** (`lib/overview.ts::buildTagTaskRows`): every tagged task would read as
de-tagged and the table would empty, rendering an empty state indistinguishable from
a genuine full de-tagging. Its sibling failure mode (`overview == null`, a transient
blob read failure) IS guarded, with exactly this rationale. Currently unreachable —
0 of ~116k date-shaped non-ad-hoc task rows in `runs-remote/` lack `tags`, and the
six zero-tag runs found are all ad-hoc (filtered upstream by id shape + `meta.adhoc`)
— so the barrier is two upstream filters rather than a check at the seam. Left
unguarded on purpose: a `if (taggedInRun.size === 0) skip the de-tag signal` guard
would also mask a real, total de-tagging. Revisit if the pipeline ever stops
stamping tags, or if a non-ad-hoc run legitimately carries zero tagged rows.

- [ ] **Discriminating-test discipline for predicate narrowings.** Two tests in this
change passed for the wrong reason — a downstream rule (the de-tag drop) masked the
mutation they claimed to catch — and the plan leaned on a `grep` acceptance criterion
that CI never runs. Both were found by mutation-testing the suite and fixed. No
mechanizable guard; the durable lesson is: when a test names a narrowing, construct
the fixture so the row SURVIVES every other rule, or the assertion proves nothing.
39 changes: 39 additions & 0 deletions .github/workflows/pr-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,45 @@ jobs:
echo "✅ Quality gate complete!"
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.
#
# 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.
name: Evalboard (Types, Tests, Build)
runs-on: ubuntu-latest
timeout-minutes: 15
steps:
- name: Checkout code
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2

- name: Set up Node.js 20
uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0
with:
node-version: "20"

- name: Enable pnpm
# The version comes from evalboard/package.json's `packageManager` field,
# so corepack pins it without a second place to keep in sync.
run: corepack enable

- name: Install dependencies (lockfile-pinned)
working-directory: evalboard
run: pnpm install --frozen-lockfile

- name: Verify (tsc --noEmit && vitest run && next build)
working-directory: evalboard
run: pnpm verify

no-uipath-extra:
# Proves that `pip install coder-eval` (without the optional `[uipath]`
# extra) yields a working framework: imports succeed, the criterion
Expand Down
6 changes: 6 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,14 @@ make typecheck # pyright
make test # pytest
make lint # custom architectural lint rules (CE001+)
make verify # All of the above + coverage check (CI equivalent)

# The JS half (evalboard/). Separate because it needs a Node/pnpm toolchain,
# but gated in CI by the `evalboard` job just like the Python side.
make evalboard-verify # tsc --noEmit + vitest + next build
```

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.

When fixing a bug, ask: *could a custom lint rule have prevented this?* If the root cause is a mechanically detectable pattern (e.g., "always import from `coder_eval.models`", "never call blocking IO in async"), add a rule to `tests/lint/rules/` following the CE001+ pattern and wire it up in `tests/lint/runner.py`. This turns a one-time fix into permanent enforcement. See `tests/test_custom_lint.py` for how rules are tested. (Doc-surface / whole-tree rules that reason over Markdown/YAML or the entire `src/` tree rather than one `.py` AST at a time — CE026–CE031 — are not `BaseRule`s in the runner; they are wired as dedicated `@pytest.mark.lint` test classes. CE031 guards against dead config: a behavior-driving field on `SimulationConfig`/`RunLimits`/`Dataset` that no code reads by name. CE026 keeps the GitHub Action's three onboarding surfaces honest: a page's *first* Action snippet must show the agent-runtime prerequisite steps (pinned to the `action-dogfood` job that proves them in CI), a zero-install absolute next to such a snippet must name the channel it means, and every `github.com/marketplace/actions/<slug>` link plus the shields badge label must match `action.yml`'s `name:`.)

Adding a user-facing field to one of the models CE030 tracks (`TaskDefinition`, `RunLimits`, `Dataset`, `SimulationConfig` — see `tests/lint/doc_schema_parity.py`) means documenting it in its guide (mention the field name as inline code) or adding an `EXEMPT` entry with a reason it is not user-authored. `make lint` fails otherwise.
Expand Down
10 changes: 9 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.PHONY: help install format check typecheck test test-live test-smoke verify verify-noextra clean run lint docs-indexes 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 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
Expand Down Expand Up @@ -56,6 +56,14 @@ verify: ## Run all verification steps (CI equivalent)
# uv run bandit -r src/ -ll --format json -o bandit-report.json
uv run pytest tests/ -n auto -m "not live and not lint" --cov=coder_eval --cov-report=term-missing --cov-report=xml --cov-fail-under=80

evalboard-verify: ## Run the evalboard (Next.js dashboard) checks: tsc + vitest + next build
# 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.
cd evalboard && pnpm install --frozen-lockfile && pnpm verify

verify-noextra: ## Verify the framework works without the optional [uipath] extra
# Build a throwaway venv that has ONLY the [dev] extra (no [uipath]); confirms
# `pip install coder-eval` (without the extra) is functional. Mirrors the
Expand Down
18 changes: 18 additions & 0 deletions evalboard/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,24 @@ show up in the index — empty shells and the `latest` symlink are filtered out.
daily success-rate chart and tag rails for filtering.
- `/trends` — per-task pass rate and avg duration/cost/turns across the
last 10 runs, with a tag filter and expandable per-task history.
- `/path-to-ga` — GA-readiness report for the tasks tagged `path-to-ga`.
Its task table deliberately answers under **stricter rules than every other
surface**, and both differences are load-bearing:
1. **De-tagged tasks are dropped.** A `run.json` tag is a historical stamp, so
elsewhere (including `/trends`) a task de-tagged upstream lingers until the
last run that predates the removal ages out. Here a task is dropped once a
*newer* run in the window carries it without the tag — proof of removal.
A task that merely stopped appearing is unknowable, so it is kept and dated.
2. **Mature carry-forwards are not passes.** Elsewhere a skipped-but-carried-
forward row counts as a pass; here it is excluded from both the numerator
and the denominator, so the rate reports only measured runs (`—` when
nothing executed).
The headline tile and chart above the table keep the ordinary mature-blind,
union-over-window semantics — they feed the front page — which is why they read
higher than the table, and why the page says so in prose.
- `/watchlist` — what needs attention, ranked over the recent-runs window: tasks
and skills scored on failures, regressions and turn-budget pressure
(`lib/watchlist.ts`).
- `/runs/latest` — shortcut that redirects to the newest run id.
- `/runs/<run-id>` — run summary (pass rate, cost, duration) + one row per task.
A "Download run (.zip)" button bundles the whole run folder.
Expand Down
138 changes: 138 additions & 0 deletions evalboard/app/path-to-ga/__tests__/task-table.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
import { describe, expect, test } from "vitest";
import { render, screen } from "@testing-library/react";
import type { TagTaskRow } from "@/lib/overview";
import { TagTaskTable } from "../task-table";

// TagTaskTable is a pure props-in/JSX-out component — no router hooks, so unlike
// run-view.render.test.tsx this needs no next/navigation stub.

function row(overrides: Partial<TagTaskRow> = {}): TagTaskRow {
return {
taskId: "skill-flow-coded-agent",
skill: "uipath-maestro-flow",
appearances: 20,
matureSkips: 0,
executed: 20,
passRate: 90,
latestStatus: "SUCCESS",
latestScore: 1.0,
latestRunId: "2026-07-31_04-38-51",
latestMatureSkipped: false,
...overrides,
};
}

function renderTable(
rows: TagTaskRow[],
harness: string | null = null,
) {
return render(
<TagTaskTable
rows={rows}
tag="path-to-ga"
windowLabel="30d"
harness={harness}
/>,
);
}

describe("TagTaskTable", () => {
test("shows a Mature pill instead of Passed when the latest run skipped the task", () => {
renderTable([row({ latestMatureSkipped: true })]);
expect(screen.getByText("Mature")).toBeInTheDocument();
expect(screen.queryByText("Passed")).not.toBeInTheDocument();
});

test("shows Passed and the numeric score for an ordinary executed row", () => {
renderTable([row({ latestScore: 0.75 })]);
expect(screen.getByText("Passed")).toBeInTheDocument();
expect(screen.getByText("0.75")).toBeInTheDocument();
});

test("dashes out the latest score on a mature row", () => {
// 1.0 on a carry-forward row is inherited, not measured — showing it
// beside a Mature pill would read as a fresh result.
renderTable([row({ latestMatureSkipped: true, latestScore: 1.0 })]);
expect(screen.queryByText("1.00")).not.toBeInTheDocument();
// Exactly one cell dashes — the score. Every other column on the default
// row is populated, so this pins WHICH cell went un-measured.
expect(screen.getAllByText("—")).toHaveLength(1);
});

test("annotates Appearances with the mature count, and only when non-zero", () => {
renderTable([row({ appearances: 24, matureSkips: 3 })]);
expect(screen.getByText("(3 mature)")).toBeInTheDocument();
// The raw count stays plain beside it (getByText matches an element's own
// direct text nodes, so this is the cell's "24", not "24 (3 mature)").
expect(screen.getByText("24")).toBeInTheDocument();
});

test("no mature annotation when nothing was skipped", () => {
renderTable([row({ appearances: 24, matureSkips: 0 })]);
expect(screen.queryByText(/mature\)/)).not.toBeInTheDocument();
});

test("renders an em dash for an unmeasured pass rate", () => {
// Every appearance was a carry-forward → nothing executed → no rate.
// Must not read as NaN% or a measured 0%. `latestMatureSkipped` is true
// by construction here: buildTagTaskRows reads latest* off one of the
// counted appearances, so matureSkips === appearances forces it — the
// score dashes too, hence two dashes rather than one.
renderTable([
row({
appearances: 4,
matureSkips: 4,
passRate: null,
latestMatureSkipped: true,
}),
]);
expect(screen.queryByText(/NaN/)).not.toBeInTheDocument();
expect(screen.queryByText("0%")).not.toBeInTheDocument();
expect(screen.getAllByText("—")).toHaveLength(2);
});

test("the pass-rate tooltip names the row's own executed denominator", () => {
// Reads `executed` off the row rather than re-deriving it, so the
// caption and the percentage can't describe different rules. The row
// below is deliberately inconsistent (executed=5 against 24-3=21) —
// only a tooltip sourced from the field can report 5.
renderTable([row({ appearances: 24, matureSkips: 3, executed: 5 })]);
expect(screen.getByText("90%").closest("td")).toHaveAttribute(
"title",
expect.stringContaining("Measured over 5 executed appearances"),
);
});

test("the mature annotation speaks in the window's voice, not one run's", () => {
renderTable([row({ appearances: 24, matureSkips: 3 })]);
const title = screen.getByText("(3 mature)").getAttribute("title");
expect(title).toContain("3 of these 24 appearances");
// The per-run string would say "this run" on a page that renders none.
expect(title).not.toContain("this run");
});

test("Last appearance shows the date half of the latest run id", () => {
renderTable([row({ latestRunId: "2026-07-16_04-24-15" })]);
expect(screen.getByText("2026-07-16")).toBeInTheDocument();
});

test("empty rows render the empty state naming the tag and window", () => {
// Newly reachable: a tag whose every task was de-tagged yields [] where
// it previously yielded stale rows.
renderTable([]);
expect(
screen.getByText(/No tasks tagged path-to-ga in the last 30d\./),
).toBeInTheDocument();
});

test("the pooled-across-harnesses note tracks the harness prop", () => {
const { unmount } = renderTable([row()], null);
expect(screen.getByText(/pooled across harnesses/)).toBeInTheDocument();
unmount();

renderTable([row()], "claude-code");
expect(
screen.queryByText(/pooled across harnesses/),
).not.toBeInTheDocument();
});
});
Loading
Loading