Compare commits

...
2 Commits
Author SHA1 Message Date
tudor 598ba9af4e Merge pull request 'fix(compare): a no-results school no longer blanks the trend chart' (#80) from fix/compare-null-year-blanks-chart into main
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 14s
Stage (build -> staging -> E2E gate) / Build Frontend (Next.js) (push) Successful in 58s
Stage (build -> staging -> E2E gate) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 13s
Stage (build -> staging -> E2E gate) / Deploy to Staging (push) Successful in 1s
Stage (build -> staging -> E2E gate) / E2E Journeys against Staging (push) Successful in 2m49s
Reviewed-on: #80
2026-07-22 14:23:25 +00:00
TudorandClaude Opus 4.8 dc85254ad2 fix(compare): a no-results school no longer blanks the trend chart
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m8s
PR Checks / Backend Smoke (pull_request) Successful in 7s
PR Checks / Build Backend (no push) (pull_request) Successful in 13s
PR Checks / Build Frontend (no push) (pull_request) Successful in 49s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 54s
Adding a school with no performance data to a comparison made every
school's trend line disappear until that school was removed.

Root cause: /api/compare returns such a school with a single phantom
yearly_data row (the dim_school LEFT JOIN) whose year is null. In
buildCompareChart, Math.trunc(null) is 0, so the axis was seeded at
year 0; fillAcademicYears then walked 0, 101, 202, … and hit its
50-step cap long before reaching the real years, leaving every
school's series mapped entirely to null.

Fix: ignore yearly rows without a real numeric year when building the
axis and the per-school year map. Regression test added.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-22 15:17:43 +01:00
2 changed files with 42 additions and 2 deletions
@@ -41,6 +41,30 @@ describe('buildCompareChart', () => {
} }
}); });
it('ignores a no-results school whose only row has a null year', () => {
// A school with no performance rows comes back from /api/compare with a
// single phantom yearly_data row (LEFT JOIN) where year and every metric
// are null. That null year must NOT pollute the axis: Math.trunc(null) is
// 0, and filling from year 0 blows past the real years, blanking every
// school's line. Regression guard for "add a no-data school → chart empty".
const withNoData = {
...THREE_SCHOOLS,
'4': school(4, [[null as unknown as number, null]]),
};
const list = [...SCHOOL_LIST, { urn: 4, school_name: 'School 4' }];
const chart = buildCompareChart(withNoData, list, 'rwm_expected_pct');
// The real years still drive the axis; the phantom year 0 is gone.
expect(chart.years).toContain(201819);
expect(chart.years).toContain(202425);
expect(chart.years).not.toContain(0);
// The three real schools still render their lines.
for (const urn of [1, 2, 3]) {
const ds = chart.schoolDatasets[urn - 1];
expect(ds.data.some((v) => v != null)).toBe(true);
}
});
it('handles float years from the API (202425.0 style)', () => { it('handles float years from the API (202425.0 style)', () => {
const floaty = { const floaty = {
'1': school(1, [[201819.0 as number, 80], [202425.0 as number, 85]]), '1': school(1, [[201819.0 as number, 80], [202425.0 as number, 85]]),
+18 -2
View File
@@ -10,6 +10,17 @@
import type { ComparisonData } from './types'; import type { ComparisonData } from './types';
/**
* A yearly row only counts once it carries a real academic year. A school with
* no performance data still comes back from /api/compare with a single phantom
* row (the dim_school LEFT JOIN) where `year` is null — and Math.trunc(null) is
* 0, which would seed the axis at year 0 and, via fillAcademicYears, blow past
* every real year and blank all schools' lines. Drop those rows up front.
*/
function hasYear(row: { year: number }): boolean {
return typeof row.year === 'number' && Number.isFinite(row.year);
}
/** 201819 → 201920 (academic-year arithmetic on YYYYYY codes). */ /** 201819 → 201920 (academic-year arithmetic on YYYYYY codes). */
function nextAcademicYear(year: number): number { function nextAcademicYear(year: number): number {
const start = Math.floor(year / 100); const start = Math.floor(year / 100);
@@ -66,14 +77,19 @@ export function buildCompareChart(
nationalByYear?: Record<number, number | null | undefined>, nationalByYear?: Record<number, number | null | undefined>,
): CompareChart { ): CompareChart {
const rawYears = schools.flatMap( const rawYears = schools.flatMap(
(s) => comparisonData[String(s.urn)]?.yearly_data.map((d) => Math.trunc(d.year)) ?? [], (s) =>
comparisonData[String(s.urn)]?.yearly_data.filter(hasYear).map((d) => Math.trunc(d.year)) ??
[],
); );
const years = fillAcademicYears(rawYears); const years = fillAcademicYears(rawYears);
const schoolDatasets: CompareChartSeries[] = schools.map((school, schoolIndex) => { const schoolDatasets: CompareChartSeries[] = schools.map((school, schoolIndex) => {
const rows = comparisonData[String(school.urn)]?.yearly_data ?? []; const rows = comparisonData[String(school.urn)]?.yearly_data ?? [];
const byYear = new Map<number, Record<string, unknown>>(); const byYear = new Map<number, Record<string, unknown>>();
for (const row of rows) byYear.set(Math.trunc(row.year), row as unknown as Record<string, unknown>); for (const row of rows) {
if (!hasYear(row)) continue;
byYear.set(Math.trunc(row.year), row as unknown as Record<string, unknown>);
}
return { return {
label: school.school_name, label: school.school_name,
data: years.map((year) => { data: years.map((year) => {