diff --git a/apps/loopover-ui/src/components/site/app-panels/calibration-card-model.ts b/apps/loopover-ui/src/components/site/app-panels/calibration-card-model.ts index de52c023b..c82309f8f 100644 --- a/apps/loopover-ui/src/components/site/app-panels/calibration-card-model.ts +++ b/apps/loopover-ui/src/components/site/app-panels/calibration-card-model.ts @@ -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 { + return bins.map((bin) => (bin.keptRate === null ? null : bin.keptRate * 100)); } export function calibrationStatus(calibration: GateCalibration): { diff --git a/apps/loopover-ui/src/components/site/app-panels/calibration-card.test.tsx b/apps/loopover-ui/src/components/site/app-panels/calibration-card.test.tsx index 6ef3eb69d..9f5fe02c0 100644 --- a/apps/loopover-ui/src/components/site/app-panels/calibration-card.test.tsx +++ b/apps/loopover-ui/src/components/site/app-panels/calibration-card.test.tsx @@ -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", () => { @@ -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(); }); }); diff --git a/apps/loopover-ui/src/components/site/app-panels/calibration-card.tsx b/apps/loopover-ui/src/components/site/app-panels/calibration-card.tsx index fbc2060ec..33c62dc2e 100644 --- a/apps/loopover-ui/src/components/site/app-panels/calibration-card.tsx +++ b/apps/loopover-ui/src/components/site/app-panels/calibration-card.tsx @@ -71,7 +71,12 @@ export function CalibrationCard({ calibration }: { calibration: GateCalibration
- +
diff --git a/apps/loopover-ui/src/components/site/app-panels/slop-duplicate-trend-card-model.ts b/apps/loopover-ui/src/components/site/app-panels/slop-duplicate-trend-card-model.ts index dffdf1352..77b18c24b 100644 --- a/apps/loopover-ui/src/components/site/app-panels/slop-duplicate-trend-card-model.ts +++ b/apps/loopover-ui/src/components/site/app-panels/slop-duplicate-trend-card-model.ts @@ -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 { + return weeks.map((week) => + series === "slop" ? week.slopFlagRatePct : week.duplicateFlagRatePct, + ); } export function seriesHasSignal( diff --git a/apps/loopover-ui/src/components/site/app-panels/slop-duplicate-trend-card.test.tsx b/apps/loopover-ui/src/components/site/app-panels/slop-duplicate-trend-card.test.tsx index 50c1daf24..15d7beaed 100644 --- a/apps/loopover-ui/src/components/site/app-panels/slop-duplicate-trend-card.test.tsx +++ b/apps/loopover-ui/src/components/site/app-panels/slop-duplicate-trend-card.test.tsx @@ -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, @@ -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", () => { @@ -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(); }); @@ -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(); }); @@ -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]); + }); +}); diff --git a/apps/loopover-ui/src/components/site/app-panels/slop-duplicate-trend-card.tsx b/apps/loopover-ui/src/components/site/app-panels/slop-duplicate-trend-card.tsx index f16975bd3..92ad6c31d 100644 --- a/apps/loopover-ui/src/components/site/app-panels/slop-duplicate-trend-card.tsx +++ b/apps/loopover-ui/src/components/site/app-panels/slop-duplicate-trend-card.tsx @@ -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" /> @@ -104,20 +106,29 @@ function TrendPanel({ values, stroke, fill, + label, }: { title: string; emptyMessage: string; hasSignal: boolean; - values: number[]; + values: ReadonlyArray; stroke: string; fill: string; + label: string; }) { return (
{title}
{hasSignal ? (
- +
) : (

{emptyMessage}

diff --git a/apps/loopover-ui/src/components/site/trend-chart.test.tsx b/apps/loopover-ui/src/components/site/trend-chart.test.tsx new file mode 100644 index 000000000..64c436877 --- /dev/null +++ b/apps/loopover-ui/src/components/site/trend-chart.test.tsx @@ -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(); + 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(); + 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(); + 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(); + const { area, line } = getPaths(container); + expect(line).toBe(""); + expect(area).toBe(""); + }); + + it("starts a single sub-path after a leading null", () => { + const { container } = render(); + 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(); + 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(); + 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().container, + ).line; + const dense = getPaths( + render().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(); + expect(screen.getByLabelText("Kept-rate curve")).toBeTruthy(); + }); + + it("renders a distinct aria-label for a second instance", () => { + render(); + expect(screen.getByLabelText("Slop flag rate trend")).toBeTruthy(); + expect(screen.queryByLabelText("Trend chart")).toBeNull(); + }); +}); diff --git a/apps/loopover-ui/src/components/site/trend-chart.tsx b/apps/loopover-ui/src/components/site/trend-chart.tsx index cfe75bb7d..fc2ea342c 100644 --- a/apps/loopover-ui/src/components/site/trend-chart.tsx +++ b/apps/loopover-ui/src/components/site/trend-chart.tsx @@ -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; 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> = []; + 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) => + 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]); @@ -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 && ( diff --git a/apps/loopover-ui/src/routes/app.analytics.test.tsx b/apps/loopover-ui/src/routes/app.analytics.test.tsx index 7b85c7095..ffd1fad52 100644 --- a/apps/loopover-ui/src/routes/app.analytics.test.tsx +++ b/apps/loopover-ui/src/routes/app.analytics.test.tsx @@ -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(); + + expect(screen.getByLabelText("False-positive rate trend")).toBeTruthy(); + expect(screen.getByLabelText("Repo coverage trend")).toBeTruthy(); + expect(screen.queryByLabelText("Trend chart")).toBeNull(); + }); +}); diff --git a/apps/loopover-ui/src/routes/app.analytics.tsx b/apps/loopover-ui/src/routes/app.analytics.tsx index c8513a2f6..b1b0ca72d 100644 --- a/apps/loopover-ui/src/routes/app.analytics.tsx +++ b/apps/loopover-ui/src/routes/app.analytics.tsx @@ -350,7 +350,7 @@ export function ProductAnalytics() { {signal.value}
- +
))} diff --git a/apps/loopover-ui/src/routes/app.index.test.tsx b/apps/loopover-ui/src/routes/app.index.test.tsx index 6f5a2bd4e..ca10719fc 100644 --- a/apps/loopover-ui/src/routes/app.index.test.tsx +++ b/apps/loopover-ui/src/routes/app.index.test.tsx @@ -78,6 +78,19 @@ describe("SparkStat loading state (#6984)", () => { }); }); +// #9674: TrendChart's aria-label was the fixed string "Trend chart" on every instance -- SparkStat +// now passes a per-tile label so each metric's chart announces itself distinctly to a screen reader. +describe("SparkStat trend chart label (#9674)", () => { + it("labels the chart with the tile's own metric label, not a shared constant", () => { + render( + + + , + ); + expect(screen.getByLabelText("Open PRs trend")).toBeTruthy(); + }); +}); + // #8668: the overview metrics ErrorState passed only title/description (both fixed strings), never // errorKind or onRetry -- so a network outage always rendered the generic AlertTriangle treatment // with no retry action, even though `overview.errorKind`/`overview.reload` were both already diff --git a/apps/loopover-ui/src/routes/app.index.tsx b/apps/loopover-ui/src/routes/app.index.tsx index 27b8acd83..01d9f5766 100644 --- a/apps/loopover-ui/src/routes/app.index.tsx +++ b/apps/loopover-ui/src/routes/app.index.tsx @@ -580,7 +580,7 @@ export function SparkStat({ {hint &&
{hint}
}
- +