fix(compare): give the trends chart a real height (was squashed to ~150px)

ComparisonChart runs Chart.js with maintainAspectRatio:false, so it sizes
to its container's height — which must be definite. TrendsExplorer gave
.chartBox a min-height, which doesn't resolve the chart wrapper's
height:100%, so Chart.js fell back to its ~150px default: a squashed
8.6:1 sliver that didn't match the mockups. Set a definite height (420px
desktop, 360px mobile where the chips row sits above the canvas).

Verified on staging by patching the live height: canvas went from
1287x150 to 1287x392 (desktop) / 284 (mobile) — proper ~3:1 proportions
matching the mockup, with the England dashed line, COVID/2021-22 gap and
table all reading correctly.

An e2e guard asserts the trends canvas is taller than 220px so the
squash can't regress.

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

e2e/tests/journeys.spec.ts

@@ -213,7 +213,12 @@ test('compare chart on mobile shows school chips with tap-to-focus', async ({ pa
   expect(bodyOverflowsX).toBe(false);
 
   // The trends chart still renders (inside the Explore trends section)…
-  await expect(page.locator('canvas:visible').first()).toBeVisible({ timeout: 15_000 });
+  const chartCanvas = page.locator('canvas:visible').first();
+  await expect(chartCanvas).toBeVisible({ timeout: 15_000 });
+  // …at a real height, not the squashed ~150px Chart.js fallback that
+  // appears when the container lacks a definite height.
+  const chartBox = await chartCanvas.boundingBox();
+  expect(chartBox && chartBox.height).toBeGreaterThan(220);
 
   // …with the mobile chart legend chips and tap-to-focus behaviour intact.
   const chipGroup = page.getByRole('group', { name: /highlight a school/i });

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/components/compare/TrendsExplorer.module.css

@@ -60,8 +60,20 @@
   margin: 0 0 1rem;
 }
 
+/* ComparisonChart runs Chart.js with maintainAspectRatio:false, so it fills
+   its container's height — which must be *definite*. A min-height alone does
+   not resolve the chart wrapper's height:100%, leaving Chart.js to fall back
+   to its ~150px default (a squashed sliver). Give it a real height. */
 .chartBox {
-  min-height: 320px;
+  height: 420px;
+}
+
+@media (max-width: 640px) {
+  /* Taller on mobile: the mobile-only school chips sit above the canvas and
+     wrap to two rows for 3+ schools, so the plot keeps a usable height. */
+  .chartBox {
+    height: 360px;
+  }
 }
 
 .tableWrapper {

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,
-  };
 }