fix(webapp): key a repeated view block by its own position

ViewBlocks looked each surviving block's index up with indexOf inside the render
loop: quadratic, and two occurrences of the same block object both answered with
the first index, so they collided on one React key. latestRevisionEntries carries
each survivor's position out instead.

Author: kathiekiwi

apps/webapp/app/components/dashboard-agent/view-blocks.test.ts

@@ -1,5 +1,10 @@
 import { describe, expect, it } from "vitest";
-import { blockIdentity, blockKey, latestRevisionBlocks } from "./view-blocks";
+import {
+  blockIdentity,
+  blockKey,
+  latestRevisionBlocks,
+  latestRevisionEntries,
+} from "./view-blocks";
 
 const enveloped = (id: string, revision: number, type = "diagnosis") => ({ type, id, revision });
 
@@ -62,6 +67,39 @@ describe("latestRevisionBlocks", () => {
   });
 });
 
+/**
+ * The positions are what `ViewBlocks` keys envelope-less blocks on. Looking one up afterwards
+ * with `indexOf` answers with the first equal block, so two of them collide on one React key.
+ */
+describe("latestRevisionEntries", () => {
+  it("reports each survivor's position in the original array", () => {
+    const legacy = { type: "chart" };
+    const blocks = [enveloped("d1", 1), legacy, enveloped("d1", 2)];
+    expect(latestRevisionEntries(blocks)).toEqual([
+      { block: legacy, index: 1 },
+      { block: enveloped("d1", 2), index: 2 },
+    ]);
+  });
+
+  it("gives two occurrences of the same block object distinct positions", () => {
+    const repeated = { type: "chart" };
+    const entries = latestRevisionEntries([repeated, repeated]);
+    expect(entries.map((entry) => entry.index)).toEqual([0, 1]);
+    expect(new Set(entries.map((entry) => blockKey(entry.block, entry.index))).size).toBe(2);
+  });
+
+  it("agrees with `latestRevisionBlocks` on which blocks survive", () => {
+    const blocks = [enveloped("d1", 1), { type: "chart" }, enveloped("d1", 2)];
+    expect(latestRevisionEntries(blocks).map((entry) => entry.block)).toEqual(
+      latestRevisionBlocks(blocks)
+    );
+  });
+
+  it("tolerates a non-array", () => {
+    expect(latestRevisionEntries(undefined as unknown as unknown[])).toEqual([]);
+  });
+});
+
 describe("blockIdentity / blockKey", () => {
   it("identifies enveloped blocks by type and id", () => {
     expect(blockIdentity(enveloped("d1", 1))).toBe("diagnosis::d1");

apps/webapp/app/components/dashboard-agent/view-blocks.ts

@@ -25,9 +25,13 @@ export function blockKey(block: unknown, index: number): string {
   return blockIdentity(block) ?? `index:${index}`;
 }
 
-// Latest-wins within one array only: highest `revision` at the winner's position,
-// ties to the last. Blocks without an envelope are all kept, in order.
-export function latestRevisionBlocks<T>(blocks: readonly T[]): T[] {
+/**
+ * Latest-wins within one array only: highest `revision` at the winner's position, ties to the
+ * last. Blocks without an envelope are all kept, in order. Each survivor keeps the index it had
+ * in `blocks`, which is what an envelope-less block is keyed on — a search for it afterwards
+ * would answer with the first equal block, not this one.
+ */
+export function latestRevisionEntries<T>(blocks: readonly T[]): { block: T; index: number }[] {
   if (!Array.isArray(blocks)) return [];
 
   const winnerIndexByIdentity = new Map<string, number>();
@@ -43,10 +47,17 @@ export function latestRevisionBlocks<T>(blocks: readonly T[]): T[] {
     }
   });
 
-  if (winnerIndexByIdentity.size === 0) return [...blocks];
-
-  return blocks.filter((block, index) => {
+  const entries: { block: T; index: number }[] = [];
+  blocks.forEach((block, index) => {
     const identity = blockIdentity(block);
-    return identity === undefined || winnerIndexByIdentity.get(identity) === index;
+    if (identity === undefined || winnerIndexByIdentity.get(identity) === index) {
+      entries.push({ block, index });
+    }
   });
+  return entries;
+}
+
+/** {@link latestRevisionEntries} without the positions. */
+export function latestRevisionBlocks<T>(blocks: readonly T[]): T[] {
+  return latestRevisionEntries(blocks).map((entry) => entry.block);
 }

apps/webapp/app/components/dashboard-agent/view-catalog.tsx

@@ -4,7 +4,7 @@ import { AgentChart } from "./AgentChart";
 import { InvestigationCard } from "./InvestigationCard";
 import { ReportView, type ResolvedUri } from "./ReportView";
 import { RunDiagnosisCard } from "./RunDiagnosisCard";
-import { blockKey, latestRevisionBlocks } from "./view-blocks";
+import { blockKey, latestRevisionEntries } from "./view-blocks";
 
 // Unknown block types are skipped, so an older or newer agent cannot render
 // arbitrary content. A new block needs a `case` here and a `viewBlockSchema` member.
@@ -25,10 +25,10 @@ export function ViewBlocks({
   if (!Array.isArray(blocks)) return null;
   return (
     <div className="space-y-2">
-      {latestRevisionBlocks(blocks).map((block) => {
-        // Index into the original array, so collapsing a revision above an
+      {latestRevisionEntries(blocks).map(({ block, index }) => {
+        // The original array's index, so collapsing a revision above an
         // envelope-less block can't shift its key.
-        const key = blockKey(block, blocks.indexOf(block));
+        const key = blockKey(block, index);
         switch (block.type) {
           case "diagnosis":
             return <RunDiagnosisCard key={key} block={block} />;