feat(sp27): server-aggregate Dashboard KPIs so 100-row sample doesn't lie
The Dashboard was hardcoded to useClaims({ limit: 100 }) and reduce
KPIs client-side. With 60k+ claims in production, every tile
(Billed $940K, Received $59K, Denial rate, Pending AR, monthly
sparkline, top providers, recent denials) was computed from a
0.16% sample of the dataset — silently wrong numbers on the
operator's primary view.
Fix: add GET /api/dashboard/kpis that aggregates server-side in
one read over the entire claim population. The new useDashboardKpis
hook consumes it and polls every 60s. Dashboard.tsx drops
useClaims({limit:100}) + useProviders() + the client-side
buildMonthly reduce.
Backend:
- store.py: dashboard_kpis() — one Claim query (selectinload on
batch to avoid N+1) + one bulk Remittance lookup, Python reduce
over the full population. Zero-filled response for empty DB.
- api.py: GET /api/dashboard/kpis behind matrix_gate, query-param
clamps (1..24 months, 0..50 top_n_*).
Frontend:
- api.ts: DashboardKpis types + getDashboardKpis() wrapper.
- useDashboardKpis.ts: TanStack Query hook, 60s refetchInterval,
bypass to data:undefined when not configured.
- Dashboard.tsx: switched to useDashboardKpis, extracted
ZERO_TOTALS constant, dropped the buildMonthly helper.
Tests:
- backend/tests/test_dashboard_kpis.py: 12 tests covering empty DB,
matched-remit math, pending-state semantics, monthly binning,
top-providers/top-denials sort + cap, orphan-claim defensive
guard, HTTP wiring + param validation.
- src/hooks/useDashboardKpis.test.ts: 3 tests for the hook
contract (configured path, unconfigured fallback, param
passthrough).
- src/pages/Dashboard.test.tsx: wrapped renders in
QueryClientProvider + stubbed useAuth + isConfigured=false. This
fixes 3 pre-existing Dashboard test failures (the page never had
a QueryClient set up because useClaims/useProviders were the
first useQuery hooks in the page).
Reviewer fixes (same commit):
1. topDenials sort placed empty submissionDate claims first under
reverse-lex. Drop them at append time.
2. r.batch lazy-load → N+1 on 60k rows. selectinload(Claim.batch).
3. pending_states rebuilt per call as a mutable set — moved to
module-level _DASHBOARD_PENDING_STATES frozenset.
4. Module-level ProviderORM import inconsistent with the
"local import inside the function" pattern — moved inline.
This commit is contained in:
@@ -16,6 +16,7 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { cleanup, fireEvent, render } from "@testing-library/react";
|
||||
import { MemoryRouter } from "react-router-dom";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import { Dashboard } from "./Dashboard";
|
||||
import { useAppStore } from "@/store";
|
||||
import type { Activity } from "@/types";
|
||||
@@ -31,6 +32,34 @@ vi.mock("sonner", () => ({
|
||||
},
|
||||
}));
|
||||
|
||||
// Stub useAuth so the Dashboard's greeting renders without spinning
|
||||
// up the full AuthProvider + /api/auth/me probe. Same pattern as
|
||||
// Inbox.test.tsx.
|
||||
vi.mock("@/auth/useAuth", () => ({
|
||||
useAuth: () => ({
|
||||
status: "authenticated" as const,
|
||||
user: { username: "tester" } as unknown as never,
|
||||
login: vi.fn(),
|
||||
logout: vi.fn(),
|
||||
refresh: vi.fn(),
|
||||
}),
|
||||
}));
|
||||
|
||||
// SP27 Task 13: stub `api.isConfigured = false` so `useDashboardKpis`
|
||||
// + `useActivity` both take their in-memory zustand fallback path.
|
||||
// These tests focus on activity-feed event routing — they don't
|
||||
// assert on KPI math, so zero-filled KPIs from the fallback are fine.
|
||||
vi.mock("@/lib/api", async () => {
|
||||
const actual = await vi.importActual<typeof import("@/lib/api")>("@/lib/api");
|
||||
return {
|
||||
...actual,
|
||||
api: {
|
||||
...actual.api,
|
||||
isConfigured: false,
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
// Capture navigation side effects so we can assert on the URL the
|
||||
// Dashboard would push. We use a `MemoryRouter` (initialEntries=["/"])
|
||||
// and observe the rendered route via a tiny listener component that
|
||||
@@ -49,6 +78,21 @@ function LocationProbe() {
|
||||
);
|
||||
}
|
||||
|
||||
// SP27 Task 13: Dashboard now reads KPIs from `useDashboardKpis`,
|
||||
// which uses TanStack Query internally. Wrap each render in a
|
||||
// QueryClientProvider so the hook doesn't throw — these tests focus
|
||||
// on activity-feed event routing, so we never resolve the KPI query.
|
||||
function renderWithQuery(ui: React.ReactNode) {
|
||||
const client = new QueryClient({
|
||||
defaultOptions: {
|
||||
queries: { retry: false, gcTime: 0, staleTime: 0 },
|
||||
},
|
||||
});
|
||||
return render(
|
||||
<QueryClientProvider client={client}>{ui}</QueryClientProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.clearAllMocks();
|
||||
@@ -70,7 +114,7 @@ describe("Dashboard · Recent activity event routing (SP21 Task 2.5)", () => {
|
||||
];
|
||||
useAppStore.setState({ activity });
|
||||
|
||||
const { getByTestId, getByRole } = render(
|
||||
const { getByTestId, getByRole } = renderWithQuery(
|
||||
<MemoryRouter initialEntries={["/"]}>
|
||||
<Dashboard />
|
||||
<LocationProbe />
|
||||
@@ -101,7 +145,7 @@ describe("Dashboard · Recent activity event routing (SP21 Task 2.5)", () => {
|
||||
];
|
||||
useAppStore.setState({ activity });
|
||||
|
||||
const { getByTestId, getByRole } = render(
|
||||
const { getByTestId, getByRole } = renderWithQuery(
|
||||
<MemoryRouter initialEntries={["/"]}>
|
||||
<Dashboard />
|
||||
<LocationProbe />
|
||||
@@ -129,7 +173,7 @@ describe("Dashboard · Recent activity event routing (SP21 Task 2.5)", () => {
|
||||
];
|
||||
useAppStore.setState({ activity });
|
||||
|
||||
const { getByTestId, getByRole } = render(
|
||||
const { getByTestId, getByRole } = renderWithQuery(
|
||||
<MemoryRouter initialEntries={["/"]}>
|
||||
<Dashboard />
|
||||
<LocationProbe />
|
||||
|
||||
+52
-88
@@ -18,71 +18,66 @@ import { DrillableCell } from "@/components/drill/DrillableCell";
|
||||
import { fmt } from "@/lib/format";
|
||||
import { eventKindToUrl } from "@/lib/event-routing";
|
||||
import { useAuth } from "@/auth/useAuth";
|
||||
import { useClaims } from "@/hooks/useClaims";
|
||||
import { useProviders } from "@/hooks/useProviders";
|
||||
import { useDashboardKpis } from "@/hooks/useDashboardKpis";
|
||||
import { useActivity } from "@/hooks/useActivity";
|
||||
import type { Claim } from "@/types";
|
||||
import { toast } from "sonner";
|
||||
|
||||
const MONTHS_BACK = 6;
|
||||
|
||||
function buildMonthly(claims: Claim[]) {
|
||||
const now = new Date();
|
||||
const months: {
|
||||
key: string;
|
||||
label: string;
|
||||
count: number;
|
||||
billed: number;
|
||||
received: number;
|
||||
denied: number;
|
||||
}[] = [];
|
||||
for (let i = MONTHS_BACK - 1; i >= 0; i--) {
|
||||
const d = new Date(now.getFullYear(), now.getMonth() - i, 1);
|
||||
months.push({
|
||||
key: `${d.getFullYear()}-${d.getMonth()}`,
|
||||
label: d.toLocaleString("en-US", { month: "short" }),
|
||||
count: 0,
|
||||
billed: 0,
|
||||
received: 0,
|
||||
denied: 0,
|
||||
});
|
||||
}
|
||||
const index = new Map(months.map((m, i) => [m.key, i]));
|
||||
for (const c of claims) {
|
||||
const d = new Date(c.submissionDate);
|
||||
const k = `${d.getFullYear()}-${d.getMonth()}`;
|
||||
const i = index.get(k);
|
||||
if (i === undefined) continue;
|
||||
months[i]!.count += 1;
|
||||
months[i]!.billed += c.billedAmount;
|
||||
months[i]!.received += c.receivedAmount;
|
||||
if (c.status === "denied") months[i]!.denied += 1;
|
||||
}
|
||||
let running = 0;
|
||||
const ar: number[] = [];
|
||||
for (const m of months) {
|
||||
running += m.billed - m.received;
|
||||
ar.push(Math.max(0, running));
|
||||
}
|
||||
return {
|
||||
count: months.map((m) => m.count),
|
||||
billed: months.map((m) => m.billed),
|
||||
received: months.map((m) => m.received),
|
||||
ar,
|
||||
denialRate: months.map((m) => (m.count ? (m.denied / m.count) * 100 : 0)),
|
||||
};
|
||||
}
|
||||
// Zero-shaped KPI totals used when the server response hasn't arrived
|
||||
// yet or the backend isn't configured. Mirrors the empty-DB shape of
|
||||
// `GET /api/dashboard/kpis` so the KPI tiles render a coherent "0"
|
||||
// instead of `undefined` during the first paint and background
|
||||
// refetches. Keeping it next to ``MONTHS_BACK`` means a future tile
|
||||
// can be added in one place rather than chasing the fallback object.
|
||||
const ZERO_TOTALS = {
|
||||
count: 0,
|
||||
billed: 0,
|
||||
received: 0,
|
||||
outstandingAr: 0,
|
||||
denied: 0,
|
||||
denialRate: 0,
|
||||
pending: 0,
|
||||
};
|
||||
|
||||
export function Dashboard() {
|
||||
// Live data: hooks fetch from /api/* when api.isConfigured; otherwise
|
||||
// they fall back to the in-memory store. Pulling from the hooks (not
|
||||
// the store directly) is what wires the Dashboard to the backend.
|
||||
const claimsQuery = useClaims({ limit: 100 });
|
||||
const providersQuery = useProviders();
|
||||
//
|
||||
// SP27 Task 13: KPIs come from the dedicated ``/api/dashboard/kpis``
|
||||
// server-side aggregate, NOT from a paginated ``/api/claims?limit=100``
|
||||
// reduced client-side. With 60k+ claims in production, the old
|
||||
// pattern silently produced wrong numbers — every Dashboard tile was
|
||||
// computed from a 100-row sample. The new endpoint does the reduce
|
||||
// in SQL once over the full population.
|
||||
const kpisQuery = useDashboardKpis({ months: MONTHS_BACK });
|
||||
const activityQuery = useActivity({ limit: 10 });
|
||||
|
||||
const claims = claimsQuery.data?.items ?? [];
|
||||
const providers = providersQuery.data?.items ?? [];
|
||||
const kpisData = kpisQuery.data;
|
||||
// Fall back to a zero-shaped object so the KpiTiles still render a
|
||||
// coherent "0" rather than NaN during the first paint and during
|
||||
// background refetches. See ``ZERO_TOTALS`` for why this is a
|
||||
// module-level constant.
|
||||
const kpis = kpisData?.totals ?? ZERO_TOTALS;
|
||||
const monthly = useMemo(() => {
|
||||
const series = kpisData?.monthly ?? [];
|
||||
return {
|
||||
count: series.map((m) => m.count),
|
||||
billed: series.map((m) => m.billed),
|
||||
received: series.map((m) => m.received),
|
||||
ar: series.map((m) => m.ar),
|
||||
denialRate: series.map((m) => m.denialRate),
|
||||
};
|
||||
}, [kpisData]);
|
||||
const topProviders = useMemo(
|
||||
() => kpisData?.topProviders ?? [],
|
||||
[kpisData]
|
||||
);
|
||||
const topDenials = useMemo(
|
||||
() => kpisData?.topDenials ?? [],
|
||||
[kpisData]
|
||||
);
|
||||
const activity = activityQuery.data?.items ?? [];
|
||||
|
||||
const navigate = useNavigate();
|
||||
@@ -96,37 +91,6 @@ export function Dashboard() {
|
||||
})();
|
||||
const operatorName = auth.user?.username ?? "there";
|
||||
|
||||
const kpis = useMemo(() => {
|
||||
const billed = claims.reduce((s, c) => s + c.billedAmount, 0);
|
||||
const received = claims.reduce((s, c) => s + c.receivedAmount, 0);
|
||||
const outstandingAr = billed - received;
|
||||
const denied = claims.filter((c) => c.status === "denied").length;
|
||||
const denialRate = claims.length > 0 ? (denied / claims.length) * 100 : 0;
|
||||
const pending = claims.filter(
|
||||
(c) => c.status === "submitted" || c.status === "pending"
|
||||
).length;
|
||||
return {
|
||||
count: claims.length,
|
||||
billed,
|
||||
received,
|
||||
outstandingAr,
|
||||
denialRate,
|
||||
pending,
|
||||
};
|
||||
}, [claims]);
|
||||
|
||||
const monthly = useMemo(() => buildMonthly(claims), [claims]);
|
||||
|
||||
const topProviders = useMemo(
|
||||
() => [...providers].sort((a, b) => b.claimCount - a.claimCount).slice(0, 4),
|
||||
[providers]
|
||||
);
|
||||
|
||||
const topDenials = useMemo(
|
||||
() => claims.filter((c) => c.status === "denied").slice(0, 5),
|
||||
[claims]
|
||||
);
|
||||
|
||||
// Stagger choreography — the hero lands first, then the KPIs in
|
||||
// a left-to-right wave, then the supporting cards. Total
|
||||
// choreography fits under 700ms.
|
||||
@@ -345,14 +309,14 @@ export function Dashboard() {
|
||||
}}
|
||||
role="button"
|
||||
tabIndex={0}
|
||||
aria-label={`View provider ${p.name}`}
|
||||
aria-label={`View provider ${p.label || p.npi}`}
|
||||
className="drillable flex items-center gap-3"
|
||||
>
|
||||
<div className="h-7 w-7 rounded-md bg-muted/60 ring-1 ring-inset ring-border/40 flex items-center justify-center mono text-[10.5px] text-muted-foreground">
|
||||
{String(i + 1).padStart(2, "0")}
|
||||
</div>
|
||||
<div className="flex-1 min-w-0">
|
||||
<div className="text-[13px] font-medium truncate">{p.name}</div>
|
||||
<div className="text-[13px] font-medium truncate">{p.label || p.npi}</div>
|
||||
<div className="mono text-[10.5px] text-muted-foreground">
|
||||
NPI {p.npi}
|
||||
</div>
|
||||
@@ -360,7 +324,7 @@ export function Dashboard() {
|
||||
<div className="text-right">
|
||||
<div className="display mono text-[15px]">{fmt.num(p.claimCount)}</div>
|
||||
<div className="mono text-[10.5px] text-muted-foreground">
|
||||
{fmt.usd(p.outstandingAr)} AR
|
||||
{fmt.usd(p.billed)} billed
|
||||
</div>
|
||||
</div>
|
||||
</li>
|
||||
@@ -416,7 +380,7 @@ export function Dashboard() {
|
||||
{fmt.usd(c.billedAmount)}
|
||||
</div>
|
||||
<div className="text-[10.5px] text-muted-foreground">
|
||||
{c.payerName}
|
||||
{fmt.date(c.submissionDate)}
|
||||
</div>
|
||||
</div>
|
||||
</li>
|
||||
|
||||
Reference in New Issue
Block a user