feat(compare): radar scale toggle, hideable usage axes, row renames by tawnymanticore · Pull Request #1765 · Kiln-AI/Kiln

tawnymanticore · 2026-09-11T16:05:13Z

What does this PR do?

TLDR: The compare radar chart said nothing with one or two run configs, because every axis was scaled to the selection and a missing score was drawn as zero. This PR adds a Relative / Full Scale toggle, plots only the scores every plotted config has, turns the cost, latency and token rows into hideable axes, and lets the user hide or rename any row. It replaces #1628 with the same feature on top of the current main, moves the chart maths into tested modules, and fixes three defects the new tests exposed. Targets main. There are no companion branches.

Before this PR, the radar chart on the compare page scaled each axis to the highest value among the selected run configs. One run config sat on the outer ring of every axis. A score with no result was plotted at the centre, which reads the same as a score of zero. Only eval sections had a hide control. After this PR, the chart has a Relative / Full Scale toggle, plots only the scores every plotted run config has a result for and reports the omitted count under the title, charts cost, latency and tokens as relative axes that the user can hide from the table, and lets the user rename any row for display. The chart logic lives in pure modules with unit tests, and the option object that reaches ECharts is asserted by a test.

Implementation

Feature, unchanged from Compare radar chart: relative/full scale toggle, removable usage axes, no false zeros, rename axes #1628 (first commit, byte-identical to that branch): the scale toggle, the shared-score filter with the omitted count, the five usage axes with "bigger means less" scoring, the hide and rename controls on every table row, and the URL parameters hidden_metrics and metric_labels that persist them.
Chart data layer: lib/utils/radar_chart_data.ts holds score normalisation, axis scaling, usage-axis inversion, the shared-key filter, label resolution and the tooltip trim. The component computes chart data once per update instead of twice. Its script drops from 629 lines to 374. The whole file is 430 lines; on main today it is 417 lines without the feature.
Shared constants: lib/utils/compare_metric_keys.ts holds the cost section id and the five usage keys. The page and the chart import them instead of repeating string literals.
Typed score range: the page's score-range lookup takes TaskOutputRatingType and ends with assertNever, so a new rating type fails the type check instead of returning null. The lookup and the axis-max builder move into compare_view.ts.
Tests: 33 component tests mock ECharts and assert on the option object that reaches setOption. 32 unit tests cover the data module. compare_view.test.ts grows from 20 to 26 cases. This is the first unit test of an ECharts component in the repo.
Fix: score labels, run config names and model names are HTML-escaped in the tooltip and the legend. Labels come from eval score names and from the metric_labels URL parameter, so a shared link could inject markup.
Fix: two run configs with the same name are separate series. Before, they collapsed into one legend entry that showed the first config's model, cost and scores.
Fix: the note under the title counts omitted axes with the correct word. It counted usage axes together with eval scores and called all of them "scores".
Fix: the radar card stays on the page when hidden rows leave fewer than three axes, and its empty state says how to get axes back. Before, the card disappeared with no message.
Fix: the radar centre and radius change in the wide layout so the left-hand axis labels fit. Before, "Input Token Efficiency" was clipped to "…en Efficiency" at the card edge.
Fix: the hide and rename controls have accessible names, and the rename input has a 60-character limit.
Fix: the chart redraws when the prompt list becomes undefined after it loaded. Before, the guard in the reactive statement stopped the redraw.
Rename: the page's hide state is named for what it hides, sections rather than evals, because the cost section is hideable too. The URL parameters keep their names. The section type is defined once, in compare_metric_keys.ts, and the chart's comparisonFeatures prop uses it: same shape, one field fewer.
Copy: the chart subtitle and the tooltip header cover the usage axes. escape_html also escapes the single quote.
Page-level, carried over from Compare radar chart: relative/full scale toggle, removable usage axes, no false zeros, rename axes #1628: updateURL() passes noScroll and keepFocus to goto(), so a state change no longer jumps the page to the top. The first table column grows from 200 px to 240 px to fit the rename control.

Before you merge

A user sees these changes: the scale toggle, the hide and rename controls on every row, the usage axes on the radar, and the omitted-score count. The old page hid eval sections only.
In the wide layout the radar is drawn smaller (radius 85 % to 70 %, centre 32 % to 36 %) so the axis labels fit. The compact layout for one or two run configs is unchanged.
Hiding rows no longer removes the radar card. The card shows an empty state with the ways to get axes back.
Not included: skipping lower_is_better or informational scores from the radar. EvalOutputScore has no direction field on main.

How the refactor was checked

The first commit reproduces the #1628 branch tip ba08b006e on main; the five changed files are byte-identical (git diff between the two is empty for those paths). A test harness was committed on top of it before any refactor. The refactor commits were then gated three ways:

The option object the component hands to ECharts, serialised with its formatter functions invoked, for six fixtures. The dump after each refactor commit is byte-identical to the dump at the baseline (diff -r is empty).
Full-page screenshots of six page states on the seeded fixture project, rendered from a checkout at the baseline commit and from the final commit, on separate ports. Two consecutive renders of the baseline are pixel-identical, so the comparison has no noise. At e9020ebab, after the three refactor commits and the first four fixes, all six states are pixel-identical to the baseline (0 pixels differ). At the final commit the differences are confined to the radar plot area in the three-config states (the resize for the labels) and to one subtitle line in the one- and two-config states, measured by the bounding box of the differing pixels.
The harness and the existing compare_view tests pass unchanged through the refactor commits. The three fix commits each change exactly the assertions they say they change.

Commits, in order:

d199675e9 feat(compare): radar chart scale toggle, hideable usage axes, no false zeros, renamable rows — Compare radar chart: relative/full scale toggle, removable usage axes, no false zeros, rename axes #1628 squashed, byte-identical in its five files
0271edcbf test(compare): characterise the compare radar chart option at baseline — 25 cases, no component change
1559d98a2 refactor(compare): extract the radar chart data layer into a tested module — golden identical
611c863af refactor(compare): share the cost section id and usage keys between page and chart — golden identical
7d3351b6b refactor(compare): type the score range lookup and move it into compare_view — golden identical
5c278e1b9 fix(compare): escape score labels in the radar tooltip — golden identical (no fixture carries markup characters); one harness assertion changed
f12bc6163 fix(compare): keep run configs that share a name apart on the radar — golden changes only in the duplicate-names fixture; one harness assertion changed
e3712603f fix(compare): count omitted axes, not scores, under the radar title — DOM text only
e9020ebab fix(compare): name axes consistently in the radar empty state — DOM text only
45b5bfbbb fix(compare): keep the radar card when hidden rows leave too few axes — DOM only, golden identical
db31ec158 fix(compare): make room for the radar axis labels — golden changes only in radar.center, radar.radius, axisName.width
3ece384c1 test(compare): assert the radar option shape instead of a tautology — tests only
4bb7c8945 fix(compare): label the hide and rename controls for assistive technology — golden identical
a4498758d refactor(compare): name the hide state for what it hides — golden identical
bc245af22 fix(compare): redraw the radar when the prompt list loads — golden identical; one new harness case
96ac35cfa fix(compare): update the radar copy for usage axes — golden changes only in tooltip header text

Screenshots

Hosted on the mike/compare-radar-legibility-v2-assets branch. Never merge that branch. The team can delete it after this PR closes.

Three run configs, relative scale. The table has hide and rename controls on every row; the radar plots one eval axis and five usage axes:

Full scale. The eval axis uses its own 0 to 1 range, and the axis labels fit:

The same view on the #1628 branch, for comparison. The left labels are clipped:

A row renamed to "Escalation" through the rename control. The name changes on the table, the axis, and the tooltip:

Two usage rows hidden from the table. They leave the radar, and the "Show Section" menu lists them:

A single run config defaults to full scale with an area fill:

Related Issues

Replaces #1628. No tracked issue.

Contributor License Agreement

Left for the PR author to complete.

Checklists

Tests have been run locally and passed (uv run ./checks.sh --agent-mode exits 0 in a clean worktree; npm run check reports 0 errors and the same 38 warnings as main; misspell is not installed locally, so CI covers that check)
New tests have been added to any work in /lib — not applicable, this PR changes the web UI only

🤖 Generated with Claude Code

…e zeros, renamable rows Squash of the six commits on mike/radar-chart-legibility (PR #1628), re-based onto main at 9893493. The five changed files are byte-identical to the old branch tip ba08b00. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Mocks echarts, renders the component with fixed props, and asserts on the option object that reaches setOption. No component change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…odule Moves score normalisation, axis scaling, usage inversion, shared-key filtering, label resolution and tooltip trimming out of compare_radar_chart.svelte into lib/utils/radar_chart_data.ts. The component computes chart data once per update. The option object that reaches echarts is byte-identical for the six golden fixtures. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…age and chart The compare page built the usage section from string literals and the radar chart re-typed the same keys, so renaming a row key would have dropped an axis with no type error. compare_metric_keys.ts now holds the section id, the five usage row keys with their table and axis names, and the usage-key predicate. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…re_view score_type_max took a string, so a rating type added to the backend would have fallen through to null and quietly disabled Full Scale for that axis. It now takes TaskOutputRatingType and ends in assertNever. It and the axis-max builder move to compare_view.ts as pure functions, with tests for the key format. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Score names, run config names, model, prompt and tool names reached the radar tooltip's HTML unescaped, and echarts renders a tooltip formatter's string as innerHTML. A score name, or a metric_labels entry carried in a shared compare link, could therefore inject markup. chat_markdown's escape helper moves to lib/utils/escape_html.ts and both call sites use it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Series and tooltip lookups keyed on the display name, so two run configs with the same name collapsed into one legend entry showing the first config's model, cost and scores. Chart data now carries the run config behind each series name, and a name two configs share is numbered so echarts can tell the series apart. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The note under the chart title counts every candidate axis dropped for want of a result, usage rows included, so calling them all scores misnamed the cost, latency and token rows it was counting. The count itself is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The note under the title counts axes. The empty state for fewer than three shared axes said "scores". Both now say axes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Hiding rows until fewer than three axes remained unmounted the whole card - title, scale toggle and all - so the chart vanished with nothing to explain it. The card now goes only when the table has no rows at all; one or two rows keep it, and the empty state names the three ways back to a third axis. The "Create and run evals" message is reserved for a table with no eval section, so it no longer greets a user whose eval rows are merely hidden, and the omitted-axis note no longer repeats what the empty state already says. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

On a card much wider than it is tall the plot's left edge sat about 0.32W - 0.425H from the left, which is less than the name gap plus the axis name box on a 1280px screen, so the left-hand names ran off the canvas. The wide layout's centre moves right and its radius comes in, and the name box narrows to a width that still fits "Output Token Efficiency" on two lines. The legend stays where it was. A new case computes the left margin from the option itself, so the next change to either number has to keep the names on the card. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The only whole-option case asserted that the keys of an object built from a list of names were that list of names, which is true of any object built that way. It proved nothing about the option, while reading as if the refactors were gated. Each fixture now gets its own case: the option carries exactly the four keys the chart sets, every series datum has one value per axis, all three formatters return something for every series name, and the option holds no undefined while its serialised form holds no function. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…logy The ✎ and ✕ buttons carry the glyph as text, so their title never becomes the accessible name and a screen reader reads out the character. Each now has an aria-label naming the row or section it acts on, matching the convention in tag_input.svelte, and the rename field has one of its own. The rename field also takes a 60 character cap: the name is JSON-stringified into the URL, and 60 is as much as a row heading can show anyway. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

hideEval now hides the cost section too and showAllHiddenEvals clears hidden rows as well, so the names had stopped describing the work. They become hideSection / showSection / hiddenSectionIds / showAllHidden / hiddenMenuItems, and the dropdown header reads "Show Section" rather than offering an eval named "Average Usage, Cost & Latency". The URL parameter names are unchanged so old links keep working. The section shape had three near-identical declarations; it now has one, in compare_metric_keys.ts, which both charts, the page's view helpers and their tests import. has_default_eval_config went with the copies - nothing on the chart side ever read it. The chart also took the user's display names by two routes at once. The page now filters first and labels afterwards, so the table gets the labelled rows and the chart gets the rows plus the metricLabels prop, which is the one route its own naming rule uses. Also: a dozen symbols nothing imports lose their export, an unreachable `|| []` goes, `|| 1` becomes `?? 1` so a legitimate zero survives, an `?.` on a non-nullable Eval goes, and a harness case is renamed to match what it checks. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The redraw guard read `prompts || prompts === null`, which is false for an undefined prop, so a caller that ever set prompts to undefined froze the chart with no error. Naming prompts in the statement is what makes the redraw depend on it, but the comparison was never what did that work. chartData and prompts are now arguments to updateChart and are threaded into the legend and tooltip builders, so the dependency is the call itself and each formatter carries the prompt list of the draw that made it. A new case walks a missing prompt list through arriving and going away again, and fails on the old guard at the last step. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The subtitle promised only evaluation scores while the chart had grown cost, speed and token axes, which is the point of the feature. It now names all four kinds of axis. In the tooltip the same list of numbers was headed "Values" or "Lowest Scores" depending on how many there were; both now say "Scores", so the two states read as one list. escapeHtml also escapes an apostrophe. Every sink today is element text or a double-quoted attribute, so nothing rendered changes, but the function promises general-purpose escaping and a single-quoted attribute would have been open. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

coderabbitai · 2026-09-11T16:05:35Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: e7aa867d-83ed-4ecc-a3f9-37a6b9dc1dce

📥 Commits

Reviewing files that changed from the base of the PR and between 9893493 and 96ac35c.

📒 Files selected for processing (13)

app/web_ui/src/lib/components/chart_no_data.svelte
app/web_ui/src/lib/components/compare_chart.svelte
app/web_ui/src/lib/components/compare_radar_chart.svelte
app/web_ui/src/lib/components/compare_radar_chart.test.ts
app/web_ui/src/lib/ui/chat/chat_markdown.svelte
app/web_ui/src/lib/utils/compare_metric_keys.ts
app/web_ui/src/lib/utils/escape_html.test.ts
app/web_ui/src/lib/utils/escape_html.ts
app/web_ui/src/lib/utils/radar_chart_data.test.ts
app/web_ui/src/lib/utils/radar_chart_data.ts
app/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/compare/+page.svelte
app/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/compare/compare_view.test.ts
app/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/compare/compare_view.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

The compare page now supports hidden sections and metrics, custom metric labels, shared comparison types, configurable radar scaling, usage metrics, safer tooltips, and extracted radar chart data utilities. New tests cover helper functions, chart rendering, scoring, formatting, and empty states.

Changes

Comparison chart controls and radar data

Layer / File(s)	Summary
Shared comparison contracts and escaping `app/web_ui/src/lib/utils/compare_metric_keys.ts`, `app/web_ui/src/lib/utils/escape_html.ts`, `app/web_ui/src/lib/ui/chat/chat_markdown.svelte`, `app/web_ui/src/lib/components/chart_no_data.svelte`, `app/web_ui/src/lib/components/compare_chart.svelte`	Shared comparison and usage metric contracts are added. HTML escaping is centralized. The no-data component now accepts title and message props.
Radar data construction and scoring `app/web_ui/src/lib/utils/radar_chart_data.ts`, `app/web_ui/src/lib/utils/radar_chart_data.test.ts`	Radar data construction now handles shared axes, usage scoring, axis limits, tooltip ranking, series names, and insufficient data states.
Comparison filtering and row controls `app/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/compare/compare_view.ts`, `app/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/compare/compare_view.test.ts`, `app/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/compare/+page.svelte`	The compare page persists hidden sections, hidden metrics, and custom labels in URL parameters. Users can hide, restore, and rename sections and metric rows.
Radar chart rendering and interaction `app/web_ui/src/lib/components/compare_radar_chart.svelte`, `app/web_ui/src/lib/components/compare_radar_chart.test.ts`	The radar chart uses extracted data, supports relative and full-scale modes, displays usage details, escapes tooltip values, adjusts layouts, and renders updated empty states.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Feature

Merge Risk: ⚪ Minimal · up to 96ac3

The comparison-page changes are covered by focused utility and component tests, with no actionable merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is detailed and covers the implementation, testing, related issue, and checklist. However, the required Contributor License Agreement confirmation is incomplete.	Complete the Contributor License Agreement section with the author’s GitHub username and confirmation that the agreement was read and accepted. Optionally mark the /lib test checklist item as checked with the stated not-applicable justifica…
Docstring Coverage	⚠️ Warning	Docstring coverage is 61.76% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 34 functions across 8 files. (5 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the primary changes: the radar scale toggle, hideable usage axes, and renamable comparison rows.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Description check

Resolution

Complete the Contributor License Agreement section with the author’s GitHub username and confirmation that the agreement was read and accepted. Optionally mark the /lib test checklist item as checked with the stated not-applicable justification.

Full details: Docstring Coverage

Explanation

Docstring coverage is 61.76% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 34 functions across 8 files. (5 skipped: 5 unsupported.)

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch mike/compare-radar-legibility-v2

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{A rabbit hops through axes bright
And trims the scores to fit just right
Hidden rows return with cheer
Safe tooltips guard each ear
Scales shift softly, clear and true
Fresh chart paths bloom anew}

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-09-11T16:08:32Z

📊 Coverage Report

Overall Coverage: 92%

Diff: origin/main...HEAD

No lines with coverage information in this diff.

📊 HTML Coverage Report - Interactive coverage report
📈 Diff Coverage Report - Detailed diff analysis
Github Actions Run - View the full coverage report

scosman

Not sure I like "a dozen pencils" UI, or an edit UI that doesn't save the edit. We should chat through the UI design.

tawnymanticore and others added 16 commits September 11, 2026 10:42

test(compare): characterise the compare radar chart option at baseline

0271edc

Mocks echarts, renders the component with fixed props, and asserts on the option object that reaches setOption. No component change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

fix(compare): name axes consistently in the radar empty state

e9020eb

The note under the title counts axes. The empty state for fewer than three shared axes said "scores". Both now say axes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

tawnymanticore mentioned this pull request Sep 11, 2026

Compare radar chart: relative/full scale toggle, removable usage axes, no false zeros, rename axes #1628

Closed

2 tasks

scosman requested changes Sep 11, 2026

View reviewed changes

Navigation Menu

feat(compare): radar scale toggle, hideable usage axes, row renames - #1765

feat(compare): radar scale toggle, hideable usage axes, row renames#1765
tawnymanticore wants to merge 16 commits into
mainfrom
mike/compare-radar-legibility-v2

tawnymanticore commented Sep 11, 2026

Uh oh!

coderabbitai Bot commented Sep 11, 2026 •

edited

Loading

❌ Failed checks (2 warnings)

Uh oh!

github-actions Bot commented Sep 11, 2026

Uh oh!

scosman left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

tawnymanticore commented Sep 11, 2026

What does this PR do?

Related Issues

Contributor License Agreement

Checklists

Uh oh!

coderabbitai Bot commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (2 warnings)

Uh oh!

github-actions Bot commented Sep 11, 2026

📊 Coverage Report

Diff: origin/main...HEAD

Uh oh!

scosman left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Sep 11, 2026 •

edited

Loading