Merge pull request #14 from avalon-vanguard/feat/neighbours-named

Name the neighbours that have a name, before the ones that only have a number

e2e/neighbour-jump.spec.ts

@@ -17,23 +17,21 @@ test.describe('Neighbour jump', () => {
     await expect(readout).toHaveText('Sol', { timeout: 30_000 });
 
     // Its nearest neighbours are named around the edge of the view; each is a button that flies
-    // there. Whichever one the ring names, not a star named here: the catalogue is refreshed on
-    // a schedule, and a refresh reorders which four are nearest — a hand-picked name made this a
-    // test of the catalogue's contents rather than of the ring. Which of them survives the
-    // declutter is a property of the view, so the first is as good as any.
-    const neighbour = page.getByRole('button', { name: /^Go to .+ pc away$/ }).first();
+    // there. Barnard's Star by name, on purpose: the ring ranks named stars ahead of survey
+    // designations, and the four nearest named to the Sun — Proxima, Rigil Kentaurus, Toliman,
+    // Barnard's — are a fact about space, not about which catalogue was refreshed last. Before
+    // that preference, a Gaia row duplicating Proxima took fourth place and this very label was
+    // the one that vanished, so it is also the regression test for it. Barnard's rather than the
+    // Alpha Centauri trio because those three share one bearing and are decluttered down to
+    // whichever the label pass reaches first.
+    const neighbour = page.getByRole('button', { name: /Barnard's Star/ });
     await expect(neighbour).toBeVisible({ timeout: 30_000 });
-    const label = (await neighbour.getAttribute('aria-label'))!;
-    const name = /^Go to (.+), [\d.]+ pc away$/.exec(label)![1];
+    await neighbour.click();
 
-    // Clicked by the name just read rather than through `neighbour`, which re-resolves `.first()`
-    // and could land on a different star if the label pass reorders the ring in between.
-    await page.getByRole('button', { name: label, exact: true }).click();
-
-    await expect(readout).toHaveText(name, { timeout: 45_000 });
+    await expect(readout).toHaveText("Barnard's Star", { timeout: 45_000 });
     // And from there the walk goes on: the new system names its own neighbours. Which ones is
-    // not asserted — neighbours sharing a bearing are decluttered, so which of them survives is
-    // a property of the view, not a fact about the catalogue.
+    // not asserted — several of Barnard's nearest share a bearing, so which of them survives
+    // the declutter is a property of the view, not a fact about the catalogue.
     await expect(page.locator('.map-label--ghost')).not.toHaveCount(0, { timeout: 30_000 });
   });
 });

src/app/features/galaxy-system/galaxy-system-scene.component.ts

@@ -18,6 +18,7 @@ import { DeepSkyRecord } from '../../shared/models/deepsky.model';
 import { ExoplanetRecord } from '../../shared/models/exoplanet.model';
 import { applyMilkyWaySkybox, createGlowSprite } from '../../shared/rendering/skybox';
 import { loadCachedTexture, MILKY_WAY_SKYBOX_PATH, SUN_TEXTURE_PATH } from '../../shared/rendering/texture-catalog';
+import { isDesignation } from '../../shared/models/star-catalog';
 import { StarRecord } from '../../shared/models/star.model';
 import { Bookmark } from '../../shared/state/bookmarks.store';
 import { NavigationStore, ViewLevel } from '../../shared/state/navigation.store';
@@ -855,7 +856,14 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy {
       // position: a neighbour whose separation rounds to what no separation prints as is not a
       // place to go, it is the same place. Compared through the formatter rather than against a
       // hand-picked epsilon, so the rule stays "would print as zero" whatever the formatter does.
-      .nearest(origin.id, NEIGHBOUR_COUNT * 2)
+      //
+      // Named stars first, survey designations only where fewer than that are in reach: a ring
+      // that exists to say where you are is not helped by "Gaia DR3 5853498713190525696" —
+      // least of all when that is the same star as the Proxima Centauri printed beside it.
+      .nearestPreferring(origin.id, NEIGHBOUR_COUNT * 2, (point) => {
+        const star = this.starsById.get(point.id);
+        return star !== undefined && !isDesignation(star);
+      })
       .filter((neighbour) => formatParsecs(neighbour.distancePc) !== formatParsecs(0))
       .slice(0, NEIGHBOUR_COUNT)
       .flatMap((neighbour) => {

src/app/shared/astro/star-neighbourhood.spec.ts

@@ -74,6 +74,16 @@ describe('StarNeighbourhood', () => {
     expect(ids(index.nearest(4, 2, (point) => point.id % 2 === 0))).toEqual([2, 6]);
   });
 
+  it('puts the stars it is told to prefer first, and fills with the rest only when short', () => {
+    const index = new StarNeighbourhood(line(10));
+    const even = (point: StarPoint) => point.id % 2 === 0;
+
+    // Enough even stars: the odd ones next door, though nearer, do not get a look in.
+    expect(ids(index.nearestPreferring(4, 2, even))).toEqual([2, 6]);
+    // Not enough: every even star in reach, then the nearest of the others.
+    expect(ids(index.nearestPreferring(4, 6, even))).toEqual([2, 6, 0, 8, 3, 5]);
+  });
+
   it('answers nothing for a star it has never heard of', () => {
     const index = new StarNeighbourhood(line(3));
 

src/app/shared/astro/star-neighbourhood.ts

@@ -70,6 +70,20 @@ export class StarNeighbourhood {
     return index === undefined ? undefined : this.points[index];
   }
 
+  /**
+   * Like `nearest`, but the stars `prefer` accepts come first, and the rest only fill what is
+   * left. The preferred pass exhausts the search before the fill runs, so a preferred star is
+   * never outranked by an ordinary one that happens to be closer — that is the point of asking.
+   */
+  nearestPreferring(id: number, count: number, prefer: (point: StarPoint) => boolean): Neighbour[] {
+    const preferred = this.nearest(id, count, prefer);
+    if (preferred.length >= count) {
+      return preferred;
+    }
+    const taken = new Set(preferred.map((neighbour) => neighbour.id));
+    return preferred.concat(this.nearest(id, count - preferred.length, (point) => !taken.has(point.id)));
+  }
+
   /**
    * The `count` stars nearest to `id`, nearest first, excluding the star itself.
    *

src/app/shared/models/star-catalog.spec.ts

@@ -1,6 +1,6 @@
 import { describe, expect, it } from 'vitest';
 
-import { BYTES_PER_STAR_META, BYTES_PER_STAR_POSITION, decodeStarCatalog, encodeStarCatalog } from './star-catalog';
+import { BYTES_PER_STAR_META, BYTES_PER_STAR_POSITION, decodeStarCatalog, encodeStarCatalog, isDesignation } from './star-catalog';
 import { StarRecord } from './star.model';
 
 const STARS: StarRecord[] = [
@@ -131,4 +131,18 @@ describe('star catalogue provenance and derived names', () => {
     expect(named.index.names).toEqual(['Some Proper Name']);
     expect(decodeStarCatalog(named.index, named.positions, named.meta)[0].name).toBe('Some Proper Name');
   });
+
+  it('tells a name somebody gave from the designation a survey generates', () => {
+    expect(isDesignation(decoded[0])).toBe(false);
+    expect(isDesignation(decoded[1])).toBe(true);
+    // The real catalogue names Gaia stars by the survey's nineteen-digit source id, which no
+    // 32-bit row id can equal — so it is the prefix that decides, not a round trip through the id.
+    expect(isDesignation({ ...MIXED[1], name: 'Gaia DR3 5853498713190525696' })).toBe(true);
+    expect(isDesignation({ ...MIXED[1], name: 'Proxima Centauri' })).toBe(false);
+    // HYG's own last resort, once every designation it knows has come up empty. Upper case,
+    // whatever case the source id is in.
+    expect(isDesignation({ ...MIXED[0], name: 'HYG 5' })).toBe(true);
+    expect(isDesignation({ ...MIXED[0], source: undefined, name: 'HYG 5' })).toBe(true);
+    expect(isDesignation({ ...MIXED[0], name: 'HD 48915' })).toBe(false);
+  });
 });

src/app/shared/models/star-catalog.ts

@@ -66,11 +66,14 @@ export interface StarCatalogIndex {
 }
 
 /**
- * How a source names a star that has no name of its own. `Gaia DR3 <id>` for Gaia; HYG's own
- * fallbacks already produce real designations, so it never needs this.
+ * How a source names a star that has no name of its own. `Gaia DR3 <id>` for Gaia; `HYG <id>`
+ * for HYG, whose ETL reaches for that only after a proper name, Bayer, Flamsteed, HD, Gliese and
+ * HIP have all come up empty (`tools/etl/fetchStars.ts`) — none in the current catalogue, but
+ * the path is there, and a name made that way is no more a name than Gaia's.
  */
 const DESIGNATION_PREFIXES: Readonly<Record<string, string>> = {
-  gaia: 'Gaia DR3'
+  gaia: 'Gaia DR3',
+  hyg: 'HYG'
 };
 
 interface StarMetaColumns {
@@ -161,6 +164,18 @@ function designationFor(prefix: string | undefined, id: number): string | undefi
   return prefix === undefined ? undefined : `${prefix} ${id}`;
 }
 
+/**
+ * Whether a star's name is only the designation its source generates, rather than anything
+ * somebody called it. Judged by the prefix alone: the number after it is the survey's own id —
+ * nineteen digits for Gaia — which the 32-bit row id `designationFor` prints cannot hold, so a
+ * round-trip through the id would call every one of those stars named.
+ */
+export function isDesignation(star: StarRecord): boolean {
+  // No source at all is a single-catalogue build, whose fallback is HYG's — see `decodeStarCatalog`.
+  const prefix = star.source === undefined ? DESIGNATION_PREFIXES['hyg'] : (DESIGNATION_PREFIXES[star.source] ?? star.source);
+  return star.name.startsWith(`${prefix} `);
+}
+
 /** Rebuilds the star records the app works with from the three loaded assets. */
 export function decodeStarCatalog(index: StarCatalogIndex, positions: Float32Array, meta: ArrayBuffer): StarRecord[] {
   const columns = metaColumns(meta, index.count);