perf: Pre-index review comments into Map to eliminate O(n) filtering

Build a Map<string, ReviewComment[]> once in ReviewTab and thread it
down through DiffViewer → FileCard → HunkRows, replacing O(n) filter
calls with O(1) map lookups. With 200 comments and 5000 diff lines,
this reduces ~1M iterations per render cycle to ~5K.

Key: "${filePath}:${lineNumber}:${lineType}" for line-level comments,
"${filePath}:file" for file-level (lineNumber === null) comments.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Lukas May
2026-03-06 19:31:57 +01:00
parent 2eccde0ee1
commit ec86b62d8d
7 changed files with 199 additions and 14 deletions

View File

@@ -1,9 +1,20 @@
import type { FileDiff, DiffLine, ReviewComment } from "./types"; import type { FileDiff, DiffLine, ReviewComment } from "./types";
import { FileCard } from "./FileCard"; import { FileCard } from "./FileCard";
function getFileCommentMap(
commentsByLine: Map<string, ReviewComment[]>,
filePath: string,
): Map<string, ReviewComment[]> {
const result = new Map<string, ReviewComment[]>();
for (const [key, val] of commentsByLine) {
if (key.startsWith(`${filePath}:`)) result.set(key, val);
}
return result;
}
interface DiffViewerProps { interface DiffViewerProps {
files: FileDiff[]; files: FileDiff[];
comments: ReviewComment[]; commentsByLine: Map<string, ReviewComment[]>;
onAddComment: ( onAddComment: (
filePath: string, filePath: string,
lineNumber: number, lineNumber: number,
@@ -21,7 +32,7 @@ interface DiffViewerProps {
export function DiffViewer({ export function DiffViewer({
files, files,
comments, commentsByLine,
onAddComment, onAddComment,
onResolveComment, onResolveComment,
onUnresolveComment, onUnresolveComment,
@@ -37,7 +48,7 @@ export function DiffViewer({
<div key={file.newPath} ref={(el) => onRegisterRef?.(file.newPath, el)}> <div key={file.newPath} ref={(el) => onRegisterRef?.(file.newPath, el)}>
<FileCard <FileCard
file={file} file={file}
comments={comments.filter((c) => c.filePath === file.newPath)} commentsByLine={getFileCommentMap(commentsByLine, file.newPath)}
onAddComment={onAddComment} onAddComment={onAddComment}
onResolveComment={onResolveComment} onResolveComment={onResolveComment}
onUnresolveComment={onUnresolveComment} onUnresolveComment={onUnresolveComment}

View File

@@ -43,7 +43,7 @@ const leftBorderClass: Record<FileChangeType, string> = {
interface FileCardProps { interface FileCardProps {
file: FileDiff; file: FileDiff;
comments: ReviewComment[]; commentsByLine: Map<string, ReviewComment[]>;
onAddComment: ( onAddComment: (
filePath: string, filePath: string,
lineNumber: number, lineNumber: number,
@@ -60,7 +60,7 @@ interface FileCardProps {
export function FileCard({ export function FileCard({
file, file,
comments, commentsByLine,
onAddComment, onAddComment,
onResolveComment, onResolveComment,
onUnresolveComment, onUnresolveComment,
@@ -70,7 +70,16 @@ export function FileCard({
onToggleViewed = () => {}, onToggleViewed = () => {},
}: FileCardProps) { }: FileCardProps) {
const [expanded, setExpanded] = useState(true); const [expanded, setExpanded] = useState(true);
const commentCount = comments.length;
const commentCount = useMemo(
() =>
Array.from(commentsByLine.values()).reduce(
(sum, arr) => sum + arr.length,
0,
),
[commentsByLine],
);
const badge = changeTypeBadge[file.changeType]; const badge = changeTypeBadge[file.changeType];
// Flatten all hunk lines for syntax highlighting // Flatten all hunk lines for syntax highlighting
@@ -158,7 +167,7 @@ export function FileCard({
key={hi} key={hi}
hunk={hunk} hunk={hunk}
filePath={file.newPath} filePath={file.newPath}
comments={comments} commentsByLine={commentsByLine}
onAddComment={onAddComment} onAddComment={onAddComment}
onResolveComment={onResolveComment} onResolveComment={onResolveComment}
onUnresolveComment={onUnresolveComment} onUnresolveComment={onUnresolveComment}

View File

@@ -6,7 +6,7 @@ import type { LineTokenMap } from "./use-syntax-highlight";
interface HunkRowsProps { interface HunkRowsProps {
hunk: { header: string; lines: DiffLine[] }; hunk: { header: string; lines: DiffLine[] };
filePath: string; filePath: string;
comments: ReviewComment[]; commentsByLine: Map<string, ReviewComment[]>;
onAddComment: ( onAddComment: (
filePath: string, filePath: string,
lineNumber: number, lineNumber: number,
@@ -23,7 +23,7 @@ interface HunkRowsProps {
export function HunkRows({ export function HunkRows({
hunk, hunk,
filePath, filePath,
comments, commentsByLine,
onAddComment, onAddComment,
onResolveComment, onResolveComment,
onUnresolveComment, onUnresolveComment,
@@ -81,9 +81,9 @@ export function HunkRows({
{hunk.lines.map((line, li) => { {hunk.lines.map((line, li) => {
const lineKey = line.newLineNumber ?? line.oldLineNumber ?? li; const lineKey = line.newLineNumber ?? line.oldLineNumber ?? li;
const lineComments = comments.filter( // O(1) map lookup — replaces the previous O(n) filter
(c) => c.lineNumber === lineKey && c.lineType === line.type, const lineComments =
); commentsByLine.get(`${filePath}:${lineKey}:${line.type}`) ?? [];
const isCommenting = const isCommenting =
commentingLine?.lineNumber === lineKey && commentingLine?.lineNumber === lineKey &&
commentingLine?.lineType === line.type; commentingLine?.lineType === line.type;

View File

@@ -7,6 +7,7 @@ import { DiffViewer } from "./DiffViewer";
import { ReviewSidebar } from "./ReviewSidebar"; import { ReviewSidebar } from "./ReviewSidebar";
import { ReviewHeader } from "./ReviewHeader"; import { ReviewHeader } from "./ReviewHeader";
import { InitiativeReview } from "./InitiativeReview"; import { InitiativeReview } from "./InitiativeReview";
import { buildCommentIndex } from "./comment-index";
import type { ReviewStatus, DiffLine } from "./types"; import type { ReviewStatus, DiffLine } from "./types";
interface ReviewTabProps { interface ReviewTabProps {
@@ -169,6 +170,11 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) {
})); }));
}, [commentsQuery.data]); }, [commentsQuery.data]);
const commentsByLine = useMemo(
() => buildCommentIndex(comments),
[comments],
);
const createCommentMutation = trpc.createReviewComment.useMutation({ const createCommentMutation = trpc.createReviewComment.useMutation({
onSuccess: () => { onSuccess: () => {
utils.listReviewComments.invalidate({ phaseId: activePhaseId! }); utils.listReviewComments.invalidate({ phaseId: activePhaseId! });
@@ -400,7 +406,7 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) {
) : ( ) : (
<DiffViewer <DiffViewer
files={files} files={files}
comments={comments} commentsByLine={commentsByLine}
onAddComment={handleAddComment} onAddComment={handleAddComment}
onResolveComment={handleResolveComment} onResolveComment={handleResolveComment}
onUnresolveComment={handleUnresolveComment} onUnresolveComment={handleUnresolveComment}

View File

@@ -0,0 +1,134 @@
// @vitest-environment happy-dom
import "@testing-library/jest-dom/vitest";
import { render, screen } from "@testing-library/react";
import { vi, describe, it, expect } from "vitest";
import { buildCommentIndex } from "./comment-index";
import type { ReviewComment } from "./types";
// ── Stub CommentThread and CommentForm so LineWithComments renders without deps ──
vi.mock("./CommentThread", () => ({
CommentThread: () => <div data-testid="comment-thread" />,
}));
vi.mock("./CommentForm", () => ({
CommentForm: vi.fn().mockReturnValue(<div data-testid="comment-form" />),
}));
vi.mock("./use-syntax-highlight", () => ({
useHighlightedFile: () => null,
}));
// ── Helpers ──────────────────────────────────────────────────────────────────
function makeComment(overrides: Partial<ReviewComment> & { id: string }): ReviewComment {
return {
id: overrides.id,
filePath: overrides.filePath ?? "src/foo.ts",
lineNumber: overrides.lineNumber !== undefined ? overrides.lineNumber : 1,
lineType: overrides.lineType ?? "added",
body: overrides.body ?? "comment body",
author: overrides.author ?? "alice",
createdAt: overrides.createdAt ?? "2024-01-01T00:00:00Z",
resolved: overrides.resolved ?? false,
parentCommentId: overrides.parentCommentId ?? null,
};
}
// ── buildCommentIndex — pure function tests ───────────────────────────────────
describe("buildCommentIndex", () => {
it("happy path — basic indexing", () => {
const c1 = makeComment({ id: "1", filePath: "src/foo.ts", lineNumber: 10, lineType: "added" });
const c2 = makeComment({ id: "2", filePath: "src/bar.ts", lineNumber: 5, lineType: "context" });
const map = buildCommentIndex([c1, c2]);
expect(map.get("src/foo.ts:10:added")).toEqual([c1]);
expect(map.get("src/bar.ts:5:context")).toEqual([c2]);
expect(map.size).toBe(2);
});
it("same-line accumulation — two comments land in same array", () => {
const c1 = makeComment({ id: "a", filePath: "src/x.ts", lineNumber: 20, lineType: "added" });
const c2 = makeComment({ id: "b", filePath: "src/x.ts", lineNumber: 20, lineType: "added" });
const map = buildCommentIndex([c1, c2]);
expect(map.get("src/x.ts:20:added")).toEqual([c1, c2]);
expect(map.size).toBe(1);
});
it("cross-type isolation — same lineNumber but different lineType produces separate entries", () => {
const added = makeComment({ id: "a", filePath: "src/x.ts", lineNumber: 10, lineType: "added" });
const removed = makeComment({ id: "r", filePath: "src/x.ts", lineNumber: 10, lineType: "removed" });
const map = buildCommentIndex([added, removed]);
expect(map.get("src/x.ts:10:added")).toEqual([added]);
expect(map.get("src/x.ts:10:removed")).toEqual([removed]);
expect(map.size).toBe(2);
});
it("null lineNumber — file-level comment stored under filePath:file", () => {
const fileComment = makeComment({ id: "f", filePath: "src/z.ts", lineNumber: null, lineType: "context" });
const map = buildCommentIndex([fileComment]);
expect(map.get("src/z.ts:file")).toEqual([fileComment]);
});
it("empty input — returns empty map", () => {
expect(buildCommentIndex([])).toEqual(new Map());
});
});
// ── LineWithComments — component tests ───────────────────────────────────────
import { LineWithComments } from "./LineWithComments";
import type { DiffLine } from "./types";
const addedLine: DiffLine = {
type: "added",
content: "const x = 1;",
oldLineNumber: null,
newLineNumber: 5,
};
const noop = () => {};
describe("LineWithComments", () => {
it("renders comment button with title when lineComments is non-empty", () => {
const lineComments = [
makeComment({ id: "c1", filePath: "src/foo.ts", lineNumber: 5, lineType: "added" }),
];
render(
<table>
<tbody>
<LineWithComments
line={addedLine}
lineKey={5}
lineComments={lineComments}
isCommenting={false}
onStartComment={noop}
onCancelComment={noop}
onSubmitComment={noop}
onResolveComment={noop}
onUnresolveComment={noop}
/>
</tbody>
</table>,
);
expect(screen.getByTitle(/1 comment/)).toBeInTheDocument();
});
it("does not render comment thread row when lineComments is empty", () => {
render(
<table>
<tbody>
<LineWithComments
line={addedLine}
lineKey={5}
lineComments={[]}
isCommenting={false}
onStartComment={noop}
onCancelComment={noop}
onSubmitComment={noop}
onResolveComment={noop}
onUnresolveComment={noop}
/>
</tbody>
</table>,
);
expect(document.querySelector("[data-comment-id]")).toBeNull();
});
});

View File

@@ -0,0 +1,25 @@
import type { ReviewComment } from "./types";
/**
* Build a Map keyed by `"${filePath}:${lineNumber}:${lineType}"` for line-level
* comments, or `"${filePath}:file"` for file-level comments (lineNumber === null).
*
* The compound key (filePath + lineNumber + lineType) is required because
* added and removed lines can share the same numeric position in a replacement
* hunk (e.g., old line 10 removed, new line 10 added).
*/
export function buildCommentIndex(
comments: ReviewComment[],
): Map<string, ReviewComment[]> {
const map = new Map<string, ReviewComment[]>();
for (const comment of comments) {
const key =
comment.lineNumber != null
? `${comment.filePath}:${comment.lineNumber}:${comment.lineType}`
: `${comment.filePath}:file`;
const existing = map.get(key);
if (existing) existing.push(comment);
else map.set(key, [comment]);
}
return map;
}

View File

@@ -28,7 +28,7 @@ export interface FileDiff {
export interface ReviewComment { export interface ReviewComment {
id: string; id: string;
filePath: string; filePath: string;
lineNumber: number; // new-side line number (or old-side for deletions) lineNumber: number | null; // null = file-level comment
lineType: "added" | "removed" | "context"; lineType: "added" | "removed" | "context";
body: string; body: string;
author: string; author: string;