Fix four defects a user hits in the first minute

Found by surveying the codebase against the plan; each was verified against the
committed assets or the running app before being touched.

TRAPPIST-1 was orbiting the Sun. The Exoplanet Archive leaves sy_dist blank for
some systems, and fetchExoplanets.ts read it with bare Number() — Number('') is
0, which is finite, so it slipped past the Number.isFinite guard in
resolveHostStarId, placed the host at the origin, and matched Sol at distance
exactly 0. 127 records shipped with hostStarId 0, all seven TRAPPIST-1 planets
among them, and the system view filters on that id, so drilling into Sol drew
127 alien worlds inside the real solar system.

Fixed in three places: resolveHostStarId now rejects a non-positive distance
(the robust guard, covering every caller), fetchExoplanets.ts uses the
parseOptionalNumber that already sat unused in that same file for ra/dec/dist,
and validateExoplanets asserts nothing ever resolves to the Sun again — the Sun
has no exoplanets, so that tripwire costs nothing and is permanent.

The archive's endpoint is blocked by this environment's egress policy, so the
ETL cannot be re-run here. The committed asset was corrected in place instead,
which is safe because the outcome is deterministic: the name path runs first and
none of the 127 resolve by name, so all of them reached id 0 positionally and
the fixed pipeline yields null for exactly that set. Cross-referenced hosts drop
from 761 to 634; record count is unchanged.

Dragging to rotate selected stars. Selection was bound to the raw click event,
which browsers fire on release however far the pointer travelled and which
OrbitControls does not suppress — so any drag ending over a star launched a
camera flight, and in system view routed away to /body/:id. Now tracks
pointerdown and ignores a release more than 5 px from it.

Ghost systems accumulated on every star-to-star hop. SystemOrbitsRenderer.dispose
released geometries and materials but never detached its group, so old orbit
lines stayed parented forever — still traversed and re-uploaded each frame with
disposed geometries, drawn over the new system and unpickable. dispose() now
detaches and clears.

Galaxy star labels stayed pinned inside the system view. They are CSS2D objects
parented to the scene rather than to galaxyGroup, so hiding the group left up to
15 parsec-space names clumped over the system's star. Cleared on entry. Also
gated the per-frame Kepler propagation on actually being in a system; it ran in
galaxy view too, because the renderer is never nulled on exit.

Tests: 116 passing, up from 112. Build, both typechecks and the Playwright suite
are green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WaySiNst4HhDXBHnMy8p5G
This commit is contained in:
Claude
2026-08-03 16:53:28 +00:00
parent 3a859360ba
commit 06cf7d2a15
7 changed files with 86 additions and 6 deletions
@@ -37,6 +37,8 @@ const DEEP_SKY_LABEL_COUNT = 12;
const LABEL_UPDATE_INTERVAL_SECONDS = 0.2; const LABEL_UPDATE_INTERVAL_SECONDS = 0.2;
/** Raycast pick tolerance around each star point, in parsecs. */ /** Raycast pick tolerance around each star point, in parsecs. */
const PICK_THRESHOLD_PC = 1.2; const PICK_THRESHOLD_PC = 1.2;
/** Pointer travel (px) above which a press counts as an orbit drag rather than a selection. */
const CLICK_DRAG_SLOP_PX = 5;
const GALAXY_OVERVIEW_POSITION = new THREE.Vector3(0, 15, 30); const GALAXY_OVERVIEW_POSITION = new THREE.Vector3(0, 15, 30);
const GALAXY_OVERVIEW_TARGET = new THREE.Vector3(0, 0, 0); const GALAXY_OVERVIEW_TARGET = new THREE.Vector3(0, 0, 0);
@@ -117,6 +119,7 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy {
private resizeObserver?: ResizeObserver; private resizeObserver?: ResizeObserver;
private unsubscribeTick?: () => void; private unsubscribeTick?: () => void;
private labelUpdateAccumulator = 0; private labelUpdateAccumulator = 0;
private pointerDownAt: { x: number; y: number } | null = null;
private ready = false; private ready = false;
private busy = false; private busy = false;
@@ -147,6 +150,7 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy {
ngOnDestroy(): void { ngOnDestroy(): void {
this.unsubscribeTick?.(); this.unsubscribeTick?.();
this.resizeObserver?.disconnect(); this.resizeObserver?.disconnect();
this.canvasRef().nativeElement.removeEventListener('pointerdown', this.handlePointerDown);
this.canvasRef().nativeElement.removeEventListener('click', this.handleClick); this.canvasRef().nativeElement.removeEventListener('click', this.handleClick);
this.controls?.dispose(); this.controls?.dispose();
this.starField?.dispose(); this.starField?.dispose();
@@ -224,6 +228,7 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy {
this.labelOverlay.setSize(width, height); this.labelOverlay.setSize(width, height);
this.raycaster.params.Points!.threshold = PICK_THRESHOLD_PC; this.raycaster.params.Points!.threshold = PICK_THRESHOLD_PC;
canvas.addEventListener('pointerdown', this.handlePointerDown);
canvas.addEventListener('click', this.handleClick); canvas.addEventListener('click', this.handleClick);
this.observeResize(canvas); this.observeResize(canvas);
@@ -246,7 +251,9 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy {
} }
} }
if (this.currentStarId !== null) {
this.systemRenderer?.update(dateToJulianDate()); this.systemRenderer?.update(dateToJulianDate());
}
this.labelOverlay?.render(camera); this.labelOverlay?.render(camera);
} }
@@ -271,11 +278,25 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy {
this.labelOverlay?.update([...starLabels, ...this.deepSkyLabels]); this.labelOverlay?.update([...starLabels, ...this.deepSkyLabels]);
} }
/** Where the current press started, so a drag can be told apart from a click. */
private readonly handlePointerDown = (event: PointerEvent): void => {
this.pointerDownAt = { x: event.clientX, y: event.clientY };
};
private readonly handleClick = (event: MouseEvent): void => { private readonly handleClick = (event: MouseEvent): void => {
if (this.rig?.isAnimating) { if (this.rig?.isAnimating) {
return; return;
} }
// The browser fires `click` on release however far the pointer travelled, and OrbitControls
// does not suppress it — so without this every drag-to-rotate that happens to finish over a
// star would launch a camera flight into its system.
const pressedAt = this.pointerDownAt;
this.pointerDownAt = null;
if (pressedAt && Math.hypot(event.clientX - pressedAt.x, event.clientY - pressedAt.y) > CLICK_DRAG_SLOP_PX) {
return;
}
const canvas = this.canvasRef().nativeElement; const canvas = this.canvasRef().nativeElement;
const camera = this.engine.getCamera(); const camera = this.engine.getCamera();
const rect = canvas.getBoundingClientRect(); const rect = canvas.getBoundingClientRect();
@@ -391,6 +412,10 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy {
this.galaxyGroup.visible = false; this.galaxyGroup.visible = false;
this.systemGroup.visible = true; this.systemGroup.visible = true;
// Labels are CSS2D objects parented to the scene, not to galaxyGroup, so hiding the group
// does not hide them: without this the galaxy-scale star names stay pinned on screen,
// clumped over the system's star.
this.labelOverlay?.update([]);
camera.near = SYSTEM_NEAR_AU; camera.near = SYSTEM_NEAR_AU;
camera.far = SYSTEM_FAR_AU; camera.far = SYSTEM_FAR_AU;
@@ -202,6 +202,12 @@ export class SystemOrbitsRenderer {
geometry.dispose(); geometry.dispose();
material.dispose(); material.dispose();
} }
// Detach as well as dispose. A star-to-star hop builds a new renderer and drops the old
// one, but without this the old orbit lines and markers stay parented to the system group
// forever — still traversed and re-uploaded every frame despite their geometries being
// disposed, and drawn over the new system while being unpickable.
this.object.removeFromParent();
this.object.clear();
} }
private addTopLevelBody(id: string, kind: SystemMemberKind, elements: OrbitalElements, gmAu3PerDay2: number, radiusKm: number | undefined): TrackedTopLevelBody { private addTopLevelBody(id: string, kind: SystemMemberKind, elements: OrbitalElements, gmAu3PerDay2: number, radiusKm: number | undefined): TrackedTopLevelBody {
@@ -6,6 +6,8 @@ import { StarRecord } from '../models/star.model';
// A small fixture standing in for a slice of the HYG star index, used to exercise the // A small fixture standing in for a slice of the HYG star index, used to exercise the
// exoplanet host-star cross-referencing logic without hitting any real API. // exoplanet host-star cross-referencing logic without hitting any real API.
const FIXTURE_STARS: StarRecord[] = [ const FIXTURE_STARS: StarRecord[] = [
// The Sun sits at the origin, exactly where a host with a missing distance lands.
{ id: 0, name: 'Sol', x: 0, y: 0, z: 0, magnitude: -26.7, spectralType: 'G2V', colorIndex: 0.656 },
{ id: 1, name: 'Proxima Centauri', x: -0.472264, y: -0.361451, z: -1.151219, magnitude: 11.01, spectralType: 'M5Ve', colorIndex: 1.807 }, { id: 1, name: 'Proxima Centauri', x: -0.472264, y: -0.361451, z: -1.151219, magnitude: 11.01, spectralType: 'M5Ve', colorIndex: 1.807 },
{ id: 2, name: 'Sirius', x: -0.494323, y: 2.476731, z: -0.758485, magnitude: -1.44, spectralType: 'A0m...', colorIndex: 0.009 }, { id: 2, name: 'Sirius', x: -0.494323, y: 2.476731, z: -0.758485, magnitude: -1.44, spectralType: 'A0m...', colorIndex: 0.009 },
{ id: 3, name: 'GJ 3512', x: 3.0, y: 4.0, z: 5.0, magnitude: 11.0, spectralType: 'M5.5', colorIndex: 1.6 } { id: 3, name: 'GJ 3512', x: 3.0, y: 4.0, z: 5.0, magnitude: 11.0, spectralType: 'M5.5', colorIndex: 1.6 }
@@ -61,4 +63,34 @@ describe('resolveHostStarId', () => {
expect(id).toBe(10); expect(id).toBe(10);
}); });
describe('missing distance column', () => {
// The Exoplanet Archive leaves `sy_dist` blank for some systems. `Number('')` is `0` —
// finite, so it slips past a naive guard — which puts the host at the origin and matches
// the Sun at distance 0. That shipped 127 alien planets, all seven TRAPPIST-1 worlds among
// them, into our own solar system.
it('does not match a host with a zero distance to the Sun', () => {
const id = resolveHostStarId({ hostname: 'TRAPPIST-1', raDeg: 346.6, decDeg: -5.04, distancePc: 0 }, FIXTURE_STARS, 0.5);
expect(id).toBeNull();
});
it('rejects a negative distance too', () => {
const id = resolveHostStarId({ hostname: 'Nowhere', raDeg: 10, decDeg: 10, distancePc: -3 }, FIXTURE_STARS, 0.5);
expect(id).toBeNull();
});
it('still matches a real host at a genuinely small distance', () => {
const id = resolveHostStarId({ hostname: 'Unmatched', raDeg: 217.4, decDeg: -62.68, distancePc: 1.2959 }, FIXTURE_STARS, 0.5);
expect(id).toBe(1);
});
it('lets a named host resolve even with no usable distance', () => {
const id = resolveHostStarId({ hostname: 'Sirius', raDeg: 101.3, decDeg: -16.7, distancePc: 0 }, FIXTURE_STARS, 0.5);
expect(id).toBe(2);
});
});
}); });
@@ -41,6 +41,14 @@ export function resolveHostStarId(
return null; return null;
} }
// A non-positive distance is never a real measurement, and it is the specific shape a
// missing CSV cell takes: `Number('')` is `0`, which passes the finiteness check above and
// then places the host exactly at the origin — where it matches the Sun at distance 0 and
// hands an alien planet to our own solar system.
if (query.distancePc <= 0) {
return null;
}
const hostPosition = raDegDecDistanceToXyz(query.raDeg, query.decDeg, query.distancePc); const hostPosition = raDegDecDistanceToXyz(query.raDeg, query.decDeg, query.distancePc);
return findNearestStarWithin(hostPosition, stars, toleranceInPc); return findNearestStarWithin(hostPosition, stars, toleranceInPc);
} }
File diff suppressed because one or more lines are too long
+8 -1
View File
@@ -3,7 +3,7 @@ import { statSync } from 'node:fs';
import { BodyRecord } from '../../src/app/shared/models/body.model'; import { BodyRecord } from '../../src/app/shared/models/body.model';
import { DeepSkyRecord } from '../../src/app/shared/models/deepsky.model'; import { DeepSkyRecord } from '../../src/app/shared/models/deepsky.model';
import { ExoplanetRecord } from '../../src/app/shared/models/exoplanet.model'; import { ExoplanetRecord } from '../../src/app/shared/models/exoplanet.model';
import { StarRecord } from '../../src/app/shared/models/star.model'; import { StarRecord, SUN_STAR_ID } from '../../src/app/shared/models/star.model';
import { fetchDeepSky } from './fetchDeepSky'; import { fetchDeepSky } from './fetchDeepSky';
import { fetchExoplanets } from './fetchExoplanets'; import { fetchExoplanets } from './fetchExoplanets';
import { fetchSolarSystem } from './fetchSolarSystem'; import { fetchSolarSystem } from './fetchSolarSystem';
@@ -61,6 +61,13 @@ function validateExoplanets(exoplanets: ExoplanetRecord[], starIds: Set<number>)
assertCondition(!!exoplanet.name, `Exoplanet ${exoplanet.id} has no name.`); assertCondition(!!exoplanet.name, `Exoplanet ${exoplanet.id} has no name.`);
if (exoplanet.hostStarId !== null) { if (exoplanet.hostStarId !== null) {
assertCondition(starIds.has(exoplanet.hostStarId), `Exoplanet ${exoplanet.id} references unknown star id ${exoplanet.hostStarId}.`); assertCondition(starIds.has(exoplanet.hostStarId), `Exoplanet ${exoplanet.id} references unknown star id ${exoplanet.hostStarId}.`);
// The Sun has no exoplanets, so any match to it is a matching failure — historically a
// blank distance column parsing as 0, which puts the host at the origin and matches Sol
// exactly. Free, permanent tripwire for that whole class of bug.
assertCondition(
exoplanet.hostStarId !== SUN_STAR_ID,
`Exoplanet ${exoplanet.id} was matched to the Sun, which has no exoplanets — the host-star match is wrong.`
);
crossReferenced++; crossReferenced++;
} }
} }
+5 -3
View File
@@ -45,9 +45,11 @@ export async function fetchExoplanets(stars?: StarRecord[]): Promise<ExoplanetRe
let matched = 0; let matched = 0;
const exoplanets: ExoplanetRecord[] = rows.map((row, index) => { const exoplanets: ExoplanetRecord[] = rows.map((row, index) => {
const raDeg = Number(row['ra']); // `parseOptionalNumber`, not `Number`: a blank cell would otherwise become 0, which is a
const decDeg = Number(row['dec']); // finite, plausible-looking coordinate rather than the "not measured" it actually means.
const distancePc = Number(row['sy_dist']); const raDeg = parseOptionalNumber(row['ra']) ?? Number.NaN;
const decDeg = parseOptionalNumber(row['dec']) ?? Number.NaN;
const distancePc = parseOptionalNumber(row['sy_dist']) ?? Number.NaN;
const hostStarId = resolveHostStarId( const hostStarId = resolveHostStarId(
{ hostname: row['hostname'], raDeg, decDeg, distancePc }, { hostname: row['hostname'], raDeg, decDeg, distancePc },