6krrt/plans/verdict-bar-update-labels-review.md

Review: verdict-bar chart breaks on its first live update

Status: done -- review of shipped work

Scope: just this one defect, introduced in 906c9e6 ("fix(admin): replace stretched verdict doughnut with partition bar and split history into per-metric mini charts"), in admin/frontend/index.html. Not a review of the rest of that commit (the history mini-charts split was checked separately and is sound).

Verdict: real regression, invisible on first load, breaks on every subsequent poll

906c9e6 replaced the Verdict Mix doughnut with a horizontal "partition bar" — one Chart.js bar dataset holding all N verdict counts, rendered as a single row split into colored segments. That design depends on the chart always having exactly one category on its index axis, with all N values living inside that one dataset's data array.

Chart creation gets this right. renderChart's verdict-bar branch (index.html:844-857) ignores whatever labels it was called with and hardcodes a single-element array:

return new Chart(canvas, { type: 'bar', data: { labels: [''], datasets }, options: opts });

But updateChart — the path taken on every render after the first, since renderVerdict calls it whenever verdictChart already exists (index.html:543) — does not:

function updateChart(name, labels, values, type, colors) {          // index.html:898
  if (name !== 'verdict' || !verdictChart) return;
  verdictChart.data.labels = labels;                                 // index.html:900
  verdictChart.data.datasets[0].data = values;
  verdictChart.data.datasets[0].backgroundColor = colors;
  verdictChart.update();
}

labels here is Object.keys(mix) (index.html:527) — e.g. ['pass', 'fail'], one entry per verdict category, not the single-element array the chart was created with. Line 900 overwrites data.labels with that real array.

For a stacked bar chart on indexAxis: 'y' with a single dataset, Chart.js uses data.labels.length to decide how many category rows to draw. stacked: true (index.html:854-855) only merges multiple datasets that share an index position — it does nothing to merge multiple values within one dataset onto a single row. So the moment data.labels goes from [''] to ['pass', 'fail'], the same one dataset that used to render as one bar with two colored segments instead renders as two separate bars, one per label. The "partition bar" concept only holds together as long as labels stays a single blank entry, which is exactly the invariant updateChart breaks.

Why this passed the round of QA that caught the last verdict-chart bug

The previous regression (verdictChart never being assigned, causing "Canvas is already in use") was caught by explicitly waiting 36+ seconds to span a REFRESH_MS poll cycle (plans/.omo/evidence/task-8-admin-visual-fixes-v3-verdict.md). That discipline wasn't applied to 906c9e6 — it's a pure frontend diff with no Playwright run, no screenshot, and no unit test attached (git show 906c9e6 --stat touches only admin/frontend/index.html). A single fresh-load screenshot would show a correct-looking single bar, since first render always goes through renderChart, not updateChart — the bug only shows up starting from the second render, i.e. the first 30-second poll or SSE-triggered refresh after page load. Same blind spot as the last bug, in a new function.

Fix

updateChart is only ever called for 'verdict' (the name !== 'verdict' guard at the top makes it single-purpose), so there's no other caller relying on the data.labels assignment. Drop it:

function updateChart(name, labels, values, type, colors) {
  if (name !== 'verdict' || !verdictChart) return;
  verdictChart.data.datasets[0].data = values;
  verdictChart.data.datasets[0].backgroundColor = colors;
  verdictChart.update();
}

labels becomes an unused parameter at that point; either drop it from the signature and its one call site (index.html:543), or leave it for signature symmetry with renderChart — cosmetic, doesn't affect correctness either way.

Verification recommendation

A screenshot alone won't catch this class of bug twice. Confirm the fix the same way the last one was confirmed: load /admin/, capture the verdict bar on first render, wait past one REFRESH_MS cycle (30s+), capture again, and diff — the bar should still be one row with the same segment count before and after, not N separate rows.