Skip to content

refactor: fold delegate-sdk into delegate, drop run-format fallbacks older than 30 days - #235

Merged
bai-uipath merged 8 commits into
mainfrom
bai/evalboard-cleanup
Oct 22, 2026
Merged

bai-uipath merged 8 commits into
mainfrom
bai/evalboard-cleanup

Conversation

@bai-uipath

@bai-uipath bai-uipath commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

  • The evalboard's Delegate button showed no run after the weekly moved from coder_eval_uipath's delegate-sdk adapter to coder_eval's built-in delegate agent: the selector pins delegate-sdk, and delegate only appears while it is among the last 12 runs.
  • Both halves of the repo carry fallbacks for run formats nothing still produces. We only need to read what was uploaded in the last 30 days.

Changes

  • One Delegate series. normalizeHarness folds delegate-sdk into delegate, delegate becomes the pinned selector entry, and old ?h=delegate-sdk links resolve to it. History reads as one line across the switch.
  • Evalboard: drop fallbacks for formats no recent run uses. Before removing each one, I checked that every run uploaded since 2026-09-22 (all three containers, plus a task.json sample per run) carries the newer field. Removed: the flat <task>/task.json layout, the wall-clock gap heuristic for grouping messages (every assistant message has a message_id), the "no branch info" state of parentToolUseId, the empty-blockTypes render path, the legacy input_tokens token fallback, and the git_commit / cli_git_commit component keys. harnessColor and the new alias lookup use Object.hasOwn, so ?h=constructor stays a plain string.
  • Python: same rule for the read side. Removed: TokenUsage adopting a pre-split input_tokens, result_kind inference from criterion_type, the simulator's route-model fallback (records have carried simulator_model since 0.10.0), the format_token_usage input_tokens fallback, and .json judge transcript siblings. With result_kind inference gone, the HTML judge section gates on JudgeCriterionResult and JUDGE_CRITERION_TYPES is deleted. Docs and notes updated to match.
  • Comments that called a still-live fallback "legacy" now name the real producer: mature carry-forward rows, a salvaged run.json, or the retired delegate-sdk adapter.
  • Docs and lint text match the code. HARNESS_PARITY.md, the lint-rules note, the CE060 message and a schema field description no longer describe the gap grouping or result_kind guessing. The token-totals pick for harnesses with no reconciliation entry (codex, antigravity, delegate) is no longer called legacy.
  • Dependency bump to unblock CI. pip-audit fails on opentelemetry-api 1.40.0 (CVE-2026-114603, fixed in 1.45.1). azure-monitor-opentelemetry-exporter 1.0.0b53 pinned OpenTelemetry ==1.40, so it moves to 1.0.0b58 (~=1.45.0). Lock-only change; every new version is at least 15 days old.

Kept on purpose

Fallback Why it stays
agent_wall_ms / setup_ms / grading_ms from iterations Fields shipped in 0.12.12 (2026-10-06); runs before that are inside the window
tasks_graded, stored tool_union_ms, harness head/tail Scribe runs on coder-eval 0.11.1 are inside the window until early November
visible_turns reconstruction, has_final_reply default A salvaged run.json (eval_runner salvage-run) still writes rows without them
variant_id / replicate_index defaults Mature carry-forward rows carry neither
Criterion defaults for evaluation_status / pass_threshold / gating The run listing still reaches older runs; without them a criterion renders as NOT EVALUATED or informational
error_details["stack_trace"] in the HTML report errors/retry.py still writes it
Prompt-regime inference in run.md The run-level environment_info never carries system_prompt_semantics
iterations alias turns The published analyze skill documents it and lint tests guard that pairing; separate change

Validation

Check Result
Field scan over every run uploaded in the last 30 days Every removed fallback's field present on every executed row / assistant message
make evalboard-verify (tsc + vitest + next build) Pass: 819 tests, build OK
make verify ruff, pyright, custom lint, prose budget pass; 6387 tests pass. 4 local-only failures: test_stats_nonfinite (3, Python 3.13.7 stdlib) and one context-window test that reads the host's CLAUDE_CODE_AUTO_COMPACT_WINDOW (passes with it unset)
Lock bump: telemetry-related tests, pyright, CI's pip-audit command 2230 passed, 0 type errors, no known vulnerabilities

🤖 Generated with Claude Code

bai-uipath and others added 3 commits October 22, 2026 11:22
The weekly Delegate run moved from coder_eval_uipath's delegate-sdk adapter to
coder_eval's built-in delegate agent. The selector pinned delegate-sdk, so the
Delegate button stopped at the last adapter run, and delegate only showed while
it was among the 12 most recent runs.

normalizeHarness now maps delegate-sdk to delegate, delegate is the pinned
selector entry and owns the reserved color, and ?h=delegate-sdk links resolve to
delegate. The Delegate trend is one series across the switch.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Each fallback was removed only after a scan of every run uploaded to the three
evalboard containers in the last 30 days (run.json plus a task.json sample per
run) showed the newer field on every executed row or assistant message.

Evalboard: the flat <task>/task.json layout, the wall-clock gap heuristic for
grouping messages (message_id is always set), the undefined state of
parentToolUseId, the empty-blockTypes render path, the legacy input_tokens
token fallback, defaults for evaluation_status / pass_threshold / gating, and
the git_commit / cli_git_commit component keys.

Python: TokenUsage adopting a pre-split input_tokens, result_kind inference
from criterion_type, the simulator's route-model fallback, the
format_token_usage input_tokens fallback, and .json judge transcript siblings.

Fallbacks a current producer still needs stay (pre-0.12.12 agent_wall_ms,
0.11.1 Scribe runs, salvaged run.json rows, mature carry-forward rows); their
comments now name that producer instead of calling them legacy.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The format cleanup removed result_kind guessing, the message gap grouping and the
legacy input_tokens adopter, but a field description, a test name, the CE060 lint
message, HARNESS_PARITY.md, the lint-rules note and the harness-candidates list still
described them. Comments that called a live path legacy (the token-totals pick for
harnesses with no reconciliation entry, mature rows without token totals) now say
what still produces those rows.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@bai-uipath
bai-uipath marked this pull request as ready for review October 22, 2026 18:45
@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Claude finished @bai-uipath's task in 1m 21s —— View job


Code Review

Working on reviewing PR #235...

Todo List

  • Read code review guidelines from .github/code_review.md
  • Review the full diff
  • Check cross-file consistency
  • Analyze what's missing
  • Provide design-level scrutiny

pip-audit fails CI on opentelemetry-api 1.40.0 (CVE-2026-114603, fixed in 1.45.1).
azure-monitor-opentelemetry-exporter 1.0.0b53 pinned opentelemetry ==1.40, so it moves
to 1.0.0b58 (~=1.45.0). All five packages are at least 15 days old.

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

@uipreliga uipreliga left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: coder_eval — pr:235 (41 files) axis:1,2,3,4,5,6,7,8 — "refactor: fold delegate-sdk into delegate, drop run-format fallbacks older than 30 days" by @bai-uipath · bai/evalboard-cleanup → main · OPEN

Scope: pr:235 (41 files) axis:1,2,3,4,5,6,7,8 — "refactor: fold delegate-sdk into delegate, drop run-format fallbacks older than 30 days" by @bai-uipath · bai/evalboard-cleanup → main · OPEN · branch bai/evalboard-cleanup (PR #235 head; reviewed via worktree) · db01977 · 2026-10-22T19:01Z · workflow variant

Change class: complex — removes read-side compatibility fallbacks in Python models (TokenUsage, CriterionResult result_kind, judge transcripts) and evalboard run/message parsing, changing which persisted run formats load; plus a harness-identity fold (delegate-sdk → delegate)

The codebase is healthy (9.7/10, no critical or high findings, and no finding changes a task's score or final_status for identical agent output), but the PR that removed the legacy shims left behind silent-drift hazards: a vacuous HTML token test, a second hand-kept list of judge types, a TokenUsage input that is dropped with no error, and evalboard readers that render wrong verdicts for older records, so fix these small items before the next release.

Summary

Axis Score 🔴 🟠 🟡 🔵 Top Issue
1. Code Quality & Style 9.1 / 10 0 0 1 4 JUDGE_CRITERION_TYPES is a hand-kept second source of truth that duplicates result_kind, and the PR deleted its drift guard
2. Type Safety 10 / 10 0 0 0 0 —
3. Test Health 9.1 / 10 0 0 1 4 Missed TokenUsage(input_tokens=...) call site makes test_task_html_renders_token_usage pass vacuously
4. Security 9.9 / 10 0 0 0 1 normalizeHarness indexes a plain-object alias map with a URL-supplied key, so ?h=constructor / ?h=proto / ?h=toString returns a non-string (inherited prototype member)
5. Architecture & Design 9.9 / 10 0 0 0 1 The delegate-sdk alias is applied at about ten consumer sites instead of once where the harness is read; RunSummary/RunListingRow still carry the raw id
6. Error Handling & Resilience 9.9 / 10 0 0 0 1 parseCriterionResults reads required criterion fields from task.json over an unchecked cast; older records render wrong verdicts
7. API Surface & Maintainability 9.8 / 10 0 0 0 2 TokenUsage now silently drops an input_tokens input (public SPI model, no error)
8. Evaluation Harness Quality 9.9 / 10 0 0 0 1 Folding delegate-sdk into delegate hides a known measurement change inside one trend series

Overall Score: 9.7 / 10 · Weakest Axis: Code Quality & Style at 9.1 / 10
Totals: 🔴 0 · 🟠 0 · 🟡 2 · 🔵 14 across 8 axes.

Blockers

None.

Non-blocking, but please consider before merge

  1. [Axis 1] JUDGE_CRITERION_TYPES is a hand-kept second source of truth that duplicates result_kind, and the PR deleted its drift guard (src/coder_eval/models/results.py:236) — The PR removed the legacy criterion_type inference from _criterion_result_discriminator. Every row now dispatches only on result_kind (return str(v.get("result_kind") or "basic")). That inference was the reason this set existed. The PR kept JUDGE_CRITERION_TYPES = frozenset({"llm_judge", "agent_judge", "system_one_judge"}) as the only gate for the HTML judge-verdict section (reports/html.py:443: if cr.criterion_type not in JUDGE_CRITERION_TYPES: continue). It also deleted the # CONVENTION: a new criterion type needing a non-"basic" result class adds its criterion_type to the matching frozenset in the same PR. comment. So the class is now known from two sources: result_kind == "judge" and a closed list of type names that nothing keeps current. A new judge criterion, in-tree or from a plugin through the SPI, that returns JudgeCriterionResult will deserialize correctly. But its findings and transcript card will silently disappear from the HTML report until someone remembers this list. Fix: gate html.py on isinstance(cr, JudgeCriterionResult) (or cr.result_kind == "judge"). Then delete JUDGE_CRITERION_TYPES and its coder_eval.models re-export. The html.py comment at line 442 (or in model_extra (round-tripped)) describes the same removed legacy path and should go too.
  2. [Axis 3] Missed TokenUsage(input_tokens=...) call site makes test_task_html_renders_token_usage pass vacuously (tests/test_reports_html.py:795) — The PR removes TokenUsage._adopt_legacy_input_tokens and changes five TokenUsage(input_tokens=...) call sites in this file to uncached_input_tokens= (lines 235, 634, 888, 979, 988). It misses test_task_html_renders_token_usage at lines 794-799: result.total_token_usage = TokenUsage(\n input_tokens=1000,\n output_tokens=500,\n cache_creation_input_tokens=200,\n cache_read_input_tokens=300, ...). Without the validator, Pydantic drops the computed-field kwarg. I ran a probe and the object builds as uncached_input_tokens=0 ... input_tokens=500. The rendered card shows Input (uncached) 0 and Total 1,000. The assertion assert "1,000" in html now matches the Total cell, not the input count. The test passes but no longer checks that uncached input renders. Fix: change line 795 to uncached_input_tokens=1000, and assert on the labelled cell (Input (uncached)</div><div class="value">1,000) so the value cannot match a different cell. Root cause: CE016 (ce016_no_computed_tokenusage_kwargs.py, whose docstring says this pattern "already bit a test") scans only src/. test_no_violations scans tests only for rules in _ALSO_SCAN_TESTS = {"CE048"}. The validator this PR removes was the only thing that made this pattern safe in tests. Add CE016 to _ALSO_SCAN_TESTS in tests/test_custom_lint.py so the next straggler fails make lint. An AST scan of tests/ finds this site as the only remaining violation.

Nits

  1. [Axis 1] Orphaned cascade-tree comment in buildThinkingModel now sits above unrelated code (evalboard/lib/thinkingSim.ts:276) — Lines 276-281 (// The prompt-cache cascade is a TREE, not a flat list. ... Each call's // branch id (parent_tool_use_id) is what models that tree.) used to sit above the branchKnown guard that the PR deleted. Now they sit directly above const perMessageOutputSum = messages.reduce( (line 282), which has nothing to do with branches. The branch-tree code they describe starts about 90 lines lower, at const branchTotal = new Map<string | null, number>(); (line 375), which already has its own comment. Delete the paragraph, or move the one useful sentence next to branchTotal.
  2. [Axis 1] format_token_usage keeps duck-typed Any + getattr + try/except after the fallback removal (src/coder_eval/formatting.py:177) — The PR deleted the docstring sentence that justified duck-typing ("for objects that predate the split / don't expose the uncached field") and collapsed the code to input_tokens = getattr(usage, "uncached_input_tokens", 0) or 0. It kept the signature def format_token_usage(usage: Any) -> str: (line 163) and the except (AttributeError, TypeError): (line 186). All three callers (streaming/renderers.py:123,226,230) pass a TokenUsage (AgentEndEvent.usage / TurnEndEvent.tokens: TokenUsage | None). A getattr with a default cannot raise AttributeError. Type the parameter as TokenUsage | None, read the attributes directly, and drop the try/except, so the defensive code does not suggest that non-TokenUsage inputs still occur.
  3. [Axis 1] harness-candidates DONE entry contradicts itself and describes the deleted validator in present tense (.claude/harness-candidates.md:549) — The entry now reads **DONE.** The validator is gone: every run uploaded in the last 30 days stores uncached_input_tokens, so no pre-split record is read. The validator copies a legacy record's full-prompt input_tokens ... (lines 549-550). It still describes the removed validator in the present tense and ends with a real bug needing its own change, with a decision about whether legacy records can be distinguished. The edit also produced a 124-char line (550) in a file wrapped at about 90. The same reflow problem occurs in .claude/notes/lint-rules.md:720 (124 chars: became contiguous (the gap is then exactly 0 ms, always). Nothing failed: ...). Collapse the DONE entry to the resolution plus one line of history (put the rest in git), and re-wrap both paragraphs.
  4. [Axis 1] Second describe("normalizeHarness") block added in a different test file (evalboard/lib/__tests__/harness.test.ts:105) — The PR adds describe("normalizeHarness", () => { test("the retired delegate-sdk id folds into delegate", ... in harness.test.ts. evalboard/lib/__tests__/overview.test.ts:538 already has describe("normalizeHarness", ...) for the same function, and this PR edited it too (the null/undefined test). normalizeHarness lives in lib/harness.ts, so move the overview.test.ts block into harness.test.ts and keep one describe for the function.
  5. [Axis 3] Discriminator round-trip test is named for the removed criterion_type rule and does not test the new rule (tests/test_criterion_result_round_trip.py:118) — test_result_kind_alone_picks_the_class has the docstring "result_kind picks the class; a judge criterion_type does not." Its payload still sets "result_kind": "basic" (line 129), so it tests the old rule that an explicit result_kind wins over inference. It also passes on main. The other remaining test, test_unknown_criterion_type_falls_to_basic, uses "future_check", which was basic under the old code too. No test fails if someone adds criterion_type inference back to _criterion_result_discriminator. The new code is return str(v.get("result_kind") or "basic"). Fix: drop result_kind from the payload, keep criterion_type: "llm_judge" plus a judge-only key (findings), and assert type(cr) is CriterionResult and that findings survives in model_extra. Also add one assertion that model_dump() of each subclass (JudgeCriterionResult, ClassificationCriterionResult) emits its result_kind. Since inference is gone, result_kind alone decides the class on reload.
  6. [Axis 3] The render tests now use 'delegate' only, so no test renders the 'delegate-sdk' id that runs before the 2026-10-07 switch still carry (evalboard/app/_components/__tests__/harness-badge.test.tsx:33) — The PR changes the badge render test to render(<HarnessBadge harness="delegate" />); (line 33) and run-identity.test.tsx to harness="delegate" (line 24). Before, both rendered "delegate-sdk". Runs from before the 2026-10-07 switch, which are inside the 30-day window, still stamp delegate-sdk. The PR also adds a second normalizeHarness(harness) call inside HarnessBadge (harness-badge.tsx:51), separate from the one in harnessShortLabel. Only the harnessShortLabel("delegate-sdk") label lookup is tested (line 17). No test renders the logo or alt text for the old id. Fix: add render(<HarnessBadge harness="delegate-sdk" />) and assert getByAltText("Delegate · UiPath"). Keep the RunIdentity case on delegate-sdk, because that is what the run pages for older runs actually pass in.
  7. [Axis 3] The 'aged-out deep link' selector test uses a pinned harness, so it never runs the branch that adds current to the options (evalboard/app/_components/__tests__/harness-selector.test.tsx:55) — a deep-linked harness that has aged out still shows as a segment renders current="delegate". HarnessSelector always adds every KNOWN_HARNESSES entry to its options (orderHarnesses([...KNOWN_HARNESSES, ...harnesses, ...(current == null ? [] : [current])])), and delegate is now in KNOWN_HARNESSES. So the Delegate segment shows even if the current part is deleted. The test passes without exercising the code it is named for, and it only repeats the next test (a known harness absent from the discovery window still shows). The same problem existed on main with delegate-sdk; this PR edits the line but leaves it in place. Fix: use a harness id that is not in KNOWN_HARNESSES (e.g. current="opencode" or "pi") and assert that its segment exists with aria-pressed="true".
  8. [Axis 3] The PR changes format_token_usage, but no test calls it; the cached= branch and the 'in= is uncached' rule have no tests (src/coder_eval/formatting.py:176) — The PR replaces the fallback with input_tokens = getattr(usage, "uncached_input_tokens", 0) or 0 (line 176). grep -rn format_token_usage tests/ returns nothing. The function runs only indirectly through streaming renderers. Routed coverage shows lines 175, 184, and 186-187 uncovered, including the cached= branch. The docstring's main rule (in= shows the uncached slice, not the derived total, so the cached prefix is not counted twice) has no test. Fix: add a small parametrized test: TokenUsage(uncached_input_tokens=100, output_tokens=5, cache_read_input_tokens=900) gives "in=100, out=5, cached=900" (not in=1000), all-zero usage gives "", and None gives "".
  9. [Axis 4] normalizeHarness indexes a plain-object alias map with a URL-supplied key, so ?h=constructor / ?h=proto / ?h=toString returns a non-string (inherited prototype member) (evalboard/lib/harness.ts:28) — PR #235 adds const HARNESS_ALIASES: Record<string, string> = { "delegate-sdk": "delegate" }; (line 24) and return HARNESS_ALIASES[h] ?? h; (line 28). It also routes the untrusted ?h= query value through this lookup: parseHarnessParam now does ? normalizeHarness(trimmed) and parseHarnessScope does ? normalizeHarness(trimmed) : null. The guard regex /^[\w.-]{1,64}$/ accepts constructor, __proto__, toString, valueOf and hasOwnProperty, and those keys resolve to inherited Object.prototype members. I checked this in node: constructor gives function Object(), __proto__ gives Object.prototype, and toString gives a function. The value is typed string, but at runtime it is a function or an object. It flows to /trends, /watchlist and / (overview), where it goes into <HarnessSelector current={harness}>, a "use client" component, and into harnessShortLabel(...) as a rendered child. Next.js cannot serialize a function or a prototype object across the server/client boundary, so a crafted link gives a 500 for whoever opens it. I checked cache poisoning and found none: the overview and listing are not cached by argument. In aggregateTaskTrends and historyForTask, the bad values serialize to null or {} in the unstable_cache key, and no legitimate request produces those keys, so no cross-user entry is overwritten. Impact is a reflected per-request crash only. Fix: change the lookup to Object.hasOwn(HARNESS_ALIASES, h) ? HARNESS_ALIASES[h] : h, or make the alias table a Map or a null-prototype object (Object.assign(Object.create(null), {...})). Add a unit test in evalboard/lib/__tests__/harness.test.ts that asserts parseHarnessParam("constructor") and parseHarnessScope("__proto__") return the input string. This was not present before the PR, because parseHarnessParam returned trimmed as is. No downstream (coder_eval_uipath) contract impact. CVSS:3.1/AV:N/AC:L/PR:L/UI:R/S:U/C:N/I:N/A:L
  10. [Axis 5] The delegate-sdk alias is applied at about ten consumer sites instead of once where the harness is read; RunSummary/RunListingRow still carry the raw id (evalboard/lib/harness.ts:24) — const HARNESS_ALIASES: Record<string, string> = { "delegate-sdk": "delegate" }; (harness.ts:24) takes effect only through normalizeHarness, and every reader must call it: overview.ts:483, 509, 786, 810, 1045, 1222; harness-badge.tsx:39, 51; harness.ts:41, 62. The one place the harness is read, extractRunConfig (runs.ts:1203; typeof r.harness === "string" && r.harness ? r.harness : mostCommonAgentType(...)), still returns raw delegate-sdk into RunSummary.harness and RunListingRow.harness. All current sites normalize, so nothing is broken today. Before this PR, a missed call only mislabeled a null harness. Now a new equality check or harnessColor(...) call that skips normalizeHarness splits the Delegate series again (gray fallback color, separate selector entry). Fold the alias once in extractRunConfig/mostCommonAgentType, so the data layer emits canonical ids and the consumer calls become redundant. Note also that the alias exists only in the TS display layer. The downstream eval_runner derives per-harness values (expected_seconds, turn budget) that it stamps into run.json from raw agent_config.type, so its history stays split at delegate-sdk/delegate even though the board shows one line. Say so in the comment, or document it in the PR as a known display-only fold.
  11. [Axis 6] parseCriterionResults reads required criterion fields from task.json over an unchecked cast; older records render wrong verdicts (evalboard/lib/runs.ts:181) — The PR made RawCriterionResult fields required in the type (runs.ts:167-169 evaluation_status: "evaluated" | "not_evaluated"; pass_threshold: number; gating: boolean;) and removed the ?? defaults at runs.ts:181-183 (evaluationStatus: criterion.evaluation_status, passThreshold: criterion.pass_threshold, gating: criterion.gating,). The data is untyped JSON read by readJson (a bare JSON.parse(raw) as T), so the new type is a cast, not a check. A missing key arrives as undefined, and the compiler can no longer flag it. Consumers in _sections.tsx then render wrong data rather than a safe default: line 136 const evaluated = c.evaluationStatus === "evaluated" shows NOT EVALUATED for an evaluated criterion; line 118 criteria.some((c) => !c.gating) labels every criterion 'informational'; line 135 (c.score ?? 0) >= c.passThreshold compares against undefined, so the result is always false. These records are still reachable. getRunListing (overview.ts:1214) paginates over every run id in the container with no age window, and EVALBOARD_LOCAL_RUNS_DIR (blob.ts:13) serves a local coder_eval runs/ tree of any age. So a pre-0.10.0 run (evaluation_status was added in 636e87d, first tag v0.10.0, 2026-08-18) still in the runs container shows every criterion as NOT EVALUATED. A pre-0.8.9 run shows every criterion as informational. The flat-layout removal in the same file (runs.ts:2482 const contentDir = replicateContentDir(taskDir, replicate);) has the same silent shape: an older flat run gets task = null and renders an empty detail page, with no sign that the format is unsupported. No still-produced path emits these shapes, so this is Low. Pick one fail mode on purpose. Option 1: keep the three ?? defaults (one line each, and they match the Python model defaults evaluated / 0.9 / True). Option 2: keep the fields optional in RawCriterionResult, and have readTaskDetail return an explicit 'run format older than supported' state when a criterion lacks evaluation_status or the nested <NN>/task.json is missing. Then an older run reads as unsupported instead of as wrong data.
  12. [Axis 7] TokenUsage now silently drops an input_tokens input (public SPI model, no error) (src/coder_eval/models/telemetry.py:57) — The PR removes _adopt_legacy_input_tokens. input_tokens is now only the @computed_field at line 57 (def input_tokens(self) -> int:), and TokenUsage keeps Pydantic's default extra='ignore'. So TokenUsage(input_tokens=1000, output_tokens=5) and TokenUsage.model_validate({"input_tokens": 1000}) both build uncached_input_tokens=0, with no warning. Verified on pr-235: both print 0. Before the PR, both forms were adopted as the uncached slice; this PR's own test_reports_html.py edits had to rewrite four TokenUsage(input_tokens=...) call sites that depended on that. CE016 guards this repo's tree and coder_eval_uipath ships its own CE016. A third-party agent plugin on the same SPI path is not guarded: it under-counts input and under-prices any rate-card backfill, with no error. Recommendation: make the drop loud. Add a small mode="before" check that raises when input_tokens is present without uncached_input_tokens; a persisted record always carries both, so the read path is unaffected. Alternative: note the read-only nature in the class docstring. Cross-repo note: the comment at coder_eval_uipath eval_runner/src/eval_runner/skills/salvage.py:144-145 ('the same adoption coder_eval's TokenUsage applies on load') is now false. That file is out of scope, but flag it to the downstream repo.
  13. [Axis 7] HARNESS_PARITY 'The collapse is a display defect' paragraph now has no antecedent and describes the old failure mode (docs/agents/HARNESS_PARITY.md:418) — The rewritten paragraph at line 411 now says an emission without a message_id "is never joined, so each of its content blocks renders as its own row", which is a split, not a collapse. The next paragraph is unchanged: "The collapse is a display defect, not an accounting one" (line 418) and "three displayed figures do move when a turn stops collapsing" (line 421). It still describes the removed gap-heuristic collapse as the current hazard. Rewrite it for the new failure mode: a harness that omits message_id now over-splits rows, which inflates the Messages count and the thinking-simulator calls. Or label it explicitly as the history of the Antigravity defect.
  14. [Axis 8] Folding delegate-sdk into delegate hides a known measurement change inside one trend series (evalboard/lib/harness.ts:24) — const HARNESS_ALIASES: Record<string, string> = { "delegate-sdk": "delegate" }; (line 24) feeds normalizeHarness (line 28: return HARNESS_ALIASES[h] ?? h;). The badge, the selector and every overview series filter then show the two adapters as the same harness. The adapters do not measure the same way. docs/agents/HARNESS_PARITY.md:518-528 says delegate-sdk records have a tool duration_ms with no execution bounds, so that time reads as Unaccounted. The built-in delegate bounds its tools, so the same time reads as Tool exec. evalboard/lib/timing.ts:184 now also says "The retired delegate-sdk adapter is the one harness that recorded only bare durations." On the merged "Delegate" line, the wall-clock and efficiency charts will therefore jump on 2026-10-07 for a reason the board does not show. Pass-rate shifts caused by the adapter change will also look like regressions in the model or skills. No board surface still shows the raw stamp, so this is a cross-run comparability gap. It does not change any score. Fix: keep the fold, but mark the switch date on the Delegate series (an annotation or a legend footnote), or show the raw delegate-sdk stamp in the run header / RunIdentity so an affected run can be identified.

What's Missing

Tests:

  • 🟡 The PR's main feature, one Delegate series across the 2026-10-07 switch, is tested only at the function level: harness.test.ts:105 checks normalizeHarness, parseHarnessParam and parseHarnessScope. No data-layer test feeds overview.ts one run stamped delegate-sdk and one stamped delegate, then asserts three things: getOverview gives one series and one selector entry, ?h=delegate returns both runs from getOverview, getRunListing and aggregateTaskTrends, and harnessColor gives the same color for both. Of the ten normalizeHarness call sites, six are in overview.ts (lines 483, 509, 786, 810, 1045, 1222). If one of them loses its call, the series splits again and every test still passes. (trigger: evalboard/lib/overview.ts) (restates: Axis 5: The delegate-sdk alias is applied at about ten consumer sites instead of once where the harness is read)

Downstream consumers:

  • 🔵 The PR removes the wall-clock gap grouping on the grounds that every message has a message_id. The 30-day field scan covered only the three evalboard containers, which hold claude-code, codex, antigravity and delegate runs. Three agents can still emit message_id=None at runtime: opencode_agent.py:618 (str(part.get("messageID") or "") or None), pi_agent.py:553 (responseId) and claude_code_agent.py:531 (when the id is not a str). The CE060 docstring lists this as BLIND SPOT 1. For those runs, parseMessages now puts every content block on its own row. This inflates the Messages count and the thinking-simulator calls. Before this PR, the gap heuristic merged those blocks. Missing: a golden or contract test that each built-in agent's emitted AssistantMessage carries a non-None message_id at runtime, or a synthesized id in place of the or None fallbacks, as Antigravity, Codex and Delegate already do. (trigger: evalboard/lib/runs.ts) (restates: Axis 7: HARNESS_PARITY 'The collapse is a display defect' paragraph now has no antecedent and describes the old failure mode)
  • 🔵 The PR justifies the Python read-side removals with the 30-day scan of evalboard blob uploads. The Python readers do not read that blob window. They read local run directories of any age: coder-eval report (report_command.py:168), evaluate <run_dir> (regrade.py:66), the --resume loader (batch.py:489, 528), harbor atif_emit.py:408 and reports/helpers.py:238. A task.json from before the uncached/result_kind split now validates without an error, but it loads with uncached_input_tokens=0 and judge rows as base CriterionResult. The findings stay in model_extra and the HTML judge card is hidden. Those formats predate the 2026-07-09 public release, so the exposure is small. But neither the PR nor docs/REPORT_SCHEMA.md states the supported record age, and nothing fails loudly. Fix: state the window in REPORT_SCHEMA.md, or add a mode="before" guard that raises on a pre-split shape. (trigger: src/coder_eval/models/telemetry.py) (restates: Axis 7: TokenUsage now silently drops an input_tokens input (public SPI model, no error))

Parallel paths:

  • 🔵 The producer still writes env_info git_commit on every run (utils.py:449), and regrade still records it as graded_by_git_commit (orchestrator.py:732). But extractComponentShas (runs.ts) now reads only coder_eval. When version("coder_eval") raises PackageNotFoundError, utils.py:473 writes coder_eval: "unknown", which happens for a source checkout that is not pip-installed. The board then shows no coder_eval chip, where before it showed the tree-linked SHA. The deleted test "an 'unknown' version still falls through to the SHA" covered this live case, not only legacy runs. Either keep the fallback key, or stop writing git_commit. Right now the producer and the consumer disagree. (trigger: evalboard/lib/runs.ts)
  • 🔵 format_token_usage keeps its documented rule that in= shows uncached_input_tokens and never the derived total, which would count the cached prefix twice. The PR removed its input_tokens fallback. The judge-transcript card in reports/html.py:563 renders the same tokens: in= label from token_usage.get("input_tokens") on the dumped TokenUsage dict. That value is the derived total and includes cache_read and cache_creation. So the static HTML report and the streaming renderer still mean different things by in=, and the PR left this parallel path as it was. (trigger: src/coder_eval/formatting.py)

Nightly pipeline:

  • 🔵 The PR changes only uv.lock for the CVE-2026-114603 fix (opentelemetry-api 1.40.0 to 1.45.1). The pyproject.toml floors stay opentelemetry-sdk>=1.30.0 and azure-monitor-opentelemetry-exporter>=1.0.0b30, so the published wheel still allows the vulnerable versions. The nightly runs from coder_eval_uipath, and its uv.lock still resolves opentelemetry-api 1.43.0 (exporter 1.0.0b55), which is below the fix. The lock bump also reaches the DockerRunner agent image through uv export --frozen (docker/Dockerfile:107), with the beta exporter moving from b53 to b58. The PR validated this with unit tests only, not with a real App Insights usage-telemetry emission. The PR does not say whether the nightly path is still exposed or whether the image's telemetry export was checked. Fix: raise the floor (opentelemetry-sdk>=1.45.1) or record the downstream follow-up. (trigger: uv.lock)

Harness & Lint Improvements

Static checks (lint / type):

  • [ce-lint] Add CE016 to _ALSO_SCAN_TESTS in tests/test_custom_lint.py:31 so the TokenUsage(input_tokens=/total_tokens=) rule (tests/lint/rules/ce016_no_computed_tokenusage_kwargs.py) also scans tests/. CE016 has no path-sensitive layering logic, so the CE004/CE066 disjointness guard at test_custom_lint.py:2307 does not apply. Also widen CE016 to flag TokenUsage.model_validate({...}) with a dict literal that has an input_tokens/total_tokens key and no uncached_input_tokens key. Its docstring says this pattern 'already bit a test', and the rule scans only src/, so it cannot catch the test-tree case. Prevents: tests/test_reports_html.py:794-795 (TokenUsage(input_tokens=1000, ...) builds uncached=0, so test_task_html_renders_token_usage passes vacuously on the Total cell). An AST scan shows this is the only remaining test-tree hit, so the change is green right after that one fix. Partly covers the in-repo half of the telemetry.py:57 silent-drop finding.
  • [ce-lint] New CE069 (CE067 is retired; do not reuse it): 'no membership test of .criterion_type against a module-level set of type-name literals'. AST: a Compare whose left side is an Attribute(attr='criterion_type') with an In/NotIn operator, where the right side is a Name bound to a frozenset/set/tuple of str literals. Dispatch on the result class must use isinstance(cr, JudgeCriterionResult) or result_kind, which the discriminator already owns. Add it as tests/lint/rules/ce069_no_criterion_type_class_gate.py, register it in ALL_RULES in tests/lint/runner.py, and add a fixture test class in tests/test_custom_lint.py. Delete JUDGE_CRITERION_TYPES and its coder_eval.models re-export in the same change. Prevents: src/coder_eval/models/results.py:236 + reports/html.py:443 (if cr.criterion_type not in JUDGE_CRITERION_TYPES: continue): a second source of truth that duplicates result_kind. A plugin or new in-tree judge type that returns JudgeCriterionResult would silently lose its HTML verdict cards.
  • [ce-lint] New CE070 (a whole-tree text rule, written as a @pytest.mark.lint class in tests/test_custom_lint.py like CE065): in evalboard/lib and evalboard/app (non-test .ts/.tsx), forbid a module-level const NAME: Record<string, …> = { … } lookup table that is later indexed as NAME[expr]. Require a Map, a null-prototype object, or an Object.hasOwn(NAME, k) ? NAME[k] : … guard. strict/noUncheckedIndexedAccess cannot catch this, because TS types inherited prototype keys (constructor, __proto__, toString) as string. Generated files (pricing.generated.ts) are exempt or must emit a Map. Prevents: evalboard/lib/harness.ts:24/28 (HARNESS_ALIASES[h] ?? h with ?h=constructor returns function Object(), which gives a 500 across the RSC boundary). A probe finds the same shape at harness.ts:82 HARNESS_COLORS, which harnessColor() indexes with the same harness string.
  • [ce-lint] After the canonical-id fold moves into extractRunConfig/mostCommonAgentType (evalboard/lib/runs.ts:1203), add a text rule: normalizeHarness( may appear only in evalboard/lib/harness.ts and evalboard/lib/runs.ts (and in the parseHarnessParam/parseHarnessScope URL entry points). A consumer-site call then fails the build, so the alias has one application point, not about ten. Prevents: evalboard/lib/harness.ts:24 architecture finding: the alias is applied at overview.ts:483/509/786/810/1045/1222 and harness-badge.tsx:39/51, while RunSummary/RunListingRow still carry raw delegate-sdk. A new equality check or harnessColor call that skips the normalize step would split the Delegate series again.
  • [ruff] Enable ANN401 (flake8-annotations any-type) for the leaf modules that do not wrap SDK payloads: formatting.py, durations.py, stats.py, result_metrics.py, run_record.py, reports/. Enabling it globally gives 161 hits, mostly in agents/*, so scope it with extend-select = ["ANN401"] plus per-file-ignores for agents/, orchestration/config_merge.py and models/criteria.py. The scoped set has 10 current hits (formatting.py:99,121,163; reports/html.py:264,300,474,506,809; reports/junit.py:130; reports/markdown.py:79). Fix them or mark each # noqa: ANN401 with a reason. A first-party model then has to be typed as itself, and the getattr(..., default) + except AttributeError duck typing has nowhere to live. Prevents: src/coder_eval/formatting.py:163/177/186: format_token_usage(usage: Any) keeps getattr(usage, "uncached_input_tokens", 0) and a dead except (AttributeError, TypeError) after the legacy fallback was removed, although every caller passes TokenUsage | None.
  • [ci-gate] Extend make docs-budget with a prose-wrap cap for .claude/notes/*.md and .claude/harness-candidates.md: no line outside a code fence or table may exceed 120 chars. Today 13 lines are over (reporting.md 10, harness-candidates.md 2, agents.md 1). Reflow them in the same change, then set the cap at or below the new max, per the 'a cap rule sets its limit below the current value' convention. Prevents: .claude/harness-candidates.md:550 and .claude/notes/lint-rules.md:720: 124-char lines left by an edit that was not reflowed. This does not catch the stale present-tense content of the DONE entry. That part is semantic (see harness bucket).
  • [ce-lint] Low-cost evalboard text rule: a describe("<identifier>" title whose identifier names an exported function may appear in only one file under evalboard/**/tests, and that file should be the test file named after the function's module (normalizeHarness → lib/tests/harness.test.ts). Prevents: evalboard/lib/tests/harness.test.ts:105 vs overview.test.ts:538: two describe("normalizeHarness") blocks for the same function in different files.

Harness improvements (not statically reachable):

  • Make the TokenUsage drop loud at runtime: add a mode="before" model validator on TokenUsage (src/coder_eval/models/telemetry.py) that raises when the input carries input_tokens or total_tokens without uncached_input_tokens. A persisted record always carries both, so the read path is unaffected. Flag the now-false comment at coder_eval_uipath eval_runner/src/eval_runner/skills/salvage.py:144-145 to the downstream repo. Why not static: CE016 covers only the trees this repo lints. A third-party agent plugin on the SPI path, or a model_validate(dict) built from runtime data, is outside any AST scan. Prevents: src/coder_eval/models/telemetry.py:57 (silent under-count and under-price for plugin agents). It is also a second line of defence for the tests/test_reports_html.py:794 vacuous test.
  • Add a legacy-format fixture suite for the evalboard: commit a minimal pre-0.10.0 task.json (no evaluation_status), a pre-0.8.9 one (no gating/pass_threshold) and a flat-layout run under evalboard/lib/tests/fixtures/. Assert that parseCriterionResults/readTaskDetail either keep the Python-model defaults (evaluated / 0.9 / True) or return an explicit 'run format older than supported' state, and never render NOT EVALUATED or 'informational' for an evaluated gating criterion. Why not static: readJson is a bare JSON.parse(raw) as T (runs.ts:807, plus runs.ts:1515 and reviews.ts:38/68). TS sees the required fields as present, so only a run against real old-shaped data shows the undefined reaching _sections.tsx:118/135/136. zod or another runtime validator is not a dependency, so a type-level gate is not available without adding one. Prevents: evalboard/lib/runs.ts:167-183 (required fields over an unchecked cast; ?? defaults removed) and the runs.ts:2482 flat-layout empty-page path.
  • Add a discriminator contract test in tests/test_criterion_result_round_trip.py, parametrized over every CriterionResult subclass: model_dump() emits its result_kind, model_validate(dump) returns the same class, and a payload that has a judge criterion_type and judge-only keys but NO result_kind loads as plain CriterionResult with those keys kept in model_extra. Name the test for the rule it tests. Why not static: Whether the discriminator infers from criterion_type is behaviour of a Pydantic callable. Only a round-trip run shows that inference came back. Prevents: tests/test_criterion_result_round_trip.py:118: a stale test, named for the removed rule, that still sets result_kind and passes on main. It is also the behavioural backstop for the JUDGE_CRITERION_TYPES removal (results.py:236).
  • Add a changed-line coverage gate to make verify/CI (e.g. diff-cover coverage.xml --compare-branch=origin/main --fail-under=90) next to the 80% global floor. Seed it with a direct parametrized test for format_token_usage: uncached=100/out=5/cache_read=900 gives in=100, out=5, cached=900; all-zero usage gives ; None gives . Why not static: The PR edited format_token_usage and left the cached= branch (formatting.py:175,184,186-187) uncovered. That shows only in coverage data from an executed test run, not in the AST. Prevents: src/coder_eval/formatting.py:176: a modified function with no direct test, whose main rule ('in= is the uncached slice') has no assertion.
  • Evalboard test hygiene for alias and known-set code: (a) keep at least one render test per retired id that runs still stamp inside the 30-day window. Render <HarnessBadge harness="delegate-sdk" />, assert getByAltText("Delegate · UiPath"), and keep RunIdentity on delegate-sdk. (b) Any test named for the 'aged-out / unknown current' branch of HarnessSelector must use an id outside KNOWN_HARNESSES (e.g. opencode) and assert aria-pressed="true". Add a guard assertion in the same test that the chosen id is not in KNOWN_HARNESSES, so a later KNOWN_HARNESSES edit cannot make the test vacuous again. Why not static: Whether a test reaches the branch it is named for depends on runtime set membership (KNOWN_HARNESSES content) and on which ids live data carries. No text pattern tells a vacuous fixture from a meaningful one. Prevents: evalboard/app/_components/tests/harness-badge.test.tsx:33 (the old id is never rendered) and harness-selector.test.tsx:55 (the current option branch is never exercised).
  • Add a regression test for the prototype-key hazard as a backstop to the proposed CE070: in evalboard/lib/tests/harness.test.ts, assert that parseHarnessParam/parseHarnessScope/normalizeHarness/harnessColor return a string for every key in Object.getOwnPropertyNames(Object.prototype). Why not static: CE070 catches the declared shape. This test also covers a lookup routed through a helper, a spread, or a computed table, which a text rule cannot follow. Prevents: evalboard/lib/harness.ts:28 (?h=constructor/__proto__/toString gives a reflected 500) and the same risk at harness.ts:82 HARNESS_COLORS.
  • Make the measurement boundary visible when the board folds harness ids: keep the delegate-sdk → delegate fold, but show the raw agent_config.type in RunIdentity and add a dated annotation (2026-10-07, 'adapter switch: tool time moves from Unaccounted to Tool exec') on the Delegate series in the trend and efficiency charts. Also record in docs/agents/HARNESS_PARITY.md that the fold is display-only, because the downstream eval_runner still keys expected_seconds and the turn budget on the raw id. Why not static: Whether two adapters are comparable is a measurement-semantics judgment (HARNESS_PARITY.md:518-528). No code pattern encodes it. Prevents: evalboard/lib/harness.ts:24 Axis 8 finding (a step change in wall-clock/efficiency or pass rate on the merged Delegate line that looks like a model or skill regression) and the cross-repo half of the Axis 5 alias finding.
  • Add a review/PR checklist item to the implement-plan final review: when a PR removes a code path (a legacy fallback, a heuristic, a validator), grep for the removed symbol and its domain words in comments, docs/ and .claude/ (e.g. collapse, legacy, model_extra, predate), and rewrite or delete every hit in the same PR. Why not static: Whether a comment or paragraph still describes current behaviour is semantic. A grep finds candidates, but only a reader can decide that 'The collapse is a display defect' is stale or that a comment is now orphaned above unrelated code. Prevents: evalboard/lib/thinkingSim.ts:276 (orphaned cascade-tree comment), docs/agents/HARNESS_PARITY.md:418 (describes the old collapse failure mode), reports/html.py:441-442 (the 'model_extra round-tripped' comment), and the present-tense DONE entry at .claude/harness-candidates.md:549.

Top 5 Priority Actions

  1. No finding changes a score or final_status. Of the rest, fix tests/test_reports_html.py:795 first: change input_tokens=1000 to uncached_input_tokens=1000 and assert on the labelled Input (uncached) cell, because the test now passes only because the Total cell happens to show 1,000. Then add CE016 to _ALSO_SCAN_TESTS in tests/test_custom_lint.py:31 so make lint catches the next case like this.
  2. Gate the HTML judge section on cr.result_kind == "judge" (or isinstance(cr, JudgeCriterionResult)) at src/coder_eval/reports/html.py:443. Then delete JUDGE_CRITERION_TYPES (src/coder_eval/models/results.py:236), its re-export from coder_eval.models, and the stale model_extra comment, so a new judge criterion's findings cannot disappear from the report.
  3. Make TokenUsage raise an error when input_tokens is given without uncached_input_tokens (src/coder_eval/models/telemetry.py:57), so a third-party SPI agent cannot silently under-count input and under-price cost. Also tell coder_eval_uipath that the adoption comment at salvage.py:144-145 is now false.
  4. In evalboard/lib/runs.ts:181, put back the ?? defaults (evaluated / 0.9 / true) for evaluation_status, pass_threshold and gating, or show an explicit 'run format older than supported' state. Today pre-0.10.0 records show every criterion as NOT EVALUATED or informational.
  5. Harden the harness alias in evalboard/lib/harness.ts:24-28. Use Object.hasOwn or a Map so ?h=constructor or ?h=__proto__ cannot cause a 500, and add a test for it. Apply the delegate-sdk to delegate mapping once in extractRunConfig (runs.ts:1203) instead of at about ten consumer sites. Mark the 2026-10-07 adapter switch on the merged Delegate series, because the two adapters measure tool time differently.

Stats: 0 🔴 · 0 🟠 · 2 🟡 · 14 🔵 across 8 axes reviewed.

@uipreliga
uipreliga self-requested a review October 22, 2026 20:24

@uipreliga uipreliga left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix what you agree with and 🚢

bai-uipath and others added 4 commits October 22, 2026 14:01
…token test

The HTML judge section checked criterion_type against JUDGE_CRITERION_TYPES, a second hand-kept list of judge types. It now checks isinstance(cr, JudgeCriterionResult), which result_kind already decides on reload, and the set is deleted.

test_task_html_renders_token_usage passed TokenUsage(input_tokens=...), which builds uncached_input_tokens=0, so its assert matched the Total cell. It now sets uncached_input_tokens and asserts on the labelled cell. CE016 also scans tests/ so the next case fails make lint.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
?h=constructor or ?h=__proto__ returned an inherited prototype member from the alias table, which cannot cross the server/client boundary. normalizeHarness and harnessColor now use Object.hasOwn.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Older runs are still reachable from the run listing. Without the defaults, a row with no evaluation_status, pass_threshold or gating rendered as NOT EVALUATED or informational. The defaults match the Python model (evaluated, 0.9, gating).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Drops an orphaned comment in thinkingSim.ts, rewrites the HARNESS_PARITY paragraph for the split failure mode, collapses the TokenUsage DONE entry, and rewraps two long lines.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@bai-uipath
bai-uipath merged commit a253f34 into main Oct 22, 2026
18 checks passed
@bai-uipath
bai-uipath deleted the bai/evalboard-cleanup branch October 22, 2026 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants