fix: /llm/costs table scrolling

[joeyorlando]

What changed

• replaced the /llm/costs table wrapper with a capped overflow-auto container
• kept the tables compact when they have only a few rows
• preserved internal scrolling once a table grows beyond roughly six visible rows
• added a focused frontend regression test for the capped scroll container behavior

Why

The previous Radix ScrollArea implementation only scrolled correctly when its root had a fixed height. That fixed the overlap, but it also forced sparse tables and empty states to reserve a large blank area. Using a normal overflow container keeps the table height content-driven up to the cap, then scrolls when needed.

Impact

This keeps /llm/costs readable across short and long datasets without pushing later sections down the page or creating oversized empty panels.

[claude]

Claude finished @joeyorlando's task in 2m 16s —— View job

---

PR Review: /llm/costs table scrolling fix

• Read PR changed files
• Review implementation correctness
• Review test coverage
• Post review feedback

---

Summary

The fix is correct and minimal. Replacing Radix ScrollArea with a plain max-h-[280px] overflow-auto container is the right call here — Radix's ScrollArea mounts an internal viewport with absolute positioning, so it only yields intuitive scroll behavior when its root has a fixed height. A native max-height + overflow-auto div gives you content-driven height that caps out, which is exactly what the PR description calls for. The import cleanup for ScrollArea/ScrollBar is tidy.

A few notes below, all minor / non-blocking.

---

Implementation observations

1. Horizontal scrolling is still covered — the previous implementation included <ScrollBar orientation="horizontal" />. Losing that could have been a regression, but Table in frontend/src/components/ui/table.tsx:7-20 already wraps the <table> in <div className="relative w-full overflow-x-auto">, so horizontal overflow is handled by the component itself. No concern.

2. Sticky table headers still work — TableHead cells use sticky top-0 z-10 bg-card (e.g. page.tsx:692-709). For sticky headers, the nearest scroll ancestor must be the vertical scroll container. Here the new <div className="max-h-[280px] overflow-auto …"> at page.tsx:1093-1095 is the vertical scroll container (the inner Table wrapper only sets overflow-x-auto), so sticky headers should continue to stick correctly. Worth a quick manual smoke test in the browser though, since JSDOM can't verify layout.

3. Height cap tightened from 360px → 280px — not mentioned in the PR description, but worth calling out. With a ~40px header row and ~36px data rows (from py-2 in TableCell, table.tsx:81-92), 280px ≈ header + 6 rows, matching the "roughly six visible rows" in the PR body. Reasonable choice.

4. Empty-state is compact — the empty TableRow uses py-8 (e.g. page.tsx:714-721), so an empty panel is ~120px tall, well under the 280px cap. This matches the PR's stated goal of keeping sparse tables from reserving a large blank area.

5. Tailwind JIT + the extracted constant — STATISTICS_TABLE_MAX_HEIGHT_CLASS = "max-h-[280px]" used via template literal at page.tsx:1094 is fine: the full literal max-h-[280px] exists statically in the source, so Tailwind's JIT will pick it up. No purge risk. That said, the constant is only used once and the indirection is primarily for the test to reference it — see next section. Inlining it would be equally clear and more idiomatic with the rest of the shadcn-styled JSX in this file.

---

Test feedback

The new test at page.test.tsx:101-168 guards the regression, but leans heavily on implementation details:

• Coupling to Tailwind class strings: container.querySelectorAll(".max-h-\\[280px\\]") and expect(tablePanel.className).toContain("max-h-[280px]") both match on raw class names. If the height cap is tweaked (e.g. to max-h-[320px]), the test breaks even though scroll behavior is still correct.
• Hardcoded toHaveLength(4): if someone adds a fifth statistics card later, this test fails in a misleading way (regression appears to be "scroll broke" when really it's "new card added").
• CLAUDE.md guidance: per platform/CLAUDE.md — "Prefer behavior-focused tests over implementation-detail tests. Do not add tests that only assert class names, prop plumbing, or incidental markup unless that detail is itself the contract." This test fits the "asserts class names" pattern.

Suggested refinement: add data-testid="statistics-table-panel" (or similar) to StatisticsTablePanel and query getAllByTestId(...) in the test. You can still assert the scroll classes with toHaveClass("max-h-[280px]", "overflow-auto"), which is at least more idiomatic than className.toContain(...). Even better would be to decouple the count assertion from the exact number of cards — e.g. expect(panels.length).toBeGreaterThan(0) combined with "every panel has scroll classes".

Also worth noting: because JSDOM does no layout, actual scroll behavior (does the container scroll when content exceeds the cap?) can't be verified here anyway — this test is really only guarding that the class names remain on the wrapper. Given that limitation, a smaller, focused unit test around StatisticsTablePanel in isolation might express intent more clearly than re-rendering the whole page with mocked query hooks.

Fix this →

---

Nits

• STATISTICS_TABLE_MAX_HEIGHT_CLASS exists mainly to be referenced from the test. If you tighten the test as suggested, you could inline the class and drop the constant to reduce indirection.
• Consider capturing a before/after screenshot (or the original bug repro) in the PR description for future readers — the "why" is clearly explained but a visual helps.

---

Verdict

The implementation is correct, minimal, and well-motivated. The main feedback is around test brittleness, which is non-blocking but worth considering. Nice cleanup overall.
• codex/fix-costs-table-scroll