perf: speed up inline-comment diff rendering with integer-keyed index

src/utils/displayLines.test.ts

@@ -0,0 +1,56 @@
+import type { Difference } from "@aws-sdk/client-codecommit";
+import { describe, expect, it } from "vitest";
+import type { CommentThread, ReactionsByComment } from "../services/codecommit.js";
+import { buildDisplayLines } from "./displayLines.js";
+
+const NO_REACTIONS: ReactionsByComment = new Map();
+
+function makeDiff(): {
+  differences: Difference[];
+  diffTexts: Map<string, { before: string; after: string }>;
+  diffTextStatus: Map<string, "loading" | "loaded" | "error">;
+  blobKey: string;
+} {
+  const differences: Difference[] = [
+    {
+      beforeBlob: { blobId: "b1", path: "src/a.ts" },
+      afterBlob: { blobId: "a1", path: "src/a.ts" },
+      changeType: "M",
+    },
+  ];
+  const blobKey = "b1:a1";
+  const diffTexts = new Map([[blobKey, { before: "old1\nold2", after: "new1\nnew2" }]]);
+  const diffTextStatus = new Map<string, "loading" | "loaded" | "error">([[blobKey, "loaded"]]);
+  return { differences, diffTexts, diffTextStatus, blobKey };
+}
+
+describe("buildDisplayLines inline threads", () => {
+  it("anchors BEFORE-version threads and renders multiple threads on the same line", () => {
+    const { differences, diffTexts, diffTextStatus } = makeDiff();
+    // Two threads anchored to the same BEFORE line (filePosition 1) on the same file.
+    const commentThreads: CommentThread[] = [
+      {
+        location: { filePath: "src/a.ts", filePosition: 1, relativeFileVersion: "BEFORE" },
+        comments: [{ commentId: "c1", content: "first", authorArn: "arn:aws:iam::1:user/alice" }],
+      },
+      {
+        location: { filePath: "src/a.ts", filePosition: 1, relativeFileVersion: "BEFORE" },
+        comments: [{ commentId: "c2", content: "second", authorArn: "arn:aws:iam::1:user/bob" }],
+      },
+    ];
+
+    const lines = buildDisplayLines(
+      differences,
+      diffTexts,
+      diffTextStatus,
+      new Map(),
+      commentThreads,
+      new Map(),
+      NO_REACTIONS,
+    );
+
+    const inlineTexts = lines.filter((l) => l.type === "inline-comment").map((l) => l.text);
+    expect(inlineTexts).toContain("💬 alice: first");
+    expect(inlineTexts).toContain("💬 bob: second");
+  });
+});

src/utils/displayLines.ts

@@ -136,37 +136,43 @@ function getSliceLimits(beforeCount: number, afterCount: number, totalLimit: num
 }
 /* v8 ignore stop */
 
+type ThreadEntry = { thread: CommentThread; index: number };
+
+/** Inline threads for a single file, bucketed by line number for each version. */
+interface FileThreadIndex {
+  before: Map<number, ThreadEntry[]>;
+  after: Map<number, ThreadEntry[]>;
+}
+
+/**
+ * Returns the inline-thread entries anchored to `line`, or undefined when none.
+ * Uses integer line-number lookups so the hot loop never concatenates keys.
+ */
 function findMatchingThreadEntries(
-  threadsByKey: Map<string, { thread: CommentThread; index: number }[]>,
-  filePath: string,
+  fileThreads: FileThreadIndex,
   line: DisplayLine,
-): { thread: CommentThread; index: number }[] {
-  const results: { thread: CommentThread; index: number }[] = [];
-
-  if (line.type === "delete" && line.beforeLineNumber) {
-    const key = `${filePath}:${line.beforeLineNumber}:BEFORE`;
-    results.push(...(threadsByKey.get(key) ?? []));
+): ThreadEntry[] | undefined {
+  if (line.type === "delete") {
+    return line.beforeLineNumber ? fileThreads.before.get(line.beforeLineNumber) : undefined;
   }
 
-  if (line.type === "add" && line.afterLineNumber) {
-    const key = `${filePath}:${line.afterLineNumber}:AFTER`;
-    results.push(...(threadsByKey.get(key) ?? []));
+  if (line.type === "add") {
+    return line.afterLineNumber ? fileThreads.after.get(line.afterLineNumber) : undefined;
   }
 
   /* v8 ignore start -- context lines always have both line numbers in practice */
   if (line.type === "context") {
-    if (line.beforeLineNumber) {
-      const key = `${filePath}:${line.beforeLineNumber}:BEFORE`;
-      results.push(...(threadsByKey.get(key) ?? []));
-    }
-    if (line.afterLineNumber) {
-      const key = `${filePath}:${line.afterLineNumber}:AFTER`;
-      results.push(...(threadsByKey.get(key) ?? []));
-    }
+    const before = line.beforeLineNumber
+      ? fileThreads.before.get(line.beforeLineNumber)
+      : undefined;
+    const after = line.afterLineNumber ? fileThreads.after.get(line.afterLineNumber) : undefined;
+    if (before && after) return [...before, ...after];
+    return before ?? after;
   }
   /* v8 ignore stop */
 
-  return results;
+  /* v8 ignore next -- diff lines are only ever add/delete/context */
+  return undefined;
 }
 
 export function buildDisplayLines(
@@ -181,15 +187,25 @@ export function buildDisplayLines(
 ): DisplayLine[] {
   const lines: DisplayLine[] = [];
 
-  // Index inline comments by file:position:version for efficient lookup
-  const inlineThreadsByKey = new Map<string, { thread: CommentThread; index: number }[]>();
+  // Index inline comments by file, then by line number per version, so the
+  // diff loop below can resolve anchors with integer lookups (no key strings).
+  const inlineThreadsByFile = new Map<string, FileThreadIndex>();
   for (let i = 0; i < commentThreads.length; i++) {
     const thread = commentThreads[i]!;
-    if (thread.location) {
-      const key = `${thread.location.filePath}:${thread.location.filePosition}:${thread.location.relativeFileVersion}`;
-      const existing = inlineThreadsByKey.get(key) ?? [];
+    const location = thread.location;
+    if (!location) continue;
+
+    let fileIndex = inlineThreadsByFile.get(location.filePath);
+    if (!fileIndex) {
+      fileIndex = { before: new Map(), after: new Map() };
+      inlineThreadsByFile.set(location.filePath, fileIndex);
+    }
+    const bucket = location.relativeFileVersion === "BEFORE" ? fileIndex.before : fileIndex.after;
+    const existing = bucket.get(location.filePosition);
+    if (existing) {
       existing.push({ thread, index: i });
-      inlineThreadsByKey.set(key, existing);
+    } else {
+      bucket.set(location.filePosition, [{ thread, index: i }]);
     }
   }
 
@@ -220,23 +236,28 @@ export function buildDisplayLines(
           afterLines.length,
           displayLimit,
         );
-        diffLines = computeSimpleDiff(
-          beforeLines.slice(0, beforeLimit),
-          afterLines.slice(0, afterLimit),
-        );
+        // Skip the slice copy when the limit already covers the whole array
+        // (the common non-truncated case); computeSimpleDiff never mutates its inputs.
+        const beforeSlice =
+          beforeLimit === beforeLines.length ? beforeLines : beforeLines.slice(0, beforeLimit);
+        const afterSlice =
+          afterLimit === afterLines.length ? afterLines : afterLines.slice(0, afterLimit);
+        diffLines = computeSimpleDiff(beforeSlice, afterSlice);
         // Enrich once here so the hot loop below can push lines without per-call copies
         for (const dl of diffLines) {
           dl.filePath = filePath;
           dl.diffKey = blobKey;
         }
         diffCache?.set(cacheKey, diffLines);
       }
-      const hasInlineThreads = inlineThreadsByKey.size > 0;
+      // Only files that actually have inline threads pay the per-line lookup cost.
+      const fileThreads = inlineThreadsByFile.get(filePath);
       for (const dl of diffLines) {
         lines.push(dl);
 
-        if (!hasInlineThreads) continue;
-        const matchingEntries = findMatchingThreadEntries(inlineThreadsByKey, filePath, dl);
+        if (!fileThreads) continue;
+        const matchingEntries = findMatchingThreadEntries(fileThreads, dl);
+        if (!matchingEntries) continue;
         for (const { thread, index: threadIdx } of matchingEntries) {
           appendThreadLines(
             lines,

src/utils/formatDiff.ts

@@ -51,16 +51,19 @@ function hasNearbyMatch(lines: string[], start: number, target: string): boolean
  */
 export function computeSimpleDiff(beforeLines: string[], afterLines: string[]): DisplayLine[] {
   const result: DisplayLine[] = [];
+  // Inputs are never mutated, so hoist lengths out of the hot loop.
+  const blen = beforeLines.length;
+  const alen = afterLines.length;
   let bi = 0; // Index for beforeLines
   let ai = 0; // Index for afterLines
 
   // Process both arrays until all lines are consumed
-  while (bi < beforeLines.length || ai < afterLines.length) {
+  while (bi < blen || ai < alen) {
     const beforeLine = beforeLines[bi];
     const afterLine = afterLines[ai];
 
     // Case 1: Lines match at current position - add as context
-    if (bi < beforeLines.length && ai < afterLines.length && beforeLine === afterLine) {
+    if (bi < blen && ai < alen && beforeLine === afterLine) {
       result.push({
         type: "context",
         text: ` ${beforeLine}`,
@@ -75,10 +78,7 @@ export function computeSimpleDiff(beforeLines: string[], afterLines: string[]):
       const startAi = ai;
 
       // Process deletions: consume lines from 'before' that don't match current 'after'
-      while (
-        bi < beforeLines.length &&
-        (ai >= afterLines.length || beforeLines[bi] !== afterLines[ai])
-      ) {
+      while (bi < blen && (ai >= alen || beforeLines[bi] !== afterLines[ai])) {
         const bl = beforeLines[bi]!;
         // Optimization: stop if this line appears within the lookahead window in 'after'
         if (hasNearbyMatch(afterLines, ai, bl)) break;
@@ -91,10 +91,7 @@ export function computeSimpleDiff(beforeLines: string[], afterLines: string[]):
       }
 
       // Process additions: consume lines from 'after' that don't match current 'before'
-      while (
-        ai < afterLines.length &&
-        (bi >= beforeLines.length || afterLines[ai] !== beforeLines[bi])
-      ) {
+      while (ai < alen && (bi >= blen || afterLines[ai] !== beforeLines[bi])) {
         const al = afterLines[ai]!;
         // Optimization: stop if this line appears within the lookahead window in 'before'
         if (hasNearbyMatch(beforeLines, bi, al)) break;
@@ -109,15 +106,15 @@ export function computeSimpleDiff(beforeLines: string[], afterLines: string[]):
       // Safety: if both loops broke without advancing, force progress to prevent infinite loop
       /* v8 ignore start -- defensive guard; the greedy algorithm always advances in normal cases */
       if (bi === startBi && ai === startAi) {
-        if (bi < beforeLines.length) {
+        if (bi < blen) {
           result.push({
             type: "delete",
             text: `-${beforeLines[bi]}`,
             beforeLineNumber: bi + 1,
           });
           bi++;
         }
-        if (ai < afterLines.length) {
+        if (ai < alen) {
           result.push({
             type: "add",
             text: `+${afterLines[ai]}`,