Compare commits

..
Author SHA1 Message Date
TudorandClaude Fable 5 9f8dba227c fix(compare): keep chart data when a client refetch fails
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 9m38s
PR Checks / Backend Smoke (pull_request) Successful in 5s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 47s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 9s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 1m9s
The compare view refetches /api/compare on the client after SSR; on any
failure the catch nulled comparisonData, destroying the working
SSR-provided chart. A transient error (or staging's broken external /api
proxy) should not blank a comparison the user is already viewing — keep
the existing data instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-05 23:08:22 +01:00
tudor 18cd805c6c Merge pull request 'feat(compare): readable comparison chart on mobile' (#11) from feat/compare-chart-mobile-readability into main
Deploy (staging -> E2E gate -> production) / Build Backend (FastAPI) (push) Successful in 13s
Deploy (staging -> E2E gate -> production) / Build Frontend (Next.js) (push) Successful in 49s
Deploy (staging -> E2E gate -> production) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 12s
Deploy (staging -> E2E gate -> production) / Deploy to Staging (push) Successful in 1s
Deploy (staging -> E2E gate -> production) / E2E Journeys against Staging (push) Failing after 1m6s
Deploy (staging -> E2E gate -> production) / Promote to Production (push) Has been skipped
Reviewed-on: #11
2026-07-05 21:26:01 +00:00
TudorandClaude Fable 5 22769b6295 feat(compare): readable comparison chart on mobile
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 9m36s
PR Checks / Backend Smoke (pull_request) Successful in 5s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 44s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 2m49s
The compare chart squashed clustered schools into a thin band (y pinned
0-100) under an in-chart title + per-school legend that ate ~40% of a
300px card, leaving converging lines indistinguishable on phones.

- Auto-fit the y-axis to the data on all viewports (computeYBounds in
  lib/utils: padded + min-span for percentages, symmetric around 0 for
  progress, fitted for scores; negative pct-named trend metrics are not
  zero-clamped).
- Distinct point style per school (circle/triangle/rect/rectRot/star)
  as secondary encoding for convergence and colour-blindness.
- Mobile: drop in-chart title/legend/axis titles; add a chip row (colour
  dot + name) that doubles as tap-to-focus — highlights one school's
  line and dims the rest. Chart card 300px -> 340px, nearly all plot.
- Fix a latent colour mismatch: datasets were built from Object.entries
  whose integer-like URN keys enumerate in ascending numeric order,
  desyncing line colours from card colours; the chart now receives the
  ordered school list.
- Union years across schools instead of taking the first school's.
- Extract PerformanceChart's matchMedia pattern into hooks/useIsMobile.

Unit tests for metricKind/computeYBounds; e2e journey covers the mobile
chips and focus toggle.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-05 22:02:26 +01:00
tudor 90f2a02e75 Merge pull request 'fix(school): make hero map fullscreen work on iOS Safari' (#10) from fix/hero-map-ios-fullscreen into main
Deploy (staging -> E2E gate -> production) / Build Backend (FastAPI) (push) Successful in 13s
Deploy (staging -> E2E gate -> production) / Build Frontend (Next.js) (push) Successful in 52s
Deploy (staging -> E2E gate -> production) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 12s
Deploy (staging -> E2E gate -> production) / Deploy to Staging (push) Successful in 1s
Deploy (staging -> E2E gate -> production) / E2E Journeys against Staging (push) Successful in 34s
Deploy (staging -> E2E gate -> production) / Promote to Production (push) Successful in 8s
Reviewed-on: #10
2026-07-05 20:52:32 +00:00
TudorandClaude Fable 5 d52d384cf2 fix(school): make hero map fullscreen work on iOS Safari
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 9m39s
PR Checks / Backend Smoke (pull_request) Successful in 5s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 47s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 9s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 4m15s
iOS Safari has no Element.requestFullscreen (fullscreen is video-only),
so tapping the map band or 'View on map' silently did nothing on
iPhones. Fall back to a fixed-position CSS overlay driven by state when
the Fullscreen API is missing or its promise rejects, locking body
scroll while open. The Leaflet map already re-measures via the shared
isFullscreen flag. New e2e journey simulates the iOS condition by
deleting the API and asserts the overlay opens and closes; it fails
against the current production build.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-05 21:34:22 +01:00
tudor ff606dad71 Merge pull request 'fix(e2e): pick the latest explicit year in the rankings year test' (#8) from fix/e2e-rankings-year-pick into main
Deploy (staging -> E2E gate -> production) / Build Backend (FastAPI) (push) Successful in 12s
Deploy (staging -> E2E gate -> production) / Build Frontend (Next.js) (push) Successful in 52s
Deploy (staging -> E2E gate -> production) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 12s
Deploy (staging -> E2E gate -> production) / Deploy to Staging (push) Successful in 1s
Deploy (staging -> E2E gate -> production) / E2E Journeys against Staging (push) Successful in 34s
Deploy (staging -> E2E gate -> production) / Promote to Production (push) Successful in 10s
Reviewed-on: #8
2026-07-05 13:54:55 +00:00
TudorandClaude Fable 5 acec8135e1 fix(e2e): pick the latest explicit year in the rankings year test
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 9m39s
PR Checks / Backend Smoke (pull_request) Successful in 5s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 41s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 9s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 1m24s
Staging doesn't always carry the full data history, so selecting the
oldest year legitimately returns no rows and fails the promotion gate.
Select the most recent explicit year instead: the default view already
proved it has rows, so an empty table after selecting it can only mean
the year query param was rejected — the regression this test guards.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-05 14:29:43 +01:00
tudor 0a370e3b63 Merge pull request 'fix(api): accept academic-year codes in rankings year filter' (#7) from fix/rankings-year-validation into main
Deploy (staging -> E2E gate -> production) / Build Backend (FastAPI) (push) Successful in 21s
Deploy (staging -> E2E gate -> production) / Build Frontend (Next.js) (push) Successful in 46s
Deploy (staging -> E2E gate -> production) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 12s
Deploy (staging -> E2E gate -> production) / Deploy to Staging (push) Successful in 1s
Deploy (staging -> E2E gate -> production) / E2E Journeys against Staging (push) Failing after 1m3s
Deploy (staging -> E2E gate -> production) / Promote to Production (push) Has been skipped
Reviewed-on: #7
2026-07-05 10:46:04 +00:00
TudorandClaude Fable 5 6c872ce726 fix(api): accept academic-year codes in rankings year filter
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 9m36s
PR Checks / Backend Smoke (pull_request) Successful in 5s
PR Checks / Build Backend (no push) (pull_request) Successful in 20s
PR Checks / Build Frontend (no push) (pull_request) Successful in 52s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 1m44s
The rankings endpoint validated year with le=2100, but the database
stores academic-year codes like 201819, so any explicit year selection
returned a 422 and the rankings page rendered its empty state. Widen
the bound to cover the codes and extend the e2e journey to pick a
specific year and assert the table stays populated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-04 22:13:56 +01:00
tudor 23b4e1c453 Merge pull request 'fix(e2e): scroll detail-page chart into view before asserting' (#5) from fix/e2e-detail-chart-scroll into main
Deploy (staging -> E2E gate -> production) / Build Backend (FastAPI) (push) Successful in 13s
Deploy (staging -> E2E gate -> production) / Build Frontend (Next.js) (push) Successful in 49s
Deploy (staging -> E2E gate -> production) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 12s
Deploy (staging -> E2E gate -> production) / Deploy to Staging (push) Successful in 1s
Deploy (staging -> E2E gate -> production) / E2E Journeys against Staging (push) Successful in 33s
Deploy (staging -> E2E gate -> production) / Promote to Production (push) Successful in 8s
2026-07-03 13:56:00 +00:00
TudorandClaude Fable 5 deeef23131 fix(e2e): assert on a visible canvas — the first canvas is hidden by design
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 9m41s
PR Checks / Backend Smoke (pull_request) Successful in 5s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 47s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 26s
The admissions card stacks year/trend views in one grid cell and keeps the
inactive view visibility:hidden; its canvas is first in the DOM. Use
canvas:visible instead of scrolling. Verified: full suite passes against prod.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PqGhF93UrpDNvXBLMjJENL
2026-07-03 14:40:13 +01:00
TudorandClaude Fable 5 4ece55b031 fix(e2e): scroll the detail-page chart into view before asserting visibility
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 9m41s
PR Checks / Backend Smoke (pull_request) Successful in 5s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 55s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 34s
The canvas renders below the fold and stays 'hidden' to Playwright until
scrolled to; wait for attachment, scroll, then assert.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PqGhF93UrpDNvXBLMjJENL
2026-07-03 14:34:17 +01:00
tudor 515494dbf0 Merge pull request 'fix(analytics): restrict Umami to production hostnames' (#4) from fix/umami-prod-domains-only into main
Deploy (staging -> E2E gate -> production) / Build Backend (FastAPI) (push) Successful in 13s
Deploy (staging -> E2E gate -> production) / Build Frontend (Next.js) (push) Successful in 50s
Deploy (staging -> E2E gate -> production) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 12s
Deploy (staging -> E2E gate -> production) / Deploy to Staging (push) Successful in 0s
Deploy (staging -> E2E gate -> production) / E2E Journeys against Staging (push) Failing after 1m7s
Deploy (staging -> E2E gate -> production) / Promote to Production (push) Has been skipped
2026-07-03 13:17:53 +00:00
TudorandClaude Fable 5 5772c54ccd fix(ci): use the documented GITHUB_TOKEN name for the run-scoped token
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 9m37s
PR Checks / Backend Smoke (pull_request) Successful in 5s
PR Checks / Build Backend (no push) (pull_request) Successful in 11s
PR Checks / Build Frontend (no push) (pull_request) Successful in 45s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 9s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 1m27s
GITEA_TOKEN worked (the review comment posted with it) but GITHUB_TOKEN is
the documented name in Gitea Actions; use it to keep the reviewer and the
docs aligned.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PqGhF93UrpDNvXBLMjJENL
2026-07-03 13:53:57 +01:00
TudorandClaude Fable 5 c62ba0ca25 fix(ci): post AI review comments with the run-scoped Gitea token
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 9m37s
PR Checks / Backend Smoke (pull_request) Successful in 5s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 45s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Failing after 1m56s
REGISTRY_TOKEN lacks issue-write scope (403 on comment post). Gitea Actions
auto-provides a repo-scoped per-run token as secrets.GITEA_TOKEN — no
user-managed secret needed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PqGhF93UrpDNvXBLMjJENL
2026-07-03 13:39:59 +01:00
TudorandClaude Fable 5 b5a63e82d4 fix(analytics): restrict Umami to production hostnames
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 9m37s
PR Checks / Backend Smoke (pull_request) Successful in 4s
PR Checks / Build Backend (no push) (pull_request) Successful in 11s
PR Checks / Build Frontend (no push) (pull_request) Successful in 48s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Failing after 36s
The same frontend image runs on staging and prod (build once, promote), so a
build-time env var can't tell them apart. Umami's data-domains attribute
scopes the tracker client-side: events only fire when location.hostname is a
production domain, so staging traffic and the E2E journeys never pollute the
dashboards.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PqGhF93UrpDNvXBLMjJENL
2026-07-03 13:24:14 +01:00
tudor f5de745a8b Merge pull request 'fix(ci): AI review via Claude Code CLI; reuse REGISTRY_TOKEN' (#3) from fix/ai-review-claude-code into main
Deploy (staging -> E2E gate -> production) / Build Backend (FastAPI) (push) Successful in 18s
Deploy (staging -> E2E gate -> production) / Build Frontend (Next.js) (push) Successful in 46s
Deploy (staging -> E2E gate -> production) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 12s
Deploy (staging -> E2E gate -> production) / Deploy to Staging (push) Failing after 1s
Deploy (staging -> E2E gate -> production) / E2E Journeys against Staging (push) Has been skipped
Deploy (staging -> E2E gate -> production) / Promote to Production (push) Has been skipped
2026-07-03 08:50:26 +00:00
TudorandClaude Fable 5 d0895c71df fix(ci): AI review via Claude Code CLI; reuse REGISTRY_TOKEN for PR comments
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 9m39s
PR Checks / Backend Smoke (pull_request) Successful in 5s
PR Checks / Build Backend (no push) (pull_request) Successful in 16s
PR Checks / Build Frontend (no push) (pull_request) Successful in 42s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 9s
PR Checks / AI Code Review (Claude) (pull_request) Failing after 27s
- 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
2026-07-03 08:28:20 +01:00
17 changed files with 513 additions and 142 deletions
+12 -5
View File
@@ -167,13 +167,20 @@ jobs:
with: with:
python-version: "3.12" python-version: "3.12"
- name: Install dependencies - name: Set up Node.js
run: pip install anthropic requests uses: actions/setup-node@v4
with:
node-version: 22
- name: Review PR diff with Claude - name: Install Claude Code
run: npm install -g @anthropic-ai/claude-code
- name: Review PR diff with Claude Code
env: env:
ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
GITEA_TOKEN: ${{ secrets.GITEA_TOKEN }} # Auto-provided per-run token from Gitea Actions (repo-scoped).
# GITHUB_TOKEN is the documented name; GITEA_TOKEN is its alias.
GITEA_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GITEA_SERVER_URL: ${{ gitea.server_url }} GITEA_SERVER_URL: ${{ gitea.server_url }}
GITEA_REPOSITORY: ${{ gitea.repository }} GITEA_REPOSITORY: ${{ gitea.repository }}
PR_NUMBER: ${{ gitea.event.pull_request.number }} PR_NUMBER: ${{ gitea.event.pull_request.number }}
+4 -1
View File
@@ -834,7 +834,10 @@ async def get_rankings(
request: Request, request: Request,
metric: str = Query("rwm_expected_pct", description="Metric to rank by", max_length=50), metric: str = Query("rwm_expected_pct", description="Metric to rank by", max_length=50),
year: Optional[int] = Query( year: Optional[int] = Query(
None, description="Specific year (defaults to most recent)", ge=2000, le=2100 None,
description="Academic year code, e.g. 201819 (defaults to most recent)",
ge=2000,
le=210100,
), ),
limit: int = Query(20, ge=1, le=100, description="Number of schools to return"), limit: int = Query(20, ge=1, le=100, description="Number of schools to return"),
local_authority: Optional[str] = Query( local_authority: Optional[str] = Query(
+8 -6
View File
@@ -57,8 +57,7 @@ fail the E2E gate. That's the point: staging absorbs the risk.
| Secret | Purpose | | Secret | Purpose |
|---|---| |---|---|
| `REGISTRY_TOKEN` | push images to privaterepo.sitaru.org (already set) | | `REGISTRY_TOKEN` | push images to privaterepo.sitaru.org (already set) |
| `ANTHROPIC_API_KEY` | Claude PR review (`scripts/ci/ai_review.py`) | | `CLAUDE_CODE_OAUTH_TOKEN` | Claude Code subscription auth for the PR review — generate with `claude setup-token` on your machine |
| `GITEA_TOKEN` | post PR review comments (needs issue-comment scope) |
| `PORTAINER_STAGING_WEBHOOK` | staging stack redeploy webhook URL | | `PORTAINER_STAGING_WEBHOOK` | staging stack redeploy webhook URL |
| `PORTAINER_PROD_WEBHOOK` | production stack redeploy webhook URL | | `PORTAINER_PROD_WEBHOOK` | production stack redeploy webhook URL |
| `STAGING_BASE_URL` | e.g. `http://10.0.1.151:3000` — health poll + E2E target | | `STAGING_BASE_URL` | e.g. `http://10.0.1.151:3000` — health poll + E2E target |
@@ -124,7 +123,10 @@ numbers, so scheduled data refreshes don't break the gate.
## AI code review ## AI code review
`scripts/ci/ai_review.py` sends the PR diff to Claude (`claude-opus-4-8`), `scripts/ci/ai_review.py` pipes the PR diff through headless Claude Code
posts the structured findings as a PR comment, and fails the check only when a (`claude -p`, authenticated with the subscription OAuth token — no API
finding is rated **severe** (would break prod, leak data, or corrupt data). billing), posts the structured findings as a PR comment using the per-run
Minor findings are informational and never block a merge. token Gitea Actions provides automatically (`secrets.GITEA_TOKEN` — no setup
needed), and fails the check only when a finding is rated
**severe** (would break prod, leak data, or corrupt data). Minor findings are
informational and never block a merge.
+77 -2
View File
@@ -43,8 +43,36 @@ test('school detail page renders name and performance data', async ({ page }) =>
await firstSchool.click(); await firstSchool.click();
await page.waitForURL(/\/school\//); await page.waitForURL(/\/school\//);
await expect(page.locator('h1').first()).toBeVisible(); await expect(page.locator('h1').first()).toBeVisible();
// The detail page renders at least one chart canvas (performance history) // The detail page renders at least one *visible* chart canvas. Plain
await expect(page.locator('canvas').first()).toBeVisible({ timeout: 15_000 }); // .first() is wrong here: the admissions card stacks its year/trend views
// in one grid cell and keeps the inactive view's canvas visibility:hidden
// by design, and that canvas comes first in the DOM.
await expect(page.locator('canvas:visible').first()).toBeVisible({ timeout: 15_000 });
});
test('school hero map opens fullscreen on mobile without the Fullscreen API', async ({ page }) => {
// iOS Safari has no Element.requestFullscreen; the map must fall back to a
// CSS overlay. Simulate that by removing the API before any page script runs.
await page.setViewportSize({ width: 390, height: 844 });
await page.addInitScript(() => {
// @ts-expect-error deliberate API removal
delete Element.prototype.requestFullscreen;
});
await searchByName(page, 'primary');
const firstSchool = schoolLinks(page).first();
await expect(firstSchool).toBeVisible({ timeout: 15_000 });
await firstSchool.click();
await page.waitForURL(/\/school\//);
const openMap = page.getByRole('button', { name: 'Open full map' });
await expect(openMap).toBeVisible({ timeout: 15_000 });
await openMap.click();
const closeMap = page.getByRole('button', { name: 'Close map' });
await expect(closeMap).toBeVisible();
await closeMap.click();
await expect(openMap).toBeVisible();
}); });
test('comparing two schools shows both side by side', async ({ page }) => { test('comparing two schools shows both side by side', async ({ page }) => {
@@ -63,6 +91,32 @@ test('comparing two schools shows both side by side', async ({ page }) => {
await expect(page.locator(`a[href*="${urns[1]}"]`).first()).toBeVisible(); await expect(page.locator(`a[href*="${urns[1]}"]`).first()).toBeVisible();
}); });
test('compare chart on mobile shows school chips with tap-to-focus', async ({ page }) => {
await page.setViewportSize({ width: 390, height: 844 });
await searchByName(page, 'primary');
await expect(schoolLinks(page).first()).toBeVisible({ timeout: 15_000 });
const hrefs = await schoolLinks(page).evaluateAll((links) =>
links.map((l) => (l as HTMLAnchorElement).getAttribute('href') || '')
);
const urns = [...new Set(hrefs.map((h) => h.match(/\/school\/(\d+)/)?.[1]).filter(Boolean))];
expect(urns.length).toBeGreaterThanOrEqual(2);
await page.goto(`/compare?urns=${urns[0]},${urns[1]}`);
await expect(page.locator('canvas:visible').first()).toBeVisible({ timeout: 15_000 });
// The mobile chart legend renders one chip per school inside the chart card.
const chipGroup = page.getByRole('group', { name: /highlight a school/i });
const chips = chipGroup.getByRole('button');
await expect(chips).toHaveCount(2);
// Tapping a chip focuses that school's line; tapping again releases it.
await chips.first().click();
await expect(chips.first()).toHaveAttribute('aria-pressed', 'true');
await chips.first().click();
await expect(chips.first()).toHaveAttribute('aria-pressed', 'false');
});
test('rankings page loads a populated table', async ({ page }) => { test('rankings page loads a populated table', async ({ page }) => {
await page.goto('/rankings'); await page.goto('/rankings');
await expect(page.getByRole('heading', { name: /rankings/i }).first()).toBeVisible(); await expect(page.getByRole('heading', { name: /rankings/i }).first()).toBeVisible();
@@ -70,3 +124,24 @@ test('rankings page loads a populated table', async ({ page }) => {
await expect(rows.first()).toBeVisible({ timeout: 15_000 }); await expect(rows.first()).toBeVisible({ timeout: 15_000 });
expect(await rows.count()).toBeGreaterThan(5); expect(await rows.count()).toBeGreaterThan(5);
}); });
test('rankings stay populated after picking a specific year', async ({ page }) => {
// Years are academic-year codes (e.g. 201819); the API must accept them
// as the `year` query param rather than rejecting with a 422.
await page.goto('/rankings');
const yearSelect = page.locator('#year-select');
await expect(yearSelect).toBeVisible({ timeout: 15_000 });
// Pick the last option — the most recent explicit year. The default view
// already proved this year has rows, so an empty table after selecting it
// can only mean the year param was rejected. (The oldest year is no good
// here: staging doesn't always carry the full data history.)
const yearValue = await yearSelect.locator('option').last().getAttribute('value');
expect(yearValue).toBeTruthy();
await yearSelect.selectOption(yearValue!);
await page.waitForURL(/year=/);
const rows = page.locator('table tbody tr');
await expect(rows.first()).toBeVisible({ timeout: 15_000 });
expect(await rows.count()).toBeGreaterThan(5);
});
+53
View File
@@ -9,6 +9,8 @@ import {
isValidPostcode, isValidPostcode,
debounce, debounce,
buildOfstedListBadge, buildOfstedListBadge,
metricKind,
computeYBounds,
} from '@/lib/utils'; } from '@/lib/utils';
describe('formatPercentage', () => { describe('formatPercentage', () => {
@@ -159,3 +161,54 @@ describe('buildOfstedListBadge', () => {
expect(badge.cssClass).toBe('ofstedPending'); expect(badge.cssClass).toBe('ofstedPending');
}); });
}); });
describe('metricKind', () => {
it('classifies metrics by key', () => {
expect(metricKind('rwm_expected_pct')).toBe('percentage');
expect(metricKind('absence_rate')).toBe('percentage');
expect(metricKind('reading_progress')).toBe('progress');
expect(metricKind('progress_8_score')).toBe('progress');
expect(metricKind('attainment_8_score')).toBe('score');
expect(metricKind('reading_avg_score')).toBe('score');
});
});
describe('computeYBounds', () => {
it('tightens clustered percentages instead of framing 0-100', () => {
const b = computeYBounds([86, 86, 86, 80, 96], 'percentage');
expect(b.min).toBeGreaterThanOrEqual(0);
expect(b.max).toBeLessThanOrEqual(100);
expect(b.min).toBeGreaterThan(50);
expect(b.max! - b.min!).toBeGreaterThanOrEqual(10);
});
it('never widens percentages beyond 0-100 for non-negative data', () => {
const b = computeYBounds([2, 5, 98], 'percentage');
expect(b.min).toBe(0);
expect(b.max).toBe(100);
});
it('does not clamp to zero when pct-named trend data is negative', () => {
const b = computeYBounds([-12, -3, 4], 'percentage');
expect(b.min).toBeLessThan(-12);
});
it('keeps progress bounds symmetric around zero', () => {
const b = computeYBounds([-1.2, 0.4, 2.1], 'progress');
expect(b.min).toBe(-b.max!);
expect(b.min).toBeLessThanOrEqual(-1.2);
expect(b.max).toBeGreaterThanOrEqual(2.1);
});
it('fits score metrics without a fixed frame', () => {
const b = computeYBounds([42.3, 48.9, 51.2], 'score');
expect(b.min).toBeGreaterThanOrEqual(0);
expect(b.min).toBeLessThanOrEqual(42.3);
expect(b.max).toBeGreaterThanOrEqual(51.2);
});
it('returns empty bounds when there is no numeric data', () => {
expect(computeYBounds([null, undefined, NaN], 'percentage')).toEqual({});
expect(computeYBounds([], 'progress')).toEqual({});
});
});
+3
View File
@@ -76,9 +76,12 @@ export default function RootLayout({
<head> <head>
<link rel="preconnect" href="https://analytics.schoolcompare.co.uk" /> <link rel="preconnect" href="https://analytics.schoolcompare.co.uk" />
<link rel="preconnect" href="https://api.postcodes.io" /> <link rel="preconnect" href="https://api.postcodes.io" />
{/* data-domains: the tracker only fires on the production hostnames,
so staging (same image, different host) never pollutes Umami */}
<Script <Script
src="https://analytics.schoolcompare.co.uk/script.js" src="https://analytics.schoolcompare.co.uk/script.js"
data-website-id="d7fb0c95-bb6c-4336-8209-bd10077e50dd" data-website-id="d7fb0c95-bb6c-4336-8209-bd10077e50dd"
data-domains="schoolcompare.co.uk,www.schoolcompare.co.uk"
data-performance="true" data-performance="true"
strategy="afterInteractive" strategy="afterInteractive"
/> />
@@ -0,0 +1,63 @@
/* Chart wrapper: chips (mobile) above, canvas filling the rest of the
parent .chartContainer, whose fixed height drives Chart.js sizing via
maintainAspectRatio: false. */
.wrapper {
display: flex;
flex-direction: column;
height: 100%;
}
.canvasBox {
position: relative;
flex: 1 1 auto;
min-height: 0;
}
/* School chips: mobile-only legend + tap-to-focus control. Desktop keeps
Chart.js's built-in legend (with per-school point shapes). */
.chips {
display: none;
}
@media (max-width: 640px) {
.chips {
display: flex;
flex-wrap: wrap;
gap: 6px;
padding-bottom: 8px;
}
.chip {
display: inline-flex;
align-items: center;
gap: 6px;
min-height: 44px;
max-width: 100%;
padding: 4px 10px;
border: 1px solid rgba(0, 0, 0, .12);
border-radius: 999px;
background: transparent;
cursor: pointer;
font-size: 12px;
font-weight: 600;
}
.chip[aria-pressed="true"] {
background: rgba(0, 0, 0, .06);
border-color: rgba(0, 0, 0, .35);
}
.chipDot {
flex: 0 0 auto;
width: 10px;
height: 10px;
border-radius: 50%;
}
.chipName {
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
max-width: 9rem;
}
}
+116 -45
View File
@@ -1,47 +1,82 @@
/** /**
* ComparisonChart Component * ComparisonChart Component
* Multi-school comparison chart using Chart.js * Multi-school comparison chart using Chart.js.
*
* Desktop: built-in legend (point-style markers double as per-school shapes).
* Mobile (≤640px): the in-chart legend and axis titles are dropped in favour
* of a chip row above the canvas; tapping a chip highlights that school's
* line and dims the rest. The y-axis auto-fits the data on all viewports so
* clustered schools stay distinguishable.
*/ */
'use client'; 'use client';
import { useEffect, useState } from 'react';
import { Line } from 'react-chartjs-2'; import { Line } from 'react-chartjs-2';
import { ChartOptions } from 'chart.js'; import { ChartOptions, ChartDataset, PointStyle } from 'chart.js';
import '@/lib/chartSetup'; import '@/lib/chartSetup';
import type { ComparisonData } from '@/lib/types'; import type { ComparisonData } from '@/lib/types';
import { CHART_COLORS, formatAcademicYear } from '@/lib/utils'; import {
CHART_COLORS,
CHART_TEXT_COLORS,
computeYBounds,
formatAcademicYear,
metricKind,
rgbToRgba,
} from '@/lib/utils';
import { useIsMobile } from '@/hooks/useIsMobile';
import { track } from '@/lib/analytics';
import styles from './ComparisonChart.module.css';
interface ComparisonChartProps { interface ComparisonChartProps {
comparisonData: Record<string, ComparisonData>; comparisonData: Record<string, ComparisonData>;
/** Ordered as displayed in the school cards, so colours match by index. */
schools: Array<{ urn: number; school_name: string }>;
metric: string; metric: string;
metricLabel: string; metricLabel: string;
} }
export function ComparisonChart({ comparisonData, metric, metricLabel }: ComparisonChartProps) { // One shape per basket slot (MAX_SCHOOLS = 5) — secondary encoding so
// Get all schools and their data // converging lines stay tellable apart without relying on hue alone.
const schools = Object.entries(comparisonData); const POINT_STYLES: PointStyle[] = ['circle', 'triangle', 'rect', 'rectRot', 'star'];
export function ComparisonChart({ comparisonData, schools, metric, metricLabel }: ComparisonChartProps) {
const isMobile = useIsMobile();
const [focusedUrn, setFocusedUrn] = useState<number | null>(null);
// A focused school that leaves the basket must not linger.
const urnKey = schools.map((s) => s.urn).join(',');
useEffect(() => {
setFocusedUrn(null);
}, [urnKey]);
if (schools.length === 0) { if (schools.length === 0) {
return <div>No data available</div>; return <div>No data available</div>;
} }
// Get years from first school (assuming all schools have same years) // Union of years across all schools — coverage differs between them.
const years = schools[0][1].yearly_data.map((d) => d.year).sort((a, b) => a - b); const years = [
...new Set(schools.flatMap((s) => comparisonData[String(s.urn)]?.yearly_data.map((d) => d.year) ?? [])),
].sort((a, b) => a - b);
// Create datasets for each school const datasets: ChartDataset<'line'>[] = schools.map((school, index) => {
const datasets = schools.map(([urn, data], index) => { const data = comparisonData[String(school.urn)];
const schoolInfo = data.school_info;
const color = CHART_COLORS[index % CHART_COLORS.length]; const color = CHART_COLORS[index % CHART_COLORS.length];
const dimmed = focusedUrn !== null && focusedUrn !== school.urn;
return { return {
label: schoolInfo.school_name, label: school.school_name,
data: years.map((year) => { data: years.map((year) => {
const yearData = data.yearly_data.find((d) => d.year === year); const yearData = data?.yearly_data.find((d) => d.year === year);
if (!yearData) return null; if (!yearData) return null;
return yearData[metric as keyof typeof yearData] as number | null; return yearData[metric as keyof typeof yearData] as number | null;
}), }),
borderColor: color, borderColor: dimmed ? rgbToRgba(color, 0.2) : color,
backgroundColor: color.replace('rgb', 'rgba').replace(')', ', 0.1)'), backgroundColor: dimmed ? 'transparent' : rgbToRgba(color, 0.1),
borderWidth: focusedUrn === school.urn ? 3 : dimmed ? 1.5 : 2,
pointStyle: POINT_STYLES[index % POINT_STYLES.length],
pointRadius: dimmed ? 2 : isMobile ? 3 : 4,
pointHoverRadius: isMobile ? 5 : 6,
tension: 0.3, tension: 0.3,
spanGaps: true, spanGaps: true,
}; };
@@ -52,9 +87,11 @@ export function ComparisonChart({ comparisonData, metric, metricLabel }: Compari
datasets, datasets,
}; };
// Determine if metric is a progress score or percentage const kind = metricKind(metric);
const isProgressScore = metric.includes('progress'); const yBounds = computeYBounds(
const isPercentage = metric.includes('pct') || metric.includes('rate'); datasets.flatMap((ds) => ds.data as Array<number | null>),
kind,
);
const options: ChartOptions<'line'> = { const options: ChartOptions<'line'> = {
responsive: true, responsive: true,
@@ -65,6 +102,7 @@ export function ComparisonChart({ comparisonData, metric, metricLabel }: Compari
}, },
plugins: { plugins: {
legend: { legend: {
display: !isMobile,
position: 'top' as const, position: 'top' as const,
labels: { labels: {
usePointStyle: true, usePointStyle: true,
@@ -74,26 +112,22 @@ export function ComparisonChart({ comparisonData, metric, metricLabel }: Compari
}, },
}, },
}, },
// No in-chart title: the section heading and metric selector above the
// chart already state the metric.
title: { title: {
display: true, display: false,
text: `${metricLabel} - Comparison`,
font: {
size: 16,
weight: 'bold',
},
padding: {
bottom: 20,
},
}, },
tooltip: { tooltip: {
backgroundColor: 'rgba(0, 0, 0, 0.8)', backgroundColor: 'rgba(0, 0, 0, 0.8)',
padding: 12, padding: isMobile ? 10 : 12,
titleFont: { titleFont: {
size: 14, size: isMobile ? 12 : 14,
}, },
bodyFont: { bodyFont: {
size: 13, size: isMobile ? 11 : 13,
}, },
usePointStyle: true,
itemSort: (a, b) => (b.parsed.y ?? -Infinity) - (a.parsed.y ?? -Infinity),
callbacks: { callbacks: {
label: function (context) { label: function (context) {
let label = context.dataset.label || ''; let label = context.dataset.label || '';
@@ -101,13 +135,7 @@ export function ComparisonChart({ comparisonData, metric, metricLabel }: Compari
label += ': '; label += ': ';
} }
if (context.parsed.y !== null) { if (context.parsed.y !== null) {
if (isProgressScore) { label += context.parsed.y.toFixed(1) + (kind === 'percentage' ? '%' : '');
label += context.parsed.y.toFixed(1);
} else if (isPercentage) {
label += context.parsed.y.toFixed(1) + '%';
} else {
label += context.parsed.y.toFixed(1);
}
} else { } else {
label += 'N/A'; label += 'N/A';
} }
@@ -121,17 +149,18 @@ export function ComparisonChart({ comparisonData, metric, metricLabel }: Compari
type: 'linear' as const, type: 'linear' as const,
display: true, display: true,
title: { title: {
display: true, display: !isMobile,
text: isPercentage ? 'Percentage (%)' : isProgressScore ? 'Progress Score' : 'Value', text: kind === 'percentage' ? 'Percentage (%)' : kind === 'progress' ? 'Progress Score' : 'Value',
font: { font: {
size: 12, size: 12,
weight: 'bold', weight: 'bold',
}, },
}, },
...(isPercentage && { ...yBounds,
min: 0, ticks: {
max: 100, font: { size: isMobile ? 10 : 12 },
}), ...(isMobile && { maxTicksLimit: 5 }),
},
grid: { grid: {
color: 'rgba(0, 0, 0, 0.05)', color: 'rgba(0, 0, 0, 0.05)',
}, },
@@ -141,16 +170,58 @@ export function ComparisonChart({ comparisonData, metric, metricLabel }: Compari
display: false, display: false,
}, },
title: { title: {
display: true, display: !isMobile,
text: 'Year', text: 'Year',
font: { font: {
size: 12, size: 12,
weight: 'bold', weight: 'bold',
}, },
}, },
ticks: {
font: { size: isMobile ? 10 : 12 },
...(isMobile && { maxRotation: 0, autoSkip: true, maxTicksLimit: 4 }),
},
}, },
}, },
}; };
return <Line data={chartData} options={options} />; const toggleFocus = (urn: number) => {
const next = focusedUrn === urn ? null : urn;
setFocusedUrn(next);
if (next !== null) track('compare_focus_school', { urn: next });
};
return (
<div className={styles.wrapper}>
{/* Mobile legend + focus control; a single series needs no legend. */}
{schools.length > 1 && (
<div className={styles.chips} role="group" aria-label="Highlight a school on the chart">
{schools.map((school, index) => (
<button
key={school.urn}
type="button"
className={styles.chip}
aria-pressed={focusedUrn === school.urn}
onClick={() => toggleFocus(school.urn)}
>
<span
className={styles.chipDot}
style={{ background: CHART_COLORS[index % CHART_COLORS.length] }}
aria-hidden="true"
/>
<span
className={styles.chipName}
style={{ color: CHART_TEXT_COLORS[index % CHART_TEXT_COLORS.length] }}
>
{school.school_name}
</span>
</button>
))}
</div>
)}
<div className={styles.canvasBox}>
<Line data={chartData} options={options} aria-label={`${metricLabel} comparison chart`} />
</div>
</div>
);
} }
@@ -454,7 +454,10 @@
} }
.chartContainer { .chartContainer {
height: 300px; /* Taller than desktop's proportion would suggest: the chip legend row
sits inside, and the in-chart title/legend/axis titles are gone, so
nearly all of this is plot area. */
height: 340px;
} }
.comparisonTable { .comparisonTable {
+4 -1
View File
@@ -111,8 +111,10 @@ export function ComparisonView({
setComparisonData(data.comparison); setComparisonData(data.comparison);
}) })
.catch((err) => { .catch((err) => {
// Keep whatever we already have (SSR data or a previous fetch) rather
// than blanking the chart — a transient refetch failure shouldn't
// destroy a working comparison the user is looking at.
console.error('Failed to fetch comparison:', err); console.error('Failed to fetch comparison:', err);
setComparisonData(null);
}); });
} else { } else {
setComparisonData(null); setComparisonData(null);
@@ -429,6 +431,7 @@ export function ComparisonView({
<div className={styles.chartContainer}> <div className={styles.chartContainer}>
<ComparisonChart <ComparisonChart
comparisonData={activeComparisonData} comparisonData={activeComparisonData}
schools={activeSchools}
metric={selectedMetric} metric={selectedMetric}
metricLabel={metricLabel} metricLabel={metricLabel}
/> />
+3 -11
View File
@@ -10,12 +10,13 @@
'use client'; 'use client';
import { useEffect, useMemo, useState } from 'react'; import { useMemo, useState } from 'react';
import { Line } from 'react-chartjs-2'; import { Line } from 'react-chartjs-2';
import { ChartOptions, ChartDataset } from 'chart.js'; import { ChartOptions, ChartDataset } from 'chart.js';
import '@/lib/chartSetup'; import '@/lib/chartSetup';
import type { SchoolResult } from '@/lib/types'; import type { SchoolResult } from '@/lib/types';
import { formatAcademicYear } from '@/lib/utils'; import { formatAcademicYear } from '@/lib/utils';
import { useIsMobile } from '@/hooks/useIsMobile';
import { track } from '@/lib/analytics'; import { track } from '@/lib/analytics';
import styles from './PerformanceChart.module.css'; import styles from './PerformanceChart.module.css';
@@ -68,16 +69,7 @@ export function PerformanceChart({
const sortedData = [...data].sort((a, b) => a.year - b.year); const sortedData = [...data].sort((a, b) => a.year - b.year);
const years = sortedData.map(d => formatAcademicYear(d.year)); const years = sortedData.map(d => formatAcademicYear(d.year));
// ── Mobile detection ───────────────────────────────────────────────── const isMobile = useIsMobile();
// Hydration-safe: SSR renders desktop; client flips to mobile after mount.
const [isMobile, setIsMobile] = useState(false);
useEffect(() => {
const mq = window.matchMedia('(max-width: 640px)');
const update = () => setIsMobile(mq.matches);
update();
mq.addEventListener('change', update);
return () => mq.removeEventListener('change', update);
}, []);
// ── Build per-year national averages ───────────────────────────────── // ── Build per-year national averages ─────────────────────────────────
const natRefRwm: (number | null)[] = sortedData.map(d => { const natRefRwm: (number | null)[] = sortedData.map(d => {
@@ -34,6 +34,15 @@
background: #fff; background: #fff;
} }
/* Fallback fullscreen (iOS Safari — no Element.requestFullscreen): the API
can't promote the element, so pin it over the page ourselves. Above the
comparison toast (3000) and everything else except modals (9999+). */
.wrapper[data-fs-fallback] {
position: fixed;
inset: 0;
z-index: 5000;
}
.skeleton { .skeleton {
width: 100%; width: 100%;
height: 100%; height: 100%;
+29 -4
View File
@@ -29,25 +29,50 @@ interface SchoolHeroMapProps {
export const SchoolHeroMap = forwardRef<SchoolHeroMapHandle, SchoolHeroMapProps>( export const SchoolHeroMap = forwardRef<SchoolHeroMapHandle, SchoolHeroMapProps>(
function SchoolHeroMap({ lat, lng }, ref) { function SchoolHeroMap({ lat, lng }, ref) {
const wrapperRef = useRef<HTMLDivElement>(null); const wrapperRef = useRef<HTMLDivElement>(null);
const [isFullscreen, setIsFullscreen] = useState(false); const [nativeFullscreen, setNativeFullscreen] = useState(false);
// iOS Safari has no Element.requestFullscreen — fall back to a
// fixed-position overlay driven by state instead of the Fullscreen API.
const [fallbackFullscreen, setFallbackFullscreen] = useState(false);
const isFullscreen = nativeFullscreen || fallbackFullscreen;
const open = useCallback(() => { const open = useCallback(() => {
wrapperRef.current?.requestFullscreen?.().catch(() => {}); const el = wrapperRef.current;
if (!el) return;
if (el.requestFullscreen) {
el.requestFullscreen().catch(() => setFallbackFullscreen(true));
} else {
setFallbackFullscreen(true);
}
}, []); }, []);
const close = useCallback(() => { const close = useCallback(() => {
if (document.fullscreenElement) document.exitFullscreen().catch(() => {}); if (document.fullscreenElement) document.exitFullscreen().catch(() => {});
setFallbackFullscreen(false);
}, []); }, []);
useImperativeHandle(ref, () => ({ open }), [open]); useImperativeHandle(ref, () => ({ open }), [open]);
useEffect(() => { useEffect(() => {
const onChange = () => setIsFullscreen(!!document.fullscreenElement); const onChange = () => setNativeFullscreen(!!document.fullscreenElement);
document.addEventListener('fullscreenchange', onChange); document.addEventListener('fullscreenchange', onChange);
return () => document.removeEventListener('fullscreenchange', onChange); return () => document.removeEventListener('fullscreenchange', onChange);
}, []); }, []);
// The fallback overlay sits on top of the page rather than replacing it,
// so lock body scroll while it is up.
useEffect(() => {
if (!fallbackFullscreen) return;
const prev = document.body.style.overflow;
document.body.style.overflow = 'hidden';
return () => { document.body.style.overflow = prev; };
}, [fallbackFullscreen]);
return ( return (
<div ref={wrapperRef} className={styles.wrapper} data-fullscreen={isFullscreen || undefined}> <div
ref={wrapperRef}
className={styles.wrapper}
data-fullscreen={isFullscreen || undefined}
data-fs-fallback={fallbackFullscreen || undefined}
>
<LeafletHeroMap lat={lat} lng={lng} interactive={isFullscreen} /> <LeafletHeroMap lat={lat} lng={lng} interactive={isFullscreen} />
{isFullscreen ? ( {isFullscreen ? (
+23
View File
@@ -0,0 +1,23 @@
/**
* Viewport hook shared by the chart components.
* Hydration-safe: SSR and the first client render report desktop; the
* media-query subscription flips the value after mount.
*/
'use client';
import { useEffect, useState } from 'react';
export function useIsMobile(maxWidth = 640): boolean {
const [isMobile, setIsMobile] = useState(false);
useEffect(() => {
const mq = window.matchMedia(`(max-width: ${maxWidth}px)`);
const update = () => setIsMobile(mq.matches);
update();
mq.addEventListener('change', update);
return () => mq.removeEventListener('change', update);
}, [maxWidth]);
return isMobile;
}
+1
View File
@@ -29,6 +29,7 @@ export type EventName =
| 'compare_viewed' | 'compare_viewed'
| 'compare_metric_changed' | 'compare_metric_changed'
| 'compare_shared' | 'compare_shared'
| 'compare_focus_school'
// Operational // Operational
| 'api_error' | 'api_error'
| 'results_load_more'; | 'results_load_more';
+51
View File
@@ -317,6 +317,57 @@ export function getTrendColor(trend: 'up' | 'down' | 'stable'): string {
} }
} }
/**
* Broad shape of a KS2/KS4 metric, used to scale chart axes and format values.
*/
export type MetricKind = 'percentage' | 'progress' | 'score';
export function metricKind(metric: string): MetricKind {
if (metric.includes('progress')) return 'progress';
if (metric.includes('pct') || metric.includes('rate')) return 'percentage';
return 'score';
}
/**
* Fit a chart y-axis to the data instead of a fixed frame, so clustered
* series remain distinguishable. Padding keeps a minimum span so noise is
* not magnified into drama.
*
* - percentage: pad and snap to 5s; cap at 100; floor at 0 only when the
* data is non-negative (some trend metrics have `pct` in the key but hold
* negative year-over-year deltas).
* - progress: symmetric around 0 so the zero line always shows.
* - score (Attainment 8, scaled scores): pad and snap to integers; floor at
* 0 only when the data is non-negative.
*/
export function computeYBounds(
values: Array<number | null | undefined>,
kind: MetricKind,
): { min?: number; max?: number } {
const nums = values.filter((v): v is number => typeof v === 'number' && Number.isFinite(v));
if (nums.length === 0) return {};
const lo = Math.min(...nums);
const hi = Math.max(...nums);
if (kind === 'progress') {
const reach = Math.max(2, Math.ceil(Math.max(Math.abs(lo), Math.abs(hi)) + 0.5));
return { min: -reach, max: reach };
}
if (kind === 'percentage') {
const pad = Math.max(5, Math.round((hi - lo) * 0.2));
const min = Math.floor((lo - pad) / 5) * 5;
const max = Math.min(100, Math.ceil((hi + pad) / 5) * 5);
return { min: lo >= 0 ? Math.max(0, min) : min, max };
}
// score
const pad = Math.max(2, (hi - lo) * 0.2);
const min = Math.floor(lo - pad);
return { min: lo >= 0 ? Math.max(0, min) : min, max: Math.ceil(hi + pad) };
}
// ============================================================================ // ============================================================================
// Local Storage Utilities // Local Storage Utilities
// ============================================================================ // ============================================================================
+49 -62
View File
@@ -1,14 +1,17 @@
#!/usr/bin/env python3 #!/usr/bin/env python3
"""AI code review for Gitea pull requests. """AI code review for Gitea pull requests, powered by Claude Code.
Reads the PR diff (base branch vs HEAD), asks Claude to review it, posts the Reads the PR diff (base branch vs HEAD), asks Claude Code (headless `claude -p`)
findings as a PR comment via the Gitea API, and exits non-zero only when the to review it, posts the findings as a PR comment via the Gitea API, and exits
review contains at least one severe finding — so the job can gate merges non-zero only when the review contains at least one severe finding — so the
without blocking on nitpicks. job can gate merges without blocking on nitpicks.
Uses only the Python standard library; the review itself runs through the
Claude Code CLI, authenticated with a subscription OAuth token.
Required environment: Required environment:
ANTHROPIC_API_KEY Anthropic API key CLAUDE_CODE_OAUTH_TOKEN token from `claude setup-token` (subscription auth)
GITEA_TOKEN Gitea token with permission to comment on PRs GITEA_TOKEN Gitea access token for posting PR comments
GITEA_SERVER_URL e.g. https://privaterepo.sitaru.org GITEA_SERVER_URL e.g. https://privaterepo.sitaru.org
GITEA_REPOSITORY owner/repo GITEA_REPOSITORY owner/repo
PR_NUMBER pull request index PR_NUMBER pull request index
@@ -19,46 +22,29 @@ import json
import os import os
import subprocess import subprocess
import sys import sys
import urllib.request
import requests
from anthropic import Anthropic
MAX_DIFF_CHARS = 150_000 MAX_DIFF_CHARS = 150_000
REVIEW_SCHEMA = { PROMPT = """You are reviewing a pull request for SchoolCompare, a UK school
"type": "object",
"properties": {
"summary": {
"type": "string",
"description": "Two or three sentences on what the change does and its overall health.",
},
"findings": {
"type": "array",
"items": {
"type": "object",
"properties": {
"severity": {"type": "string", "enum": ["severe", "minor"]},
"file": {"type": "string"},
"issue": {"type": "string"},
},
"required": ["severity", "file", "issue"],
"additionalProperties": False,
},
},
},
"required": ["summary", "findings"],
"additionalProperties": False,
}
SYSTEM_PROMPT = """You are reviewing a pull request for SchoolCompare, a UK school
comparison site (FastAPI backend, Next.js frontend, Airflow/dbt data pipeline, comparison site (FastAPI backend, Next.js frontend, Airflow/dbt data pipeline,
deployed via Gitea Actions to a staging-then-production Docker setup). deployed via Gitea Actions to a staging-then-production Docker setup).
The PR diff is provided on stdin.
Report correctness bugs, security issues, data-loss risks, and broken deploy/CI Report correctness bugs, security issues, data-loss risks, and broken deploy/CI
configuration. Mark a finding "severe" only if it would break production, leak configuration. Mark a finding "severe" only if it would break production, leak
data, or corrupt data — severe findings block the merge. Everything else data, or corrupt data — severe findings block the merge. Everything else
(style, performance suggestions, minor cleanups) is "minor". Do not invent (style, performance suggestions, minor cleanups) is "minor". Do not invent
findings: an empty findings list is a perfectly good review of a clean diff.""" findings: an empty findings list is a perfectly good review of a clean diff.
Respond with ONLY a JSON object (no markdown fences, no prose) of this shape:
{
"summary": "two or three sentences on what the change does and its health",
"findings": [
{"severity": "severe" | "minor", "file": "path", "issue": "description"}
]
}"""
def get_diff(base_ref: str) -> str: def get_diff(base_ref: str) -> str:
@@ -79,29 +65,25 @@ def get_diff(base_ref: str) -> str:
def review(diff: str) -> dict: def review(diff: str) -> dict:
client = Anthropic() proc = subprocess.run(
with client.messages.stream( ["claude", "-p", PROMPT, "--output-format", "json"],
model="claude-opus-4-8", input=diff,
max_tokens=16000, capture_output=True,
thinking={"type": "adaptive"}, text=True,
system=SYSTEM_PROMPT, timeout=900,
output_config={"format": {"type": "json_schema", "schema": REVIEW_SCHEMA}}, )
messages=[ if proc.returncode != 0:
{ raise RuntimeError(f"claude CLI failed:\n{proc.stderr}")
"role": "user", envelope = json.loads(proc.stdout)
"content": f"Review this pull request diff:\n\n```diff\n{diff}\n```", result = envelope["result"].strip()
} # Defensive: strip markdown fences if the model added them anyway
], if result.startswith("```"):
) as stream: result = result.split("\n", 1)[1].rsplit("```", 1)[0]
message = stream.get_final_message() return json.loads(result)
if message.stop_reason == "refusal":
raise RuntimeError("Claude declined to review this diff")
text = next(b.text for b in message.content if b.type == "text")
return json.loads(text)
def format_comment(result: dict) -> str: def format_comment(result: dict) -> str:
lines = ["## 🤖 AI Code Review (Claude)", "", result["summary"], ""] lines = ["## 🤖 AI Code Review (Claude Code)", "", result["summary"], ""]
severe = [f for f in result["findings"] if f["severity"] == "severe"] severe = [f for f in result["findings"] if f["severity"] == "severe"]
minor = [f for f in result["findings"] if f["severity"] == "minor"] minor = [f for f in result["findings"] if f["severity"] == "minor"]
if severe: if severe:
@@ -121,13 +103,18 @@ def post_comment(body: str) -> None:
server = os.environ["GITEA_SERVER_URL"].rstrip("/") server = os.environ["GITEA_SERVER_URL"].rstrip("/")
repo = os.environ["GITEA_REPOSITORY"] repo = os.environ["GITEA_REPOSITORY"]
pr = os.environ["PR_NUMBER"] pr = os.environ["PR_NUMBER"]
resp = requests.post( req = urllib.request.Request(
f"{server}/api/v1/repos/{repo}/issues/{pr}/comments", f"{server}/api/v1/repos/{repo}/issues/{pr}/comments",
headers={"Authorization": f"token {os.environ['GITEA_TOKEN']}"}, data=json.dumps({"body": body}).encode(),
json={"body": body}, headers={
timeout=30, "Authorization": f"token {os.environ['GITEA_TOKEN']}",
"Content-Type": "application/json",
},
method="POST",
) )
resp.raise_for_status() with urllib.request.urlopen(req, timeout=30) as resp:
if resp.status >= 300:
raise RuntimeError(f"Comment post failed: HTTP {resp.status}")
def main() -> int: def main() -> int: