Merge pull request 'fix(pipeline): keep the destinations marts out of the scheduled builds' (#181) from fix/scheduled-dbt-selectors into main

Reviewed-on: #181

e2e/tests/journeys.spec.ts

@@ -410,6 +410,29 @@ test('search and the school page agree on how many pupils a secondary has', asyn
   expect(school.total_pupils).toBe(detail.school_info.total_pupils);
 });
 
+test('a secondary search row compares its Attainment 8 with the LA average', async ({ page }) => {
+  // The comparison vanished unnoticed: the averages were fetched with
+  // force-cache, so one stored failure hid it in that browser for good.
+  // Playwright disables the HTTP cache when it intercepts requests, so this
+  // guards the comparison itself; the unit test pins the cache mode.
+  const la = await (await page.request.get('/api/la-averages')).json();
+  const averages: Record<string, number> = la.secondary?.attainment_8_by_la ?? {};
+  const res = await page.request.get('/api/schools?search=school&phase=secondary&page_size=50');
+  expect(res.ok()).toBeTruthy();
+  const school = ((await res.json()).schools ?? []).find(
+    (s: { attainment_8_score?: number | null; local_authority?: string; school_type?: string }) =>
+      s.attainment_8_score != null && s.local_authority != null && averages[s.local_authority] != null
+      && !/special|pupil referral|alternative provision/i.test(s.school_type ?? ''));
+  test.skip(!school, 'no mainstream secondary with an LA average here');
+
+  await searchByName(page, school.school_name);
+  const link = page.locator(`a[href^="/school/${school.urn}-"]`).first();
+  await expect(link).toBeVisible({ timeout: 15_000 });
+  const stats = link.locator('xpath=ancestor::div[contains(@class, "__rowContent")][1]')
+    .locator('[class*="__line3"]');
+  await expect(stats.getByText(/vs LA avg/)).toBeVisible();
+});
+
 test('a phase outside primary/secondary filters to that phase, not to everything', async ({ page }) => {
   // The search page offers every GIAS phase, but the API only knew the grouped
   // ones and silently dropped the rest — so "Nursery" returned primaries.

nextjs-app/__tests__/components/HomeView.staleFetch.test.tsx

@@ -1,6 +1,6 @@
 import { act, fireEvent, render, screen } from '@testing-library/react';
 import { HomeView } from '@/components/HomeView';
-import { fetchSchools } from '@/lib/api';
+import { fetchLAaverages, fetchSchools } from '@/lib/api';
 import { primaryFixture } from '../support/schoolFixtures';
 import type { SchoolsResponse, School } from '@/lib/types';
 
@@ -84,3 +84,23 @@ test('failed map requests can be retried by reopening the map', async () => {
   expect(fetchSchools).toHaveBeenCalledTimes(2);
   expect(screen.getByTestId('map')).toHaveTextContent('Retry result');
 });
+
+test('LA averages are not fetched with force-cache, so one failure is not replayed for good', async () => {
+  // force-cache serves any stored response, however old, without asking the
+  // server. A request that failed once (a staging deploy restart, the July
+  // proxy outage) was stored and replayed on every later visit, and the
+  // "vs LA avg" delta vanished from every secondary row in that browser.
+  // The default mode honours the API's Cache-Control and never reuses an
+  // error.
+  params = new URLSearchParams('search=high');
+  const secondary: SchoolsResponse = {
+    ...response('Alpha High'),
+    schools: [{ ...primaryFixture.schoolInfo, school_name: 'Alpha High', phase: 'Secondary', attainment_8_score: 50 }],
+  };
+  render(<HomeView initialSchools={secondary} filters={filters} />);
+  await act(async () => {});
+  expect(fetchLAaverages).toHaveBeenCalled();
+  for (const [options] of jest.mocked(fetchLAaverages).mock.calls) {
+    expect(options?.cache).not.toBe('force-cache');
+  }
+});

nextjs-app/__tests__/components/NearbySchoolsSection.test.tsx

@@ -8,7 +8,6 @@
 import { render, screen } from '@testing-library/react';
 
 import {
-  nearbyNoun,
   NearbySchoolsSection,
   shouldRenderNearby,
 } from '@/components/school/NearbySchoolsSection';
@@ -43,13 +42,7 @@ function school(overrides: Partial<NearbySchool> = {}): NearbySchool {
 
 function renderSection(nearby: NearbySchool[]) {
   return render(
-    <NearbySchoolsSection
-      urn={100001}
-      schoolName="Meadowbrook Primary School"
-      phase="Primary"
-      thisMetricValue={72}
-      nearby={nearby}
-    />,
+    <NearbySchoolsSection urn={100001} thisMetricValue={72} nearby={nearby} />,
   );
 }
 
@@ -90,15 +83,10 @@ describe('what the section claims', () => {
   });
 
   it('shows no chips at all when nothing is shared, rather than inventing one', () => {
-    const { container } = render(
-      <NearbySchoolsSection
-        urn={100001}
-        schoolName="Meadowbrook Primary School"
-        phase="Primary"
-        thisMetricValue={72}
-        nearby={[school({ shared: [] }), school({ urn: 100003, shared: [] })]}
-      />,
-    );
+    const { container } = renderSection([
+      school({ shared: [] }),
+      school({ urn: 100003, shared: [] }),
+    ]);
     // The card still carries its distance, name, type and figure — just no
     // claim of likeness.
     expect(container.querySelectorAll('li ul').length).toBe(0);
@@ -106,37 +94,6 @@ describe('what the section claims', () => {
   });
 });
 
-describe('what the lede calls the set', () => {
-  it.each([
-    ['Primary', 'primary schools'],
-    ['Middle deemed primary', 'primary schools'],
-    ['Secondary', 'secondary schools'],
-    ['Middle deemed secondary', 'secondary schools'],
-    ['All-through', 'all-through schools'],
-    // GIAS phase 6. Its candidates span the whole secondary group, so no
-    // single noun fits and it takes the honest general one.
-    ['16 plus', 'schools and colleges'],
-    ['', 'schools'],
-    [null, 'schools'],
-  ])('calls a %s school\'s neighbours "%s"', (phase, expected) => {
-    expect(nearbyNoun(phase)).toBe(expected);
-  });
-
-  it('never calls a sixth form college\'s neighbours primary schools', () => {
-    render(
-      <NearbySchoolsSection
-        urn={100001}
-        schoolName="Barnet Sixth Form College"
-        phase="16 plus"
-        thisMetricValue={null}
-        nearby={[school(), school({ urn: 100003 })]}
-      />,
-    );
-    expect(screen.getByText(/Other schools and colleges near Barnet Sixth Form College/)).toBeInTheDocument();
-    expect(screen.queryByText(/primary schools/)).not.toBeInTheDocument();
-  });
-});
-
 describe('cards', () => {
   it('links each school to its canonical slug', () => {
     renderSection([school(), school({ urn: 100003, school_name: 'Oakfield Primary School' })]);

nextjs-app/components/HomeView.tsx

@@ -358,10 +358,14 @@ export function HomeView({ initialSchools, filters, totalSchools, howItWorks, ed
     return () => controller.abort();
   }, [resultsView, searchParams, initialSchools.schools]);
 
-  // Fetch LA averages when secondary or mixed schools are visible
+  // Fetch LA averages when secondary or mixed schools are visible. Default
+  // cache mode, never force-cache: force-cache replays any stored response
+  // without asking the server, so one failed request (a deploy restart) hid
+  // every "vs LA avg" delta in that browser for good. The API's Cache-Control
+  // already lets the browser reuse a good answer for five minutes.
   useEffect(() => {
     if (!isSecondaryView && !isMixedView) return;
-    fetchLAaverages({ cache: 'force-cache' })
+    fetchLAaverages()
       .then(data => setLaAverages(data.secondary.attainment_8_by_la))
       .catch(() => {});
   }, [isSecondaryView, isMixedView]);

nextjs-app/components/school/NearbySchools.module.css

@@ -1,8 +1,7 @@
 .heading { font-family: var(--font-display); font-size: 1.4rem; letter-spacing: -0.4px; margin: 0; }
-.lede { margin: 0.5rem 0 1.25rem; color: var(--text-secondary); max-width: 64ch; }
 .caption { margin: 1rem 0 0; font-size: 0.72rem; color: var(--text-muted); }
 
-.top { display: flex; align-items: flex-start; justify-content: space-between; gap: 1rem; }
+.top { display: flex; align-items: center; justify-content: space-between; gap: 1rem; margin-bottom: 1.25rem; }
 .arrows { display: flex; gap: 0.5rem; flex: none; }
 .arrow { width: 44px; height: 44px; display: grid; place-items: center; cursor: pointer; border: 1px solid var(--border-strong); border-radius: 999px; background: var(--bg-card); color: var(--brand); }
 .arrow:hover:not(:disabled) { border-color: var(--brand); background: var(--brand-bg); }
@@ -15,10 +14,10 @@
 .scroller { display: grid; grid-auto-flow: column; grid-auto-columns: calc((100% - 1.8rem) / 3); gap: 0.9rem; overflow-x: auto; scroll-snap-type: x mandatory; padding: 2px; margin: -2px; list-style: none; scrollbar-width: none; -ms-overflow-style: none; }
 .scroller::-webkit-scrollbar { display: none; }
 @media (max-width: 820px) { .scroller { grid-auto-columns: calc((100% - 0.9rem) / 2); } }
-/* Touch widths (MOBILE.md): the arrows would take 96px from a 328px card and
-   crush the lede into four lines, for a control swiping already provides. They
-   go, and the documented right-edge fade carries the affordance — lifting at
-   the end of the travel, where there is nothing more to hint at. */
+/* Touch widths (MOBILE.md): the arrows would take 96px from a 328px card, for
+   a control swiping already provides. They go, and the documented right-edge
+   fade carries the affordance — lifting at the end of the travel, where there
+   is nothing more to hint at. */
 @media (max-width: 640px) {
   .top { display: block; }
   .arrows { display: none; }

nextjs-app/components/school/NearbySchoolsCarousel.tsx

@@ -4,7 +4,7 @@
  * The scroller and its arrows.
  *
  * `children` are the server-rendered cards and `header` the server-rendered
- * heading and lede: both stay server components, passed through, so this file
+ * heading: both stay server components, passed through, so this file
  * owns a DOM ref and nothing else. That is what keeps all six links in the
  * initial HTML — a carousel that mounted cards on click would put four of the
  * six beyond a crawler and beyond a reader with no JavaScript.

nextjs-app/components/school/NearbySchoolsSection.tsx

@@ -13,8 +13,10 @@
  * reached on their behalf.
  *
  * There is deliberately no "how these are chosen" panel: the method is already
- * visible in the lede, the chips and the distances. The single caption line is
- * not a method note — it is the one thing a card cannot self-correct.
+ * visible in the chips and the distances. The single caption line is not a
+ * method note — it is the one thing a card cannot self-correct.
+ *
+ * Nor is there a lede: "Other primary schools near X" only restated the heading.
  */
 
 import Link from 'next/link';
@@ -32,28 +34,6 @@ export function shouldRenderNearby(nearby?: NearbySchool[] | null): boolean {
   return (nearby?.length ?? 0) >= MINIMUM;
 }
 
-/**
- * What the lede calls the set of schools it is showing.
- *
- * Derived from the school's own GIAS phase rather than the template it renders
- * with, because those disagree for "16 plus" (GIAS phase 6): a sixth-form
- * college renders the primary template — computeSchoolFlags tests for the
- * substring "secondary" — while the backend correctly matches it against the
- * secondary group. Taking the noun from the template would print "Other primary
- * schools near <sixth form college>" above a row of secondaries.
- *
- * A 16-plus school's candidates span the whole secondary group, so no single
- * noun fits and it gets the honest general one.
- */
-export function nearbyNoun(phase: string | null | undefined): string {
-  const text = (phase ?? '').trim().toLowerCase();
-  if (text === 'all-through') return 'all-through schools';
-  if (text === '16 plus') return 'schools and colleges';
-  if (text.includes('secondary')) return 'secondary schools';
-  if (text.includes('primary')) return 'primary schools';
-  return 'schools';
-}
-
 function metricLabel(key: string): string {
   return key === 'attainment_8_score' ? 'Attainment 8' : 'Reading, writing & maths';
 }
@@ -65,25 +45,16 @@ function formatMetric(value: number | null, key: string): string {
 
 export function NearbySchoolsSection({
   urn,
-  schoolName,
-  phase,
   thisMetricValue,
   nearby,
 }: {
   urn: number;
-  schoolName: string;
-  /** The school's own GIAS phase, not the template it renders with. */
-  phase: string | null | undefined;
   thisMetricValue: number | null;
   nearby?: NearbySchool[] | null;
 }) {
   if (!shouldRenderNearby(nearby)) return null;
   const schools = nearby as NearbySchool[];
-
-  // One card matched on phase alone, so the section may not claim the set
-  // shares an intake with this school.
   const metricKey = schools[0].metric_key;
-  const noun = nearbyNoun(phase);
 
   return (
     <Section id="nearby">
@@ -91,12 +62,9 @@ export function NearbySchoolsSection({
         count={schools.length}
         labelledBy="nearby-schools-heading"
         header={
-          <div>
-            <h2 id="nearby-schools-heading" className={styles.heading}>
-              Other schools nearby
-            </h2>
-            <p className={styles.lede}>{`Other ${noun} near ${schoolName}.`}</p>
-          </div>
+          <h2 id="nearby-schools-heading" className={styles.heading}>
+            Other schools nearby
+          </h2>
         }
       >
         {schools.map((school) => (

nextjs-app/components/school/PrimarySchoolSections.tsx

@@ -154,8 +154,6 @@ export function PrimarySchoolSections({
       {/* Last: it is where the reader goes next, not part of this school. */}
       <NearbySchoolsSection
         urn={schoolInfo.urn}
-        schoolName={schoolInfo.school_name}
-        phase={schoolInfo.phase}
         thisMetricValue={flags.latestResults?.rwm_expected_pct ?? null}
         nearby={nearbySchools}
       />

nextjs-app/components/school/SecondarySchoolSections.tsx

@@ -148,8 +148,6 @@ export function SecondarySchoolSections({
       {/* Last: it is where the reader goes next, not part of this school. */}
       <NearbySchoolsSection
         urn={schoolInfo.urn}
-        schoolName={schoolInfo.school_name}
-        phase={schoolInfo.phase}
         thisMetricValue={flags.latestResults?.attainment_8_score ?? null}
         nearby={nearbySchools}
       />

pipeline/dags/school_data_pipeline.py

@@ -106,9 +106,12 @@ print(f'Validation passed: {{count}} GIAS rows')
         """,
     )
 
+    # Marts fed by annual EES staging models are rebuilt by the EES DAG, even
+    # when they join dim_school. Selecting them here fails in any database
+    # where that DAG hasn't run (pipeline/tests/test_dag_selectors.py).
     dbt_build = BashOperator(
         task_id="dbt_build",
-        bash_command=f"cd {PIPELINE_DIR}/transform && {DBT_BIN} build --profiles-dir . --target production --select stg_gias_establishments+ stg_gias_links+ gias_code_names+ --exclude int_ks2_with_lineage+ int_ks4_with_lineage+",
+        bash_command=f"cd {PIPELINE_DIR}/transform && {DBT_BIN} build --profiles-dir . --target production --select stg_gias_establishments+ stg_gias_links+ gias_code_names+ --exclude int_ks2_with_lineage+ int_ks4_with_lineage+ stg_ees_ks4_destinations+ stg_ees_ks5_destinations+",
     )
 
     sync_typesense = BashOperator(
@@ -143,7 +146,7 @@ with DAG(
 
     dbt_build_ofsted = BashOperator(
         task_id="dbt_build",
-        bash_command=f"cd {PIPELINE_DIR}/transform && {DBT_BIN} build --profiles-dir . --target production --select stg_ofsted_inspections+ int_ofsted_latest+ fact_ofsted_inspection+ dim_school+",
+        bash_command=f"cd {PIPELINE_DIR}/transform && {DBT_BIN} build --profiles-dir . --target production --select stg_ofsted_inspections+ int_ofsted_latest+ fact_ofsted_inspection+ dim_school+ --exclude stg_ees_ks4_destinations+ stg_ees_ks5_destinations+",
     )
 
     sync_typesense_ofsted = BashOperator(

pipeline/tests/test_dag_selectors.py

@@ -0,0 +1,98 @@
+"""Every scheduled dbt build must only build models whose parents exist.
+
+The daily GIAS build selects `stg_gias_establishments+`, so any mart that joins
+dim_school joins the daily build too. When such a mart also reads a staging
+model that only the manually triggered EES DAG builds, the daily build fails in
+any database where that DAG has not run since. Sync and cache invalidation then
+never run either. The destinations marts did this from late August 2026.
+
+The graph is read from the model SQL, because CI has no dbt.
+"""
+import re
+from collections import defaultdict
+from pathlib import Path
+
+import pytest
+
+PIPELINE = Path(__file__).resolve().parents[1]
+MODELS = PIPELINE / 'transform' / 'models'
+DAG_FILE = PIPELINE / 'dags' / 'school_data_pipeline.py'
+
+REF = re.compile(r"ref\(\s*'([a-z0-9_]+)'\s*\)")
+DBT_BUILD = re.compile(r'dbt_build\w*\s*=\s*BashOperator\(.*?build --profiles-dir \. --target production ([^"]+)"', re.S)
+DAG_ID = re.compile(r'dag_id="([a-z0-9_]+)"')
+
+DAILY = 'school_data_daily'
+
+# dim_school reads int_ofsted_latest only when the relation exists
+# (adapter.get_relation), so a missing table is not a failure.
+OPTIONAL_PARENTS = {'int_ofsted_latest'}
+
+
+def model_parents():
+    """{model: models it refs}. Seeds are left out: they are loaded once and always exist."""
+    sql = {p.stem: p.read_text() for p in MODELS.rglob('*.sql')}
+    return {name: set(REF.findall(text)) & set(sql) for name, text in sql.items()}
+
+
+def downstream(node, children):
+    seen, stack = {node}, [node]
+    while stack:
+        for child in children[stack.pop()]:
+            if child not in seen:
+                seen.add(child)
+                stack.append(child)
+    return seen
+
+
+def expand(tokens, children):
+    out = set()
+    for token in tokens:
+        out |= downstream(token[:-1], children) if token.endswith('+') else {token}
+    return out
+
+
+def scheduled_builds():
+    """{dag_id: dbt selection arguments} for every dbt build in the DAG file."""
+    text = DAG_FILE.read_text()
+    starts = [(m.start(), m.group(1)) for m in DAG_ID.finditer(text)]
+    builds = {}
+    for i, (start, dag_id) in enumerate(starts):
+        end = starts[i + 1][0] if i + 1 < len(starts) else len(text)
+        found = DBT_BUILD.search(text, start, end)
+        if found:
+            builds[dag_id] = found.group(1)
+    return builds
+
+
+def selected_models(args, parents):
+    children = defaultdict(set)
+    for model, ps in parents.items():
+        for p in ps:
+            children[p].add(model)
+    select = re.search(r'--select (.+?)(?= --exclude|$)', args).group(1).split()
+    excluded = re.search(r'--exclude (.+)$', args)
+    exclude = excluded.group(1).split() if excluded else []
+    return (expand(select, children) - expand(exclude, children)) & set(parents)
+
+
+PARENTS = model_parents()
+BUILDS = scheduled_builds()
+DAILY_MODELS = selected_models(BUILDS[DAILY], PARENTS)
+
+
+def test_every_dag_with_a_dbt_build_is_parsed():
+    assert set(BUILDS) == {
+        'school_data_daily', 'school_data_monthly_ofsted', 'school_data_annual_ees',
+        'school_data_annual_idaci', 'school_data_annual_distance',
+    }
+
+
+@pytest.mark.parametrize('dag_id', sorted(BUILDS))
+def test_selected_models_only_read_models_that_exist(dag_id):
+    selected = selected_models(BUILDS[dag_id], PARENTS)
+    # The daily build is the base layer: other DAGs may rely on what it builds.
+    available = selected | OPTIONAL_PARENTS | (DAILY_MODELS if dag_id != DAILY else set())
+    missing = {model: sorted(PARENTS[model] - available) for model in sorted(selected)
+               if PARENTS[model] - available}
+    assert missing == {}, f'{dag_id} builds models whose parents it never builds: {missing}'