fix(compare): show SSR data on refresh; drop dead per-page comparison fetch

Refresh bug: on mount the basket is empty for a beat before it hydrates
from the URL. The fetch effect nulled comparisonData on that transient
empty urnKey, then the one-shot 'SSR covers it' skip suppressed the
refetch — leaving the page blank on reload. The effect is now gated on
isInitialized, never blanks on empty (the render already shows the empty
state when nothing is selected), and decides fetch-vs-skip by whether it
already holds each requested school's data (SSR or a prior fetch).

Perf: useComparison ran a useSWR('/api/compare') whose result nothing
consumed — dead weight that fired on every page (Navigation + Toast are
global) whenever the basket was non-empty, and duplicated ComparisonView's
own fetch on the compare page. Removed; the hook now exposes basket state
only.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB

nextjs-app/__tests__/components/ComparisonView.refresh.test.tsx

@@ -0,0 +1,82 @@
+/**
+ * Regression: on refresh, the compare page must show the SSR-rendered data.
+ *
+ * The basket hydrates from the URL a beat after mount (selectedSchools is
+ * empty for the first render), so the fetch effect must not blank the
+ * SSR payload during that window — and must not refetch data the server
+ * already provided.
+ */
+
+import { render, screen, waitFor } from '@testing-library/react';
+
+import { ComparisonView } from '@/components/ComparisonView';
+import { ComparisonProvider } from '@/context/ComparisonProvider';
+import type { ComparisonData, School } from '@/lib/types';
+
+const fetchComparison = jest.fn();
+jest.mock('@/lib/api', () => ({
+  fetchComparison: (...args: unknown[]) => fetchComparison(...args),
+}));
+jest.mock('@/lib/analytics', () => ({ track: jest.fn() }));
+
+function school(urn: number, name: string): School {
+  return {
+    urn,
+    school_name: name,
+    local_authority: 'Testshire',
+    school_type: 'Community school',
+    rwm_expected_pct: 80,
+    phase: 'Primary',
+  } as School;
+}
+
+function data(urn: number, name: string): ComparisonData {
+  return {
+    school_info: school(urn, name),
+    yearly_data: [{ year: 202425, rwm_expected_pct: 80 }] as ComparisonData['yearly_data'],
+    ofsted: null,
+    census: null,
+    admissions: null,
+    admissions_history: [],
+    deprivation: null,
+  };
+}
+
+const INITIAL_DATA = {
+  '100': data(100, 'Alpha Primary'),
+  '200': data(200, 'Beta Primary'),
+};
+
+beforeEach(() => {
+  fetchComparison.mockReset();
+});
+
+test('renders SSR data on refresh without wiping it or refetching', async () => {
+  render(
+    <ComparisonProvider>
+      <ComparisonView
+        initialData={INITIAL_DATA}
+        initialNationalAverages={{
+          year: 202425,
+          primary: { rwm_expected_pct: 62 },
+          secondary: {},
+          by_year: [],
+        }}
+        initialBenchmarks={undefined}
+        initialUrns={[100, 200]}
+        metrics={[]}
+        selectedMetric="rwm_expected_pct"
+      />
+    </ComparisonProvider>,
+  );
+
+  // Both SSR-provided schools appear (data was not blanked during hydration)
+  await waitFor(() => {
+    expect(screen.getAllByText('Alpha Primary').length).toBeGreaterThan(0);
+  });
+  expect(screen.getAllByText('Beta Primary').length).toBeGreaterThan(0);
+  expect(screen.getByRole('heading', { name: 'At a glance' })).toBeInTheDocument();
+
+  // …and the client never refetched data the server already rendered.
+  expect(fetchComparison).not.toHaveBeenCalled();
+});

nextjs-app/components/ComparisonView.tsx

@@ -107,24 +107,26 @@ export function ComparisonView({
     router.replace(newUrl, { scroll: false });
   }, [urnKey, selectedMetric, pathname, searchParams, router]);
 
-  // Fetch only when the school set changes. The very first run is skipped
+  // Fetch when the school set changes, but only for schools we don't already
-  // when the SSR payload already covers the current set — no double-fetch
+  // have data for. This skips the refetch of SSR-rendered data on load AND
-  // of data the server just rendered.
+  // avoids a network call when a school is merely removed. A ref holds the
-  const firstFetchRef = useRef(true);
+  // latest data so the effect can read it without re-running on every fetch.
-  useEffect(() => {
+  //
-    if (!urnKey) {
+  // Correctness note: we must NOT null the data on a transient empty urnKey.
-      setComparisonData(null);
+  // On mount the basket is empty for a beat before it hydrates from the URL,
-      setNationalAverages(undefined);
+  // and blanking here (then skipping the refetch because SSR "covers" the set)
-      setBenchmarks(undefined);
+  // was leaving the page empty on refresh. The render already shows the empty
-      return;
+  // state whenever `selectedSchools` is empty, so stale data for deselected
-    }
+  // schools is harmless — it's simply unused.
+  const comparisonDataRef = useRef(comparisonData);
+  comparisonDataRef.current = comparisonData;
 
-    if (firstFetchRef.current) {
+  useEffect(() => {
-      firstFetchRef.current = false;
+    if (!isInitialized || !urnKey) return;
-      const ssrUrns = new Set(Object.keys(initialData ?? {}));
+
-      const covered = urnKey.split(',').every((urn) => ssrUrns.has(urn));
+    const have = comparisonDataRef.current ?? {};
-      if (covered && ssrUrns.size > 0) return;
+    const covered = urnKey.split(',').every((urn) => have[urn] != null);
-    }
+    if (covered) return;
 
     fetchComparison(urnKey, { cache: 'no-store' })
       .then((data) => {
@@ -138,8 +140,7 @@ export function ComparisonView({
         // destroy a working comparison the user is looking at.
         console.error('Failed to fetch comparison:', err);
       });
-    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [urnKey, isInitialized]);
-  }, [urnKey]);
 
   // Classify schools by phase using comparison data
   const classifySchool = (school: School): 'primary' | 'secondary' => {

nextjs-app/hooks/useComparison.ts

@@ -1,50 +1,18 @@
 /**
- * Custom hook for managing school comparison state
+ * Custom hook for managing school comparison state.
- * Uses shared context for real-time updates across components
+ *
+ * This hook is mounted on every page via the global Navigation and
+ * ComparisonToast, so it must stay cheap — it exposes basket state only.
+ * The compare page fetches `/api/compare` itself (ComparisonView); nothing
+ * ever read the comparison payload from here, so the previous per-page SWR
+ * fetch (which fired on every page whenever the basket was non-empty) was
+ * dead weight and has been removed.
  */
 
 'use client';
 
-import useSWR from 'swr';
-import { fetcher } from '@/lib/api';
 import { useComparisonContext } from '@/context/ComparisonContext';
-import type { ComparisonResponse } from '@/lib/types';
 
 export function useComparison() {
-  const {
+  return useComparisonContext();
-    selectedSchools,
-    addSchool,
-    removeSchool,
-    replaceSchools,
-    clearAll,
-    isSelected,
-    canAddMore,
-    isInitialized,
-  } = useComparisonContext();
-
-  // Fetch comparison data for selected schools
-  const urns = selectedSchools.map((s) => s.urn).join(',');
-  const { data, error, isLoading, mutate } = useSWR<ComparisonResponse>(
-    selectedSchools.length > 0 ? `/compare?urns=${urns}` : null,
-    fetcher,
-    {
-      revalidateOnFocus: false,
-      dedupingInterval: 10000,
-    }
-  );
-
-  return {
-    selectedSchools,
-    comparisonData: data?.comparison,
-    isLoading,
-    error,
-    addSchool,
-    removeSchool,
-    replaceSchools,
-    clearAll,
-    isSelected,
-    canAddMore,
-    isInitialized,
-    mutate,
-  };
 }