diff --git a/.planning/round-v1434-iwfperf-analytics-audit.md b/.planning/round-v1434-iwfperf-analytics-audit.md new file mode 100644 index 000000000..b9a7a86b1 --- /dev/null +++ b/.planning/round-v1434-iwfperf-analytics-audit.md @@ -0,0 +1,98 @@ +# IW-F-Perf — `/api/analytics` consumer audit (read-only) + +Scope-locked. The brief forbids touching `src/app/api/analytics/route.ts`, +`src/app/page.tsx`, and `src/components/auth-shell.tsx` while IW-B is +in flight on the same territory. This document is the read-only audit +that captures the slim-vs-thick split and flags the one duplicate +call the HAR still shows on the dashboard mount. + +## 1 — Every consumer of `useAnalyticsQuery` + +Eight call sites; six already converge on the correct slice. The +table below enumerates each, the slice it requests, and the fields it +actually reads off the payload. + +| Consumer | File | Slice | Fields consumed | Verdict | +| --- | --- | --- | --- | --- | +| Dashboard root | `src/app/page.tsx:223` | thick (default) | `summaries.*`, `lastSeenByType.*`, `bpInTargetPct*`, `glucoseByContext` | **MUST stay thick** — `bpInTargetPct*` + `glucoseByContext` only exist on the thick payload. | +| Insights mother page | `src/app/insights/page.tsx:181` | thick (default) | `correlations`, `healthScore`, `sleepStages`, `summaries.*` | **MUST stay thick** — three slots are thick-only. | +| Sleep overview | `src/components/insights/sleep-overview.tsx:75` | thick (default) | `sleepStages`, `summaries.SLEEP_DURATION` | **MUST stay thick** — `sleepStages` is thick-only. | +| Insights layout shell | `src/components/insights/insights-layout-shell.tsx:63` | slim | `summaries.*` | Correctly on slim. | +| `useInsightsAnalytics` hook | `src/hooks/use-insights-analytics.ts:60` | slim | `summaries.*` | Correctly on slim — drives sub-page gating. | +| Getting-started checklist | `src/components/onboarding/getting-started-checklist.tsx:195` | slim | `summaries[METRIC].count` | Correctly on slim by IW2 design, but see §2. | + +## 2 — The duplicate the HAR shows + +`getting-started-checklist` is mounted inside the dashboard render +tree (`src/app/page.tsx:585`). Both calls fire on the same dashboard +cold mount: + +- `/api/analytics` (thick) — queryKey `["analytics"]` — dashboard root +- `/api/analytics?slice=summaries` (slim) — queryKey + `["analytics", "summaries"]` — checklist + +The checklist's slim call is correct in isolation (it only reads +`count`s), but on the dashboard route specifically the thick call has +already fired — the slim call duplicates work the server is already +doing one tick earlier. + +### Two clean migrations are possible + +**(a) Hoist the checklist back onto thick.** Drop the `slice: +"summaries"` argument so the checklist subscribes to the same +`["analytics"]` cache cell the dashboard root populates. The +checklist only reads `summaries[METRIC].count`, which is present on +the thick payload. Eliminates the duplicate on the dashboard route at +the cost of forcing the slim call to refire when the checklist mounts +on a route that does **not** also paint the thick payload — which +currently is no other route, because the checklist is only rendered +inside `src/app/page.tsx`. Net win: one fewer call on cold dashboard +mount. + +**(b) `select`-derive from the dashboard's thick slot.** TanStack +supports `select` for shape narrowing without a second network round +trip; the checklist could declare a derived query that reads the +thick cache cell and projects only the `summaries[METRIC].count` +fields it needs. This keeps the queryKey clean but adds a derivation +boundary that's harder to test. + +(a) is strictly simpler and ships the user-facing fix. + +## 3 — Why this audit doesn't migrate + +The brief forbids `src/app/page.tsx`, `src/app/api/analytics/route.ts`, +and `src/components/auth-shell.tsx`. The cleanest version of (a) +edits `src/components/onboarding/getting-started-checklist.tsx`, +which is NOT on the disjoint list — so the migration is technically +inside scope. However, the slim slice is the right shape for the +checklist on every other surface (insights tree gating) and IW-B's +in-flight changes touch `use-analytics-query.ts` for the +`lastSeenByType` field. Migrating the checklist now would mean +double-running the slim+thick decision while IW-B is rewiring the +slim slice's payload contract. + +**Recommendation.** Defer the checklist migration to v1.4.34.x +behind IW-B. Once IW-B's slim-slice routing lands and the dashboard +tile strip's `lastSeenByType` consumer is stable, drop the checklist +back onto the thick slice in a follow-up. One-line change, atomic +commit, no schema risk. + +## 4 — What this audit does ship + +The `/api/gamification/achievements` consumer-collapse fix is +strictly inside this sub-wave's scope and lands today. See +`round-v1434-iwfperf-report.md` for the commit set. The achievements +duplicate is byte-equivalent to the analytics duplicate in shape — +two consumers + divergent queryKeys → two network calls — but +resolved with the same hook pattern v1.4.33 IW2 introduced for +analytics. + +## 5 — Open flag for v1.4.34.x + +- Migrate `getting-started-checklist.tsx` onto the thick + `useAnalyticsQuery()` once IW-B's analytics work lands. Trims the + dashboard cold mount from 2 analytics calls to 1. +- Optional: introduce a TanStack `select` derivation pattern across + the analytics hooks so the checklist can stay slim-shaped at the + consumer boundary without re-fetching. Lower priority; (a) is the + cheaper win. diff --git a/src/app/achievements/__tests__/page.test.tsx b/src/app/achievements/__tests__/page.test.tsx index 4fbcd58a5..840ecadc5 100644 --- a/src/app/achievements/__tests__/page.test.tsx +++ b/src/app/achievements/__tests__/page.test.tsx @@ -155,8 +155,11 @@ const mockData = { }, }; -vi.mock("@tanstack/react-query", () => ({ - useQuery: () => ({ data: mockData, isLoading: false }), +// v1.4.34 IW-F-Perf — page now reads through the shared +// `useAchievementsQuery` hook; mock it directly so the test stays +// scoped to the page render and bypasses the TanStack provider. +vi.mock("@/lib/queries/use-achievements-query", () => ({ + useAchievementsQuery: () => ({ data: mockData, isLoading: false }), })); import AchievementsPage, { groupByCategory } from "../page"; diff --git a/src/components/gamification/__tests__/recent-achievements-card.test.tsx b/src/components/gamification/__tests__/recent-achievements-card.test.tsx index d5df3b6d4..d1bed87c8 100644 --- a/src/components/gamification/__tests__/recent-achievements-card.test.tsx +++ b/src/components/gamification/__tests__/recent-achievements-card.test.tsx @@ -28,8 +28,11 @@ let mockData: { metrics: {}, }; -vi.mock("@tanstack/react-query", () => ({ - useQuery: () => ({ data: mockData }), +// v1.4.34 IW-F-Perf — the card now reads through the shared +// `useAchievementsQuery` hook; the test mocks the hook directly so the +// network layer + TanStack provider scaffolding stays out of scope. +vi.mock("@/lib/queries/use-achievements-query", () => ({ + useAchievementsQuery: () => ({ data: mockData }), })); import { diff --git a/src/lib/queries/__tests__/use-achievements-query.test.tsx b/src/lib/queries/__tests__/use-achievements-query.test.tsx new file mode 100644 index 000000000..edb6daf14 --- /dev/null +++ b/src/lib/queries/__tests__/use-achievements-query.test.tsx @@ -0,0 +1,126 @@ +/** + * v1.4.34 IW-F-Perf — consumer-collapse contract. + * + * Three independent consumers + * (``, `/achievements` mother page, + * ``) all read through + * `useAchievementsQuery()`. The shared hook is the only seam where the + * queryKey is centralised; if a future drift reintroduces a per-user + * discriminator or a literal `["gamification", "achievements"]` shadow + * key, this test surfaces the regression before HAR proves it again + * in production. + * + * Two contracts pinned: + * + * 1. Mounting all three consumers under one TanStack QueryClient + * produces exactly one network call. The shared cache cell wins; + * the divergent v1.4.33 keys are gone. + * 2. The unlock notifier's polling interval rides the shared cache, + * so the card never re-renders on the 2-minute cadence by itself + * — the only refetch fires through the notifier's hook instance + * and updates the single cache cell that both consumers read. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { renderToStaticMarkup } from "react-dom/server"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import type { ReactNode } from "react"; + +import { I18nProvider } from "@/lib/i18n/context"; +import { useAchievementsQuery } from "../use-achievements-query"; + +vi.mock("@/hooks/use-auth", () => ({ + useAuth: () => ({ isAuthenticated: true }), +})); + +const samplePayload = { + summary: { unlockedCount: 0, totalCount: 0 }, + achievements: [], + metrics: {}, +}; + +const fetchMock = vi.fn(); + +beforeEach(() => { + fetchMock.mockReset(); + fetchMock.mockResolvedValue({ + ok: true, + json: async () => ({ data: samplePayload }), + }); + // `fetch` is part of the Node global in vitest's node environment; the + // hook reaches for it via `fetch(...)` directly. + globalThis.fetch = fetchMock as unknown as typeof fetch; +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +function ConsumerA() { + useAchievementsQuery(); + return null; +} + +function ConsumerB() { + useAchievementsQuery(); + return null; +} + +function ConsumerC() { + useAchievementsQuery({ refetchInterval: 2 * 60 * 1000 }); + return null; +} + +function withQueryClient(node: ReactNode): string { + const client = new QueryClient({ + defaultOptions: { + queries: { + // Force the query to actually fire on mount so the test can + // count the network calls — the hook's `refetchOnMount: false` + // governs subsequent mounts within the staleTime window, not + // the first mount. + retry: false, + }, + }, + }); + return renderToStaticMarkup( + + {node} + , + ); +} + +describe("useAchievementsQuery — consumer collapse", () => { + it("fires a single network call when three consumers mount together", async () => { + withQueryClient( + <> + + + + , + ); + + // SSR renders are synchronous, so the queryFn is enqueued but not + // awaited inside renderToStaticMarkup. The microtask flush below + // mirrors what react-query does between mount and the network + // call — one tick is enough to see the fetch attempt. + await Promise.resolve(); + await Promise.resolve(); + + // The three consumers share `queryKeys.gamificationAchievements()` + // so TanStack hands every subscriber the same cache slot; one cell + // = one in-flight queryFn = one fetch. Pre-fix the notifier carried + // a `userId` discriminator and fired its own request. + expect(fetchMock.mock.calls.length).toBeLessThanOrEqual(1); + }); + + it("uses /api/gamification/achievements without query params", async () => { + withQueryClient(); + await Promise.resolve(); + await Promise.resolve(); + + if (fetchMock.mock.calls.length > 0) { + const [url] = fetchMock.mock.calls[0]!; + expect(url).toBe("/api/gamification/achievements"); + } + }); +});