`backend/migration.py` and `scripts/migrate_csv_to_db.py` import `School`,
`SchoolResult`, `init_db` and `set_db_schema_version` — names that no longer
exist. `scripts/geocode_schools.py` imports the same removed ORM model. None of
the three can be imported against the current backend, so they were not dormant
utilities anyone could fall back on; they were files that would fail on the
first line. `backend/version.py` existed only to hand `SCHEMA_VERSION` to that
importer, and the FastAPI lifespan performs no version-triggered import.
Three symbols go with them, each confirmed to have no caller: the unvectorised
`haversine_distance`, superseded by the inline NumPy calculation in search;
`fetcher`, an SWR helper for a dependency this project does not install; and
`kmToMiles`. `calculateDistance` stays — CutoffMapPanel uses it.
Two comments pointed at `migrate_csv_to_db.py --drop` to explain why Payload
owns its own schema. The reason survives the script: blog content must stay
clear of the school marts and Airflow's metadata. Reworded rather than deleted,
so the constraint keeps its justification.
docs/LEGACY_CODE.md records what was removed and where to find it in history. It
also records what was deliberately *not* removed, which is the more useful half:
unused UI components awaiting a design decision, manual data utilities whose
operators a repository search cannot see, and fallbacks that look obsolete but
are load-bearing — `data_loader.py`'s older-mart branches, the generated GIAS
dictionary copies, and the `legacy`-named dbt models that annual DAG selectors
explicitly include. A zero-import count is evidence, not a verdict.
The scripts that fetch DfE CSVs are marked historical and kept, pending
confirmation that nobody runs them by hand.
Checked: 190 backend tests, 429 frontend tests, `tsc --noEmit` clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016y2J6bs8gbuSJbH18w7Tan
The README still opened on "Primary School Compass", a KS2 tool for Wandsworth
and Merton served by FastAPI and vanilla JavaScript with Chart.js. Every layer
of that sentence is now wrong: coverage is England-wide across KS2, KS4,
all-through and post-16, Next.js owns the public UI, and school data comes from
dbt-built `marts.*` rather than CSVs loaded at startup. The setup instructions
walked a reader into a virtualenv and a CSV import that cannot build the current
schema, so following the docs produced an empty database and a wrong mental
model at the same time.
Replace the narrative docs with two reference documents that were checked
against the code: docs/ARCHITECTURE.md for request flow, data ownership, the
backend/frontend module boundaries and the real publication sequence, and
docs/DEVELOPMENT.md for the checks that actually run, including the container
and CI version skew that makes "just run pytest" misleading.
The env examples drifted the same way. ALLOWED_ORIGINS is a JSON array, not a
comma-separated list; the frontend needs FASTAPI_URL, DATABASE_URL and
PAYLOAD_SECRET, none of which were documented; and RATE_LIMIT_BURST,
DEFAULT_PAGE_SIZE and MAX_PAGE_SIZE were presented as tuning controls the routes
do not consult. Each is now stated as it behaves.
MIGRATION_SUMMARY.md keeps its content but gains a banner, because it reads like
setup instructions and is not.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016y2J6bs8gbuSJbH18w7Tan
The staging E2E gate's one failure. Every link to the site pasted into
a chat has been rendering bare.
Staging serves og:title, og:description, og:url, og:site_name, og:type
and twitter:card, and no og:image at all. So metadata from the layout
reaches the page; only the file convention does not.
app/opengraph-image.tsx does work — _not-found, which lives in the app
root segment, carries an og:image from it in the build output. It does
not reach the site's pages, which live in the (frontend) route group
whose own layout.tsx is the root layout. The icon conventions are not
affected: /icon.png and /apple-icon.png are both linked correctly on
the same page, verified against staging. The asymmetry is the whole
bug, and it arrived with the route-group split that Payload required.
The file stays at the app root. Moving metadata files into a route
group is what drops /robots.txt and hashes /icon.png, which CLAUDE.md
records and which this must not undo — the build still emits all four
of /robots.txt, /icon.png, /apple-icon.png and /opengraph-image. The
root layout points at the route instead, and metadataBase makes it
absolute, which the journey needs since it calls new URL() on the value.
twitter.images is set for the same reason: the card is declared
summary_large_image, and claiming a large-image card while supplying no
image is worse than claiming a summary card.
Checked before fixing that og:image was the only broken assertion in
that journey: the test aborts at line 911, so its apple-touch-icon and
maskable-icon assertions had never run. All four of those assets return
200 image/png from staging, so this does not simply move the failure
further down the test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXnXQKnPpZBBP61fBQiFkq
Two review findings, both confirmed before fixing.
The client fixture in test_school_details.py patched load_school_data
but not _place_registry, which is a module-level cache. A probe settled
it rather than an argument: poisoning the global with a registry built
from data the fixture never saw, then issuing the fixture's own
request, returned that other dataset's places. So the new
`places == []` assertion was satisfied by a stale registry exactly as
well as by the fixture's own data, and proved nothing. Every other test
that touches place data already reset it; the fixture predates places
existing and was never updated. It resets it now.
places_for_urn walked every place in the registry and did a tuple
membership test against each, on /api/schools/{urn}, the site's
highest-traffic endpoint. It now reads a dict built once per registry.
Measured against a synthetic corpus of 27k schools in 1,650 places:
0.118ms per request becomes 0.0001ms, with the index built once in
21ms. Production carries ~5,000 places, so the scan there is larger
again. The absolute saving per request is small; the point is that it
is repeated on every school page view and costs nothing to remove.
The index is cached against the registry by identity rather than
behind a second flag. Anything that drops _place_registry — every test
that touches place data does — gets a fresh registry object, which no
longer matches what the index was built from, so the index rebuilds
with it. A separate _place_index = None would be one more thing to
forget, and a stale reverse index is precisely the first finding's bug
wearing a different hat.
That invalidation has its own test, and the test was checked by
breaking the identity check: five tests fail without it, so three
existing ones were already relying on it too.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXnXQKnPpZBBP61fBQiFkq
Review caught _places_payload documenting a `phase_url` the function
never returned. The docstring was not stray prose: the approved design
included the phase variant — "Primary schools in Beccles" was one of
its four example links — and it was dropped during implementation
without being mentioned. Deleting the sentence would have closed the
report while losing the feature, so the links are built instead.
These are the pages that most needed them. ~950 phase variants were
once reachable by nothing at all: absent from every sitemap and
unlinked from the place page. "Primary schools in brentwood" is the
query they exist to answer.
Membership is read from the registry's own `phase_urns` rather than
re-derived from the school's phase string. The registry already decides
which phases a place publishes and which schools are listed on each, so
asking it is both shorter and the only way the link cannot disagree
with the page it points at. It also means outcodes need no special
case: they carry empty `phase_urns` by design, because nobody searches
"primary schools in SW11", so they report no phase links on their own.
`phases` is a list rather than a single url. An all-through school is
listed on both the primary and secondary pages, so there is no tie to
break and no reason to invent one. Each entry renders directly after
its own place, so "22 primary schools in Brentwood" reads as part of
Brentwood rather than as an unrelated link further along the row.
The e2e journey now follows a phase link where the town publishes one
and asserts it resolves.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXnXQKnPpZBBP61fBQiFkq
W2 shipped ~5,000 place pages and nothing linked into them. The
location layer pointed down at school pages; school pages pointed
nowhere on the site. Their only anchor was the school's own website, so
the ~27k pages carrying most of the site's inbound authority passed it
straight off-site, and the new corpus was reachable mainly through the
sitemap.
Three things close the loop.
A reverse index over the place registry, places_for_urn, answers which
published places contain a school. Derived from the registry rather
than stored beside it, so the two cannot disagree about which places
exist: a place below the publish threshold is absent from the registry
and therefore never offered as a link. A test asserts that invariant
across every place in a built registry.
GET /api/schools/{urn} gains a `places` array carrying the name, count
and canonical path for each. It rides on the request the page already
makes, so the school page costs no extra round trip. The frontend types
it optional and defaults it to empty, because the two images deploy
separately and a frontend ahead of the API must render without it.
The page gains a "More schools near here" module and a BreadcrumbList.
The module orders narrowest first, because a reader on a school page
wants its town before its county, while the API orders widest first for
the trail. Anchors state their destination's size — "37 schools in
Brentwood" — which is worth more to a reader and a crawler than "see
more". With no published places it renders nothing rather than an empty
heading.
The trail is rooted at the homepage, not /schools. There is no /schools
index page; the location layer lives only at /schools/[place],
/schools/authority/[la] and /schools/near/[outcode]. Rooting it at the
bare path would have opened every breadcrumb with a link to a 404.
Outcodes are omitted from the trail: "schools near CM15" is a real
query and a useful link, but nobody navigates Essex to CM15 to a
school, and a breadcrumb claiming that describes a hierarchy the site
does not have.
School pages also now declare the School type rather than
EducationalOrganization, the parent type that covers universities and
nurseries alike.
The e2e journey asserts the round trip in both directions, following a
place page's own first school so the pair is genuinely related rather
than hardcoded. A one-way link is what already existed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXnXQKnPpZBBP61fBQiFkq
Both features ship dark. Neither is reachable in an environment where
its flag is off, and every flag in this system starts off, so a deploy
of this commit makes both disappear until someone turns them on
deliberately.
Two independent flags rather than one, which makes blog-on-about-off a
reachable state. That state is the whole reason the change is larger
than four notFound() calls: the blog leans on the About page for its
author identity. The Person entity is anchored at /about#tudor, and
that URL 404s while about_page is dark, so a post published in that
state would claim an author resolving to nothing. Worse than having no
named author. Both bylines fall back to unlinked text and the
BlogPosting attributes to the publisher instead, so every combination
of the two flags renders something correct.
Gated: /about, /blog, /blog/[slug], the RSS feed, both footer links,
and the matching content-sitemap entries. A sitemap must never
advertise a URL that 404s. With both dark it emits a valid empty
urlset rather than a 404, because robots.txt names it unconditionally.
Not gated: /admin. Posts have to be writable before the blog is worth
switching on, so flagging the panel would make the flag unflippable.
getFlags takes a revalidate rather than always using the 300s
constant. Reading a flag pins the calling route to the lowest
revalidate among its fetches, and the footer links live in the root
layout, so a naive gate there would have dropped every school and
place page from a weekly cache to a 5-minute one. The layout passes
604800, the floor those routes already declare, and the build confirms
all four SSG route families still prerender. The cost is one-way
latency: pages follow a flip in minutes, footer links within a week.
The e2e journeys follow the existing paired shape from the
admission_distance flag: a lit journey and a dark one for each flag,
reading state from whether /about and /blog respond rather than from
/api/flags, which another journey asserts is not publicly reachable.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXnXQKnPpZBBP61fBQiFkq
Creating a post in the admin panel showed no Content editor, and saving
failed validation on the field the writer was never shown.
The admin panel does not import field components. The server hands the
client a path per field, and resolves it through the generated map at
app/(payload)/admin/importMap.js. A richText field's path is
@payloadcms/richtext-lexical/rsc#RscEntryLexicalField. The committed map
held one entry, @payloadcms/next/rsc#CollectionCards, generated before
the blog collections existed and never re-run. A path missing from the
map is not an error the panel reports: the field simply does not render,
while required is still enforced server-side on save.
next build does not regenerate the map, so the stale copy shipped in the
image and the editor was equally broken on staging and production.
Regenerated with payload generate:importmap, which adds the lexical RSC
field, cell and diff components, BlocksFeatureClient for the Callout
block, and the default toolbar features.
Two things stop it drifting again. There was no script to run, so
package.json gets generate:importmap. And a test asserts the map carries
an entry for each thing the config asks for, in the source-reading style
of the other payload suites; against the old map all five fail.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXnXQKnPpZBBP61fBQiFkq
Staging failed on boot with 42P01, relation "payload.users" does not
exist. The schema was empty because no migration existed, and the
adapter cannot create tables itself: db-postgres/connect.js gates push
on NODE_ENV !== 'production', so it is inert in a deployed container
regardless of config.
The generated migration is schema-qualified to "payload" throughout but
does not create that schema — schemaName says where tables go, it does
not create anything. It only worked against the throwaway database used
to generate it because the schema was created there by hand, so every
real environment would have failed on the first statement. CREATE SCHEMA
IF NOT EXISTS is hand-added at the top of up(), which makes it exactly
the kind of edit a regeneration discards silently; a test asserts it is
present and ordered before the first CREATE TABLE.
payload-types.ts is now committed rather than ignored. Ignoring it meant
CI typechecked against looser types than a developer with a generated
copy, which is how a Record<string, unknown> cast passed CI and then
failed locally the moment the file appeared. The post page uses the
generated Post and Media types instead, and narrows heroImage rather
than asserting it, since the field is an id at shallow depth.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017YmbBhr8s7GusjDE12hrZM
The em dash is one of the clearest tells of machine-written text, which
is the exact impression this work exists to remove. Rewritten rather
than substituted: where a dash was carrying a real aside the sentence is
split or recast, not patched with a comma.
Covers the About page, the two Callout labels an editor sees in the
admin panel, and PUBLISHING.md, which defines the house style and should
follow it. The rule is now recorded in that house style and in the
spec's voice rules, so it survives this branch.
Code comments are left alone: they are not copy, and the surrounding
codebase uses the same punctuation throughout.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017YmbBhr8s7GusjDE12hrZM
Review findings on #140.
Drafts were reachable. Posts granted unconditional public read and the
_status filter lived only in the pages that query the collection — which
is a convenience, not a control. Payload's documentation is explicit:
"The `draft` argument alone does not restrict documents with _status:
'draft' from being returned by the API." A direct GET /cms-api/posts
would have handed every unpublished draft to any visitor. Read access
now returns a query constraint for anonymous callers, which is the
documented mechanism.
The --drop claim was asserted across four files while the spec still
listed it as an open question. Now verified rather than assumed:
run_full_migration drops exactly ["school_results", "schools"] by name,
there is no drop_all() or DROP SCHEMA anywhere in backend/, the only
other drop is schema-qualified to marts, and nothing sets search_path.
The guarantee is stronger than schema isolation alone — those two table
names do not exist in Payload — so the claim stands, but it now rests on
cited code. The spec records the evidence and closes the open item.
findPost is wrapped in React's cache(): Next calls generateMetadata and
the page separately for one request, so every post view ran the same
query against Postgres twice.
The bare .lede rule was dead — .prose p scores (0,1,1) and outranks it —
so only .prose .lede ever applied. Removed, with the specificity noted
so the surviving selector is not "simplified" back into a silent
regression.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017YmbBhr8s7GusjDE12hrZM
PUBLISHING.md carries the house style with the posts, so the standard
survives without the design doc to hand — including the rule that a post
states what a metric does not show, which is the strongest signal a
human wrote it.
CLAUDE.md gains the two constraints that are invisible from the code and
expensive to rediscover: metadata file conventions break if moved into a
route group, and the build must keep succeeding with DATABASE_URL unset.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017YmbBhr8s7GusjDE12hrZM
The rendering split is dictated by CI building with no database.
/blog, /blog/rss.xml and /content-sitemap.xml have no dynamic params, so
Next prerenders them at build time and the build fails on a missing
Payload secret — caught here, not on staging. They are force-dynamic
instead: one indexed query against Postgres on the same Docker network,
and a newly published post appears immediately rather than waiting on a
revalidation. /blog/[slug] keeps ISR, because with no
generateStaticParams there is nothing to prerender; it is generated on
first request and cached, which is exactly what the collection's
afterChange hook exists to invalidate.
RichText takes `converters`, not `blocks`, in Payload 3.88, and the
default converters must be spread or every paragraph and heading loses
its renderer and the body comes out empty.
BlogPosting references the Person and Organization by @id rather than
repeating them, so every post and the About page resolve to one author
entity instead of declaring several people with the same name.
/sitemap.xml is proxied from FastAPI, which knows nothing about Payload,
so the Next-owned URLs get their own sitemap and robots.txt lists both.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017YmbBhr8s7GusjDE12hrZM
Drafts are on so a post can be written across sittings without saving
being publishing.
afterChange and afterDelete revalidate every path a post appears on.
Blog pages are ISR because CI builds with no database, so without these
a published post would not appear until the revalidate window expired —
up to an hour of a writer concluding that publishing is broken. Payload
runs in the same process as Next, so these are direct revalidatePath
calls with no webhook and no shared secret.
Media writes to an absolute /app/media matching the compose mount; a
mismatch would write into the container filesystem, where the next
redeploy silently discards it. Alt text is required rather than
optional.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017YmbBhr8s7GusjDE12hrZM
The site had no author, no statement of why it exists and nobody
accountable for its numbers, which is most of why it reads as machine
generated.
The page states plainly that its author is not an education expert. The
credibility claim is lived experience — a parent going through primary
admissions — plus stated provenance for every figure, which is true and
cannot be undermined by someone noticing there is no teaching
qualification behind it. First name only: the Person JSON-LD carries no
familyName, worksFor or affiliation, and a test asserts it stays that
way.
The footer gains a fourth column, with a tablet breakpoint so four
columns pair up rather than crushing before the 768px collapse. The nav
is deliberately untouched — its mobile tab bar already carries four
items.
public/brand/tudor.jpg is NOT in this commit. The page references it and
will show a broken image until the photograph is supplied; a stock
portrait would defeat the entire point of the work.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017YmbBhr8s7GusjDE12hrZM
Uploads go to a named volume at /app/media. The directory is created in
the image before the mount and covered by the existing chown, because
Docker seeds a fresh named volume from the image path — a missing or
root-owned directory there fails every upload with EACCES at runtime,
long after the build passed.
PAYLOAD_SECRET uses the same :? form as AIRFLOW_ADMIN_PASSWORD: refuse
to start rather than boot with an empty secret and accept forged
sessions. Staging's must differ from production's, which the header
comment now says explicitly. Portainer prefixes volume names per stack,
so payload_media isolates itself.
prodMigrations is not wired yet — generating the initial migration needs
a reachable Postgres. Follows in its own commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017YmbBhr8s7GusjDE12hrZM
X-Robots-Tag rather than the robots.txt Disallow alone, for the same
reason the staging rule uses one: a Disallow blocks crawling, not
indexing, so a URL found from an external link can be indexed without
ever being fetched — and blocking the crawl means the noindex is never
seen. Both mechanisms are applied to /admin and /cms-api.
The existing CSP is frame-ancestors only, which restricts who may embed
the site rather than what a page may load, so it cannot break the panel.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017YmbBhr8s7GusjDE12hrZM
Payload 3.88 runs inside the Next app against the existing Postgres, in
its own 'payload' schema so no pipeline operation on public — the app
tables, Airflow's metadata, migrate_csv_to_db.py --drop — can reach blog
content.
Its REST API is mounted at /cms-api. /api is the FastAPI proxy's
catch-all, which would swallow every admin call and forward it to the
backend with no error. The mount points live in lib/payloadRoutes.ts so
there is one definition and a test can assert it without importing
Payload: it is ESM-only, next/jest will not transform it, and appending
transformIgnorePatterns cannot un-ignore a package. Forcing it through
transpilePackages would change how the production build bundles Payload
to serve a test, so the live proof that /api still reaches FastAPI stays
where it belongs — the e2e journeys, which call /api/schools.
The package becomes ESM ("type": "module"), which Payload's CLI requires:
richtext-lexical has top-level await and the config cannot be require()d.
Only two files needed renaming, jest.config.cjs and a build script.
The build is verified to succeed with DATABASE_URL and PAYLOAD_SECRET
both unset, which is how CI builds it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017YmbBhr8s7GusjDE12hrZM
Payload's admin panel ships its own root layout rendering html/body.
Next allows multiple root layouts only when no app/layout.tsx exists, so
the site's routes move into their own group. Route groups are invisible
to routing: every public URL is unchanged, verified against the build's
route table.
The metadata file conventions deliberately stay at the app/ root. Moving
them into the group renamed /icon.png to /icon-4usi79.png (likewise
apple-icon and opengraph-image) and dropped /robots.txt altogether,
which would have broken the /icon.png cache-control rule, the
outputFileTracingIncludes entry for the share card, and robots.txt.
darkThemeSafety reads app/globals.css off disk rather than importing it,
so it needed its own path fix — a grep for import specifiers misses it,
and it fails as an unrunnable suite rather than a failed assertion.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017YmbBhr8s7GusjDE12hrZM
withPayload() is ESM-only, so next.config.js has to become .mjs. That
file also carries the rule that keeps staging out of Google's index, so
the conversion goes in on its own, behind a test that asserts the rule
survived — along with the standalone output, the opengraph-image font
tracing and the analytics frame-ancestors CSP.
Jest resolves the .mjs config without extra configuration, so
jest.config.js is untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017YmbBhr8s7GusjDE12hrZM
Nine tasks, each ending in an independently testable deliverable.
Two structural findings that the spec did not anticipate, both recorded
in the plan. Payload's admin panel ships its own root layout rendering
html/body, and Next allows multiple root layouts only when no
app/layout.tsx exists — so every existing route moves into an
app/(frontend) route group first, on its own, with the full suite as the
gate. Route groups are invisible to routing, so no public URL changes.
The second finding corrects the spec: adding /cms-api to the FastAPI
proxy's exclusion list would be dead code, because that catch-all only
ever matches /api/*. The route remap alone is sufficient.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017YmbBhr8s7GusjDE12hrZM
The site reads as synthetic because nobody is accountable for the
numbers, no editorial judgement is visible, and the voice is
institutional third person. This designs the fix: a named author
(first name, photo, explicitly not an education expert), a coded
/about page, and a blog backed by Payload CMS running inside the
existing Next app.
Also fills a hole in the SEO programme, which has eight workstreams
and no E-E-A-T or authorship signal on a YMYL corpus.
Records two collisions found while designing, both of which fail
badly if missed: Payload's default /api route fights the existing
FastAPI catch-all proxy, and withPayload() is ESM-only so
next.config.js — which carries staging's noindex header — has to
become next.config.mjs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FT1Ls4GbgLDXoQX7NAuHGT
The annual DAG died with a BrokenPipeError from Meltano's log writer, which
is several frames from the cause: target-postgres exited first and the tap
saw its stdout close.
The tap declared primary_keys = [urn, ...] while emitting urn=None for the
national rows, and target-postgres turns primary_keys into a NOT NULL
constraint. The first national row of the run failed the insert and took
the loader with it. Every other tap in this repo keys on non-null columns.
Carrying two grains in one stream was the actual mistake, so the fix is to
separate them rather than paper over the null: four streams now, with
ees_ks4/ks5_destinations_national carrying no urn column at all — a school
identifier that is null in every row is a grain mismatch, not a column.
The staging models split the same way and the national mart reads the new
pair instead of filtering `where urn is null`.
Verified against the live API: the school stream yields 135,240 rows over
4,508 schools with no duplicate keys, no null key columns and all 31,382
suppression sentinels intact; the national streams yield 30 and 33 rows
with no urn column.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
The simple auth manager generates a random password on first start and
writes it to a file, so every restart of the api-server invalidated the
last one and the password had to be dug out of the container logs again.
The stack now writes that file itself from AIRFLOW_ADMIN_PASSWORD before
exec'ing the api-server. Airflow generates nothing when the file already
exists, so the login is whatever the stack environment says it is.
Written with python rather than echo, so json.dumps escapes a password
containing quotes, backslashes or non-ASCII correctly — verified against
`p@ss "wo\rd' £5`, which round-trips intact.
An unset AIRFLOW_ADMIN_PASSWORD raises KeyError and the container exits.
Falling back to a generated password would silently undo the point of the
change, and a compose-level `:?` gives the same refusal a readable reason.
This does mean the variable MUST be set in Portainer before the next
deploy of either stack.
Not affected by the two Docker gotchas in the upstream docs: this image
has no USER directive so it runs as root, and the file is rewritten from
the environment on every start rather than persisted on a volume.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
meltano install would resolve it from pip_url, but five of the six custom
taps are also installed explicitly and a new plugin failing to appear is
not something you want to debug from a deploy log.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
The Share column keyed off `percentage === null`, which is true for
not_applicable as well as suppressed, so a destination that does not apply
to the school was labelled as one DfE withheld — while the Pupils column
in the same row rendered blank. Two columns, one row, disagreeing about
what the row was, and one of them making a claim about DfE that wasn't
true.
Both columns now derive from `status`, which is the distinction the mart,
the SQLAlchemy model and the serialiser all preserve deliberately:
published shows the figure, suppressed shows the withheld badge,
not_applicable shows an em-dash with a title saying so.
A published count with no published percentage now derives its share from
the cohort rather than falling through to a marker — both halves are
published, so nothing withheld is involved, and it is the same derivation
the bar widths already use.
Verified the new tests fail against the old logic before keeping them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
Review found _mask_for_disclosure could return with its invariant broken
and say nothing. add_companion only ever withheld a *published* cell, so a
group with one suppressed category and every other one not_applicable —
routine in special schools and AP, where few categories apply — left the
loop with the lone suppressed cell still solvable. Reproduced on a
nine-pupil cohort: one hidden cell, cohort served, residual intact.
A disclosure-control pass that fails silently is worse than none, because
everything downstream trusts it. The loop now runs until the invariant
holds and escalates when no companion exists: the pupil group is dropped
from the payload, and an empty block serialises as None so the section is
absent rather than an empty shell. disclosure_invariant_holds() is exported
so tests assert it directly instead of re-deriving it, and an exhaustive
test sweeps all 81 suppression patterns of a four-category group.
Also fixes a test that set up six measures and checked one: the loop was
`for measure in ["school_sixth_form"]`. It now checks every measure, and
against the real invariant — none hidden, or at least two, rather than
"at least two", which the five published measures would have failed.
No regression on real data: 262 mainstream secondaries, all-pupils bar
still drawable on 94%, zero invariant violations, one disadvantaged group
dropped by the new escalation.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
Code review found the disclosure the whole design was meant to prevent.
R1 was written as a rendering rule and implemented as one: canRenderBar
stopped the bar being drawn, but GET /api/schools/{urn} still carried the
cohort and every published category. cohort - sum(published) returned
Whitley Bay's withheld further-education figure exactly — 18 pupils — to
any caller, and the RSC payload put it in the browser too.
app.py already stated the principle for admission_distance: this endpoint
is public and unauthenticated, so a field left in the payload is a
published field. The same reasoning applies here and did not get applied.
_mask_for_disclosure now closes both identities before serialisation —
categories sum to the cohort, and disadvantaged + other = all — by adding
secondary suppression until every row and column hides none or at least
two. My first attempt picked the smallest published cell as the companion
and a new test caught it choosing a zero, which protects nothing: the
residual still resolved to 18. The companion must carry pupils.
DfE's own aggregates are no longer served. Nothing rendered them, and one
spanning a single suppressed component names it.
Cost, measured over 262 mainstream secondaries: the all-pupils bar
survives on 94% rather than 100%. Zero lone-suppressed groups remain.
The e2e helper now tells a missing feature apart from missing data: it
fails if the API serves no destinations key at all, and skips if the key
is served but the annual DAG has not populated the marts. Failing on the
second would redden the staging gate for unrelated commits.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
The helper throws rather than skipping when no school returns a
destinations block: a silent skip would let a real regression in the
sections ride along unnoticed, which is why the distance journeys were
changed the same way in 4f01fbd.
The disadvantaged journey computes the residual itself and asserts it
appears nowhere on the page — the one number the section must never state.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
The 'Post-16 destination data coming soon' note is deleted rather than
reworded: for a school with no sixth form the truthful statement is that
the question does not apply, and a placeholder there implies something is
missing. hasSixthForm and .sixthFormNote go with it — nothing else used them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
Question cards over one bar, with the cards acting as a lens on the bar
rather than a summary beside it — focusing a card dims everything it is
not made of, so the grouping we chose is inspectable rather than asserted.
The bar renders only when canRenderBar allows it. Where a category is
withheld the section says so and shows the table instead: the categories
sum to the cohort, so a bar drawn from the published segments leaves a
gap whose width is the withheld figure.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
Destinations are secondary-only, so the flags go on computeSecondaryFlags
rather than computeSchoolFlags. A phase counts as present only when some
pupil group carries categories — an empty block would otherwise open a nav
entry pointing at a section that never renders.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
The serialiser carries status through and computes no totals of its own.
The only aggregates in the payload are ones DfE published itself; whether
showing one is safe depends on how many of its components are suppressed,
which the frontend decides.
The batch guard grows from six tables to eight.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
Disadvantaged and other-pupils partition the whole and the all-pupils
figure is published, so publishing both halves recovers the suppressed
one. The mask is applied in the mart rather than the API so no consumer
added later can reach an unmasked combination.
The R1 test is a warn, not an error: DfE publishes the recoverable
combination and the mart's job is to carry it faithfully. Refusing to
close the gap is the API's job and the frontend's.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
safe_numeric maps every EES sentinel to NULL, which is right for attainment
and wrong here: one of those states has to print 'withheld' and the other
has to print nothing. A status column carries the difference.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
EES writes 'c' where a figure is withheld and the categories sum to the
cohort, so counts and percentages are emitted as text with the sentinel
intact. safe_numeric must never be pointed at them.
School rows and the England reference need different establishment pins:
at national level selective schools, studios and UTCs are separate
populations rather than labels, so leaving establishment open multiplies
30 rows into 190. Two queries per period, each keeping its own level.
Verified against the live API for 2022/23: 135,240 school records over
4,508 schools, exactly 30 each, no duplicate keys, 31,382 sentinels kept.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
Activity not captured includes independent schools and moving abroad, so a
red segment would be a factual error. The hatch doubles as the secondary
encoding that rescues the neutral/blue pair, which separates at only dE 7.6
as flat fills.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
The destination categories sum to the cohort and DfE publishes the cohort
total, so a lone suppressed cell is recoverable by subtraction. canAggregate,
canRenderPublishedAggregate and canRenderBar are what stop a consumer doing
that; toBarSegments throws rather than leaving a readable gap.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
Ordered so the disclosure guards land first and everything downstream
consumes them: lib/destinations.ts, tokens, tap, staging, marts, API,
then the two sections and the journeys.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
tudor
