Skip to content

Commit 48733ee

Browse files
satraclaude
andcommitted
harden(026): make the density sampler robust to non-finite year/lod_level
Gemini review follow-up. countsByYear + the bucketing loop now skip points without a finite `year` (a NaN/undefined Map key would make the ascending-year sort non-deterministic), and the within-year comparator uses `Number.isFinite` (the previous `?? +Inf` did NOT catch NaN, which could make the comparator return NaN → non-deterministic sort). Guards the pure module's determinism contract regardless of caller input. +2 unit tests. Declined the suggested `if (!p) continue` in +page.svelte's targetBudget loop: atlasBackdrop is a typed, dense parquet-derived array, so null elements are structurally impossible — that guard would be dead code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent d8d918e commit 48733ee

2 files changed

Lines changed: 42 additions & 5 deletions

File tree

site/src/lib/atlas/year_density.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,15 @@ export interface DensityCalibration {
3333
k: number;
3434
}
3535

36-
/** Per-year article counts from a point list. */
36+
/** Per-year article counts from a point list. Points without a finite
37+
* `year` are ignored so a NaN/undefined key can never enter the map and
38+
* break the deterministic year sort downstream. */
3739
function countsByYear(points: readonly DensityPoint[]): Map<number, number> {
3840
const counts = new Map<number, number>();
39-
for (const p of points) counts.set(p.year, (counts.get(p.year) ?? 0) + 1);
41+
for (const p of points) {
42+
if (!Number.isFinite(p.year)) continue;
43+
counts.set(p.year, (counts.get(p.year) ?? 0) + 1);
44+
}
4045
return counts;
4146
}
4247

@@ -74,9 +79,12 @@ export function yearAwareSample(
7479
): DensityPoint[] {
7580
if (points.length === 0 || calib.k <= 0) return [];
7681

77-
// Bucket by year.
82+
// Bucket by year, skipping points without a finite year so the year
83+
// keys stay strictly numeric (a NaN/undefined key would make the
84+
// ascending-year sort below non-deterministic).
7885
const byYear = new Map<number, DensityPoint[]>();
7986
for (const p of points) {
87+
if (!Number.isFinite(p.year)) continue;
8088
const bucket = byYear.get(p.year);
8189
if (bucket) bucket.push(p);
8290
else byYear.set(p.year, [p]);
@@ -97,8 +105,12 @@ export function yearAwareSample(
97105
// tiebreak ascending pubmed_id. Missing lod_level sorts as +∞ so
98106
// legacy builds fall back to a deterministic pubmed_id order.
99107
const ranked = [...bucket].sort((a, b) => {
100-
const la = a.lod_level ?? Number.POSITIVE_INFINITY;
101-
const lb = b.lod_level ?? Number.POSITIVE_INFINITY;
108+
// `?? +Inf` handles null/undefined; `Number.isFinite` additionally
109+
// treats NaN (and ±Inf) as the rest tier, so the comparator can
110+
// never return NaN → the sort stays deterministic. Legacy builds
111+
// (no lod_level) fall through to the pubmed_id tiebreak.
112+
const la = Number.isFinite(a.lod_level) ? (a.lod_level as number) : Number.POSITIVE_INFINITY;
113+
const lb = Number.isFinite(b.lod_level) ? (b.lod_level as number) : Number.POSITIVE_INFINITY;
102114
if (la !== lb) return la - lb;
103115
return a.pubmed_id - b.pubmed_id;
104116
});

site/src/tests/unit/year_density.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,31 @@ describe('yearAwareSample() — case 7 (empty input)', () => {
143143
});
144144
});
145145

146+
describe('yearAwareSample() — invalid year / lod_level are ignored deterministically', () => {
147+
it('drops points with non-finite year (never a NaN/undefined bucket key)', () => {
148+
const dirty: DensityPoint[] = [
149+
...pts(2010, 100),
150+
{ pubmed_id: 999999, year: NaN as unknown as number, lod_level: 0 },
151+
{ pubmed_id: 999998, year: undefined as unknown as number, lod_level: 0 }
152+
];
153+
const calib = calibrate(dirty, 10); // Σ√ counts only the valid year (2010)
154+
const out = yearAwareSample(dirty, calib);
155+
expect(out.every((p) => p.year === 2010)).toBe(true);
156+
expect(out.some((p) => p.pubmed_id === 999999 || p.pubmed_id === 999998)).toBe(false);
157+
});
158+
it('non-finite lod_level sorts as the rest tier (comparator never returns NaN)', () => {
159+
const yr: DensityPoint[] = [
160+
{ pubmed_id: 1, year: 2010, lod_level: NaN as unknown as number },
161+
{ pubmed_id: 2, year: 2010, lod_level: 0 },
162+
{ pubmed_id: 3, year: 2010, lod_level: 1 }
163+
];
164+
const calib = { targetBudget: 2, k: 2 / Math.sqrt(3) }; // quota = round(k√3) = 2
165+
const out = yearAwareSample(yr, calib);
166+
// lowest finite lod_levels (0,1 → pubmed 2,3) win; the NaN one is last.
167+
expect(out.map((p) => p.pubmed_id).sort((a, b) => a - b)).toEqual([2, 3]);
168+
});
169+
});
170+
146171
describe('yearAwareSample() — case 8 (legacy: no lod_level)', () => {
147172
it('deterministic, reproducible selection when lod_level is absent', () => {
148173
const yr: DensityPoint[] = Array.from({ length: 10 }, (_, i) => ({

0 commit comments

Comments
 (0)