Answer the review: share a graph request asked again, by its range and its drawn list

Graph requests were never shared, on the grounds that the scene never asks for the same graph
twice. It does: turning the layer off and on while the worker is busy asks again for the graph
already waiting. The new request superseded the old one, and the old one's rejection handler,
which finds its request by range and drawn list, wiped the state of the new one: the layer stayed
on with no graph. An identical request now shares the outstanding promise, a graph being the same
when its range matches and its drawn list is the very same array.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016jxMkwA2rbicdGxHosecYi

src/app/features/galaxy-system/routing-client.spec.ts

@@ -148,12 +148,38 @@ describe('RoutingClient with a worker', () => {
     const { client, worker } = clientWithFake();
     const once = client.route(100, 104, 1.5, 8);
     const again = client.route(100, 104, 1.5, 8);
+    const widerRange = client.route(100, 104, 2.5, 8);
 
     expect(worker.requests).toHaveLength(1);
     worker.answer({ kind: 'route', requestId: worker.requests[0].requestId, route: null, neededRangePc: 4 });
 
     expect(await again).toEqual(await once);
-    expect(worker.requests).toHaveLength(1);
+    await flush();
+    // The same two stars at another range is another question.
+    expect(worker.requests.map((request) => request.rangePc)).toEqual([1.5, 2.5]);
+    worker.answer({ kind: 'route', requestId: worker.requests[1].requestId, route: null, neededRangePc: null });
+    await expect(widerRange).resolves.toEqual({ route: null, neededRangePc: null });
+    client.dispose();
+  });
+
+  // Turning the layer off and on again while the worker is busy asks for the same graph twice. Were
+  // the second to replace the first, the first's rejection would wipe the scene's record of the second.
+  it('shares a graph already on its way for the same range and the same list of drawn stars', async () => {
+    const { client, worker } = clientWithFake();
+    const drawn = Uint32Array.of(0, 1, 2);
+    const building = client.links(3, drawn);
+    const waiting = client.links(5, drawn);
+    const again = client.links(5, drawn);
+    const sameAsBuilding = client.links(3, drawn);
+
+    worker.answer({ kind: 'links', requestId: worker.requests[0].requestId, segments: new Float32Array(6) });
+    await expect(building).resolves.toHaveLength(6);
+    await expect(sameAsBuilding).resolves.toHaveLength(6);
+    await flush();
+    worker.answer({ kind: 'links', requestId: worker.requests[1].requestId, segments: new Float32Array(12) });
+    await expect(waiting).resolves.toHaveLength(12);
+    await expect(again).resolves.toHaveLength(12);
+    expect(worker.requests.map((request) => request.kind === 'links' && request.rangePc)).toEqual([3, 5]);
     client.dispose();
   });
 

src/app/features/galaxy-system/routing-client.ts

@@ -18,12 +18,6 @@ export class SupersededRequest extends Error {
 /** A request made and not yet answered: what was asked, and the promise whoever asked is holding. */
 interface Outstanding {
   readonly request: RoutingRequest;
-  /**
-   * A route question without its id, so the same route asked for twice can be recognised. Empty for
-   * a graph: the scene never asks for the same graph twice, and spelling out 70 000 drawn stars to
-   * compare costs more than the comparison could save.
-   */
-  readonly question: string;
   readonly promise: Promise<RoutingResponse>;
   readonly resolve: (response: RoutingResponse) => void;
   readonly reject: (error: Error) => void;
@@ -36,9 +30,19 @@ function outstanding(request: RoutingRequest): Outstanding {
     resolve = onResolve;
     reject = onReject;
   });
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  return { request, promise, resolve, reject };
-  const { requestId, ...question } = request;
+}
-  return { request, question: request.kind === 'route' ? JSON.stringify(question) : '', promise, resolve, reject };
+
+/**
+ * Whether two requests ask the same question. A graph is the same when it is for the same range and
+ * the very same list of drawn stars: the star field replaces that list whenever the set changes, so
+ * one array is one set, and comparing 70 000 indices would cost more than sharing could save.
+ */
+function asksTheSame(a: RoutingRequest, b: RoutingRequest): boolean {
+  if (a.kind === 'links' || b.kind === 'links') {
+    return a.kind === 'links' && b.kind === 'links' && a.rangePc === b.rangePc && a.drawn === b.drawn;
+  }
+  return a.fromId === b.fromId && a.toId === b.toId && a.rangePc === b.rangePc && a.ceilingPc === b.ceilingPc;
 }
 
 /** The routing worker, where this environment has one. */
@@ -50,12 +54,13 @@ function startRoutingWorker(): Worker | undefined {
  * Asks the route questions of a worker holding its own copy of the catalogue, and hands back
  * promises.
  *
- * The worker answers one request at a time and cannot drop one it has started: a jump-link graph
+ * The worker answers one request at a time and cannot drop one it has started: a route with no path
- * at 8 pc is seconds of work. So requests are held here and sent one by one, and while one is out,
+ * can be seconds of work, and a graph of the drawn stars at 8 pc a few hundred milliseconds. So
- * only the latest of each kind waits behind it — a newer graph replaces an older one before it is
+ * requests are held here and sent one by one, and while one is out, only the latest of each kind
- * ever built, and the older promise is rejected with {@link SupersededRequest}. Routes go ahead of
+ * waits behind it — a newer graph replaces an older one before it is ever built, and the older
- * graphs, being quick and asked for by a click. The same route asked for again while it is still
+ * promise is rejected with {@link SupersededRequest}. Routes go ahead of graphs, being quick to ask
- * outstanding shares the answer rather than being worked out twice.
+ * for and asked for by a click. The same question asked again while it is still outstanding shares
+ * the answer rather than being worked out twice; see `asksTheSame`.
  *
  * Where there is no worker — the unit tests' DOM has none, and a worker can fail to load or crash —
  * the same answers are worked out in place, from the index the scene already holds.
@@ -115,11 +120,13 @@ export class RoutingClient {
     if (!this.worker) {
       return new Promise((resolve) => resolve(answerRouting(this.localIndex, request)));
     }
-    const asked = outstanding(request);
+    // Shared rather than replaced: an identical request superseding the one it repeats would reject it,
-    const same = asked.question ? [this.inFlight, this.waiting[request.kind]].find((other) => other?.question === asked.question) : undefined;
+    // and whoever holds that promise would take the rejection for its own question.
+    const same = [this.inFlight, this.waiting[request.kind]].find((other) => other !== undefined && asksTheSame(other.request, request));
     if (same) {
       return same.promise;
     }
+    const asked = outstanding(request);
     this.waiting[request.kind]?.reject(new SupersededRequest());
     this.waiting[request.kind] = asked;
     this.sendNext();