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
The published files suppress individual cells, not whole cohorts, and the
categories sum to the cohort — so on 22% of mainstream secondaries the
withheld figure can be recovered by subtraction. Three disclosure rules
fall out of that, and the rest of the design is downstream of them.
Verified against the EES API rather than assumed: both datasets carry
school-level rows keyed by URN, with the disadvantage split and every
destination category the display needs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvdDKvFFSZuMVDH5fEyTob
Code review, both findings valid.
The design doc claimed Cloudflare "replaces the header, so a browser
cannot forge it", and that only the X-Forwarded-For fallback was
forgeable. That is true only for traffic that actually passed through
Cloudflare, and nothing in this process can verify that it did. Reaching
the origin directly, both headers are equally attacker-controlled — and
rotating CF-Connecting-IP mints a fresh rate-limit bucket per request,
defeating per-client limits on every endpoint including the
DataFrame-heavy /api/schools. Against abuse that is worse than the
shared bucket it replaced, which at least capped everyone together.
So the ceiling comes back. I dropped it earlier arguing it belonged at
Cloudflare; that argument assumed the keying was sound, and it is not.
GlobalRateLimitMiddleware counts all /api/ traffic in a fixed window
against a total, independent of client identity, outermost so it refuses
before any work happens. Written by hand because slowapi cannot express
a global cap: default_limits and application_limits are both keyed by
key_func, and the latter needs middleware this app does not install.
It does not make the header trustworthy — it makes trusting it
survivable. The real fix is Authenticated Origin Pulls or an origin
firewall, now documented in DEPLOY.md as the open gap it is.
127.0.0.1 is exempt: the healthcheck curls localhost from inside the
container, and starving it would restart the container and turn a load
spike into an outage loop. Keyed on the peer address, never the Host
header, which the caller sets.
Second finding: suggest_schools_typesense promised "never raises" while
the parsing loop sat outside the try, so int(None) on a malformed
document would have made a keystroke a 500. The loop now skips bad rows
rather than dropping the whole list — and a hit with no document no
longer becomes a suggestion pointing at /school/0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
The runbook said a flag 'appears in the Unleash UI after the backend has
evaluated it once'. That is wrong. SDKs read definitions from the server
and never register anything, and metrics for an unknown flag are
discarded — so a declared flag is evaluated on every request, stays
False forever, and never shows up until someone creates it by hand.
Found the way these things usually are: staging had been running the
flag code for a while and the UI was still empty.
Also names the environment trap while here — each stack's token is
scoped to one environment, so toggling the other does nothing visible
and looks like the flag is broken.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
Self-review caught three defects in the plan. .omniBoxContainer, the
wrapper the dropdown positions against, does not declare position:
relative — without it the list anchors to the page. The postcode
suppression test typed character by character, so it would have asserted
no request while 'NW1' legitimately fires one; it now sets the value in
one go. And the Enter-submits-search assertion needed waitFor, because
updateURL pushes inside startTransition.
Task 1 is the one to review hardest: it is the only unflagged change and
it alters rate limiting for every endpoint.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
Reading slowapi rather than assuming: default_limits and
application_limits are both evaluated with the same key_func, so they
are per-client across routes, not global. And application_limits only
apply 'if in_middleware' — this app installs no SlowAPIMiddleware, so
they would never have fired at all.
A genuine global cap would need a second Limiter with a constant key
plus that middleware. Cloudflare is already in the path on both
environments and does this at the right layer, so the ceiling is named
as a follow-up there rather than built badly here.
The risk that leaves is stated plainly in the risks section instead of
being papered over with a mechanism that does not do the job.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
The load-bearing finding is not about autosuggest. The rate limiter keys
on request.client.host, which in staging and prod is the Next container
— so all browser users share one 60/min bucket per route. Measured
against staging: 70 concurrent requests gave exactly 60 x 200 and
10 x 429. Eight concurrent searchers would 429 the site once each
keystroke costs a request, so the keying fix is part of this work.
Both environments are behind Cloudflare, which sets CF-Connecting-IP and
overwrites any client-supplied value — trustworthy in a way a parsed
X-Forwarded-For chain is not, and the backend is unreachable except
through the Next proxy.
Named honestly: the shared bucket has been an accidental global throttle
on a single-process backend, so correct per-user keying removes a
protection. A global ceiling ships with it rather than instead of it.
Suggestions come from Typesense alone. The existing search path filters
a 25,000-row DataFrame per query, which is exactly the cost a keystroke
endpoint cannot pay, so there is deliberately no DataFrame fallback.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
Its own Portainer stack, belonging to neither application stack: a
staging redeploy must not be able to disturb production's flag state.
One instance serves both. OSS Unleash ships development and production
environments with environment-scoped client tokens, so the same flag
holds independent state in each — which is what lets a feature be on in
staging, where the E2E journeys exercise it, while production stays dark.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
Task 1 is the Unleash stack and ends with a human step — the Portainer
deploy and the token generation cannot be automated from here. Nothing
else blocks on it: an unset UNLEASH_URL means every flag is False, which
is what local development and CI get, so the whole suite runs without a
flag server existing.
Self-review caught three defects in the plan itself. get_supplementary_data
takes (db, urn), not (urn), and the test DataFrame was minimised to the
point where the endpoint would have failed for reasons unrelated to
flags — both now copy the known-good shape from test_school_details.py.
The proxy test needs the node jest environment, since NextRequest wants
Fetch API globals jsdom does not provide. And the e2e off-state check
hardcoded a URN, so a 404 page would have satisfied it without proving
anything.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
Next uses the LOWEST revalidate among a route's fetches, not the segment
value. School pages fetch school details at 300s and place pages fetch
national averages at 3600s, so the effective ISR period is five minutes
and one hour respectively — not the seven days the segment declares.
A flag flip therefore propagates on its own, well inside the monthly,
by-hand cadence these flags are for. That deletes two webhook
integrations, a revalidate route, a secret-in-query-string scheme, an
idempotency requirement, and the rule that every fetch carry a cache
tag — which was the part most likely to rot as fetches are added.
Two constraints survive: a flag must never gate content on a
force-static page, because app/admissions never revalidates; and a
route-family flag must rebuild the sitemap, deferred with the route case
since no flag in scope touches it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
Unleash self-hosted in its own Portainer stack, with FastAPI holding the
only SDK and Next reading flags through a tagged fetch.
The two hard parts are consequences of putting flag state in a service
rather than the repo: main stops being the whole truth about what is on,
and a flag can now change without the deploy that would have cleared the
caches. A code-declared registry bounds the first; webhook-driven
revalidateTag handles the second.
Cache tagging is deliberately coarse — every server fetch carries the
flags tag, not just the flags fetch itself. The first consumer proves
why: admission_distance changes the shape of /api/schools/{urn}, so a
narrow purge would leave ~25,000 school pages serving the pre-flip
render for a week, invisibly.
First consumer is the last-distance-offered feature, which is on main
and staging and has never reached production. It needs one gate, at the
API, because the frontend already no-ops on a missing field.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
Every generateStaticParams is gated behind PRERENDER_PLACES and wrapped in the
same try/catch the school route uses. The plan claimed authority pages were
'few enough to always prebuild' — but few enough still means the API must be
reachable at build time, and in CI it is not: the build failed with
ECONNREFUSED rather than degrading to ISR.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
Two namespaces because 67 town names collide with an authority name and
neither set contains the other — postal towns cross authority boundaries, so
Bedford the town holds 104 schools against the authority's 86.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
Seven tasks: the place registry, London localities and outcodes, the places
API, per-family sitemaps, the shared place view, the four route families, and
structured data plus the e2e gate.
Two things the plan corrects against the spec. The backend image does not
contain pipeline/, so the curated locality list cannot live only in a dbt
seed — it follows the gias_codes.py precedent instead, canonical in backend
with the seed as a mirror. And NationalAverages is nested by phase rather than
flat, which the first draft read wrongly and would have rendered every page
without its England comparison.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
Supersedes the original spec's W2. The Search Console baseline inverted its
ordering: every measured location query is town or district level, none is an
administrative area, and phase is part of the query rather than a filter.
Two problems the original design did not anticipate. 67 viable towns share a
name with a local authority, and the authority is the larger set in only 43 of
them — postal towns cross authority boundaries, so neither can absorb the
other. Two namespaces resolve it by construction. And the GIAS town field
collapses 1,819 London schools into one value, which a curated
locality-to-outcode seed solves without new ingestion.
Sizing is measured against the live 25,185-school corpus rather than
estimated: 783 viable towns, 1,760 outcodes, 154 authorities.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
79 remote branches deleted: 77 fully merged into main, plus
feat/seo-crawl-hygiene and feat/england-only-corpus, whose content is
preserved on feat/seo-crawl-hygiene-main (PR #110).
Each line carries the SHA, so any branch can be restored with
git push origin <sha>:refs/heads/<name>
The 14 branches left standing all carry content that differs from main and
none of them is mine to judge abandoned.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
Next only treats a whole bracketed path segment as dynamic, so the planned
app/sitemap-[...parts]/route.ts would have been read as a literal static
folder and never matched. Children move under /sitemaps/.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
Five tasks: one canonical host, canonicals on every route, noindex on
parameterised comparisons, a sitemap that drops dataless schools and invented
priorities, and a per-family sitemap index.
Planning turned up a fault the spec had missed: the apex 301s to www, but
metadataBase, the school-page canonical, robots.txt's Sitemap: line and every
sitemap <loc> named the apex. Task 1 fixes it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
GIAS ships the whole UK plus overseas and offshore establishments. None of
them carry comparable DfE performance data — Wales does not publish on the
English measures at all — so every one of these pages rendered with null
results, null Ofsted and null phase. There were 2,036 of them: 1,569 Welsh,
123 offshore (Jersey, Guernsey, Isle of Man, Gibraltar), 316 British schools
overseas and 28 service children's schools. All 2,036 were being submitted to
search engines, alongside 29 local authorities that existed in the filters
purely to list them.
Filter at the mart boundary rather than the view layer. dim_school and
dim_location both exclude TypeOfEstablishment in {25, 26, 30, 37}, listed once
as vars.non_england_school_type_codes. Everything downstream reads those two
marts — search, the school page, /api/filters, rankings, Typesense and
build_sitemap() — so one filter removes them from the site and the sitemap
together, and Typesense drops them on its next rebuild since it recreates the
collection and swaps the alias rather than upserting in place.
coalesce rather than a bare NOT IN: a null type code would make the predicate
null and drop the row silently, and an unknown type is not grounds for
exclusion. No establishment has a null type today, but a future GIAS refresh
could ship one and the loss would be invisible.
assert_england_only_schools guards both directions: no excluded type survives
in dim_school, and dim_location holds no URN dim_school lacks — the API
inner-joins them, so the two filters drifting apart would silently shrink the
corpus.
Corpus goes from 27,229 schools to 25,193, and the authority list from 182 to
153. The 1,569 Welsh URLs now 404.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
Two unrelated working-tree changes, committed as one at the owner's request.
Removes "Built for parents, by a parent." and its comment from the hero. It
was added two commits ago; taking it out is the owner's call, and the reasons
it was placed under the search rather than in the footer no longer apply.
The .heroByline rules in HomeView.module.css are deliberately left in place.
They are now unreferenced, but the class was purpose-built for this one line
and keeping it makes restoring the byline a one-line change. If the removal is
permanent, that block (and its 640px media query) should go with it.
Also updates two quoted figures in the 2026-07-02 UX audit notes from
"24,000+" to "27,000+".
Worth noting for the record: that file documents what the page said when it was
audited, and at that time it genuinely did say "24,000+" — which was itself the
bug later fixed by reading unique_schools instead of a field the API never
sent. Editing the quoted evidence makes the note read consistently with the
current site, at the cost of no longer being a verbatim record of what was
observed. Left as the owner edited it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nine tasks mapping to the spec's commit sequence, each ending in an
independently testable deliverable.
Also corrects the spec's section-sharing table: measured similarity shows
Admissions (14%) and History (40%) are not shareable between the two views,
only Ofsted (80%) and Finances (91%). This costs no bytes -- the win comes
from sections being server components, not from sharing them -- and cuts the
diff and regression risk.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Planning surfaced a gap: the two detail views' CSS modules share 79 class
names, 22 with differing rules, 16 of those used in section markup by both
views. Shared sections cannot use one stylesheet without visual change.
Resolution: union the 14 incidental-drift differences (defensive overflow
properties never back-ported between the views), and keep the 2 genuine
visual differences (.genderBar, .heroStatValue) as distinct classes behind a
variant prop. Adds an isolated CSS-merge commit so a visual regression
bisects to the merge rather than a JSX move.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Design for splitting SchoolDetailView (65KB) and SecondarySchoolDetailView
(46KB) into server section components behind a small client shell, following
the existing components/compare/ decomposition pattern.
Key constraint: a server component imported by a client component becomes
client, so sections are composed in page.tsx and passed through the shell as
children. Moving /api/national-averages server-side is a prerequisite, since
the England-comparison deltas feed most sections.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The final review claimed DfE published school-level 2021/22 KS2 in Dec
2022. Re-verified: the GOV.UK announcement 'Primary school performance
tables: 2022' is CANCELLED ('will not be published in key stage 2
performance tables in academic year 2021/22'), and the EES 2021/22
release carries the same statement. No code change.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB
Removes the 'What Parents Say' section and all supporting elements:
Frontend:
- Drop the OfstedParentView type, the parent_view field, the survey
section and the 'X% would recommend' callouts in the primary and
secondary detail views, the Parents nav item, and the parent-view CSS.
Backend:
- Remove the FactParentView model, its loading in data_loader, and
parent_view from the school-details API response.
- Bump SCHEMA_VERSION to 6 and add an idempotent drop step
(DROP TABLE IF EXISTS marts.fact_parent_view) to the CLI migration;
add scripts/sql/drop_fact_parent_view.sql to apply directly to the
dbt-owned marts DBs on staging and prod.
Pipeline:
- Delete the stg_parent_view + fact_parent_view dbt models and their
source/schema entries, the tap-uk-parent-view Meltano extractor, and
the monthly Parent View DAG; drop it from the Dockerfile and the
staging bootstrap docs.
The rest of dbt (which builds every mart the app reads) is untouched.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- ai_review.py now pipes the diff through headless Claude Code (claude -p,
--output-format json) authenticated with CLAUDE_CODE_OAUTH_TOKEN from
'claude setup-token' — subscription auth, no Anthropic API billing
- stdlib-only script (urllib instead of requests/anthropic)
- PR comments posted with the existing REGISTRY_TOKEN secret; the separate
GITEA_TOKEN secret is no longer needed
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PqGhF93UrpDNvXBLMjJENL
- pr-checks.yml: PR gate — frontend typecheck+jest, backend import smoke,
image builds (no push), Claude AI review posted as PR comment (severe
findings block merge)
- deploy.yml (replaces build-and-push.yml): merge to main builds+pushes
images tagged sha-<sha>/staging, deploys the staging Portainer stack via
webhook, runs Playwright E2E journeys against staging, then retags the
verified images :prod (previous kept as :prod-previous) and deploys prod
- docker-compose.portainer.staging.yml: second Portainer stack — :staging
images, sc_staging_* names, own macvlan IPs, Airflow on 8081; data
bootstrapped from source via the staging Airflow DAGs
- prod compose now pins :prod instead of :latest (only the promotion step
moves it; :latest is no longer published)
- e2e/: 6 Playwright journeys (search, postcode, detail, compare, rankings)
driven by BASE_URL — the promotion gate
- scripts/ci/ai_review.py: Claude review with structured JSON findings
- docs/DEPLOY.md: full SDLC doc incl. one-time setup checklist and rollback
- replaced removed 'next lint' with tsc typecheck; fixed stale jest tests
(slug URLs, N/A formatting, stable trend, fake-timer setup)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PqGhF93UrpDNvXBLMjJENL
Recheck with 'B91 3DL' via the search box shows the geocoded radius
search works (distances, nearest-first, radius selector, map toggle).
Original P0.3 tested only a partial postcode, which silently falls back
to text search — refiled as P2.0 (silent mode switch on partial input).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
tudor