Compare commits
17
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
7b5f4fdd62 | ||
|
|
22769b6295 | ||
|
|
90f2a02e75 | ||
|
|
d52d384cf2 | ||
|
|
ff606dad71 | ||
|
|
acec8135e1 | ||
|
|
0a370e3b63 | ||
|
|
6c872ce726 | ||
|
|
23b4e1c453 | ||
|
|
deeef23131 | ||
|
|
4ece55b031 | ||
|
|
515494dbf0 | ||
|
|
5772c54ccd | ||
|
|
c62ba0ca25 | ||
|
|
b5a63e82d4 | ||
|
|
f5de745a8b | ||
|
|
d0895c71df |
@@ -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
@@ -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
@@ -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.
|
||||||
|
|||||||
@@ -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);
|
||||||
|
});
|
||||||
|
|||||||
@@ -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({});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -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;
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -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 {
|
||||||
|
|||||||
@@ -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}
|
||||||
/>
|
/>
|
||||||
|
|||||||
@@ -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,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 ? (
|
||||||
|
|||||||
@@ -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;
|
||||||
|
}
|
||||||
@@ -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';
|
||||||
|
|||||||
@@ -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
@@ -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:
|
||||||
|
|||||||
Reference in New Issue
Block a user