fix(webapp): the reads × freshness chart's busiest dot fits inside its own frame (BEA-60) - #111
Merged
Merged
Conversation
…s own frame (BEA-60) Three defects on one panel, all of them stopping it from delivering its one insight — "these docs are hot but stale, go fix them": - The hottest file plotted at exactly y = M.t with a radius up to 7px, so the most important point on the chart straddled the top border. The plotted box is now inset on both axes by the largest radius the size formula can produce, derived from that formula rather than hardcoded, so a future radius change can't quietly reintroduce the clipping. Thresholds, the danger rect and the dots all read X/Y and shift together; the axis lines use M and stay. - No dot said which file it was — identity lived only in the hover tooltip. The six busiest hot+stale files now carry their basename beside their dot, flipped to the dot's other side rather than leaving the frame and stacked when two would print on one baseline. Placement is a pure function in lib/heat.ts so it can be unit-tested; every dot keeps its title and click. - "dot size = agent share of reads" was drawn inside the <svg> directly under the right-anchored "hot + stale" label, 14px apart. It moved to the panel heading row, capped at the chart's own max-width. The e2e seed grew three hot-but-months-old files: every seeded file was hours old, so the danger quadrant — the whole reason this panel exists — was empty in every test that had ever run against it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
Before / after
Same seeded
wikiproject, same data, both shots — only the rendering differs.main)On
mainthe top dot renders atcy = 20, r = 5.9—M.tis 20, so 5.9px of it is outside the frame. After:cy = 27, r = 5.9.The heading row on a phone, since the caption now lives there (no horizontal overflow —
scrollWidthis exactly the 390px viewport):What changed
1. Radius headroom (
Insights.tsx,Scatter).Y(maxReads)landed at exactlyM.twhile the dot radius runs up to 7px, so the hottest file always straddled the top border. The plotted box is now inset on both axes by the largest radius the size formula can produce:Derived rather than hardcoded so a future radius change can't quietly reintroduce the clipping. The thresholds, the danger rect and the dots all read
X/Yand move together by a few px; the axis and frame lines useMdirectly and stay put.2. Names on the danger dots. The six busiest hot+stale files get their basename beside their dot: to the right, flipped to the left when the text would leave the frame, stacked (down, or up if down would run out of frame) when two would print on one baseline. Placement is
placeLabels()inlib/heat.ts— pure, so it is unit-tested on node;Insights.tsxitself can't be, which is why the rest of that arithmetic already lives there. Labels arepointer-events: none, so every dot keeps its<title>and its click.3. Caption out of the plot.
dot size = agent share of readswas a<text>aty = M.t + 28, right-anchored, 14px under the right-anchoredhot + stalelabel. It is now a<span className="in-cap">in the panel's<h3>, capped at the chart's own 760px max-width so it lands over the plot's right edge rather than the column's.Deviations from the reviewed plan
Two, both forced by the real code:
.in-pt-labelcount is between 1 and 6 on the seededwikiproject"). Every file inseedE2Eis at most 72h old andSTALE_DAYSis 30, so the seed has zero hot+stale points — the danger quadrant has been empty in every test run against this panel. Rather than weaken the assertion, the seed gained three hot-but-months-old files underarchive/. Blast radius checked: no spec asserts an exact root-listing or project-history count (home.spec.tsuses>= 6), and all 109 e2e tests pass.lib/heat.ts, not inline inScatter(). Same logic the plan specified, moved so it can be unit-tested — the plan's own sketch was pseudo-code for a loop with three failure modes (flip, stack, clamp) worth pinning down.Architecture changes
architecture/webapp-frontend.md:libgained one exported member line —heat.ts placeLabels LABEL_MAX— and thecomponents --> libedge label gainedplaceLabels. No new types, no new seams, nothing removed.flowchart LR components["<div style='text-align:left'><b>components</b><br/>Insights (Scatter, Treemap, HotPath)<br/>FileView FolderListing FileTree<br/>HistoryView DiffView Palette ...</div>"] lib["<div style='text-align:left'><b>lib</b><br/>+diff.ts splitLines lcsDiff diffText<br/>+runs.ts groupRuns runFileCount<br/>+heat.ts heatFor heatTotal heatText heatLevel hotPathSplit<br/>+heat.ts ageRange isFlatRange ageSpanLabel (treemap scale)<br/>+heat.ts orphanPaths (reads whose file left the tree)<br/><span style='background:#22c55e55;padding:0 4px;border-radius:3px'>✅ +heat.ts placeLabels LABEL_MAX (scatter danger-dot labels)</span><br/>+sniff.ts sniffBytes BlobText MAX_BYTES<br/>+utils.ts</div>"] Note["placeLabels is pure, so the danger-dot<br/>placement is unit-tested on node —<br/>Insights.tsx itself cannot be (JSX)"] components -- "diffText groupRuns hotPathSplit <span style='background:#22c55e55;padding:0 5px;border-radius:3px'>✅ placeLabels</span>" --> lib lib -.- Note classDef added fill:#22c55e22,stroke:#22c55e,stroke-width:2px classDef noteBox fill:#88888822,stroke:#888888,stroke-dasharray:2 2 class Note noteBox linkStyle 0 stroke:#22c55e,stroke-width:2pxPre-existing, not fixed here:
architecture/webapp-frontend.md's mermaid block does not parse at all onmain— thenote for componentsline contains{name, template}, and the literal braces break the classDiagram grammar, so GitHub renders that file as an error box. The other three diagrams parse fine. Out of scope for this issue; flagged separately.Verification
go build ./...,go vet ./...go test ./...npm test(frontend unit)placeLabelscasesnpm run e2efrontend/check-dist.shinternal/webapp/static is freshRebased onto
origin/mainafter BEA-57 (#110) landed mid-build; theinternal/webapp/staticconflict was resolved by re-runningnpm run build, not by picking a side.Build session
(only works on the machine this ran on)