Skip to content

[Power] role-local measured energy axes + per-worker power drilldown | 前端:新增角色级实测能耗坐标轴与每 worker 功耗下钻 - #935

Open
edwingao28 wants to merge 24 commits into
masterfrom
feat/worker-power-drilldown
Open

[Power] role-local measured energy axes + per-worker power drilldown | 前端:新增角色级实测能耗坐标轴与每 worker 功耗下钻#935
edwingao28 wants to merge 24 commits into
masterfrom
feat/worker-power-drilldown

Conversation

@edwingao28

@edwingao28 edwingao28 commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR makes role-specific measured power data usable in the InferenceX frontend:

  • Adds Measured Prefill Joules per Input Token and Measured Decode Joules per Output Token to the Measured Energy y-axis group.
  • Adds a pinned-tooltip Measured Worker Power table for official results, unofficial-run overlays, and GPU/date comparisons when a row contains workers[].
  • Shows a specific bilingual empty-state explanation when the selected dataset does not report role-level prefill/decode energy, instead of suggesting that the chart is generically broken.
  • Keeps metric switching non-destructive: configurations without the selected role metric leave the rendered chart and legend, then return when the user switches back to a metric they report.

Data availability

Current role-energy coverage is sparse. The preview has real role-level points for selected DeepSeek V4 Pro 8K/1K FP4 and GLM-5.2 AgentX FP8 runs, but most historical rows do not contain the two role-energy scalars.

Current AgentX rows also do not contain workers[], so the AgentX worker drilldown is waiting for producer-side emission. The same tooltip path can be verified today on fixed-sequence historical disaggregated rows, including DeepSeek R1 MI355X results. The UI does not derive role energy from worker watts when the producer has not published the role-energy fields.

Validation

  • bun run typecheck
  • bun run lint
  • bun run fmt
  • App unit suite: 281 files / 4,727 tests ✅
  • Focused component coverage: ScatterGraph and GPUGraph, 35/35 tests ✅
  • English desktop and Simplified Chinese 375 px empty states rendered and checked ✅
  • GitHub CI re-runs the complete unit and browser matrix on this commit.

中文说明

本 PR 让 InferenceX 前端可以展示并解释按角色拆分的实测功耗数据:

  • 在“实测能耗”纵轴分组中新增“每输入 token 实测 Prefill 能耗”和“每输出 token 实测 Decode 能耗”。
  • 当数据行包含 workers[] 时,在点击固定的 tooltip 中展示“各 worker 实测功耗”表格;官方结果、非官方运行叠加层和 GPU/日期对比均支持。
  • 如果当前数据集未提供 Prefill/Decode 各角色的能耗数据,空状态会明确说明缺少这类数据,不再让用户误以为图表出现了通用故障。
  • 切换指标不会破坏芯片选择:缺少当前角色指标的配置会暂时从图表和图例中移除,切回其支持的指标后会自动恢复。

数据覆盖情况

目前按角色拆分的能耗数据仍然较少。预览环境中,部分 DeepSeek V4 Pro 8K/1K FP4 和 GLM-5.2 AgentX FP8 运行包含真实数据点,但大多数历史数据行尚未提供这两个角色能耗字段。

当前 AgentX 数据行也尚未包含 workers[],因此 AgentX 的 worker 下钻仍需等待生产端输出。现阶段可以使用固定序列的历史分离式推理数据验证同一套 tooltip 路径,例如 DeepSeek R1 的 MI355X 结果。如果生产端未发布角色能耗字段,前端不会根据 worker 功耗自行推算角色能耗。

验证

  • bun run typecheck
  • bun run lint
  • bun run fmt
  • App 单元测试:281 个文件 / 4,727 项测试 ✅
  • ScatterGraph 与 GPUGraph 定向组件测试:35/35 ✅
  • 已实际检查英文桌面端和 375 px 简体中文空状态 ✅
  • GitHub CI 将在本次提交上重新运行完整的单元测试和浏览器矩阵。

…filter | 推理图表:派生实测功耗认证层级并新增快捷筛选

Compute a per-entry power_tier ('certified' | 'legacy') in
rowToAggDataEntry from the producer verdict and whole-deployment energy
semantics, without changing which values render. Add a 'Measured Power'
quick-filter category (Certified/Legacy pills, i_power URL param) wired
through QuickFilters, InferenceContext, the dialog, and GPUGraph.

在 rowToAggDataEntry 中根据生产端验证结论与整体部署能耗语义派生
power_tier(certified/legacy),不改变任何渲染值。新增“实测功耗”
快捷筛选(已认证/旧版,URL 参数 i_power)。
… axes | 推理图表:在实测能耗坐标轴上标记旧版功耗数据点

On the nine Measured Energy y-axes, draw a dotted ring around points
whose telemetry lacks a producer validation verdict (official and
overlay render paths), show a localized footer legend key when such
points are on screen, state the tier in all three tooltip generators
(EN/ZH), and append a tier note to the measured axis explanations.

在九个实测能耗 Y 轴上,为缺少生产端验证结论的数据点绘制虚线圆环
(官方与叠加渲染路径),当此类点可见时在图表底部显示本地化图例,
在三种工具提示中标注认证层级(中英文),并在实测指标说明中补充
层级说明。
…and tooltips | 测试:覆盖实测功耗层级派生、筛选、圆环与工具提示

Table-driven resolvePowerTier cases, tier derivation through
rowToAggDataEntry/transformBenchmarkRows, quick-filter matching and
availability, measured-energy key classification, decoration rendering
per axis, tier tooltip lines (EN/ZH), and the i_power URL default.

覆盖 resolvePowerTier 表驱动用例、转换层层级派生、快捷筛选匹配与
可用性、实测能耗指标判定、按坐标轴的圆环渲染、中英文工具提示层级
行以及 i_power URL 默认值。
…与旧版实测功耗筛选

Deterministic intercepted fixtures (one certified, one legacy config)
drive the measured-axis ring, footer legend key, Certified/Legacy quick
filter pills, clear-filters reset, and i_power share-link restore.

使用确定性的拦截数据(一个已认证、一个旧版配置)验证实测坐标轴
圆环、底部图例、已认证/旧版快捷筛选、清除筛选以及 i_power 分享
链接恢复。
…ick-filter reset | 推理图表:图例快捷筛选计数与重置纳入实测功耗层级

ScatterGraph keeps its own copy of the quick-filter badge count and the
legend "Reset filter" handler; both omitted the new power category, so a
Certified/Legacy selection was invisible in the legend badge and
survived the reset click. Mirror the GPUGraph edit.
…rilldown | 推理图表:新增角色级实测能耗坐标轴与每 worker 功耗下钻

Add y_measuredPrefillJPerInputToken and y_measuredDecodeJPerOutputToken to
the Measured Energy group (bilingual labels, lower polarity, explanations),
map prefill_joules_per_input_token / decode_joules_per_output_token into
chart fields, and render a pinned-only per-worker power table (role, chips,
watts, optional temp/util/mem/hosts, 8-row cap, HTML-escaped JSONB strings)
in all three tooltip generators.

在“实测能耗”分组新增每输入 token 实测 Prefill 能耗与每输出 token 实测
Decode 能耗两个 Y 轴(双语标签、越低越好、附说明),将角色级能耗字段映射
为图表字段,并在三种工具提示中新增仅固定时显示的每 worker 功耗表
(角色、芯片数、功耗,可选温度/利用率/显存/主机,上限 8 行,HTML 转义)。
…worker drilldown | 测试:覆盖 AgentX 实测功耗、角色级能耗坐标轴与 worker 下钻

Agentic-traces transform pass-through and power_valid=0 scrub (including
workers), role-energy chart-field emission/absence/zero-preservation,
Pareto corners and bilingual labels for the two new axes, input/output
token classification and share-link round-trip, and the pinned worker
table across all three tooltip generators (cap, ZH, HTML escaping).

覆盖 agentic_traces 变换透传与 power_valid=0 清除(含 workers)、角色级
能耗图表字段的生成/缺省/零值保留、两个新坐标轴的帕累托方向与双语标签、
输入/输出 token 分类与分享链接解析回环,以及三种工具提示中固定状态下
的 worker 表(行数上限、中文、HTML 转义)。
…使用合成 worker 数据的 AgentX 实测功耗

Spec-scoped intercepts inject a disagg agentic series (role energy +
workers[]) and an aggregate series (whole-run metrics, workers: null):
the two role-energy axes render under Measured Energy, the aggregate
series coverage-filters off while the legend stays intact, the pinned
tooltip shows the worker table, and a workers-less point stays clean.

通过 spec 级拦截注入分离式 agentic 序列(角色级能耗 + workers[])与
聚合序列(全程指标、workers 为 null):两个角色级能耗坐标轴显示在
“实测能耗”分组下,聚合序列因无该指标从图中过滤但图例保持完整,
固定的工具提示显示 worker 表,无 workers 的点保持正常。
…n-destructive axis switch | 端到端测试修复:断言覆盖过滤会移出图例,并验证坐标轴切换可无损恢复

The rendered legend is metric-aware (hwTypesWithData), so a series without
the selected role-energy metric drops out of the legend too — the previous
assertion expected it to stay and failed deterministically. Re-selecting a
whole-run measured axis now proves the selection universe survives the
switch. Also documents the MiB reading of avg_mem_used_mb in the worker
tooltip.

图例渲染依据所选指标(hwTypesWithData),缺少角色级能耗指标的系列会同时从图例中
移除,原断言必然失败。现改为断言其不在图例中,并通过重新选择全局实测坐标轴验证
选择集在切换后无损恢复。同时为 worker 工具提示中 avg_mem_used_mb 的 MiB 语义
补充说明。
@edwingao28
edwingao28 requested a review from adibarra as a code owner August 28, 2026 02:53
@vercel

vercel Bot commented Aug 28, 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 Aug 31, 2026 10:24pm

Request Review

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

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


Review: PR #935 — role-local measured energy axes + per-worker power drilldown

  • Read the diff against feat/measured-power-tiering
  • Review new registry axes + chart field mapping
  • Review generateWorkerPowerHTML (escaping, unit conversion, caps)
  • Review coverage filtering / legend behavior
  • Check test coverage
  • Check CI status
  • Runtime verification (Playwright)

🟢 LGTM — no blocking issues found

Static review + runtime verification both clean. This is a tight, well-scoped, well-tested addition.

What I verified statically

  • Registry wiring — both new keys (measuredPrefillJPerInputToken, measuredDecodeJPerOutputToken) carry bilingual labels/titles, polarity: 'lower', METRIC_EXPLANATIONS entries with the tier note, and membership in MEASURED_ENERGY_METRIC_CONFIG_KEYS (so the tier ring / tooltip line / footer key apply automatically). tokenMetricTypeForConfigKey correctly classifies them input/output via substring — no output substring collision on the prefill key.
  • Transform — role-local energy and workers[] are gated on measuredPowerValid (benchmark-transform.ts:237-242, :253), so power_valid: 0 scrubs them consistently with the role-watts fields. Gating role-local joules on power_valid only (not hasWholeDeploymentEnergySemantics) is correct — the role-prefixed fields have unambiguous role-local semantics and never had the whole-deployment drift problem. This is the shared transform, so the overlay/?unofficialrun= path is covered too.
  • Tooltip XSSw.role and w.hosts (artifact JSONB) are HTML-escaped before interpolation (escapeHtml, tooltipUtils.ts); the remaining interpolated values are typed numbers. isPinned is in scope in all three generators (official/overlay/GPU-compare) and correctly gates rendering; 8-row cap + +N more overflow logic is correct (no line at exactly 8).
  • avg_mem_used_mb / 1024 → GiB — the MiB-despite-_mb assumption is explicitly commented as a re-check item for PLAN-09's producer. Fine to ship as documented.

Runtime (Playwright, real read-only DB)

  • Both new options render under the Measured Energy group in the y-axis selector.
  • Selecting Measured Prefill Joules per Input Token produces no console errors; the chart coverage-filters to an empty state (the default agentic model in prod has no role-local energy yet — the expected graceful path). The synthetic Cypress fixtures exercise the populated render.

Tests — comprehensive: transform pass-through + scrub, chart-field emission/absence/zero-preservation, Pareto corners + bilingual labels, token-type classification + share-link round-trip, tooltip table across all three generators (pinned/unpinned, empty/absent, 8-row cap, ZH, HTML escaping), and a new AgentX e2e spec covering both axes, non-destructive coverage filtering, the worker drilldown, and graceful absence.

Note: stacked PR — base is feat/measured-power-tiering (#934), which must land first.
· feat/worker-power-drilldown

中文:合并 master,并解决实测功耗功能的冲突。
…icts

中文:合并 measured-power-tiering 分支并解决 tooltip 冲突
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

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


Re-review: PR #935 — role-local measured energy axes + per-worker power drilldown

  • Read the diff against feat/measured-power-tiering
  • Identify new commits since the last review (ce14eec)
  • Review new registry axes + chart field mapping
  • Review generateWorkerPowerHTML (escaping, units, caps)
  • Review coverage filtering / legend behavior
  • Check test coverage
  • Check CI status
  • Runtime verification (Playwright)

🟢 LGTM — no blocking issues found

The only change since my last review (ce14eec) is the merge commit 393d5b36 ("merge measured-power-tiering, resolve tooltip conflicts"). I focused this pass there.

What the merge changed in scope

  • The conflict resolution in tooltipUtils.ts cleanly integrates master's tooltip localization (Chinese dates via formatTooltipDate, DPA: 是/否, worker 数, GitHub Actions 运行记录, runLinkHTML(runUrl, locale)) alongside this PR's generateWorkerPowerHTML. All three generators (official / overlay / GPU-compare) still call generateWorkerPowerHTML(d, isPinned, locale) with isPinned in scope; the escaping, 8-row cap + +N more, and MiB→GiB conversion are unchanged from the version I already approved.
  • No conflict markers, and the feature files carried by this PR (metric-registry.ts, chart-utils.ts, types.ts) are untouched by the merge — the two new axes (measuredPrefillJPerInputToken / measuredDecodeJPerOutputToken), their MEASURED_ENERGY_METRIC_CONFIG_KEYS membership, and buildMeasuredPowerChartFields mapping remain exactly as reviewed.
  • The Chinese tooltip strings introduced by the merge (数据并行注意力 (DPA), worker 数, etc.) originate from the base/master side, not this PR's authored work; they read naturally and are consistent with the established register — nothing to flag.

Runtime (Playwright, real read-only DB)

  • /inference renders with 0 console errors after the merge — the build is healthy, no regression from the conflict resolution. The two new axes and worker-drilldown behavior were already verified in the prior LGTM pass; the merge introduces no UI-surface change requiring re-verification (worker table needs a pinned point with populated workers[], which prod AgentX rows still don't ship — the synthetic Cypress fixtures cover that path).

CIClaude (route) pass, Vercel pass; tests-* / lint / Cursor Bugbot still pending at review time (nothing failing). No new code was added without tests.

Nothing blocking. Reminder for the merge order: this is a stacked PR — base feat/measured-power-tiering (#934) must land first; do not squash into master before it.
· feat/worker-power-drilldown

edwingao28 and others added 6 commits August 31, 2026 13:22
…filter | 推理图表:派生实测功耗认证层级并新增快捷筛选

Compute a per-entry power_tier ('certified' | 'legacy') in
rowToAggDataEntry from the producer verdict and whole-deployment energy
semantics, without changing which values render. Add a 'Measured Power'
quick-filter category (Certified/Legacy pills, i_power URL param) wired
through QuickFilters, InferenceContext, the dialog, and GPUGraph.

在 rowToAggDataEntry 中根据生产端验证结论与整体部署能耗语义派生
power_tier(certified/legacy),不改变任何渲染值。新增“实测功耗”
快捷筛选(已认证/旧版,URL 参数 i_power)。
… axes | 推理图表:在实测能耗坐标轴上标记旧版功耗数据点

On the nine Measured Energy y-axes, draw a dotted ring around points
whose telemetry lacks a producer validation verdict (official and
overlay render paths), show a localized footer legend key when such
points are on screen, state the tier in all three tooltip generators
(EN/ZH), and append a tier note to the measured axis explanations.

在九个实测能耗 Y 轴上,为缺少生产端验证结论的数据点绘制虚线圆环
(官方与叠加渲染路径),当此类点可见时在图表底部显示本地化图例,
在三种工具提示中标注认证层级(中英文),并在实测指标说明中补充
层级说明。
…and tooltips | 测试:覆盖实测功耗层级派生、筛选、圆环与工具提示

Table-driven resolvePowerTier cases, tier derivation through
rowToAggDataEntry/transformBenchmarkRows, quick-filter matching and
availability, measured-energy key classification, decoration rendering
per axis, tier tooltip lines (EN/ZH), and the i_power URL default.

覆盖 resolvePowerTier 表驱动用例、转换层层级派生、快捷筛选匹配与
可用性、实测能耗指标判定、按坐标轴的圆环渲染、中英文工具提示层级
行以及 i_power URL 默认值。
…与旧版实测功耗筛选

Deterministic intercepted fixtures (one certified, one legacy config)
drive the measured-axis ring, footer legend key, Certified/Legacy quick
filter pills, clear-filters reset, and i_power share-link restore.

使用确定性的拦截数据(一个已认证、一个旧版配置)验证实测坐标轴
圆环、底部图例、已认证/旧版快捷筛选、清除筛选以及 i_power 分享
链接恢复。
…ick-filter reset | 推理图表:图例快捷筛选计数与重置纳入实测功耗层级

ScatterGraph keeps its own copy of the quick-filter badge count and the
legend "Reset filter" handler; both omitted the new power category, so a
Certified/Legacy selection was invisible in the legend badge and
survived the reset click. Mirror the GPUGraph edit.
Draw historical-measurement rings in GPU comparisons, use clearer validated/historical copy with accessible help, expose visible coverage counts, and prevent singleton previews from hiding fuller curves. Add focused official, overlay, and end-to-end regressions.\n\n中文:在 GPU 对比图中绘制历史测量圆环,采用更清晰的“已验证/历史测量”文案并补充无障碍说明,显示当前可见数据覆盖率,同时避免单点预览遮蔽更完整的曲线。新增官方数据、非官方叠加与端到端回归测试。
@edwingao28
edwingao28 force-pushed the feat/measured-power-tiering branch from 5f4ddbb to a5162ad Compare August 31, 2026 20:24
中文:合并更新后的实测功耗基础分支
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

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


Re-review: PR #935 — role-local measured energy axes + per-worker power drilldown

  • Diff against feat/measured-power-tiering (13 files, +668/-1 — matches PR)
  • Identify new commits since last review (393d5b36)
  • Re-check core feature files (registry axes, chart fields, tooltip)
  • Check CI status
  • Runtime verification (Playwright)

🟢 LGTM — no blocking issues found

The only new commit since the last LGTM (393d5b36) is the base merge 225163cb ("chore: merge updated measured-power base"), which pulls the refreshed feat/measured-power-tiering base in. The PR's own diff against the base is unchanged — 13 files, +668/−1, identical to what was already reviewed. The merge resolved cleanly: no conflict markers, and this PR's authored files (metric-registry.ts, chart-utils.ts, types.ts, tooltipUtils.ts) are untouched by the merge itself.

Independently re-verified in this pass

  • Registry axesmeasuredPrefillJPerInputToken / measuredDecodeJPerOutputToken carry bilingual labels/titles, polarity: 'lower', and membership in MEASURED_ENERGY_METRIC_CONFIG_KEYS (so the tier ring / tooltip line / footer key apply). No output substring collision on the prefill key nor input on the decode key — tokenMetricTypeForConfigKey classifies each correctly.
  • Chart fields (chart-utils.ts:575) — role-local joules are mapped straight through chartMetric(...); the comment correctly notes rowToAggDataEntry already scrubbed them on power_valid, so no double-gating.
  • Tooltip XSSw.role and w.hosts (artifact JSONB) are escapeHtml-escaped before interpolation; remaining interpolated values are typed numbers. generateWorkerPowerHTML(d, isPinned, locale) is gated on isPinned && workers?.length in all three generators (official / overlay / GPU-compare), with the 8-row cap + +N more overflow.
  • avg_mem_used_mb / 1024 → GiB — MiB-despite-_mb assumption stays explicitly commented as a re-check item for the feat/agentx-power-artifacts producer. Fine to ship as documented.

Runtime/inference renders with 0 console errors after the merge; build is healthy, no regression from the base update. The two new axes and worker-drilldown paths were runtime-verified in the prior passes (worker table needs a pinned point with populated workers[], which prod AgentX rows don't yet ship — the synthetic Cypress fixtures cover it).

CIClaude route in progress at review time; tests-* / lint / Bugbot not yet reported. Nothing failing. No new untested code was introduced by the merge.

Nothing blocking. Merge-order reminder still stands: this is a stacked PR — base feat/measured-power-tiering (#934) must land first; do not squash into master before it.
· feat/worker-power-drilldown

Base automatically changed from feat/measured-power-tiering to master August 31, 2026 20:38
@edwingao28 edwingao28 changed the title feat(app): role-local measured energy axes + per-worker power drilldown | 前端:新增角色级实测能耗坐标轴与每 worker 功耗下钻 [Power] role-local measured energy axes + per-worker power drilldown | 前端:新增角色级实测能耗坐标轴与每 worker 功耗下钻 Aug 31, 2026
中文:合并最新 master 并解决 PowerX 冲突
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

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


Re-review: PR #935 — role-local measured energy axes + per-worker power drilldown

  • Identify new commits since last review (225163cb)
  • Review any new/changed feature code
  • Check CI status
  • Runtime verification (Playwright)
  • Post findings

🟢 LGTM — no blocking issues found

The only new commit since the last LGTM is the merge 88558843 ("chore: merge latest master"). It's a clean mergegit show 88558843 carries no conflict-resolution hunks, and this PR's own delta is unchanged: 13 files, +668/−1, byte-identical to what the three prior LGTM passes reviewed (git diff 225163cb..HEAD is empty). None of the authored feature files (metric-registry.ts, chart-utils.ts, types.ts, tooltipUtils.ts) were touched by the merge.

Nothing new to review substantively. Carrying forward the previously verified conclusions:

  • Registry axes measuredPrefillJPerInputToken / measuredDecodeJPerOutputToken — bilingual labels/titles, polarity: 'lower', MEASURED_ENERGY_METRIC_CONFIG_KEYS membership; tokenMetricTypeForConfigKey classifies each correctly with no substring collision.
  • Chart fields — role-local joules pass straight through chartMetric(...), no double-gating (already scrubbed on power_valid in rowToAggDataEntry).
  • Tooltip XSSw.role / w.hosts escapeHtml-escaped; generateWorkerPowerHTML gated on isPinned && workers?.length across all three generators; 8-row cap + +N more.
  • avg_mem_used_mb / 1024 → GiB — MiB-despite-_mb assumption remains explicitly commented as a re-check for the feat/agentx-power-artifacts producer. Fine to ship as documented.

CI — Lint & Format ✅, Tests (Unit) ✅, Code Quality ✅; Tests (E2E) + Claude in progress at review time, nothing failing.

Runtime/inference renders with 0 console errors after the master merge; build healthy, no regression from the base update. The worker table needs a pinned point with populated workers[] (prod AgentX rows still don't ship it) — the synthetic Cypress fixtures cover that path.

Merge-order reminder still stands: this is a stacked PR — base feat/measured-power-tiering (#934) must land first; do not squash into master before it.
· feat/worker-power-drilldown

中文:移除使用合成数据的 AgentX 功耗端到端测试
中文:将 PowerX 内部规划编号改为公开可读的技术说明
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

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


Re-review: PR #935 — role-local measured energy axes + per-worker power drilldown

  • Identify new commits since last review (225163cb)
  • Review core feature files (registry, chart fields, tooltip)
  • Check CI status
  • Runtime verification
  • Post review

🟢 LGTM — no blocking issues found

New commits since the last LGTM (225163cb): a master merge (88558843), a docs/comment relabel (c760e8bb), and the removal of the synthetic AgentX e2e spec (e5e04d39). The PR's authored feature files (metric-registry.ts, chart-utils.ts, types.ts, tooltipUtils.ts logic) are otherwise unchanged from the version already reviewed.

What the new commits changed

  • c760e8bb — pure comment/text relabel: swaps internal PLAN-02 / gap G2 / PLAN-09 planning labels for human-readable prose in three files (certified-power-filter.cy.ts, tooltipUtils.ts MiB→GiB comment, benchmark-transform.test.ts). No behavior change.
  • e5e04d39removes cypress/e2e/agentic-measured-power.cy.ts (−166) and its timings.json entries. Note this is a reversal of the e2e coverage the PR body still describes. It's a defensible call — the spec depended entirely on synthetic fixtures (prod AgentX rows don't ship workers[] yet), and the feature logic remains well-covered at the unit level (tooltip-utils.test.ts across all three generators incl. escaping/8-row cap/ZH; chart-utils.test.ts emission/absence/zero; benchmark-transform.test.ts pass-through + power_valid scrub; metric-registry.test.ts classification + share-link round-trip). Non-blocking, but worth updating the PR description so it no longer advertises the removed spec.

Re-verified

  • Registry/chart fieldsmeasuredPrefillJPerInputToken / measuredDecodeJPerOutputToken carry bilingual labels/titles, polarity: 'lower', and MEASURED_ENERGY_METRIC_CONFIG_KEYS membership. No output substring on the prefill key nor input on the decode key, so tokenMetricTypeForConfigKey classifies each correctly. Chart fields map straight through chartMetric(...); the comment correctly notes rowToAggDataEntry already scrubbed on power_valid (no double-gating).
  • Tooltip XSSw.role and w.hosts (artifact JSONB) are escapeHtml-escaped; remaining interpolated values are typed numbers. generateWorkerPowerHTML(d, isPinned, locale) gated on isPinned && workers?.length in all three generators, 8-row cap + +N more, MiB→GiB /1024.

Runtime/inference renders with 0 console errors after the merge; build healthy.

CILint & Format ✅; Tests (Unit) / Tests (E2E) in progress at review time, nothing failing.

Nothing blocking.
· feat/worker-power-drilldown

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

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


Re-review: PR #935 — role-local measured energy axes + per-worker power drilldown

  • Identify new commits since last review (88558843)
  • Review changed feature code
  • Check CI status
  • Runtime verification
  • Post findings

🟢 LGTM — no blocking issues found

Two new commits since the last LGTM, both low-risk:

  • c760e8bb (docs: replace internal PowerX planning labels) — comment-text only. Swaps internal PLAN-02/PLAN-09/G2 planning labels for reader-facing prose in certified-power-filter.cy.ts, tooltipUtils.ts, and benchmark-transform.test.ts. No functional change — the avg_mem_used_mb / 1024 → GiB line and all logic are byte-identical apart from the comment.
  • e5e04d39 (test: remove synthetic AgentX power E2E) — deletes cypress/e2e/agentic-measured-power.cy.ts (166 lines) and its timings.json entry. Intentional: the spec relied on synthetic workers[] fixtures because prod AgentX rows don't yet ship worker telemetry (producer lands with feat/agentx-power-artifacts).

Note (non-blocking): removing that spec leaves the worker drilldown with no E2E coverage, but it retains comprehensive unit coverage — tooltip-utils.test.ts exercises generateWorkerPowerHTML across all three generators (official / overlay / GPU-compare), pinned/unpinned, empty/absent, the 8-row cap, ZH strings, and HTML escaping, which satisfies the AGENTS.md overlay-path test requirement. Worth re-adding a real-data E2E once feat/agentx-power-artifacts populates workers[] in fixtures.

Carried-forward verification (unchanged by these commits):

  • Registry axes measuredPrefillJPerInputToken / measuredDecodeJPerOutputToken — bilingual labels/titles, polarity: 'lower', MEASURED_ENERGY_METRIC_CONFIG_KEYS membership; tokenMetricTypeForConfigKey classifies each correctly (no input/output substring collision).
  • Chart fields (chart-utils.ts:575) — role-local joules pass straight through chartMetric(...), correctly not double-gated (already scrubbed on power_valid in rowToAggDataEntry).
  • Tooltip — w.role / w.hosts (JSONB) escapeHtml-escaped; other interpolated values are typed numbers; gated on isPinned && workers?.length.

CI — Lint & Format ✅, Tests (Unit) ✅, Code Quality ✅, PR checks ✅; Tests (E2E) + Claude in progress at review time, nothing failing.

Runtime/inference renders with 0 console errors after these commits; build healthy, no regression.

Merge-order reminder still stands: this is a stacked PR — base feat/measured-power-tiering (#934) must land first; do not squash into master before it.
· feat/worker-power-drilldown

中文:精简 PowerX 实现中的重复测试说明和易过时注释
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

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


Re-review: PR #935 — role-local measured energy axes + per-worker power drilldown

  • Identify new commits since last review (e5e04d39)
  • Review changed code
  • Check CI status
  • Runtime verification
  • Post findings

🟢 LGTM — no blocking issues found

The only new commit since the last LGTM is 0c5c96fb (docs: simplify PowerX implementation comments). It is comment/text-only — no functional change:

  • certified-power-filter.cy.ts — trims the file header and several inline explanatory comments; all cy.* assertions are byte-identical.
  • tooltip-utils.test.ts — removes a section banner and two inline comments; test bodies unchanged.
  • tooltipUtils.ts — condenses the WORKER_ROWS_LIMIT and generateWorkerPowerHTML doc comments. The gating condition (!isPinned || !Array.isArray(d.workers) || d.workers.length === 0), escaping, 8-row cap, and MiB→GiB /1024 conversion are all unchanged.

git diff confirms no logic lines were touched. Nothing to review substantively.

Carried-forward verification (unchanged by this commit):

  • Registry axes measuredPrefillJPerInputToken / measuredDecodeJPerOutputToken — bilingual labels/titles, polarity: 'lower', MEASURED_ENERGY_METRIC_CONFIG_KEYS membership; tokenMetricTypeForConfigKey classifies each correctly (no input/output substring collision).
  • Chart fields — role-local joules pass straight through chartMetric(...), not double-gated (already scrubbed on power_valid in rowToAggDataEntry).
  • Tooltip XSS — w.role / w.hosts (artifact JSONB) escapeHtml-escaped; other interpolated values are typed numbers.

CI — all six runs pending/in-progress at review time (freshly triggered by this push); nothing failing.

Merge-order reminder still stands: this is a stacked PR — base feat/measured-power-tiering (#934) must land first; do not squash into master before it.
· feat/worker-power-drilldown

中文:移除 PowerX 测试中的重复分段与断言说明
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

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


Re-review: PR #935 — role-local measured energy axes + per-worker power drilldown

  • Identify new commits since last review (e5e04d39)
  • Review changed code
  • Check CI status
  • Post findings

🟢 LGTM — no blocking issues found

Two new commits since the last LGTM, both comment/narration only — no behavior change:

  • 0c5c96fb (docs: simplify PowerX implementation comments) — trims comments in certified-power-filter.cy.ts, tooltip-utils.test.ts, and tooltipUtils.ts. In tooltipUtils.ts it drops the WORKER_ROWS_LIMIT inline note and condenses the generateWorkerPowerHTML docblock; the guard (!isPinned || !Array.isArray(d.workers) || d.workers.length === 0), 8-row cap, +N more, escaping, and MiB→GiB conversion are byte-identical.
  • 3c80df4e (docs: remove redundant PowerX test narration) — trims comments in quickFilters.test.ts, tooltip-utils.test.ts, and benchmark-transform.ts. The benchmark-transform.ts change only reworded the comment above the measuredPowerValid ? … : undefined hoists — the gating logic is unchanged.

Confirmed via git show that neither commit touches any executable line. The core feature files (metric-registry.ts, chart-utils.ts, types.ts, and the generateWorkerPowerHTML logic itself) are unchanged from the versions approved in the prior passes.

Carried-forward verification (unchanged):

  • Registry axes measuredPrefillJPerInputToken / measuredDecodeJPerOutputToken — bilingual labels/titles, polarity: 'lower', MEASURED_ENERGY_METRIC_CONFIG_KEYS membership; tokenMetricTypeForConfigKey classifies each correctly (no input/output substring collision).
  • Chart fields — role-local joules pass straight through chartMetric(...), not double-gated (already scrubbed on power_valid in rowToAggDataEntry).
  • Tooltip XSSw.role / w.hosts (JSONB) escapeHtml-escaped; remaining interpolated values are typed numbers; gated on isPinned && workers?.length across all three generators.

CI — Lint & Format ✅, Tests (Unit) ✅, Code Quality ✅, PR checks ✅; Tests (E2E) + Claude in progress at review time, nothing failing.

Nothing blocking. Merge-order reminder still stands: this is a stacked PR — base feat/measured-power-tiering (#934) must land first; do not squash into master before it.

中文:当所选数据集未提供 Prefill/Decode 分角色能耗时,在普通图表和 GPU 对比图中显示明确的双语空状态,并补充组件回归测试。
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

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


Re-review: PR #935 — role-local measured energy axes + per-worker power drilldown

  • Identify new commits since last review (3c80df4e)
  • Review changed feature code
  • Check CI status
  • Runtime verification (Playwright)
  • Post findings

🟢 LGTM — no blocking issues found

The only new commit since the last LGTM is 62095602 (fix: explain unavailable role-local energy). It's a focused, well-tested addition:

  • New helper isRoleLocalMeasuredEnergyConfigKey (metric-registry.ts) — matches exactly the two role-local keys (y_measuredPrefillJPerInputToken / y_measuredDecodeJPerOutputToken), disjoint from the whole-deployment measured-energy keys. Covered by a new registry test.
  • Empty-state copy — both ScatterGraph.tsx and GPUGraph.tsx swap noDataHint for a role-energy-specific bilingual message when a role-local axis is selected and no series carry role energy. This is the expected path today (prod AgentX rows don't yet ship role-local energy). It's a heuristic keyed on the selected axis rather than the reason data is empty, so an unrelated empty selection on that axis would also show this hint — acceptable UX tradeoff, not a bug.
  • Tests — new component specs for both graphs (EN + ZH) assert the role-energy explanation renders and the generic hint does not; the metric-registry unit test locks the helper's classification.
  • Chinese copy当前数据集未提供 Prefill/Decode 各角色的能耗数据。请选择其他模型、场景、精度、日期或实测能耗指标。 reads naturally, preserves the source meaning, and correctly keeps Prefill/Decode in English per convention. Nothing to flag.

Runtime (Playwright, real read-only DB) — loaded /inference?i_metric=y_measuredPrefillJPerInputToken; the new empty state renders ("This dataset does not report role-level prefill/decode energy…") with 0 console errors, and the share-link param round-trips to the axis. Carried-forward verification of the two axes, chart-field mapping, and generateWorkerPowerHTML escaping/8-row-cap/gating from the prior passes is unchanged.

CI — Lint & Format ✅, Tests (Unit) ✅, PR checks ✅; Tests (E2E) / Code Quality / Claude in progress at review time, nothing failing.

Nothing blocking. Merge-order reminder still stands: this is a stacked PR — base feat/measured-power-tiering (#934) must land first; do not squash into master before it.
· feat/worker-power-drilldown

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