diff --git a/apps/web/src/components/review/DiffViewer.tsx b/apps/web/src/components/review/DiffViewer.tsx index 5b9c1e2..d4cfd1e 100644 --- a/apps/web/src/components/review/DiffViewer.tsx +++ b/apps/web/src/components/review/DiffViewer.tsx @@ -1,9 +1,20 @@ import type { FileDiff, DiffLine, ReviewComment } from "./types"; import { FileCard } from "./FileCard"; +function getFileCommentMap( + commentsByLine: Map, + filePath: string, +): Map { + const result = new Map(); + for (const [key, val] of commentsByLine) { + if (key.startsWith(`${filePath}:`)) result.set(key, val); + } + return result; +} + interface DiffViewerProps { files: FileDiff[]; - comments: ReviewComment[]; + commentsByLine: Map; onAddComment: ( filePath: string, lineNumber: number, @@ -21,7 +32,7 @@ interface DiffViewerProps { export function DiffViewer({ files, - comments, + commentsByLine, onAddComment, onResolveComment, onUnresolveComment, @@ -37,7 +48,7 @@ export function DiffViewer({
onRegisterRef?.(file.newPath, el)}> c.filePath === file.newPath)} + commentsByLine={getFileCommentMap(commentsByLine, file.newPath)} onAddComment={onAddComment} onResolveComment={onResolveComment} onUnresolveComment={onUnresolveComment} diff --git a/apps/web/src/components/review/FileCard.tsx b/apps/web/src/components/review/FileCard.tsx index d7056c8..4d6ed6f 100644 --- a/apps/web/src/components/review/FileCard.tsx +++ b/apps/web/src/components/review/FileCard.tsx @@ -43,7 +43,7 @@ const leftBorderClass: Record = { interface FileCardProps { file: FileDiff; - comments: ReviewComment[]; + commentsByLine: Map; onAddComment: ( filePath: string, lineNumber: number, @@ -60,7 +60,7 @@ interface FileCardProps { export function FileCard({ file, - comments, + commentsByLine, onAddComment, onResolveComment, onUnresolveComment, @@ -70,7 +70,16 @@ export function FileCard({ onToggleViewed = () => {}, }: FileCardProps) { 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]; // Flatten all hunk lines for syntax highlighting @@ -158,7 +167,7 @@ export function FileCard({ key={hi} hunk={hunk} filePath={file.newPath} - comments={comments} + commentsByLine={commentsByLine} onAddComment={onAddComment} onResolveComment={onResolveComment} onUnresolveComment={onUnresolveComment} diff --git a/apps/web/src/components/review/HunkRows.tsx b/apps/web/src/components/review/HunkRows.tsx index 86cf6dd..9bec866 100644 --- a/apps/web/src/components/review/HunkRows.tsx +++ b/apps/web/src/components/review/HunkRows.tsx @@ -6,7 +6,7 @@ import type { LineTokenMap } from "./use-syntax-highlight"; interface HunkRowsProps { hunk: { header: string; lines: DiffLine[] }; filePath: string; - comments: ReviewComment[]; + commentsByLine: Map; onAddComment: ( filePath: string, lineNumber: number, @@ -23,7 +23,7 @@ interface HunkRowsProps { export function HunkRows({ hunk, filePath, - comments, + commentsByLine, onAddComment, onResolveComment, onUnresolveComment, @@ -81,9 +81,9 @@ export function HunkRows({ {hunk.lines.map((line, li) => { const lineKey = line.newLineNumber ?? line.oldLineNumber ?? li; - const lineComments = comments.filter( - (c) => c.lineNumber === lineKey && c.lineType === line.type, - ); + // O(1) map lookup — replaces the previous O(n) filter + const lineComments = + commentsByLine.get(`${filePath}:${lineKey}:${line.type}`) ?? []; const isCommenting = commentingLine?.lineNumber === lineKey && commentingLine?.lineType === line.type; diff --git a/apps/web/src/components/review/ReviewTab.tsx b/apps/web/src/components/review/ReviewTab.tsx index 099f380..8d0a2c0 100644 --- a/apps/web/src/components/review/ReviewTab.tsx +++ b/apps/web/src/components/review/ReviewTab.tsx @@ -7,6 +7,7 @@ import { DiffViewer } from "./DiffViewer"; import { ReviewSidebar } from "./ReviewSidebar"; import { ReviewHeader } from "./ReviewHeader"; import { InitiativeReview } from "./InitiativeReview"; +import { buildCommentIndex } from "./comment-index"; import type { ReviewStatus, DiffLine } from "./types"; interface ReviewTabProps { @@ -169,6 +170,11 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) { })); }, [commentsQuery.data]); + const commentsByLine = useMemo( + () => buildCommentIndex(comments), + [comments], + ); + const createCommentMutation = trpc.createReviewComment.useMutation({ onSuccess: () => { utils.listReviewComments.invalidate({ phaseId: activePhaseId! }); @@ -400,7 +406,7 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) { ) : ( ({ + CommentThread: () =>
, +})); +vi.mock("./CommentForm", () => ({ + CommentForm: vi.fn().mockReturnValue(
), +})); +vi.mock("./use-syntax-highlight", () => ({ + useHighlightedFile: () => null, +})); + +// ── Helpers ────────────────────────────────────────────────────────────────── + +function makeComment(overrides: Partial & { 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( + + + + +
, + ); + expect(screen.getByTitle(/1 comment/)).toBeInTheDocument(); + }); + + it("does not render comment thread row when lineComments is empty", () => { + render( + + + + +
, + ); + expect(document.querySelector("[data-comment-id]")).toBeNull(); + }); +}); diff --git a/apps/web/src/components/review/comment-index.ts b/apps/web/src/components/review/comment-index.ts new file mode 100644 index 0000000..cc486ca --- /dev/null +++ b/apps/web/src/components/review/comment-index.ts @@ -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 { + const map = new Map(); + 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; +} diff --git a/apps/web/src/components/review/types.ts b/apps/web/src/components/review/types.ts index 2b99452..2f9a1ad 100644 --- a/apps/web/src/components/review/types.ts +++ b/apps/web/src/components/review/types.ts @@ -28,7 +28,7 @@ export interface FileDiff { export interface ReviewComment { id: 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"; body: string; author: string;