Skip to content

feat(overview): create overview page / 创建总览页面 - #611

Merged
edwingao28 merged 13 commits into
masterfrom
feat/executive-overview
Jul 23, 2026
Merged

feat(overview): create overview page / 创建总览页面#611
edwingao28 merged 13 commits into
masterfrom
feat/executive-overview

Conversation

@edwingao28

@edwingao28 edwingao28 commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Adds /overview (and /zh/overview): a server-rendered scorecard that answers one question per model — if you deploy it today, how much capacity does each platform deliver?

It mounts as the first Dashboard tab, to the left of Inference Performance, sharing the dashboard shell, tab bar and heading styles; the Dashboard header link highlights on it.

What it shows

  • One row per active model, one column per platform: B200 as the reference, then MI355X, B300, GB200 NVL72, GB300 NVL72. Fixed workload: 8K input / 1K output single-turn, speculative decode only.
  • Best validated stack per platform. Each cell is that platform's own best validated deployment — framework, precision and speculative method chosen freely per platform — labeled FRAMEWORK · PRECISION, with the full stack on the link title. This measures deployable capacity, not isolated silicon, and the methodology note on the page says so.
  • Service-level switcher. ?tier=30|75|100 re-renders the whole matrix at another tok/s/user target (default 50). The switcher is plain links, no client JS, so every view is a shareable URL.
  • Deltas never mix stacks quietly. The signed percentage against B200 appears only when both displayed results share one precision and one model release. Otherwise the cell says why: FP4 vs FP8 · no comparable delta, Different releases · no comparable delta.
  • Honest gaps. A side with no exact result at the displayed tier shows plus its reason (no 8K/1K data, cannot reach @50, standard decode only, …). There is never a percentage against a missing number.
  • Leader flips. When the winner changes at 100 tok/s/user, the lower-tier views add one line: At 100, X leads.
  • Every value links to the dashboard filtered to its exact configuration, and every result carries the run dates of the frontier points behind it.

Testing

  • Data rules pinned in overview-data.test.ts: per-platform bucket selection, tier parameterization, mismatch and missing-reason classification. Expected values come from running the real builder — tier values are spline-interpolated, so nothing is hand-computed.
  • A small synthetic fixture is served through the real assembler in fixtures mode; overview.cy.ts covers the matrix, the tier links, mismatch and states, the Chinese sibling, and 390/320px overflow, on Chrome and Firefox.
  • Desktop fits a 1440×900 fold. Mobile renders the same cell components as a two-column list.

中文说明

新增 /overview/zh/overview:服务端渲染的对比页,回答一个问题——现在部署这个模型,各平台能交付多少容量。页面作为 Dashboard 第一个标签页挂载(在「推理性能」左侧),共享仪表板外壳、标签栏与标题样式。

  • 每行一个活跃模型,每列一个平台:B200 为参照,另有 MI355X、B300、GB200 NVL72、GB300 NVL72。固定 8K→1K 单轮负载,仅投机解码。
  • 每格是该平台自己的最佳已验证部署(框架、精度、投机方式各自取最优),标注 框架 · 精度,完整配置在链接悬浮提示中。对比对象是完整服务栈,不是单独的芯片,页面方法说明中已写明。
  • ?tier=30|75|100 切换服务档位(默认 50)。切换器是纯链接,无客户端 JS,任何视图都是可转发的 URL。
  • 相对 B200 的差值只在同精度、同版本之间计算;不可比时写明原因,绝不静默混比。
  • 无精确结果的一侧显示 与原因,绝不对缺失数字给出百分比。
  • 100 档赢家换人时,低档视图给出一行提示。
  • 每个数值链接到按该配置精确过滤的仪表板;每个结果携带自己的证据日期。
  • 测试:数据规则逐条单元断言,期望值由真实 builder 生成(档位值为样条插值,不手算);合成 fixture 走真实装配器;Cypress 覆盖矩阵、档位切换、中文页与 390/320px,Chrome 与 Firefox 双跑。

Note

Medium Risk
New user-facing benchmark matrix with nuanced delta and missing-data rules—logic bugs could show misleading comparisons, though behavior is heavily unit- and e2e-tested.

Overview
Adds /overview and /zh/overview: a server-rendered matrix of active models vs B200, MI355X, B300, GB200, and GB300 on fixed 8K→1K speculative-decode workloads, with ?tier= (30/50/75/100) re-rendering the whole page via plain links.

Overview is the first dashboard tab; the header treats /overview as part of Dashboard and the language switcher keeps the current query string (e.g. tier). Sitemap and zh mirror routes are wired; cell values link to filtered /inference URLs (plain <a> where needed).

A new overview-data pipeline picks per-platform best stacks (finer config identity including GPU topology), interpolates tiers via tco-feed (adds per-tier evidence_date), and only shows B200 deltas when precision and model release match—otherwise mismatch copy or with reasons. Leader-flip hints at @100 are included.

Header/mobile: 44px touch targets, GitHub stars hidden below sm, Minecraft audio toggles moved into the mobile menu to avoid 320px overflow; Cypress covers 320×700 layout.

Tests: Vitest for data/link rules and fixture drift guard; large overview.cy.ts e2e suite. getCachedBenchmarks moved to benchmark-data.server; computeToggle extracted to toggle-set for server reuse.

Reviewed by Cursor Bugbot for commit 5aade72. Bugbot is set up for automated code reviews on this repo. Configure here.

@vercel

vercel Bot commented Jul 21, 2026

Copy link
Copy Markdown

Deployment failed with the following error:

You don't have permission to create a Preview Deployment for this Vercel project: inferencemax-app.

View Documentation: https://vercel.com/docs/accounts/team-members-and-roles

@edwingao28

Copy link
Copy Markdown
Collaborator Author

Preview status update:

  • The Vercel build has not started because my current SemiAnalysisAI Vercel seat is Pro Viewer, which can inspect previews but cannot create them. Both the Git integration and a manual CLI deploy return You don't have permission to create a Preview Deployment. I need Developer (or equivalent preview-deploy) permission before retriggering.
  • As an interim real-data check, I fetched the current public benchmark rows for all five active model families and ran them through this PR's exact reducer. It completed successfully with one precision per model: DeepSeek V4 Pro (FP4 / GB300), Kimi (FP4 / GB200), MiniMax M3 (FP8 / GB200), Qwen3.5 (FP8 / GB300), while GLM5.2 correctly remained the empty fixed-sequence 8K/1K row.

The protected Vercel Preview is still required for final server-loader and responsive validation.

@edwingao28 edwingao28 changed the title feat(overview): add executive inference scorecard / 新增高管推理总览评分页 feat(overview): add overview inference scorecard / 新增推理总览评分页 Jul 21, 2026
@edwingao28
edwingao28 force-pushed the feat/executive-overview branch from 5160d07 to a854318 Compare July 21, 2026 20:42
@edwingao28 edwingao28 changed the title feat(overview): add overview inference scorecard / 新增推理总览评分页 feat(overview): create overview page / 创建总览页面 Jul 21, 2026
@edwingao28
edwingao28 force-pushed the feat/executive-overview branch from a854318 to 785c1af Compare July 21, 2026 21:16
@vercel

vercel Bot commented Jul 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
inferencemax-app Ready Ready Preview, Comment Jul 23, 2026 6:23pm

Request Review

Comment thread packages/app/src/lib/tco-feed.ts
Comment thread packages/app/src/components/overview/overview-scorecard.tsx Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eec67f33dd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/app/src/components/overview/overview-scorecard.tsx Outdated
Comment thread packages/app/src/lib/tco-feed.ts
@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Claude finished @edwingao28's task in 5m 20s —— View job


Review — PR #611 Executive Overview page

Runtime verified on localhost:3000 (real read-only DB): /overview and /zh/overview both render real ranked data, evidence dates, leader-change badges, coverage lines and not-ranked reasons; 0 console errors; no horizontal overflow at 320px. CI green (Lint / Unit / E2E). This is careful, well-tested work — the data-layer invariants are pinned by focused unit tests and a builder-driven e2e fixture.

I confirmed the two open bot findings (Cursor Bugbot + Codex agree on both) and posted precise fixes inline:

  1. 🟡 tco-feed.ts bracketKnots — an exact hit on an interior frontier knot still returns a bracketing pair, so evidence_date becomes a cross-day range for a value that was actually a single measured point. Contradicts the PR's "same-day read shows one date" promise. Low trigger probability on the live overview (float interactivity rarely lands exactly on 30/50/75/100) but real in the shared helper behind the public tco-feed API. → collapse to the single knot on an exact interior match.
  2. 🟡 overview-scorecard.tsx NotRankedLine — in ranked secondary state the dedup against the full measuredHardware list drops secondary-measured hardware that has no exact @50 read at either precision, so it appears in neither the ranked chips nor the not-ranked line — breaking the "every hardware accounted for" guarantee. Not reproducible on today's real data (all secondaries are coverage), but the ranked branch is live code. → only dedup against measuredHardware in coverage state; suggest a unit test for the ranked case.

Both are 🟡 (correctness/honesty of displayed evidence, not crashes). No other blocking issues found — no missing-test gaps (new lib code has colocated tests, the UI has an e2e), no resource/hook/security concerns in the new code.


  • Read the diff & new modules
  • Assess the two open bot findings — both confirmed
  • Check CI status — ✅ green
  • Runtime verify /overview and /zh/overview
  • Post review

Comment thread packages/app/src/lib/tco-feed.ts
Comment thread packages/app/src/components/overview/overview-scorecard.tsx Outdated
@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Claude finished @edwingao28's task in 2m 13s —— View job


Re-review — PR #611 Executive Overview page

Re-reviewing the one new commit since my last review (69ae73c — "infinity placeholder chips for missing results").

What changed: NotRankedLine was reworked from a Not ranked: … sentence into per-hardware <label> ∞ chips, with the reason carried in title/aria-label and a ∞ = no comparable result legend added to the methodology footer. E2E updated accordingly.

Runtime verified on localhost:3000 (real read-only DB): /overview renders 50 ∞ chips, each with its reason in title/aria (no exact @50 result, no 8K/1K data, standard decode only, …), the legend is present, no ∞% is ever rendered, and there are 0 console errors. CI is green (Lint / Unit / Typecheck / E2E chrome+firefox / Component). The new rendering itself is correct.

However — neither open finding from the last round was addressed:

  1. 🟡 overview-scorecard.tsx:553 — ranked secondary still drops hardware. The ∞ rewrite touched these exact lines but kept the unconditional new Set(model.secondary?.measuredHardware) dedup. In a ranked secondary (which renders only exact-@50 chips), secondary-measured hardware without an exact @50 read now disappears from every surface — no chip, no reason. buildNotRanked already excludes the exact-@50 ranked hardware, so the dedup is only correct in coverage state. Re-flagged inline with a suggestion + unit-test note. Not reproducible on today's data (all secondaries are coverage) but the branch is live.
  2. 🟡 tco-feed.ts:226 bracketKnots — exact interior-knot reads still report a cross-day range. Unchanged since eec67f3; the existing inline thread + suggestion (collapse to the single knot on an exact interior match) still applies. Contradicts the PR's "same-day reads show one date" promise for the public tco-feed API.

No new issues introduced by the ∞-chip commit. Both remaining items are 🟡 (honesty of displayed evidence, not crashes) — recommend fixing both before merge and adding the ranked-secondary unit test.

  • Diff new commit 69ae73c vs reviewed eec67f3
  • Check the two open findings — both still unaddressed
  • Review new ∞-chip logic — correct, no new issues
  • Check CI status — ✅ green
  • Runtime verify /overview — ∞ chips + legend render, 0 console errors
  • Post review
    · feat/executive-overview

Comment thread packages/app/src/components/overview/overview-scorecard.tsx Outdated
@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @edwingao28's task in 2m 11s —— View job


Re-review — PR #611 Executive Overview page

Re-reviewing the new commit 1725e80 (per-platform matrix with tier switcher and stack labels), which is a substantial rewrite of the overview data + UI layers.

Both previously-flagged findings are resolved:

  1. tco-feed.ts bracketKnots interior-knot range — now collapses to a single knot on an exact interior match (tco-feed.ts:224: if (frontier[i].interactivity === tier) return [frontier[i], frontier[i]]). An exact measured point at an interior tier no longer reports a cross-day evidence range. tco-feed.test.ts adds coverage for this.
  2. overview-scorecard.tsx ranked-secondary drops hardware — the entire primary/secondary/NotRankedLine/measuredHardware structure the finding was about is gone. The rewrite is a per-platform matrix: each hardware column independently selects its own best qualified bucket (dbModel × precision, exact-first, tie → FP4), and any hardware with no exact read shows with an explicit reason (no exact @tier result, standard decode only, no 8K/1K data, cannot reach @tier, INT4/BF16 only). The "every hardware accounted for" guarantee now holds structurally — every cell is either an exact read or a reasoned .

Runtime verified on localhost:3000 (real read-only DB):

  • /overview renders the full matrix with real ranked data, precision badges (TRTLLM · FP4, SGLang · FP4, ATOM¹ · FP4), signed deltas vs B200, per-result evidence dates, reasons, and At 100, <hw> leads transition lines. Filtered dashboard links carry correct g_model/i_gpus/i_spec/i_prec/g_runid params.
  • Tier switcher (?tier=30/75/100) re-renders server-side; /zh/overview?tier=75 renders fully translated. 0 console errors on all views.

CI: green — Lint, Typecheck+Unit, Component, E2E chrome (all shards) + firefox all passing (a couple of jobs still finishing, none failing).

No new blocking or warning issues found in the rewrite. The data-layer invariants (exact-vs-clamped classification via isExactTierRead, cohort/precision isolation, evidence-date sourcing) are pinned by the updated overview-data.test.ts, and the builder-driven overview.cy.ts fixture exercises the UI. 🟢 LGTM.

Comment thread packages/app/src/components/overview/overview-scorecard.tsx Fixed
Comment thread packages/app/src/components/overview/overview-detail-link.tsx Outdated
@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @edwingao28's task in 56s —— View job


Re-review — PR #611 Executive Overview page

Re-reviewing the one new commit 7378a3b (drop dead dashboard-link helper and unreachable date guard) since my last review of 1725e80 (LGTM). This commit only touches two files (+7 / −49) and addresses the two low-severity findings from the previous round.

Both findings resolved, and both changes verified safe:

  1. Unused OverviewDashboardLink / EVIDENCE_LINK_CLASS removed (overview-detail-link.tsx) — confirmed no remaining references anywhere in packages/app/src/ (grep → none). The matrix cells build their evidence anchors inline via buildOverviewDashboardHref (overview-scorecard.tsx:224), which is unchanged. Dead export cleanly excised; the surviving OverviewDetailLink comment was tidied, no behavior change.
  2. Unreachable evidenceDate === null guard removed (overview-scorecard.tsx:126) — formatEvidenceDate now takes a non-null { from; to } and returns string. The sole caller already guards null before rendering (overview-scorecard.tsx:240: {evidenceDate === null ? null : (…formatEvidenceDate(…)…)}), so TypeScript narrows evidenceDate to non-null at the call site — the removed branch was genuinely dead. Matches the CodeQL finding.

CI: green across the board — Lint, Typecheck+Unit, Component, CodeQL, E2E chrome (4 shards) + firefox (4 shards), Vercel Preview all passing. (Claude review / Cursor Bugbot still pending, as expected.)

No new issues introduced. The two data-honesty findings from earlier rounds (tco-feed interior-knot range, ranked-secondary hardware drop) were already resolved by the 1725e80 rewrite. 🟢 LGTM.

  • Diff 7378a3b vs previously-reviewed 1725e80
  • Verify the two low-severity findings (unused helper, unreachable guard) are resolved
  • Confirm no new issues introduced — call-site null-guard makes the tightened signature safe; removed exports have zero references
  • Check CI status — ✅ green
    · feat/executive-overview

Comment thread packages/app/src/lib/overview-data.ts Outdated
Comment thread packages/app/src/components/header/header.tsx
@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @edwingao28's task in 2m 6s —— View job


Re-review — PR #611 Executive Overview page

Re-reviewing the one new commit 03e6359 ("cannot-reach reason requires every speculative bucket unreachable") since my last LGTM on 69b1530. Small data-layer fix (+67 / −16 across 2 files) addressing the last open Bugbot finding ("Wrong infinity reason for gaps").

Finding resolved, fix verified correct:

  • cannot_reach_at_tier is now a whole-platform claim. missingReasonForHeadlineMember takes the reads across all candidate buckets and only reports cannot_reach_at_tier when bucketReads.length > 0 && bucketReads.every((r) => r.boundary === 'unreachable') (overview-data.ts:332). If any one qualified speculative stack merely tops out on the low side (clamped_low — under-swept, not fundamentally unable to reach), the gap correctly downgrades to no_exact_at_tier. This fixes the Bugbot case where nonComparableAsMissing had already nulled the value sort key, letting an FP4 unreachable bucket win over an FP8 clamped_low one and mislabel the reason.

I traced the supporting pieces to confirm the fix is sound, not just plausible:

  • nonComparableAsMissing (overview-data.ts:267) preserves boundary on non-exact reads (only nulls value/evidenceDate), so r.boundary === 'unreachable' is a valid discriminator on the passed reads.
  • Boundary semantics (tco-feed.ts:45): unreachable = tier above the frontier's max interactivity; clamped_low = tier below the min. The new unit test's unreachable fixture (knots at 20–45, below tier 50) and underSwept fixture (knots at 60–90, above tier 50) genuinely produce those two boundaries, so the mixed → no_exact_at_tier and allUnreachable → cannot_reach_at_tier assertions exercise the real branch — not a hand-mocked value.

Runtime verified on localhost:3000 (real read-only DB): /overview?tier=100 renders the full matrix — real deltas vs B200, precision badges, evidence dates, and ∞ chips each carrying their reason in title/aria (no exact @100 result, standard decode only, no 8K/1K data); no ∞% ever rendered; 0 console errors. (The cannot_reach_at_tier path isn't triggered by today's live data, exactly as the finding noted — it's now pinned by the unit test instead.)

CI: green on completed jobs — Lint & Format, Tests (Unit), Code Quality, PR checks all passing; E2E still finishing (none failing).

No new issues introduced. Every finding across all prior rounds (tco-feed interior-knot range, ranked-secondary hardware drop, unused dashboard-link helper, unreachable date guard, unused config fields, undersized audio controls, unused exactCount, language-toggle tier drop, and now the wrong--reason gap) is resolved. 🟢 LGTM.

  • Diff 03e6359 vs previously-reviewed 69b1530
  • Verify the cannot_reach_at_tier reason fix + new unit test (boundary semantics + fixtures traced end-to-end)
  • Check CI status — ✅ green
  • Runtime verify /overview?tier=100 — renders, ∞ reasons present, 0 console errors

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @edwingao28's task in 5m 6s —— View job


Re-review — PR #611 Executive Overview page

The branch was rebased onto latest master (8070120, the Kimi-K2.7-Code ETL fix) since my last LGTM on 03e6359 — the prior commit SHAs no longer exist, and HEAD is now 7299875 with the same content. No new feature commits; nothing new to flag. I re-verified the tree at HEAD rather than trusting the rebase.

Both data-honesty findings confirmed still fixed at HEAD:

  1. tco-feed.ts:224 — interior-knot evidence range. bracketKnots collapses to a single knot on an exact interior hit (if (frontier[i].interactivity === tier) return [frontier[i], frontier[i]]), so a measured point at an interior tier no longer reports a cross-day range.
  2. overview-data.ts:332 — wrong reason. missingReasonForHeadlineMember only returns cannot_reach_at_tier when bucketReads.length > 0 && bucketReads.every(r => r.boundary === 'unreachable'); one under-swept (clamped_low) stack correctly downgrades to no_exact_at_tier. Both pinned by unit tests.

The earlier structural findings (ranked-secondary hardware drop) remain moot — the per-platform matrix rewrite made every cell either an exact read or a reasoned .

CI: ✅ green on completed jobs — Lint & Format, Tests (Unit), Code Quality, PR checks all passing; E2E still running (none failing).

Runtime verified on localhost:3000 (real read-only DB):

  • /overview?tier=100 — full matrix renders real data, 0 console errors.
  • /zh/overview?tier=75 — fully translated (headers, methodology line, tier switcher, 打开筛选后的仪表板… deep-link titles), real ranked values (e.g. DeepSeek V4 Pro 255 · TRTLLM · FP4 · MTP), evidence dates, deep links carrying correct g_model/g_runid/i_gpus/i_spec/i_prec. 0 console errors.

No new issues. Every finding across all prior rounds is resolved. 🟢 LGTM.

  • Determine what changed since the last LGTM (03e6359) — rebase only, no new feature commits
  • Re-confirm the two data-honesty findings stay fixed at HEAD
  • Check CI status — ✅ green
  • Runtime verify /overview?tier=100 + /zh/overview?tier=75 — render real data, 0 console errors
  • Post review

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @edwingao28's task in 1m 59s —— View job


Re-review — PR #611 Executive Overview page

Reviewing the one new commit e2b7324 ("mount as first dashboard tab") since my last LGTM on 7299875. Small, well-scoped change (+39 / −29 across 9 files) that turns /overview into the first dashboard tab.

What changed — and why it's safe:

  • Routes moved into the (dashboard) route group (app/(dashboard)/overview/page.tsx, app/zh/(dashboard)/overview/page.tsx). The pages dropped their local <main className="relative"><div className="container …"> wrapper — that chrome (plus TabNav, GlobalFilterProvider, UnofficialRunProvider, NudgeEngine) is now supplied by DashboardShell via the group layout, so the wrapper removal is correct, not a regression. Server component under client providers is fine.
  • Tab registered everywhere it needs to be: VISIBLE_TABS (first entry, tab-nav.tsx), DASHBOARD_TABS (header.tsx, so the Dashboard nav link highlights), and TAB_LABELS_ZH (overview: '总览'). /overview was already in ZH_MIRRORED_ROUTES, so localizedPath correctly resolves the zh tab to /zh/overview.
  • activeTab() handles itsegments[0] = overview for both /overview and /zh/overview, so the active-tab underline lights up without touching that helper.
  • Header wrapped in a Card for visual parity with the other tabs; Card is already imported in overview-page.tsx. No logic change.
  • Tests updated in lockstep: tab-nav.cy.tsx (bare + unofficialruns hrefs), navigation.cy.ts (tab click → URL), and overview.cy.ts (desktop tab active state, mobile select, /zh/overview label 总览).

Runtime verified on localhost:3000 (real read-only DB):

  • /overviewchart-section-tabs renders, tab-trigger-overview is first with href="/overview" and the active border-secondary class, Dashboard nav link is text-brand highlighted. 0 console errors.
  • /zh/overview — overview tab links to /zh/overview with label 总览, page title AI 推理总览. 0 console errors. (Only benign SVG-preload warnings on both.)

CI: green on completed jobs — Lint & Format, Typecheck + Unit, Component, CodeQL (all analyzers), E2E chrome (shards 1–3) + firefox (2, 4), Vercel Preview all passing; remaining E2E shards still running, none failing.

No new issues introduced. Every finding from prior rounds remains resolved. 🟢 LGTM.

  • Diff e2b7324 vs previously-reviewed 7299875
  • Static review of the new commit
  • Check CI status — ✅ green
  • Runtime verify /overview + /zh/overview tab mounting — render correctly, 0 console errors
  • Post review

Comment thread packages/app/src/lib/overview-data.ts
@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @edwingao28's task in 2m 39s —— View job


Re-review — PR #611 Executive Overview page

Reviewed the commits since my last LGTM on e2b7324 plus the one open Bugbot finding.

New commits (f3d5ae0, 69a0a1a) are comment/format-onlyf3d5ae0 reformats the overview-rows.json fixture; 69a0a1a trims doc comments in overview-scorecard.tsx, overview-data.ts, overview-config-identity.ts, and header.tsx. No functional change (verified by reading the diff), and .prettierignore drops the now-conformant fixture. Nothing new to flag there.

One open finding confirmed — At 100, X leads can disagree with /overview?tier=100 🟡

  • overview-data.ts:336-341highRead is read off best.bucket, the bucket that won at the displayed tier. /overview?tier=100 re-selects the best bucket at 100 and may pick a different precision/release for a platform, so headlineLeaderTransition and the highLeadLine it gates can name a leader that contradicts what the 100 view renders. Not triggerable on today's single-precision data, but the branch is live and it's the same data-honesty class as the earlier -reason / evidence-date findings. Flagged inline with a fix (select the high-tier winner independently) + unit-test note.

CI: ✅ green — Lint & Format, Typecheck+Unit, Component, CodeQL (all analyzers), E2E chrome (4 shards) + firefox (4 shards), Vercel all passing. (Claude review / Cursor Bugbot pending, as expected.)

Runtime verified on localhost:3000 (real read-only DB): /overview renders the full matrix, 0 console errors.

No blocking issues. The single 🟡 is a correctness edge for a cross-tier hint line that today's data doesn't reach — recommend fixing before merge for the honesty invariant, but not a merge blocker on current data.

  • Diff 69a0a1a/f3d5ae0 vs previously-reviewed e2b7324 — comment/format-only
  • Assess open Bugbot finding: leader line vs tier-100 winner — confirmed, flagged inline
  • Check CI status — ✅ green
  • Runtime verify /overview — renders, 0 console errors
  • Post review

Comment thread packages/app/src/lib/overview-data.ts Outdated
Comment thread packages/app/src/components/header/header.tsx
@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @edwingao28's task in 2m 5s —— View job


Re-review — PR #611 Executive Overview page

Reviewed the two new commits since my last review of 69a0a1a. They resolve the two open findings from that round — the tier-100 leader line and the language-toggle href. Both are correct.

1. ✅ b507220At 100, X leads can no longer disagree with /overview?tier=100.
The tier-selection step is factored into selectRead(hardware, atTier) (overview-data.ts:323), and buildMember now calls it once at the displayed tier and independently at OVERVIEW_HIGH_TIER (:350-351), so highRead is the 100 view's own re-selected bucket — not best.bucket reused from tier 50. On top of that, headlineLeaderTransition (:288) now also requires the two @100 reads to share precision and dbModel (:296-299); when a different precision/release wins at 100 the line is suppressed (returns null) rather than naming a cross-stack winner — the same rule the direct delta already follows. I traced the wiring: the transition is fed candidate.highRead/baseline.highRead (:402-403), which are exactly the reads the tier-100 view renders, so the claim and the 100 matrix can't diverge. Pinned by the new overview-data.test.ts case (:297) where FP4 overtakes FP8 at 100 → highRead.config.precision === FP4 and highLeaderTransition is null.

2. ✅ 5aade72 — language toggle href carries query state.
LanguageToggle now syncs window.location.search into state (header.tsx:88-96) and sets href={target + search}, so modified-click / open-in-new-tab / copy-link keep ?tier= instead of dropping to the default tier — the earlier fix only covered the left-click navigateInApp path. The tier switcher uses plain <a href> full navigations (overview-scorecard.tsx:463), so each tier change re-mounts the toggle and the useEffect re-reads the fresh search — the [pathname]-only dep isn't a staleness risk here. SSR renders the bare sibling (correct hreflang fallback for no-JS crawlers), hydration fills in the query. E2E asserts href === '/zh/overview?tier=100' before the click (overview.cy.ts:263).

CI: ✅ green — Lint & Format, Typecheck+Unit, Component, CodeQL (all analyzers), E2E chrome (4 shards) + firefox (4 shards), Vercel all passing. (Claude review / Cursor Bugbot pending, as expected.)

Runtime verified on localhost:3000 (real read-only DB): /overview?tier=100 renders the full matrix; the language-toggle href is /zh/overview?tier=100 with hreflang="zh-CN"; 0 console errors. (The leader-line branch isn't triggered by today's single-precision data — pinned by the unit test instead, as the finding noted.)

No new issues introduced. Every finding across all prior rounds is now resolved. 🟢 LGTM.

  • Diff b507220 + 5aade72 vs previously-reviewed 69a0a1a
  • Verify the leader-line fix (highRead independent @100 selection + cross-stack suppression)
  • Verify the language-toggle href query-string fix
  • Check CI status — ✅ green
  • Runtime verify /overview?tier=100 — href carries ?tier=100, 0 console errors
  • Post review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5aade72. Configure here.

Comment thread packages/app/src/lib/overview-data.ts
@edwingao28
edwingao28 merged commit 61128b4 into master Jul 23, 2026
41 checks passed
@edwingao28
edwingao28 deleted the feat/executive-overview branch July 23, 2026 20:21
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.

1 participant