mirror of
https://github.com/runbear-io/beardrive.git
synced 2026-08-25 08:08:08 +02:00
fix(webapp): the reads × freshness chart's busiest dot fits inside its own frame (BEA-60) (#111)
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 commit is contained in:
co-authored by
Claude Opus 5
parent
d7772a22df
commit
7861b755ac
@@ -83,6 +83,7 @@ classDiagram
|
||||
+heat.ts heatFor heatTotal heatText heatLevel hotPathSplit
|
||||
+heat.ts ageRange isFlatRange ageSpanLabel (treemap scale)
|
||||
+heat.ts orphanPaths (reads whose file left the tree)
|
||||
+heat.ts placeLabels LABEL_MAX (scatter danger-dot labels)
|
||||
+sniff.ts sniffBytes BlobText MAX_BYTES
|
||||
+utils.ts
|
||||
}
|
||||
@@ -97,7 +98,7 @@ classDiagram
|
||||
Browser --> components
|
||||
HubApp --> components
|
||||
components --> nav : linkProps navigate
|
||||
components --> lib : diffText groupRuns hotPathSplit
|
||||
components --> lib : diffText groupRuns hotPathSplit placeLabels
|
||||
hooks --> lib : re-exports heat.ts, sniffBytes
|
||||
hooks --> api
|
||||
Browser --> hooks
|
||||
|
||||
@@ -231,6 +231,12 @@ func seedE2E(t *testing.T, state, prefix, projectID string) {
|
||||
png := "\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01\x08\x06\x00\x00\x00\x1f\x15\xc4\x89" +
|
||||
"\x00\x00\x00\nIDATx\x9cc\x00\x01\x00\x00\x05\x00\x01\r\n-\xb4\x00\x00\x00\x00IEND\xaeB`\x82"
|
||||
put("assets/logo.png", png, 24*time.Hour)
|
||||
// Hot but stale: read a lot, unchanged for months. Every other seeded file
|
||||
// is hours old, so without these the Dashboard's danger quadrant — the one
|
||||
// thing that panel exists to surface — was empty in every test.
|
||||
put("archive/retired-spec.md", "# Retired spec\n\nNobody has touched this in months.\n", 210*24*time.Hour)
|
||||
put("archive/old-runbook.md", "# Old runbook\n\nStill read, never maintained.\n", 150*24*time.Hour)
|
||||
put("archive/legacy-notes.md", "# Legacy notes\n\nStale, still consulted.\n", 95*24*time.Hour)
|
||||
ops[2].Note = "expanded the guide — https://claude.ai/session/e2e" // the one row with a note expander
|
||||
// One agent run that touched two files — the history feed groups it into
|
||||
// a single card. One file it edited and one it created (whose undo is a
|
||||
@@ -272,6 +278,10 @@ func seedE2E(t *testing.T, state, prefix, projectID string) {
|
||||
{"guide.md", ReadKindHuman, "bob@x.io", 5},
|
||||
{"guide.md", ReadKindAgent, "seed", 9},
|
||||
{"notes/readme.md", ReadKindAgent, "seed", 2},
|
||||
// The danger quadrant: hot enough to matter, stale enough to worry.
|
||||
{"archive/retired-spec.md", ReadKindAgent, "seed", 14},
|
||||
{"archive/old-runbook.md", ReadKindHuman, "bob@x.io", 11},
|
||||
{"archive/legacy-notes.md", ReadKindAgent, "seed", 8},
|
||||
// Read history for the file the seed deletes above: heat rows outlive
|
||||
// their file, and the Dashboard must say so rather than drop them.
|
||||
{"scratch.md", ReadKindHuman, "alice@x.io", 4},
|
||||
|
||||
@@ -14,6 +14,74 @@ test("reads × freshness names all four quadrants", async ({ page }) => {
|
||||
]);
|
||||
});
|
||||
|
||||
/* Three defects on one chart (BEA-60): the busiest dot sat half outside the
|
||||
frame, no dot said which file it was, and the size caption was drawn on top
|
||||
of the "hot + stale" label. The frame numbers come off the viewBox rather
|
||||
than being repeated here. */
|
||||
test("every dot sits inside the frame, and the hot+stale ones say which file they are", async ({
|
||||
page,
|
||||
}) => {
|
||||
await login(page);
|
||||
const pid = await wikiId(page);
|
||||
await page.goto(`/${pid}/dashboard`);
|
||||
|
||||
const svg = page.locator("svg.in-chart:not(.in-treemap):not(.in-matrix)");
|
||||
const [, , W, H] = (await svg.getAttribute("viewBox"))!.split(" ").map(Number);
|
||||
const M = { l: 44, r: 16, t: 20, b: 34 }; // Scatter's margins
|
||||
|
||||
const dots = await svg.locator(".in-pt").evaluateAll((els) =>
|
||||
els.map((e) => ({
|
||||
cx: +e.getAttribute("cx")!,
|
||||
cy: +e.getAttribute("cy")!,
|
||||
r: +e.getAttribute("r")!,
|
||||
})),
|
||||
);
|
||||
expect(dots.length).toBeGreaterThan(0);
|
||||
// The repro: the hottest file landed at cy === M.t with r up to 7.
|
||||
expect(Math.min(...dots.map((d) => d.cy - d.r))).toBeGreaterThanOrEqual(M.t);
|
||||
expect(Math.max(...dots.map((d) => d.cx + d.r))).toBeLessThanOrEqual(W - M.r);
|
||||
expect(Math.max(...dots.map((d) => d.cy + d.r))).toBeLessThanOrEqual(H - M.b);
|
||||
expect(Math.min(...dots.map((d) => d.cx - d.r))).toBeGreaterThanOrEqual(M.l);
|
||||
|
||||
// The seeded archive/ files are hot and months old — the danger quadrant.
|
||||
const labels = svg.locator(".in-pt-label");
|
||||
const names = await labels.allTextContents();
|
||||
expect(names.length).toBeGreaterThan(0);
|
||||
expect(names.length).toBeLessThanOrEqual(6);
|
||||
expect(names).toContain("retired-spec.md"); // basename, not the full path
|
||||
for (const n of names) {
|
||||
await expect(page.locator(".in-hp-row", { hasText: n })).not.toHaveCount(0);
|
||||
}
|
||||
// Inside the frame, and no two labels on the same baseline.
|
||||
const boxes = await labels.evaluateAll((els) =>
|
||||
els.map((e) => e.getBoundingClientRect()),
|
||||
);
|
||||
const plot = (await svg.boundingBox())!;
|
||||
const s = plot.width / W; // viewBox units → screen px
|
||||
for (const b of boxes) {
|
||||
expect(b.left).toBeGreaterThanOrEqual(plot.x + M.l * s - 1);
|
||||
expect(b.right).toBeLessThanOrEqual(plot.x + (W - M.r) * s + 1);
|
||||
expect(b.top).toBeGreaterThanOrEqual(plot.y + M.t * s - 1);
|
||||
expect(b.bottom).toBeLessThanOrEqual(plot.y + (H - M.b) * s + 1);
|
||||
}
|
||||
for (let i = 0; i < boxes.length; i++)
|
||||
for (let j = i + 1; j < boxes.length; j++)
|
||||
expect(
|
||||
boxes[i].right < boxes[j].left ||
|
||||
boxes[j].right < boxes[i].left ||
|
||||
boxes[i].bottom < boxes[j].top ||
|
||||
boxes[j].bottom < boxes[i].top,
|
||||
).toBe(true);
|
||||
|
||||
// A dot keeps its tooltip and its click even where a label now sits.
|
||||
await expect(svg.locator(".in-pt").first().locator("title")).toHaveCount(1);
|
||||
await expect(svg.locator(".in-pt.danger").first()).toBeVisible();
|
||||
|
||||
// The caption moved out of the plot, where it overprinted "hot + stale".
|
||||
await expect(svg).not.toContainText("dot size");
|
||||
await expect(page.locator(".in-cap")).toHaveText("dot size = agent share of reads");
|
||||
});
|
||||
|
||||
/* A heat row outlives its file. Before this, the file panels joined heat onto
|
||||
the tree and dropped whatever didn't match — so the page could say "no reads"
|
||||
while the agent-coverage panel beside it rendered those same reads. */
|
||||
|
||||
@@ -3,7 +3,7 @@ import { useQuery } from "@tanstack/react-query";
|
||||
import { getJSON } from "../api/http";
|
||||
import type { HeatMap, Node } from "../api/types";
|
||||
import { heatTotal, hotPathSplit } from "../hooks/useBrowse";
|
||||
import { ageRange, ageSpanLabel, isFlatRange, orphanPaths } from "../lib/heat";
|
||||
import { ageRange, ageSpanLabel, isFlatRange, orphanPaths, placeLabels } from "../lib/heat";
|
||||
import { linkProps } from "../nav";
|
||||
|
||||
/* ---- the project Dashboard: the read×write matrix ----
|
||||
@@ -185,7 +185,12 @@ export function Insights(props: {
|
||||
<Treemap pts={pts} onOpenFile={props.onOpenFile} onOpenFolder={props.onOpenFolder} isFolder={props.isFolder} />
|
||||
{orphanNote}
|
||||
|
||||
<h3 className="dl-h3">Reads × freshness</h3>
|
||||
{/* The size caption belongs in the header, not in the plot: inside the
|
||||
<svg> it was drawn on top of the "hot + stale" quadrant label. */}
|
||||
<h3 className="dl-h3 in-h3-row">
|
||||
Reads × freshness
|
||||
<span className="in-cap">dot size = agent share of reads</span>
|
||||
</h3>
|
||||
<Scatter pts={pts} onOpenFile={props.onOpenFile} />
|
||||
{orphanNote}
|
||||
|
||||
@@ -439,8 +444,30 @@ function Scatter({ pts, onOpenFile }: { pts: Pt[]; onOpenFile: (p: string) => vo
|
||||
const maxReads = Math.max(HOT_READS * 2, ...pts.map((p) => p.reads));
|
||||
const lx = (d: number) => Math.log10(d + 1) / Math.log10(maxDays + 1);
|
||||
const ly = (r: number) => Math.log10(r + 1) / Math.log10(maxReads + 1);
|
||||
const X = (d: number) => M.l + lx(d) * (W - M.l - M.r);
|
||||
const Y = (r: number) => H - M.b - ly(r) * (H - M.t - M.b);
|
||||
/* Radius encodes the agent share of the file's reads. The plotted box is
|
||||
inset by the largest radius the formula can produce, so the busiest file
|
||||
— which lands at the very top of the range — sits wholly inside the frame
|
||||
instead of straddling its border. Derived, not hardcoded: a future radius
|
||||
change can't quietly reintroduce the clipping. The thresholds, the danger
|
||||
rect and the dots all read X/Y, so they move together; the axis lines use
|
||||
M directly and stay put. */
|
||||
const dotR = (share: number) => 3 + 4 * share;
|
||||
const rMax = dotR(1);
|
||||
const X = (d: number) => M.l + rMax + lx(d) * (W - M.l - M.r - 2 * rMax);
|
||||
const Y = (r: number) => H - M.b - rMax - ly(r) * (H - M.t - M.b - 2 * rMax);
|
||||
|
||||
const labels = placeLabels(
|
||||
pts
|
||||
.filter((p) => p.danger)
|
||||
.map((p) => ({
|
||||
path: p.path,
|
||||
reads: p.reads,
|
||||
cx: X(p.days),
|
||||
cy: Y(p.reads),
|
||||
r: dotR(p.total ? (p.agent || 0) / p.total : 0),
|
||||
})),
|
||||
{ right: W - M.r, top: M.t + 8, bottom: H - M.b - 4 },
|
||||
);
|
||||
|
||||
return (
|
||||
<svg viewBox={`0 0 ${W} ${H}`} className="in-chart">
|
||||
@@ -478,19 +505,15 @@ function Scatter({ pts, onOpenFile }: { pts: Pt[]; onOpenFile: (p: string) => vo
|
||||
<text x={M.l + 6} y={H - M.b - 8} className="in-quad">
|
||||
cold + fresh
|
||||
</text>
|
||||
<text x={W - M.r - 6} y={M.t + 28} className="in-label" textAnchor="end">
|
||||
dot size = agent share of reads
|
||||
</text>
|
||||
{pts.map((p) => {
|
||||
// Radius encodes the agent share of the file's reads; translucent
|
||||
// dots keep the cloud readable at hundreds of files.
|
||||
// Translucent dots keep the cloud readable at hundreds of files.
|
||||
const share = p.total ? (p.agent || 0) / p.total : 0;
|
||||
return (
|
||||
<circle
|
||||
key={p.path}
|
||||
cx={Number(X(p.days).toFixed(1))}
|
||||
cy={Number(Y(p.reads).toFixed(1))}
|
||||
r={Number((3 + 4 * share).toFixed(1))}
|
||||
r={Number(dotR(share).toFixed(1))}
|
||||
className={"in-pt" + (p.danger ? " danger" : p.reads ? "" : " cold")}
|
||||
onClick={() => onOpenFile(p.path)}
|
||||
>
|
||||
@@ -500,6 +523,19 @@ function Scatter({ pts, onOpenFile }: { pts: Pt[]; onOpenFile: (p: string) => vo
|
||||
</circle>
|
||||
);
|
||||
})}
|
||||
{/* The danger quadrant's story is unreadable if you have to hover dot by
|
||||
dot to learn which file is which; the tooltips stay for the rest. */}
|
||||
{labels.map((l) => (
|
||||
<text
|
||||
key={l.path}
|
||||
x={Number(l.x.toFixed(1))}
|
||||
y={Number(l.y.toFixed(1))}
|
||||
textAnchor={l.anchor}
|
||||
className="in-pt-label"
|
||||
>
|
||||
{l.name}
|
||||
</text>
|
||||
))}
|
||||
</svg>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -5,6 +5,7 @@ import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { heatFor, heatLevel, heatText, heatTotal, hotPathSplit } from "./heat.ts";
|
||||
import { ageRange, ageSpanLabel, isFlatRange, FLAT_AGE_SPREAD, orphanPaths } from "./heat.ts";
|
||||
import { placeLabels, LABEL_MAX } from "./heat.ts";
|
||||
import type { HeatMap } from "../api/types.ts";
|
||||
|
||||
// One fixture, read by both surfaces: the file header (heatText/heatTotal) and
|
||||
@@ -127,3 +128,56 @@ test("orphanPaths: heat rows whose file left the tree, sorted", () => {
|
||||
assert.deepEqual(orphanPaths({}, known), []);
|
||||
assert.deepEqual(orphanPaths(null, known), []);
|
||||
});
|
||||
|
||||
/* ---- placeLabels: the danger dots' basenames (BEA-60) ---- */
|
||||
|
||||
const BOUNDS = { right: 704, top: 28, bottom: 322 }; // W - M.r, M.t + 8, H - M.b - 4
|
||||
const dot = (path: string, reads: number, cx: number, cy: number, r = 3) => ({
|
||||
path,
|
||||
reads,
|
||||
cx,
|
||||
cy,
|
||||
r,
|
||||
});
|
||||
|
||||
test("placeLabels: basenames, busiest first, capped at six", () => {
|
||||
const dots = Array.from({ length: 9 }, (_, i) => dot(`archive/f${i}.md`, i, 400, 40 + i * 40));
|
||||
const out = placeLabels(dots, BOUNDS);
|
||||
assert.equal(out.length, LABEL_MAX);
|
||||
assert.deepEqual(
|
||||
out.map((l) => l.name),
|
||||
["f8.md", "f7.md", "f6.md", "f5.md", "f4.md", "f3.md"],
|
||||
);
|
||||
assert.deepEqual(placeLabels([], BOUNDS), []);
|
||||
});
|
||||
|
||||
test("placeLabels: sits right of the dot, flips left rather than leave the frame", () => {
|
||||
const [near] = placeLabels([dot("a/near-edge.md", 5, 400, 100, 7)], BOUNDS);
|
||||
assert.equal(near.anchor, "start");
|
||||
assert.equal(near.x, 411); // cx + r + 4
|
||||
const [far] = placeLabels([dot("a/a-very-long-filename.md", 5, 690, 100, 7)], BOUNDS);
|
||||
assert.equal(far.anchor, "end");
|
||||
assert.equal(far.x, 679); // cx - r - 4, text runs back into the frame
|
||||
});
|
||||
|
||||
test("placeLabels: colliding labels stack instead of overprinting", () => {
|
||||
// Three dots within a couple of px of each other: the naive placement puts
|
||||
// all three basenames on the same baseline.
|
||||
const out = placeLabels(
|
||||
[dot("a/one.md", 9, 500, 100), dot("a/two.md", 8, 505, 101), dot("a/three.md", 7, 510, 99)],
|
||||
BOUNDS,
|
||||
);
|
||||
const ys = out.map((l) => l.y).sort((a, b) => a - b);
|
||||
for (let i = 1; i < ys.length; i++) assert.ok(ys[i] - ys[i - 1] >= 11, `overlap: ${ys}`);
|
||||
assert.ok(ys.every((y) => y >= BOUNDS.top && y <= BOUNDS.bottom));
|
||||
});
|
||||
|
||||
test("placeLabels: stacks upward when downward would leave the frame", () => {
|
||||
const out = placeLabels(
|
||||
Array.from({ length: 5 }, (_, i) => dot(`a/f${i}.md`, 9 - i, 500, BOUNDS.bottom - 1)),
|
||||
BOUNDS,
|
||||
);
|
||||
const ys = out.map((l) => l.y).sort((a, b) => a - b);
|
||||
assert.ok(ys.every((y) => y >= BOUNDS.top && y <= BOUNDS.bottom), `out of frame: ${ys}`);
|
||||
for (let i = 1; i < ys.length; i++) assert.ok(ys[i] - ys[i - 1] >= 11, `overlap: ${ys}`);
|
||||
});
|
||||
|
||||
@@ -108,3 +108,55 @@ export function ageSpanLabel(min: number, max: number): string {
|
||||
b = Math.round(max);
|
||||
return a === b ? `${a}d` : `${a}–${b}d`;
|
||||
}
|
||||
|
||||
/* ---- scatter dot labels ---- */
|
||||
|
||||
// Only the danger quadrant gets named: it is the one the panel exists to
|
||||
// surface, and a label per dot at hundreds of files is unreadable.
|
||||
export const LABEL_MAX = 6;
|
||||
const LABEL_LH = 11; // line height at the 11px label size = the collision gap
|
||||
const LABEL_CH = 5.5; // ~average glyph width; only picks which side of the dot
|
||||
|
||||
export interface Dot {
|
||||
path: string;
|
||||
reads: number;
|
||||
cx: number;
|
||||
cy: number;
|
||||
r: number;
|
||||
}
|
||||
export interface PlacedLabel {
|
||||
path: string;
|
||||
name: string;
|
||||
x: number;
|
||||
y: number;
|
||||
anchor: "start" | "end";
|
||||
}
|
||||
|
||||
/* Basenames for the busiest danger dots, placed beside their dot: to the
|
||||
right, flipped left when the text would leave the frame, and stacked down
|
||||
(up, if down runs out of frame) when two would collide. Pure so it can be
|
||||
unit-tested — Insights.tsx can't be, see above. */
|
||||
export function placeLabels(
|
||||
dots: Dot[],
|
||||
bounds: { right: number; top: number; bottom: number },
|
||||
): PlacedLabel[] {
|
||||
const out: PlacedLabel[] = [];
|
||||
for (const d of [...dots].sort((a, b) => b.reads - a.reads).slice(0, LABEL_MAX)) {
|
||||
const name = d.path.split("/").pop()!;
|
||||
let x = d.cx + d.r + 4,
|
||||
anchor: "start" | "end" = "start";
|
||||
if (x + name.length * LABEL_CH > bounds.right) {
|
||||
x = d.cx - d.r - 4;
|
||||
anchor = "end";
|
||||
}
|
||||
const free = (v: number) => out.every((o) => Math.abs(o.y - v) >= LABEL_LH);
|
||||
let y = d.cy;
|
||||
while (y <= bounds.bottom && !free(y)) y += LABEL_LH;
|
||||
if (y > bounds.bottom) {
|
||||
y = d.cy;
|
||||
while (y >= bounds.top && !free(y)) y -= LABEL_LH;
|
||||
}
|
||||
out.push({ path: d.path, name, x, y: Math.min(bounds.bottom, Math.max(bounds.top, y)), anchor });
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
@@ -626,6 +626,13 @@ a.ai-main:hover { color: var(--accent); }
|
||||
.in-pt:hover { opacity: 1; }
|
||||
.in-pt.cold { fill: var(--text-ghost); opacity: .25; }
|
||||
.in-pt.danger { fill: #e05d5d; opacity: .6; }
|
||||
/* basename beside a hot+stale dot; pointer-events off so it never eats the click */
|
||||
.in-pt-label { fill: var(--text-faint); font-size: 11px; pointer-events: none; }
|
||||
/* the "dot size = …" caption, right-aligned in the panel heading — same
|
||||
max-width as .in-chart so it lands over the plot's right edge, not the
|
||||
column's */
|
||||
.in-h3-row { display: flex; justify-content: space-between; align-items: baseline; gap: 12px; max-width: 760px; }
|
||||
.in-cap { font-size: 11.5px; color: var(--text-faint); font-weight: 400; text-transform: none; letter-spacing: 0; }
|
||||
/* treemap */
|
||||
.in-treemap { background: #0c0d10; }
|
||||
.in-tm-group { fill: none; stroke: var(--border); stroke-width: 1; cursor: pointer; pointer-events: all; }
|
||||
|
||||
+15
-15
File diff suppressed because one or more lines are too long
+1
-1
File diff suppressed because one or more lines are too long
@@ -5,8 +5,8 @@
|
||||
<meta name="viewport" content="width=device-width, initial-scale=1">
|
||||
<title>BearDrive</title>
|
||||
<link rel="icon" href="data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 32 32' fill='%23f5a623'><rect x='4' y='4' width='5.6' height='24'/><rect x='11.2' y='4' width='14.4' height='11.2'/><rect x='11.2' y='16.8' width='16.8' height='11.2'/></svg>">
|
||||
<script type="module" crossorigin src="/assets/index-MK6uEuUN.js"></script>
|
||||
<link rel="stylesheet" crossorigin href="/assets/index-B-zSoeQX.css">
|
||||
<script type="module" crossorigin src="/assets/index-C2ypx3jZ.js"></script>
|
||||
<link rel="stylesheet" crossorigin href="/assets/index-CO632k9V.css">
|
||||
</head>
|
||||
<body>
|
||||
<div id="root"></div>
|
||||
|
||||
Reference in New Issue
Block a user