Compare commits

...
Author SHA1 Message Date
tudor b6e48c4930 Merge pull request 'fix(pipeline): decode GIAS extracts as Windows-1252' (#182) from fix/gias-encoding into main
Stage (build -> staging -> E2E gate) / prepare (push) Successful in 1s
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 21s
Stage (build -> staging -> E2E gate) / Build Frontend (Next.js) (push) Successful in 1m32s
Stage (build -> staging -> E2E gate) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 1m37s
Stage (build -> staging -> E2E gate) / Deploy to Staging (push) Successful in 5s
Stage (build -> staging -> E2E gate) / E2E Journeys against Staging (push) Successful in 3m21s
Reviewed-on: #182
2026-10-05 06:31:30 +00:00
tudor 9f4f2507cc Merge pull request 'fix(pipeline): keep the destinations marts out of the scheduled builds' (#181) from fix/scheduled-dbt-selectors into main
Stage (build -> staging -> E2E gate) / prepare (push) Successful in 1s
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 47s
Stage (build -> staging -> E2E gate) / Build Frontend (Next.js) (push) Successful in 1m35s
Stage (build -> staging -> E2E gate) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 1m20s
Stage (build -> staging -> E2E gate) / Deploy to Staging (push) Successful in 3s
Stage (build -> staging -> E2E gate) / E2E Journeys against Staging (push) Successful in 3m26s
Reviewed-on: #181
2026-10-05 06:19:44 +00:00
tudor e1373fb6df Merge pull request 'fix(school): drop the nearby section's lede, which repeated its heading' (#180) from fix/nearby-drop-lede into main
Stage (build -> staging -> E2E gate) / prepare (push) Successful in 1s
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 19s
Stage (build -> staging -> E2E gate) / Build Frontend (Next.js) (push) Successful in 1m31s
Stage (build -> staging -> E2E gate) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 2m8s
Stage (build -> staging -> E2E gate) / Deploy to Staging (push) Successful in 4s
Stage (build -> staging -> E2E gate) / E2E Journeys against Staging (push) Failing after 45s
Reviewed-on: #180
2026-10-04 09:13:48 +00:00
TudorandClaude Opus 5.5 65a2619e1d fix(pipeline): keep the destinations marts out of the scheduled builds
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m18s
PR Checks / Backend Smoke (pull_request) Successful in 11s
PR Checks / Build Backend (no push) (pull_request) Successful in 18s
PR Checks / Build Frontend (no push) (pull_request) Successful in 1m35s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 1m21s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 19s
fact_ks4_destinations and fact_ks5_destinations join dim_school, so the
daily build's stg_gias_establishments+ and the monthly Ofsted build's
dim_school+ both selected them. They also read stg_ees_ks4/ks5_destinations,
which only the manually triggered EES DAG builds. Where that DAG hasn't run
since the destinations models landed, dbt_build fails with "relation
staging.stg_ees_ks4_destinations does not exist", and sync_typesense and
invalidate_cache never run. Production's register data has been stuck at
about 25 Aug 2026.

Both builds now exclude the descendants of the two EES staging models, as
the daily build already does for the KS2/KS4 lineage models. The EES DAG
still rebuilds the marts when their data changes.

test_dag_selectors reads the model graph from the SQL (CI has no dbt) and
checks that every scheduled build only reads models it or the daily build
builds. It failed for the daily and monthly Ofsted builds before this change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-03 22:21:27 +01:00
TudorandClaude Opus 5.5 ccfa44389e fix(school): drop the nearby section's lede, which repeated its heading
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m19s
PR Checks / Backend Smoke (pull_request) Successful in 10s
PR Checks / Build Backend (no push) (pull_request) Successful in 19s
PR Checks / Build Frontend (no push) (pull_request) Successful in 1m32s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 1m18s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 14s
"Other schools nearby" was followed by "Other primary schools near
<school>.", which says the same thing again. The heading now stands alone.

nearbyNoun() and the phase and schoolName props existed only to build that
line, so they go with it. Its bottom margin was the only gap between the
heading and the cards, so the header row carries that gap now, and centres
the heading against the carousel arrows now that it is a single line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-03 21:10:36 +01:00
8 changed files with 121 additions and 100 deletions

No files matched your search

@@ -8,7 +8,6 @@
import { render, screen } from '@testing-library/react'; import { render, screen } from '@testing-library/react';
import { import {
nearbyNoun,
NearbySchoolsSection, NearbySchoolsSection,
shouldRenderNearby, shouldRenderNearby,
} from '@/components/school/NearbySchoolsSection'; } from '@/components/school/NearbySchoolsSection';
@@ -43,13 +42,7 @@ function school(overrides: Partial<NearbySchool> = {}): NearbySchool {
function renderSection(nearby: NearbySchool[]) { function renderSection(nearby: NearbySchool[]) {
return render( return render(
<NearbySchoolsSection <NearbySchoolsSection urn={100001} thisMetricValue={72} nearby={nearby} />,
urn={100001}
schoolName="Meadowbrook Primary School"
phase="Primary"
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', () => { it('shows no chips at all when nothing is shared, rather than inventing one', () => {
const { container } = render( const { container } = renderSection([
<NearbySchoolsSection school({ shared: [] }),
urn={100001} school({ urn: 100003, shared: [] }),
schoolName="Meadowbrook Primary School" ]);
phase="Primary"
thisMetricValue={72}
nearby={[school({ shared: [] }), school({ urn: 100003, shared: [] })]}
/>,
);
// The card still carries its distance, name, type and figure — just no // The card still carries its distance, name, type and figure — just no
// claim of likeness. // claim of likeness.
expect(container.querySelectorAll('li ul').length).toBe(0); 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', () => { describe('cards', () => {
it('links each school to its canonical slug', () => { it('links each school to its canonical slug', () => {
renderSection([school(), school({ urn: 100003, school_name: 'Oakfield Primary School' })]); renderSection([school(), school({ urn: 100003, school_name: 'Oakfield Primary School' })]);
@@ -1,8 +1,7 @@
.heading { font-family: var(--font-display); font-size: 1.4rem; letter-spacing: -0.4px; margin: 0; } .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); } .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; } .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 { 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); } .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 { 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; } .scroller::-webkit-scrollbar { display: none; }
@media (max-width: 820px) { .scroller { grid-auto-columns: calc((100% - 0.9rem) / 2); } } @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 /* Touch widths (MOBILE.md): the arrows would take 96px from a 328px card, for
crush the lede into four lines, for a control swiping already provides. They a control swiping already provides. They go, and the documented right-edge
go, and the documented right-edge fade carries the affordance — lifting at fade carries the affordance — lifting at the end of the travel, where there
the end of the travel, where there is nothing more to hint at. */ is nothing more to hint at. */
@media (max-width: 640px) { @media (max-width: 640px) {
.top { display: block; } .top { display: block; }
.arrows { display: none; } .arrows { display: none; }
@@ -4,7 +4,7 @@
* The scroller and its arrows. * The scroller and its arrows.
* *
* `children` are the server-rendered cards and `header` the server-rendered * `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 * 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 * 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. * six beyond a crawler and beyond a reader with no JavaScript.
@@ -13,8 +13,10 @@
* reached on their behalf. * reached on their behalf.
* *
* There is deliberately no "how these are chosen" panel: the method is already * 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 * visible in the chips and the distances. The single caption line is not a
* not a method note — it is the one thing a card cannot self-correct. * 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'; import Link from 'next/link';
@@ -32,28 +34,6 @@ export function shouldRenderNearby(nearby?: NearbySchool[] | null): boolean {
return (nearby?.length ?? 0) >= MINIMUM; 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 { function metricLabel(key: string): string {
return key === 'attainment_8_score' ? 'Attainment 8' : 'Reading, writing & maths'; 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({ export function NearbySchoolsSection({
urn, urn,
schoolName,
phase,
thisMetricValue, thisMetricValue,
nearby, nearby,
}: { }: {
urn: number; urn: number;
schoolName: string;
/** The school's own GIAS phase, not the template it renders with. */
phase: string | null | undefined;
thisMetricValue: number | null; thisMetricValue: number | null;
nearby?: NearbySchool[] | null; nearby?: NearbySchool[] | null;
}) { }) {
if (!shouldRenderNearby(nearby)) return null; if (!shouldRenderNearby(nearby)) return null;
const schools = nearby as NearbySchool[]; 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 metricKey = schools[0].metric_key;
const noun = nearbyNoun(phase);
return ( return (
<Section id="nearby"> <Section id="nearby">
@@ -91,12 +62,9 @@ export function NearbySchoolsSection({
count={schools.length} count={schools.length}
labelledBy="nearby-schools-heading" labelledBy="nearby-schools-heading"
header={ header={
<div> <h2 id="nearby-schools-heading" className={styles.heading}>
<h2 id="nearby-schools-heading" className={styles.heading}> Other schools nearby
Other schools nearby </h2>
</h2>
<p className={styles.lede}>{`Other ${noun} near ${schoolName}.`}</p>
</div>
} }
> >
{schools.map((school) => ( {schools.map((school) => (
@@ -154,8 +154,6 @@ export function PrimarySchoolSections({
{/* Last: it is where the reader goes next, not part of this school. */} {/* Last: it is where the reader goes next, not part of this school. */}
<NearbySchoolsSection <NearbySchoolsSection
urn={schoolInfo.urn} urn={schoolInfo.urn}
schoolName={schoolInfo.school_name}
phase={schoolInfo.phase}
thisMetricValue={flags.latestResults?.rwm_expected_pct ?? null} thisMetricValue={flags.latestResults?.rwm_expected_pct ?? null}
nearby={nearbySchools} nearby={nearbySchools}
/> />
@@ -148,8 +148,6 @@ export function SecondarySchoolSections({
{/* Last: it is where the reader goes next, not part of this school. */} {/* Last: it is where the reader goes next, not part of this school. */}
<NearbySchoolsSection <NearbySchoolsSection
urn={schoolInfo.urn} urn={schoolInfo.urn}
schoolName={schoolInfo.school_name}
phase={schoolInfo.phase}
thisMetricValue={flags.latestResults?.attainment_8_score ?? null} thisMetricValue={flags.latestResults?.attainment_8_score ?? null}
nearby={nearbySchools} nearby={nearbySchools}
/> />
+5 -2
View File
@@ -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( dbt_build = BashOperator(
task_id="dbt_build", 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( sync_typesense = BashOperator(
@@ -143,7 +146,7 @@ with DAG(
dbt_build_ofsted = BashOperator( dbt_build_ofsted = BashOperator(
task_id="dbt_build", 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( sync_typesense_ofsted = BashOperator(
+98
View File
@@ -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}'