From dc85254ad2ddf134b4434065d4762cef374d2220 Mon Sep 17 00:00:00 2001 From: Tudor Date: Wed, 22 Jul 2026 15:17:43 +0100 Subject: [PATCH] fix(compare): a no-results school no longer blanks the trend chart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../__tests__/lib/compareChartData.test.ts | 24 +++++++++++++++++++ nextjs-app/lib/compareChartData.ts | 20 ++++++++++++++-- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/nextjs-app/__tests__/lib/compareChartData.test.ts b/nextjs-app/__tests__/lib/compareChartData.test.ts index 7c59bc5..8c5b168 100644 --- a/nextjs-app/__tests__/lib/compareChartData.test.ts +++ b/nextjs-app/__tests__/lib/compareChartData.test.ts @@ -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)', () => { const floaty = { '1': school(1, [[201819.0 as number, 80], [202425.0 as number, 85]]), diff --git a/nextjs-app/lib/compareChartData.ts b/nextjs-app/lib/compareChartData.ts index c3e783e..03cb559 100644 --- a/nextjs-app/lib/compareChartData.ts +++ b/nextjs-app/lib/compareChartData.ts @@ -10,6 +10,17 @@ 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). */ function nextAcademicYear(year: number): number { const start = Math.floor(year / 100); @@ -66,14 +77,19 @@ export function buildCompareChart( nationalByYear?: Record, ): CompareChart { 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 schoolDatasets: CompareChartSeries[] = schools.map((school, schoolIndex) => { const rows = comparisonData[String(school.urn)]?.yearly_data ?? []; const byYear = new Map>(); - for (const row of rows) byYear.set(Math.trunc(row.year), row as unknown as Record); + for (const row of rows) { + if (!hasYear(row)) continue; + byYear.set(Math.trunc(row.year), row as unknown as Record); + } return { label: school.school_name, data: years.map((year) => {