Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ export function calibrationHasSamples(calibration: GateCalibration): boolean {
return calibration.bins.some((bin) => bin.sampleSize > 0);
}

/** Kept-rate curve values for TrendChart — empty-sample bins read as 0 on the chart axis. */
export function calibrationTrendValues(bins: CalibrationBin[]): number[] {
return bins.map((bin) => (bin.keptRate === null ? 0 : bin.keptRate * 100));
/** Kept-rate curve values for TrendChart — an empty-sample bin's null `keptRate` is preserved as
* null so TrendChart renders a gap there instead of a fabricated 0% point. */
export function calibrationTrendValues(bins: CalibrationBin[]): Array<number | null> {
return bins.map((bin) => (bin.keptRate === null ? null : bin.keptRate * 100));
}

export function calibrationStatus(calibration: GateCalibration): {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,33 @@ describe("calibrationStatus", () => {
});

describe("calibrationTrendValues", () => {
it("maps null keptRate bins to 0 for the chart axis", () => {
expect(calibrationTrendValues(emptyBins())).toEqual([0, 0, 0, 0, 0]);
it("preserves null keptRate bins as null so TrendChart renders a gap", () => {
expect(calibrationTrendValues(emptyBins())).toEqual([null, null, null, null, null]);
});

it("preserves a mix of null and numeric bins in order", () => {
expect(
calibrationTrendValues([
{
label: "50–60%",
minConfidence: 0.5,
maxConfidence: 0.6,
sampleSize: 0,
keptCount: 0,
revertedCount: 0,
keptRate: null,
},
{
label: "90–100%",
minConfidence: 0.9,
maxConfidence: 1,
sampleSize: 2,
keptCount: 2,
revertedCount: 0,
keptRate: 0.8,
},
]),
).toEqual([null, 80]);
});

it("scales kept rates to percentage points for TrendChart", () => {
Expand Down Expand Up @@ -255,5 +280,6 @@ describe("CalibrationCard", () => {
expect(screen.getByText(/Raise confidenceFloor 0.9 → 0.94/)).toBeTruthy();
expect(screen.getByText("70–80%")).toBeTruthy();
expect(screen.getByText("90–100%")).toBeTruthy();
expect(screen.getByLabelText("Kept-rate curve by confidence band")).toBeTruthy();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,12 @@ export function CalibrationCard({ calibration }: { calibration: GateCalibration
</span>
</div>
<div className="mt-3 h-24 w-full">
<TrendChart values={trendValues} height={96} showAxis />
<TrendChart
values={trendValues}
height={96}
showAxis
label="Kept-rate curve by confidence band"
/>
</div>
</div>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,10 @@ export function formatTrendRatePct(value: number | null | undefined): string {
export function chartValuesForSeries(
weeks: SlopDuplicateTrendWeek[],
series: "slop" | "duplicate",
): number[] {
return weeks.map((week) => {
const value = series === "slop" ? week.slopFlagRatePct : week.duplicateFlagRatePct;
return value ?? 0;
});
): Array<number | null> {
return weeks.map((week) =>
series === "slop" ? week.slopFlagRatePct : week.duplicateFlagRatePct,
);
}

export function seriesHasSignal(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest";

import { SlopDuplicateTrendCard } from "@/components/site/app-panels/slop-duplicate-trend-card";
import {
chartValuesForSeries,
latestWeekWithSignal,
type MaintainerSlopDuplicateTrend,
type SlopDuplicateTrendWeek,
Expand Down Expand Up @@ -35,7 +36,8 @@ describe("SlopDuplicateTrendCard", () => {
expect(screen.getByText(/latest: 25%/i)).toBeTruthy();
expect(screen.getByText(/fresh snapshot/i)).toBeTruthy();
expect(screen.getByText(/generated/i)).toBeTruthy();
expect(screen.getAllByLabelText("Trend chart")).toHaveLength(2);
expect(screen.getByLabelText("Slop flag rate trend")).toBeTruthy();
expect(screen.getByLabelText("Duplicate flag rate trend")).toBeTruthy();
});

it("shows a one-series-empty branch when only duplicate samples exist", () => {
Expand All @@ -54,7 +56,8 @@ describe("SlopDuplicateTrendCard", () => {
/>,
);
expect(screen.getByText("No slop-flag samples in the snapshot window yet.")).toBeTruthy();
expect(screen.getByLabelText("Trend chart")).toBeTruthy();
expect(screen.getByLabelText("Duplicate flag rate trend")).toBeTruthy();
expect(screen.queryByLabelText("Slop flag rate trend")).toBeNull();
expect(screen.getByText(/latest: 50%/i)).toBeTruthy();
});

Expand All @@ -81,7 +84,8 @@ describe("SlopDuplicateTrendCard", () => {
/Queue-health snapshot history will appear here after signal snapshot jobs run/i,
),
).toBeTruthy();
expect(screen.queryByLabelText("Trend chart")).toBeNull();
expect(screen.queryByLabelText("Slop flag rate trend")).toBeNull();
expect(screen.queryByLabelText("Duplicate flag rate trend")).toBeNull();
// The header action slot (freshness pill + generated-at stamp) renders in every state (#6175).
expect(screen.getByText("fresh snapshot")).toBeTruthy();
});
Expand Down Expand Up @@ -193,3 +197,26 @@ describe("latestWeekWithSignal", () => {
expect(latestWeekWithSignal(weeks, "duplicate")?.weekStart).toBe("2026-06-02");
});
});

describe("chartValuesForSeries", () => {
const week = (
weekStart: string,
slopFlagRatePct: number | null,
duplicateFlagRatePct: number | null,
): SlopDuplicateTrendWeek => ({
weekStart,
slopFlagRatePct,
slopBandLabel: slopFlagRatePct === null ? null : "low",
duplicateFlagRatePct,
});

it("preserves a null slop rate as null instead of substituting 0", () => {
const weeks = [week("2026-06-02", null, 25), week("2026-06-09", 10, 20)];
expect(chartValuesForSeries(weeks, "slop")).toEqual([null, 10]);
});

it("preserves a null duplicate rate as null instead of substituting 0", () => {
const weeks = [week("2026-06-02", 40, null), week("2026-06-09", 30, 20)];
expect(chartValuesForSeries(weeks, "duplicate")).toEqual([null, 20]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export function SlopDuplicateTrendCard({ trend }: { trend: MaintainerSlopDuplica
values={chartValuesForSeries(trend.weeks, "slop")}
stroke="var(--mint)"
fill="color-mix(in oklab, var(--mint) 18%, transparent)"
label="Slop flag rate trend"
/>
<TrendPanel
title="Duplicate flag rate"
Expand All @@ -89,6 +90,7 @@ export function SlopDuplicateTrendCard({ trend }: { trend: MaintainerSlopDuplica
values={chartValuesForSeries(trend.weeks, "duplicate")}
stroke="var(--warning)"
fill="color-mix(in oklab, var(--warning) 18%, transparent)"
label="Duplicate flag rate trend"
/>
</div>

Expand All @@ -104,20 +106,29 @@ function TrendPanel({
values,
stroke,
fill,
label,
}: {
title: string;
emptyMessage: string;
hasSignal: boolean;
values: number[];
values: ReadonlyArray<number | null>;
stroke: string;
fill: string;
label: string;
}) {
return (
<div className="rounded-token border border-border bg-background/40 p-3">
<div className="text-token-xs font-medium text-foreground">{title}</div>
{hasSignal ? (
<div className="mt-2 h-20">
<TrendChart values={values} stroke={stroke} fill={fill} height={80} showAxis />
<TrendChart
values={values}
stroke={stroke}
fill={fill}
height={80}
showAxis
label={label}
/>
</div>
) : (
<p className="mt-2 text-token-xs text-muted-foreground">{emptyMessage}</p>
Expand Down
85 changes: 85 additions & 0 deletions apps/loopover-ui/src/components/site/trend-chart.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { render, screen } from "@testing-library/react";
import { describe, expect, it } from "vitest";

import { TrendChart } from "@/components/site/trend-chart";

function getPaths(container: HTMLElement) {
const paths = container.querySelectorAll("path");
return { area: paths[0]?.getAttribute("d") ?? "", line: paths[1]?.getAttribute("d") ?? "" };
}

describe("TrendChart", () => {
// The named regression test for this bug (#9674): a null between two numeric points must break
// the polyline into two separate `M`-started sub-paths, never an `L` segment drawn through it.
it("breaks [10, null, 30] into two sub-paths instead of interpolating through the gap", () => {
const { container } = render(<TrendChart values={[10, null, 30]} label="Test trend" />);
const { line } = getPaths(container);
expect(line).toBe("M 0.0 52.0 M 120.0 4.0");
});

it("closes a filled area per contiguous run, not once across the whole series", () => {
const { container } = render(<TrendChart values={[10, null, 30]} label="Test trend" />);
const { area } = getPaths(container);
expect((area.match(/M /g) ?? []).length).toBe(2);
expect((area.match(/Z/g) ?? []).length).toBe(2);
});

it("renders empty d and area attributes for an all-null series", () => {
const { container } = render(<TrendChart values={[null, null]} label="Test trend" />);
const { area, line } = getPaths(container);
expect(line).toBe("");
expect(area).toBe("");
});

it("renders empty d and area attributes for an empty series", () => {
const { container } = render(<TrendChart values={[]} label="Test trend" />);
const { area, line } = getPaths(container);
expect(line).toBe("");
expect(area).toBe("");
});

it("starts a single sub-path after a leading null", () => {
const { container } = render(<TrendChart values={[null, 10, 20]} label="Test trend" />);
const { line } = getPaths(container);
expect((line.match(/M /g) ?? []).length).toBe(1);
expect(line).not.toContain("NaN");
});

it("ends a single sub-path before a trailing null", () => {
const { container } = render(<TrendChart values={[10, 20, null]} label="Test trend" />);
const { line } = getPaths(container);
expect((line.match(/M /g) ?? []).length).toBe(1);
expect(line).not.toContain("NaN");
});

it("renders one continuous sub-path when no null is present", () => {
const { container } = render(<TrendChart values={[10, 20, 30]} label="Test trend" />);
const { line } = getPaths(container);
expect((line.match(/M /g) ?? []).length).toBe(1);
expect((line.match(/L /g) ?? []).length).toBe(2);
});

it("excludes null entries from the min/max/range computation", () => {
// If a null were coerced into the range computation (e.g. via `?? 0`), the numeric points here
// would be plotted relative to a fabricated 0 floor instead of their own 10..30 span.
const withGap = getPaths(
render(<TrendChart values={[10, null, 30]} label="Test trend" />).container,
).line;
const dense = getPaths(
render(<TrendChart values={[10, 30]} label="Test trend" />).container,
).line;
expect(withGap).toBe("M 0.0 52.0 M 120.0 4.0");
expect(dense).toBe("M 0.0 52.0 L 120.0 4.0");
});

it("renders the required label as the svg's aria-label", () => {
render(<TrendChart values={[1, 2, 3]} label="Kept-rate curve" />);
expect(screen.getByLabelText("Kept-rate curve")).toBeTruthy();
});

it("renders a distinct aria-label for a second instance", () => {
render(<TrendChart values={[4, 5, 6]} label="Slop flag rate trend" />);
expect(screen.getByLabelText("Slop flag rate trend")).toBeTruthy();
expect(screen.queryByLabelText("Trend chart")).toBeNull();
});
});
50 changes: 36 additions & 14 deletions apps/loopover-ui/src/components/site/trend-chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,34 +12,56 @@ export function TrendChart({
stroke = "var(--mint)",
fill = "color-mix(in oklab, var(--mint) 18%, transparent)",
showAxis = false,
label,
}: {
values: number[];
values: ReadonlyArray<number | null>;
className?: string;
height?: number;
stroke?: string;
fill?: string;
showAxis?: boolean;
label: string;
}) {
const { d, area, w } = useMemo(() => {
const w = Math.max(values.length * 14, 120);
if (!values.length) return { d: "", area: "", w };
const max = Math.max(...values, 1);
const min = Math.min(...values, 0);
const numeric = values.filter((v): v is number => v !== null);
if (!numeric.length) return { d: "", area: "", w };
const max = Math.max(...numeric, 1);
const min = Math.min(...numeric, 0);
const range = Math.max(max - min, 1);
const step = w / Math.max(values.length - 1, 1);
const pts = values.map((v, i) => {
// A null breaks the polyline into a fresh sub-path (a new `M`) instead of connecting through it
// -- a fabricated straight line would misrepresent a below-sample-floor gap as real data.
const runs: Array<Array<readonly [number, number]>> = [];
let prevIndex = -1;
values.forEach((v, i) => {
if (v === null) return;
const x = i * step;
const y = height - ((v - min) / range) * (height - 8) - 4;
return [x, y] as const;
if (prevIndex === i - 1 && runs.length) {
runs[runs.length - 1]?.push([x, y]);
} else {
runs.push([[x, y]]);
}
prevIndex = i;
});
const d = pts
.map(([x, y], i) =>
i === 0 ? `M ${x.toFixed(1)} ${y.toFixed(1)}` : `L ${x.toFixed(1)} ${y.toFixed(1)}`,
)
const runPath = (pts: Array<readonly [number, number]>) =>
pts
.map(([x, y], i) =>
i === 0 ? `M ${x.toFixed(1)} ${y.toFixed(1)}` : `L ${x.toFixed(1)} ${y.toFixed(1)}`,
)
.join(" ");
const d = runs.map(runPath).join(" ");
// Each contiguous run closes its own filled region -- a single area across the whole series
// would draw a fill panel spanning the gap the line correctly leaves open.
const area = runs
.map((pts) => {
const first = pts[0];
const last = pts[pts.length - 1];
if (!first || !last) return "";
return `${runPath(pts)} L ${last[0].toFixed(1)} ${height} L ${first[0].toFixed(1)} ${height} Z`;
})
.join(" ");
const last = pts[pts.length - 1];
const first = pts[0];
const area = `${d} L ${last[0].toFixed(1)} ${height} L ${first[0].toFixed(1)} ${height} Z`;
return { d, area, w };
}, [values, height]);

Expand All @@ -50,7 +72,7 @@ export function TrendChart({
className={cn("h-full w-full", className)}
style={{ height }}
role="img"
aria-label="Trend chart"
aria-label={label}
>
{showAxis && (
<line x1={0} x2={w} y1={height - 0.5} y2={height - 0.5} stroke="var(--border)" />
Expand Down
26 changes: 26 additions & 0 deletions apps/loopover-ui/src/routes/app.analytics.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,3 +99,29 @@ describe("ProductAnalytics window preference rebrand migration (#8699)", () => {
expect(window.localStorage.getItem("gittensory.analytics.windowDays")).toBeNull();
});
});

// #9674: TrendChart's aria-label was the fixed string "Trend chart" on every instance -- each
// operational trend signal now passes its own label so screen readers hear which chart is which.
describe("ProductAnalytics operational trend signal chart labels (#9674)", () => {
it("labels each signal's chart with its own metric label, not a shared constant", () => {
useApiResource.mockReturnValue({
status: "ready",
data: {
metrics: [{ label: "Active repos", value: "12", delta: "+2" }],
noiseReduction: [
{ label: "False-positive rate", value: 4, spark: [10, 8, 6] },
{ label: "Repo coverage", value: 92, spark: [80, 85, 92] },
],
},
error: null,
loadedAt: "2026-07-17T00:00:00.000Z",
reload: () => {},
});

render(<ProductAnalytics />);

expect(screen.getByLabelText("False-positive rate trend")).toBeTruthy();
expect(screen.getByLabelText("Repo coverage trend")).toBeTruthy();
expect(screen.queryByLabelText("Trend chart")).toBeNull();
});
});
Loading
Loading