Skip to content

feat(evalboard): make analytics surfaces harness-aware and stream tables#45

Merged
bai-uipath merged 5 commits into
mainfrom
bai/evalboard-trends-harness-scope
Jul 22, 2026
Merged

feat(evalboard): make analytics surfaces harness-aware and stream tables#45
bai-uipath merged 5 commits into
mainfrom
bai/evalboard-trends-harness-scope

Conversation

@bai-uipath

@bai-uipath bai-uipath commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

What

Make the evalboard's analytics surfaces harness-aware. Trends, the front-page success chart + tag rail, Path to GA, and the Watchlist are each scoped to a single harness (default claude-code) with a Claude Code / Codex / Antigravity switcher, instead of aggregating every recent pipeline run together. The front-page run list stays all-harness — showing every recent run is its job, and its Harness column already disambiguates each row. Trends and the Watchlist now stream their tables behind a Suspense boundary so the page shell paints immediately on a cold load.

Why

The nightly now rotates three harnesses — claude-code most days, codex and antigravity a few times a week — each uploaded as its own date-shaped pipeline run. The analytics surfaces took the last N pipeline runs regardless of harness and collapsed them into one per-task pass rate, one status strip, and one set of duration/cost/turn averages. Those harnesses have very different pass rates and cost profiles, so the blend made tasks that pass reliably on the primary nightly read as flaky or broken, painted half-red status strips, and produced meaningless averages. A trend, a streak, and a volatility score are only comparable within one harness.

Scoping to one harness restores a correct pass rate, a single-harness status strip, and comparable averages, and the switcher keeps the codex/antigravity views one click away. The switcher is populated from the harnesses actually present in recent runs rather than a hardcoded list, so a new harness (e.g. a future delegate) shows up automatically, and an unknown harness degrades to its raw id instead of breaking. Streaming the heavy tables behind Suspense also fixes the trends page feeling slow on a cold load — the header and switcher render right away while the table fills in.

Screenshots

image
image

bai-uipath and others added 3 commits July 22, 2026 14:58
Trends aggregated the last N pipeline runs regardless of harness, but the
nightly now rotates claude-code / codex / antigravity as separate runs — so a
single per-task pass rate, status strip, and averages were blended across
harnesses with incomparable pass rates and cost profiles, making
reliably-passing tasks read as broken.

Scope the page to one harness (default claude-code, the daily primary) with a
switcher, and fold the tag-rail counts into the cached aggregate so the window
loads once instead of twice per view.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Extend harness scoping beyond trends to the whole board and make the harness
set data-driven so a new harness (e.g. delegate) appears on its own:

- lib/harness.ts: shared leaf module (constants + normalize + a whitelist-free
  param parser) so client and server can import without bundle contamination.
- listRecentHarnesses(): cached discovery of which harnesses have recent runs;
  the switcher lists exactly those (plus the active one), no hardcoded set.
- Watchlist and path-to-ga: scoped to one harness (default claude-code) with a
  selector, like trends — their blended pass rates / streaks / charts were
  mixing incomparable harnesses.
- Front page: analytics (success chart + tag rail) scoped to a harness; the run
  LIST stays all-harness (its job is to show every recent run, and the Harness
  column already disambiguates each row).
- Tag rail preserves the active harness across chip clicks (fixes a reset on
  trends too).

Also stream the trends and watchlist tables behind a Suspense boundary so the
header + selector paint immediately instead of blocking on the cold data load.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The Suspense fallback rendered only the harness selector, while the loaded
view adds a "last N … runs" pill in the same right-aligned group, so the
selector jumped left when the pill appeared. Mirror the pill (pulsing count)
in the skeleton so the header geometry is stable through streaming.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@bai-uipath
bai-uipath marked this pull request as ready for review July 22, 2026 22:44
@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown

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


PR Review In Progress

Todo List:

  • Read .github/code_review.md for review standards
  • Read CLAUDE.md for project conventions
  • Get full PR diff (git diff origin/main...HEAD)
  • Read and analyze all 16 changed files
  • Check cross-file consistency
  • Analyze what might be missing
  • Provide comprehensive review feedback

Starting review now...

@bai-uipath bai-uipath changed the title feat(evalboard): scope task trends to one harness feat(evalboard): make analytics surfaces harness-aware and stream tables Jul 22, 2026

@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:45

Scope: pr:45 · branch bai/evalboard-trends-harness-scope · b78b5d1 · 2026-07-22T22:52Z · workflow variant

Change class: complex — changes analytics data-filtering/aggregation and cache-key logic (harness scoping) that governs which runs feed pass-rate/trend/watchlist numbers; correctness needs reasoning about filtering, scan-factor sizing, and per-harness cache isolation

coder_eval remains exceptionally healthy (9.7/10, with Security, Architecture, Error Handling, and Harness Quality all clean at 10) and this PR ships no correctness, type, or security regressions — the only real risks are concentrated in the evalboard frontend, where the harness-scoping layer that decides which runs count toward pass-rate/trend numbers went untested and the default-harness value is now duplicated across several modules, so the bottom line is a solid merge that needs a focused round of test coverage and DRY cleanup on the new harness plumbing rather than any structural fix.

Summary

Axis Score 🔴 🟠 🟡 🔵 Top Issue
1. Code Quality & Style 9.3 / 10 0 0 1 2 Default harness "claude-code" hardcoded instead of DEFAULT_HARNESS (second source of truth across trends.ts/overview.ts/harness-badge.tsx)
2. Type Safety 9.9 / 10 0 0 0 1 Redundant type assertion harnesses as readonly string[] on an already-readonly string[] value
3. Test Health 8.9 / 10 0 0 2 1 New public validator parseHarnessParam ships with zero test coverage
4. Security 10 / 10 0 0 0 0
5. Architecture & Design 10 / 10 0 0 0 0
6. Error Handling & Resilience 10 / 10 0 0 0 0
7. API Surface & Maintainability 9.8 / 10 0 0 0 2 Suspense skeletons hardcode KNOWN_HARNESSES for the selector while streamed content uses the discovered harness set, so the segment set can shift on load
8. Evaluation Harness Quality 10 / 10 0 0 0 0

Overall Score: 9.7 / 10 · Weakest Axis: Test Health at 8.9 / 10
Totals: 🔴 0 · 🟠 0 · 🟡 3 · 🔵 6 across 8 axes.

Blockers

None.

Non-blocking, but please consider before merge

  1. [Axis 1] Default harness "claude-code" hardcoded instead of DEFAULT_HARNESS (second source of truth across trends.ts/overview.ts/harness-badge.tsx) (evalboard/lib/trends.ts:248) — harness.ts (a leaf module, freely importable) defines export const DEFAULT_HARNESS = "claude-code" and its own comment even flags the duplication ("Mirrors HarnessBadge's default."). Yet this PR re-hardcodes the literal instead of importing the constant in several places: trends.ts:248 harness: string = "claude-code", (aggregateTaskTrends), trends.ts:265 (historyForTaskInner), trends.ts:303 (historyForTask); harness-badge.tsx:34 const key = harness ?? "claude-code";; and overview.ts:305 return ordered.length > 0 ? ordered : [...KNOWN_HARNESSES.slice(0, 1)];, whose empty-list fallback silently assumes KNOWN_HARNESSES[0] === DEFAULT_HARNESS even though that array's own comment calls itself a mutable "display order" hint. Changing the app's default harness (or reordering KNOWN_HARNESSES) would leave these defaults pointing at the old value while parseHarnessParam/tag-rail/page.tsx (which correctly use DEFAULT_HARNESS) point at the new one — a silent drift a reader must keep in sync by hand. Import DEFAULT_HARNESS into trends.ts and harness-badge.tsx and use it for the defaults; make the overview.ts fallback [DEFAULT_HARNESS].
  2. [Axis 3] New public validator parseHarnessParam ships with zero test coverage (evalboard/lib/harness.ts:26) — harness.ts adds TWO exported functions; its sibling normalizeHarness got two direct tests (overview.test.ts:353-362), but parseHarnessParam — the new public validation/sanitization gate — got none. It is consumed by 5 route handlers on untrusted URL query input (params.h): page.tsx:113, path-to-ga/page.tsx:40, trends/page.tsx:44, watchlist/page.tsx:18, actions.ts:17. Its body const v = Array.isArray(raw) ? raw[0] : raw; if (typeof v !== "string") return DEFAULT_HARNESS; const trimmed = v.trim(); return /^[\w.-]{1,64}$/.test(trimmed) ? trimmed : DEFAULT_HARNESS; has four untested branches (array→first element, non-string→default, trim, regex accept/reject incl. the 1-64 length bound). Because the returned harness decides which runs are counted toward the pass-rate/trend numbers, a silent regression in this gate (e.g. an over-permissive charset or a bad default) would mis-scope benchmark numbers with no failing assertion. Add a harness.test.ts asserting: array input picks index 0; undefined/empty-array/non-string → DEFAULT_HARNESS; whitespace is trimmed; a valid id ("codex", "delegate") passes through; and the reject branch ("has space", "a/b", 65-char string, empty string) falls back to DEFAULT_HARNESS.
  3. [Axis 3] listRecentHarnesses ordering and never-empty fallback branch untested (evalboard/lib/overview.ts:291) — listRecentHarnessesInner (overview.ts:291-306) is new behavior with three untested rules: known-harnesses-first ordering (const known = KNOWN_HARNESSES.filter((h) => seen.has(h))), extras appended alphabetically (const extras = [...seen].filter(...).sort()), and the never-empty fallback gate at line 305 return ordered.length > 0 ? ordered : [...KNOWN_HARNESSES.slice(0, 1)]. None is exercised — the fallback branch (which guarantees the default is always selectable when no runs load) never runs in any test. As written it is not unit-testable because the ordering/fallback logic is inlined with the blob-backed loadRecentRuns call rather than extracted into a pure helper. Extract the seen-set→ordered-list logic into a pure exported function (e.g. orderHarnesses(seen: Set<string>): string[]) and add tests for: known ordering preserved, an unknown harness ("delegate") appended alphabetically after knowns, and the empty-seen fallback returning ["claude-code"].

Nits

  1. [Axis 1] Duplicated harness+adhoc filter block across getOverview and getTagTaskBreakdown (evalboard/lib/overview.ts:544) — The identical five-line predicate (await loadWindowData(window)).filter((r) => !r.adhoc && (harness == null || normalizeHarness(r.overview?.harness) === harness)) appears verbatim at overview.ts:544-549 (getOverview) and again at 629-634 (getTagTaskBreakdown). Extract a small filterPipelineRuns(perRun, harness) helper so the adhoc+harness scoping rule lives in one place and the two windowed views can't drift apart.
  2. [Axis 1] Exported type KnownHarness is unused (dead public API) (evalboard/lib/harness.ts:11) — export type KnownHarness = (typeof KNOWN_HARNESSES)[number]; (harness.ts:11) is re-exported again from harness-badge.tsx:6 but has zero consumers anywhere in evalboard (grep finds only the definition + re-export). Drop the type (and its re-export) until a caller needs it — YAGNI; KNOWN_HARNESSES itself is used, only the derived type is dead.
  3. [Axis 2] Redundant type assertion harnesses as readonly string[] on an already-readonly string[] value (evalboard/app/_components/harness-selector.tsx:27) — The harnesses prop is already declared harnesses: readonly string[] (line 12), so the assertion in const opts = (harnesses as readonly string[]).includes(current) is a no-op cast — .includes(current: string) type-checks without it. Drop the cast to const opts = harnesses.includes(current) ? harnesses : [current, ...harnesses];. (The sibling cast in overview.ts:301, (KNOWN_HARNESSES as readonly string[]).includes(h), is legitimate — KNOWN_HARNESSES is a literal tuple there and needs widening — so only the harness-selector one is redundant.) Harmless but reads as author uncertainty about the type; no runtime effect.
  4. [Axis 3] trends-view test passes new harness props but asserts nothing about the selector (evalboard/app/trends/__tests__/trends-view.test.tsx:47) — The trends-view test was updated only to keep rendering green: it now passes activeHarness="claude-code" and harnesses={KNOWN_HARNESSES} (renderView, lines 44-48) but no test asserts the HarnessSelector renders the harness options or reflects the active harness. Contrast with the watchlist-view test, which does add a real assertion tying activeHarness to output (expect(screen.getByText("last 0 Claude Code runs"))). Add one assertion to trends-view.test.tsx that the selector surfaces the passed harnesses (or that the active harness is marked selected), so the new harness-selector wiring on the trends surface has a direct assertion rather than a render-smoke pass.
  5. [Axis 7] Suspense skeletons hardcode KNOWN_HARNESSES for the selector while streamed content uses the discovered harness set, so the segment set can shift on load (evalboard/app/trends/page.tsx:165) — TrendsSkeleton passes <HarnessSelector current={activeHarness} harnesses={KNOWN_HARNESSES} /> and watchlist/page.tsx's WatchlistSkeleton does the same, but the real content feeds the selector the data-driven listRecentHarnesses() result. The PR's own design comment in lib/harness.ts says the switcher is "data-driven ... so a new harness like 'delegate' surfaces automatically" — the skeleton bypasses that, so a discovered harness (e.g. delegate) is missing from the fallback selector and a known-but-inactive harness appears only in the skeleton, causing the segment row to change/reflow when content streams in. The skeleton's stated purpose is "keeps the harness selector live so the user can re-scope without waiting" and "nothing shifts horizontally" — using KNOWN_HARNESSES defeats both. Consider rendering the selector shell without the full segment set, or accept a brief mismatch as a known tradeoff and note it.
  6. [Axis 7] trends-view renders raw harness id instead of harnessShortLabel in provenance and empty-state prose, inconsistent with the same component's header (evalboard/app/trends/trends-view.tsx:629) — Line 616 correctly renders {harnessShortLabel(activeHarness)} runs ("Claude Code"), but line 629 renders Last {provenance.count} {activeHarness} runs and line 673 renders No recent {activeHarness} runs. using the raw id ("claude-code"). Same view shows both "Claude Code" and "claude-code" for the same harness. Wrap both with harnessShortLabel(activeHarness) for consistent user-facing labels.

What's Missing

Tests:

  • 🟡 New public URL-param validator parseHarnessParam (harness.ts:26) — consumed by 5 untrusted-input route handlers (page.tsx:113, path-to-ga/page.tsx:40, trends/page.tsx:44, watchlist/page.tsx:18, actions.ts:17) and deciding which runs are counted toward pass-rate/trend numbers — ships with zero tests. Add harness.test.ts covering array-input picks index 0, non-string/empty/undefined -> DEFAULT_HARNESS, whitespace trim, valid-id pass-through, and the reject branch (space, slash, 65-char, empty) falling back to DEFAULT_HARNESS. (trigger: evalboard/lib/harness.ts) (restates: Axis 3: New public validator parseHarnessParam ships with zero test coverage)
  • 🟡 listRecentHarnessesInner (overview.ts:291) adds three untested rules — known-first ordering, alphabetical append of newcomers, and the never-empty fallback at line 305 (return ordered.length>0 ? ordered : [...KNOWN_HARNESSES.slice(0,1)]) that guarantees the default is always selectable when no runs load. None is exercised; the fallback branch never runs in any test. Extract the seen-set->ordered-list logic into a pure exported helper and test known ordering, an unknown harness appended alphabetically, and empty-seen -> ["claude-code"]. (trigger: evalboard/lib/overview.ts) (restates: Axis 3: listRecentHarnesses ordering and never-empty fallback branch untested)
  • 🟡 getOverview (overview.ts:544) and getTagTaskBreakdown (overview.ts:629) each gained a new inline harness-scoping predicate (harness == null || normalizeHarness(r.overview?.harness) === harness) that filters the headline pass-rate + tag-rail numerators, but neither is directly tested (overview.test.ts references getTagTaskBreakdown zero times and only exercises the harness filter at the collectPipelineRuns layer, not these two consumers). Add a test that a mixed-harness window scopes the returned pass-rate/tag counts to the selected harness and that harness=null preserves legacy all-harness behavior. (trigger: evalboard/lib/overview.ts)
  • 🔵 The trends-view test was updated only to render green — it passes activeHarness="claude-code" and harnesses={KNOWN_HARNESSES} but asserts nothing about the HarnessSelector (options rendered, active harness marked selected), unlike the watchlist-view test which ties activeHarness to output ("last 0 Claude Code runs"). Add one assertion on the new selector wiring on the trends surface. (trigger: evalboard/app/trends/tests/trends-view.test.tsx) (restates: Axis 3: trends-view test passes new harness props but asserts nothing about the selector)

Parallel paths:

  • 🟡 The leaf constant DEFAULT_HARNESS="claude-code" (harness.ts) is re-hardcoded as the literal "claude-code" in the parallel data-layer paths instead of imported: trends.ts:248/265/303 (aggregateTaskTrends/historyForTaskInner/historyForTask defaults) and the overview.ts:305 fallback [...KNOWN_HARNESSES.slice(0,1)] (which silently assumes KNOWN_HARNESSES[0] === DEFAULT_HARNESS). parseHarnessParam/tag-rail/page.tsx correctly use DEFAULT_HARNESS, so changing the app default or reordering KNOWN_HARNESSES leaves these four sites pointing at the old value — a hand-synced second source of truth. Import DEFAULT_HARNESS into trends.ts and make the overview.ts fallback [DEFAULT_HARNESS]. (trigger: evalboard/lib/trends.ts) (restates: Axis 1: Default harness "claude-code" hardcoded instead of DEFAULT_HARNESS)
  • 🔵 TrendsSkeleton (trends/page.tsx:140) and WatchlistSkeleton (watchlist/page.tsx:59) feed the HarnessSelector a hardcoded KNOWN_HARNESSES while the real streamed content feeds the data-driven listRecentHarnesses() result. A discovered harness (e.g. delegate) is missing from the fallback selector and a known-but-inactive harness appears only in the skeleton, so the segment row changes/reflows when content streams in — defeating the skeleton's stated "nothing shifts horizontally" purpose. Render the selector shell without the full segment set, or accept the brief mismatch as a documented tradeoff. (trigger: evalboard/app/trends/page.tsx) (restates: Axis 7: Suspense skeletons hardcode KNOWN_HARNESSES for the selector)

Downstream consumers:

  • 🔵 The new harness filter scopes the analytics numerators (chart, tag-rail, trends, watchlist streaks/volatility, path-to-ga readiness) — all four surfaces correctly thread harness — but the run LIST (getRunListing/summarizeListing, overview.ts:682) and per-run tables deliberately stay all-harness (documented in page.tsx). This split is intentional but untested; add a note/assertion that the run-list totals remain harness-agnostic while the analytics block is scoped, so a future reviewer doesn't 'helpfully' scope the list and change its meaning. (trigger: evalboard/lib/overview.ts)

Display & mapping dicts:

  • 🔵 trends-view renders the raw harness id in provenance ("Last N {activeHarness} runs", trends-view.tsx:629) and empty-state prose ("No recent {activeHarness} runs", ~:673) while the same view's header uses harnessShortLabel(activeHarness) at ~:616 — so one view shows both "Claude Code" and "claude-code" for the same harness. Wrap both prose sites with harnessShortLabel for consistent user-facing labels. (trigger: evalboard/app/trends/trends-view.tsx) (restates: Axis 7: trends-view renders raw harness id instead of harnessShortLabel)
  • 🔵 The HARNESS_LOGO / harnessShortLabel dict (harness-badge.tsx) maps only the three KNOWN_HARNESSES; no new harness value is added by this PR so no extension is required, but the data-driven switcher can now surface a discovered harness with no HARNESS_LOGO entry (e.g. a future "delegate"), which falls through to a raw-id label and no logo in both the selector and badge. This is deliberate and documented ('show the raw id rather than a misleading logo') — flag so the reviewer confirms the raw-id fallback is the intended UX for an unmapped discovered harness rather than an oversight. _(trigger: evalboard/app/components/harness-badge.tsx)

Daily/nightly:

  • 🔵 Harness scoping is a pure evalboard (cross-repo consumer) change that depends on the nightly pipeline — which lives in the separate coder-eval-uipath/eval-runner repo — stamping each run's harness into the run overview/RunConfig. Runs predating the stamp fold to claude-code via normalizeHarness, so a window straddling the stamp rollout silently buckets old codex/antigravity runs as claude-code and blends their pass rates into the default segment. The PR does not state this cross-repo backfill/skew impact; note it (and whether historical runs get backfilled) so the nightly dashboards aren't read as authoritative during the transition window. (trigger: evalboard/lib/overview.ts)

Harness & Lint Improvements

Static checks (lint / type):

  • [ce-lint] evalboard has no ESLint today (verify = tsc + vitest + next build only). Add ESLint (eslint-config-next is one dep away) and a no-restricted-syntax rule that bans the string literal "claude-code" anywhere under evalboard/ except lib/harness.ts, forcing import { DEFAULT_HARNESS } from "@/lib/harness". lib/harness.ts already owns DEFAULT_HARNESS and its own comment flags the duplication risk. Wire ESLint into the verify npm script so it runs in CI. Prevents: Finding 1 (DEFAULT_HARNESS second source of truth): the four PR-introduced re-hardcodings — trends.ts:248/265/303 harness: string = "claude-code" defaults and overview.ts:305 [...KNOWN_HARNESSES.slice(0,1)] fallback — plus the pre-existing harness-badge.tsx:34 ?? "claude-code", would all fail lint and be forced to import the constant, killing the silent-drift class if the app default ever changes.
  • [ce-lint] In the same new ESLint config, enable the type-aware rule @typescript-eslint/no-unnecessary-type-assertion (requires parserOptions.project pointed at tsconfig.json so the linter has type info). tsc alone does not flag no-op casts. Prevents: Finding 4: the redundant (harnesses as readonly string[]) in harness-selector.tsx:27, where harnesses is already typed readonly string[]. The rule leaves the legitimate widening cast in overview.ts:301 (literal tuple KNOWN_HARNESSES) untouched, exactly matching the finding's distinction.
  • [ce-lint] Add unused-export detection to evalboard verify — either knip/ts-prune, or the ESLint rule import/no-unused-modules with { unusedExports: true }. tsconfig noUnusedLocals does NOT catch exported-but-unimported symbols, which is why this slipped through typecheck. Prevents: Finding 3: export type KnownHarness (harness.ts:11) and its re-export from harness-badge.tsx:6 have zero consumers; an unused-exports gate flags dead public API like this the moment it lands.

Harness improvements (not statically reachable):

  • Add a coverage-threshold gate to evalboard: @vitest/coverage-v8 is already installed but neither test (vitest run) nor verify runs --coverage or enforces a floor. Change verify to vitest run --coverage with per-file line/branch thresholds in vitest.config.ts, so a newly-exported function that ships with no test fails CI. Why not static: Coverage is a runtime property — a static/type check cannot know that a branch (e.g. parseHarnessParam's regex-reject path, or listRecentHarnesses's empty-seen fallback) is never exercised; it requires executing the test suite and measuring hit lines. Prevents: Findings 5 and 6: parseHarnessParam (harness.ts:26, consumed by 5 untrusted-input route handlers) and listRecentHarnessesInner's ordering/fallback (overview.ts:291-306) both shipped with zero coverage; a threshold gate would have blocked the merge.
  • Extract the pure seen-set to ordered-list logic out of the blob-backed listRecentHarnessesInner into an exported orderHarnesses(seen: Set<string>): string[], and add unit tests: known-first ordering preserved, unknown harness appended alphabetically, empty-seen -> [DEFAULT_HARNESS]. Add a harness.test.ts for parseHarnessParam's four branches (array->index 0; non-string/empty->DEFAULT_HARNESS; trim; regex accept/reject incl. the 1-64 length bound). Why not static: The correctness of ordering, sanitization, and the never-empty fallback needs runtime assertions on actual outputs; and the logic is currently un-unit-testable because it is inlined with the async loadRecentRuns IO call — a refactor plus behavioral tests, neither reachable by tsc or ESLint. Prevents: Findings 5 and 6: gives the pass-rate/trend scoping gate (which decides which runs count) direct assertions instead of the render-smoke pass it has now.
  • Treat assertion-free tests as a review/process gap (optionally a mutation-testing gate such as Stryker on evalboard/lib). The trends-view test passes activeHarness/harnesses props but asserts nothing about the HarnessSelector, unlike the watchlist-view test which ties activeHarness to visible output. Add a direct assertion (selector surfaces the passed harnesses / active harness marked selected). Why not static: "A test contains a meaningful assertion" is not statically detectable and not coverage-detectable — the render lines are counted as covered while the contract is unverified; only mutation testing mechanically surfaces a test that would pass with the behavior removed. Prevents: Finding 7: the new harness-selector wiring on the trends surface would get a real assertion rather than a green-but-empty render test.
  • Handle the residual DRY / label-consistency / skeleton items as manual refactors, not new gates: extract the duplicated adhoc+harness filter predicate into one filterPipelineRuns(perRun, harness) helper (overview.ts:544-549 and 629-634); route provenance/empty-state prose through harnessShortLabel(activeHarness) (trends-view.tsx:629,673); and align the Suspense skeleton selector with the data-driven harness set (trends/page.tsx:165, watchlist skeleton) or record the mismatch as a deliberate tradeoff. Why not static: Duplication across only 2 sites is below a useful jscpd threshold and a copy-paste detector would be noisy there; the raw-id-vs-label choice needs semantic judgment about user-facing text vs. prop context (a lint rule cannot tell a display string from a data value); and the skeleton/content set divergence is a design tradeoff, not a mechanical pattern. Prevents: Findings 2, 7 (label/9), and 8: DRY drift between the two windowed views, inconsistent Claude Code vs claude-code labels in one component, and the selector segment-row reflow on stream-in.

Top 5 Priority Actions

  1. Add evalboard/lib/harness.test.ts covering parseHarnessParam (harness.ts:26) — it gates untrusted URL params.h at 5 route handlers and its result decides which runs are counted toward pass-rate/trend numbers, so an over-permissive charset or bad default silently mis-scopes benchmark figures with no failing assertion.
  2. Extract the seen-set ordering + never-empty fallback in listRecentHarnessesInner (overview.ts:291-306, fallback at 305) into a pure orderHarnesses(seen) helper and test known-first ordering, alphabetical extras, and the empty-seen [claude-code] branch, which currently never executes in any test yet also drives which harnesses are selectable.
  3. Replace the re-hardcoded "claude-code" literals with the existing DEFAULT_HARNESS constant at trends.ts:248/265/303 and change the overview.ts:305 fallback to [DEFAULT_HARNESS], eliminating the 4 PR-introduced second sources of truth that would silently drift if the app's default harness or KNOWN_HARNESSES order changes.
  4. Fix the Suspense skeletons (trends/page.tsx:165 and watchlist/page.tsx WatchlistSkeleton) that hardcode KNOWN_HARNESSES while streamed content uses the data-driven listRecentHarnesses() set, so discovered harnesses like delegate are absent and the selector segment row reflows on load — render the shell without the full segment set or accept and note the mismatch.
  5. Wrap the raw harness id with harnessShortLabel(activeHarness) in the provenance (trends-view.tsx:629) and empty-state (trends-view.tsx:673) prose so the same view stops showing both "Claude Code" and "claude-code", matching the already-correct header at line 616.

Stats: 0 🔴 · 0 🟠 · 3 🟡 · 6 🔵 across 8 axes reviewed.

@uipreliga
uipreliga self-requested a review July 22, 2026 23:16

@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 2 commits July 22, 2026 16:53
- Use the DEFAULT_HARNESS constant for the default-harness value in
  trends.ts and overview.ts's never-empty fallback instead of re-hardcoding
  the "claude-code" literal, removing a silent drift trap.
- Render harnessShortLabel in the trends provenance + empty-state prose so
  the view no longer shows both "Claude Code" and "claude-code".
- Drop a no-op readonly cast in the harness selector.
- Add parseHarnessParam tests (array/absent/whitespace/valid/reject/length).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@bai-uipath
bai-uipath merged commit acc1c86 into main Jul 22, 2026
12 checks passed
@bai-uipath
bai-uipath deleted the bai/evalboard-trends-harness-scope branch July 22, 2026 23:59
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