Share comment thread building between patch views

Revision and ReviewPage built threads and checked who may resolve a
comment with their own near-identical copies.

src/components/ReviewPage.svelte

@@ -39,13 +39,18 @@
   import { anchorOf } from "@app/lib/pierreComments";
   import * as roles from "@app/lib/roles";
   import { push } from "@app/lib/router";
+  import {
+    buildThreads,
+    groupThreadsByFile,
+    isCodeRoot,
+    reviewDiscussionThreads,
+  } from "@app/lib/threads";
   import {
     absoluteTimestamp,
     authorForNodeId,
     didFromPublicKey,
     formatTimestamp,
     pluralize,
-    publicKeyFromDid,
     revisionPosition,
     verdictAction,
     verdictBadge,
@@ -247,20 +252,9 @@
     }
   }
 
-  // Comments on the review itself rather than on a line. The summary is a
-  // field, not the root of this thread, so a comment replying to the review id
-  // starts a root thread of its own.
-  const commentThreads: Thread[] = $derived.by(() => {
-    const comments = (review.comments ?? []) as Comment<CodeLocation>[];
-    return comments
-      .filter(c => (!c.location && !c.replyTo) || c.replyTo === review.id)
-      .map(root => ({
-        root,
-        replies: comments
-          .filter(c => c.replyTo === root.id)
-          .sort((a, b) => a.edits[0].timestamp - b.edits[0].timestamp),
-      })) as unknown as Thread[];
-  });
+  const commentThreads = $derived(
+    reviewDiscussionThreads(review) as unknown as Thread[],
+  );
 
   async function createDiscussionComment(
     body: string,
@@ -328,21 +322,15 @@
   // revision comments have no resolve action at all.
   function canResolveComment(commentId: string): boolean {
     if (ownerOf(commentId)?.kind !== "review") return false;
-    if (
-      roles.isDelegate(
-        config.publicKey,
-        repoDelegates.map(d => d.did),
-      )
-    ) {
-      return true;
-    }
-    const comment = (review.comments ?? []).find(c => c.id === commentId);
-    return [
-      comment?.author.did,
-      review.author.did,
-      reviewedRevision?.author.did,
-    ].some(
-      did => did !== undefined && publicKeyFromDid(did) === config.publicKey,
+    return roles.canResolveReviewComment(
+      config.publicKey,
+      repoDelegates.map(d => d.did),
+      {
+        comment: (review.comments ?? []).find(c => c.id === commentId)?.author
+          .did,
+        review: review.author.did,
+        revision: reviewedRevision?.author.did,
+      },
     );
   }
 
@@ -357,35 +345,22 @@
     }),
   );
 
-  const fileGroups: FileGroup[] = $derived.by(() => {
-    // eslint-disable-next-line svelte/prefer-svelte-reactivity
-    const groups = new Map<string, Thread<CodeLocation>[]>();
-    const addThreads = (
-      comments: Comment<CodeLocation>[],
-      excludeId: string,
-    ) => {
-      const roots = comments.filter(
-        c => c.location && !c.replyTo && c.id !== excludeId,
-      );
-      for (const root of roots) {
-        const replies = comments
-          .filter(c => c.replyTo === root.id)
-          .sort((a, b) => a.edits[0].timestamp - b.edits[0].timestamp);
-        const thread = { root, replies } as Thread<CodeLocation>;
-        const path = root.location!.path;
-        const list = groups.get(path) ?? [];
-        list.push(thread);
-        groups.set(path, list);
-      }
-    };
-    addThreads(review.comments as Comment<CodeLocation>[], review.id);
-    // Standalone code comments left directly on the reviewed revision (not part
-    // of the review itself) also belong on this page.
-    if (reviewedRevision?.discussion) {
-      addThreads(reviewedRevision.discussion, reviewedRevision.id);
-    }
-    return [...groups.entries()].map(([path, threads]) => ({ path, threads }));
-  });
+  // Standalone code comments left directly on the reviewed revision (not part
+  // of the review itself) also belong on this page.
+  const fileGroups: FileGroup[] = $derived(
+    groupThreadsByFile([
+      ...buildThreads(
+        review.comments as Comment<CodeLocation>[],
+        c => isCodeRoot(c) && c.id !== review.id,
+      ),
+      ...(reviewedRevision
+        ? buildThreads(
+            reviewedRevision.discussion ?? [],
+            c => isCodeRoot(c) && c.id !== reviewedRevision.id,
+          )
+        : []),
+    ]),
+  );
 
   // Standalone comments render next to the review's own and look identical,
   // but nothing can resolve them and their edits target the revision, so they

src/components/Revision.svelte

@@ -34,6 +34,16 @@
   import { patchTimeline } from "@app/lib/patchTimeline";
   import * as roles from "@app/lib/roles";
   import { push } from "@app/lib/router";
+  import {
+    buildThreads,
+    codeThreadsByReview,
+    discussionThreadsByReview as discussionThreadsByReviewOf,
+    groupThreadsByFile,
+    isCodeRoot,
+    reviewIdByComment,
+    revisionDiscussionThreads,
+    revisionIdByComment,
+  } from "@app/lib/threads";
   import {
     authorForNodeId,
     didFromPublicKey,
@@ -255,14 +265,7 @@
   const hasPublishedReview = $derived(Boolean(ownPublishedReview));
 
   const codeCommentThreads: Thread<CodeLocation>[] = $derived(
-    draftReview
-      ? (draftReview.comments
-          .filter(c => c.location && !c.replyTo)
-          .map(root => ({
-            root,
-            replies: draftReview.comments.filter(c => c.replyTo === root.id),
-          })) as Thread<CodeLocation>[])
-      : [],
+    draftReview ? buildThreads(draftReview.comments, isCodeRoot) : [],
   );
 
   async function createCodeComment(
@@ -379,72 +382,18 @@
     }
   }
 
-  // One header per file with its hunks stacked beneath, rather than a
-  // duplicate file block per comment.
-  function groupThreadsByFile(
-    list: Thread<CodeLocation>[],
-  ): { path: string; threads: Thread<CodeLocation>[] }[] {
-    // eslint-disable-next-line svelte/prefer-svelte-reactivity
-    const groups = new Map<string, Thread<CodeLocation>[]>();
-    for (const thread of list) {
-      const path = thread.root.location?.path;
-      if (!path) continue;
-      const existing = groups.get(path) ?? [];
-      existing.push(thread);
-      groups.set(path, existing);
-    }
-    return [...groups.entries()].map(([path, threads]) => ({ path, threads }));
-  }
-
   // Comments on a review itself rather than on a line. Kept apart from
   // `threadsByReview`, which feeds the diff and must only carry threads that
   // can be anchored to a line.
-  const discussionThreadsByReview = $derived.by(() => {
-    // eslint-disable-next-line svelte/prefer-svelte-reactivity
-    const map = new Map<string, Thread<CodeLocation>[]>();
-    (revision.reviews ?? []).forEach(review => {
-      const comments = review.comments ?? [];
-      const threads = comments
-        .filter(c => (!c.location && !c.replyTo) || c.replyTo === review.id)
-        .map(root => ({
-          root,
-          replies: comments
-            .filter(c => c.replyTo === root.id)
-            .sort((a, b) => a.edits[0].timestamp - b.edits[0].timestamp),
-        })) as Thread<CodeLocation>[];
-      if (threads.length > 0) {
-        map.set(review.id, threads);
-      }
-    });
-    return map;
-  });
+  const discussionThreadsByReview = $derived(
+    discussionThreadsByReviewOf(revision.reviews ?? []),
+  );
 
-  const commentToReviewId = $derived.by(() => {
-    // eslint-disable-next-line svelte/prefer-svelte-reactivity
-    const map = new Map<string, string>();
-    for (const source of [threadsByReview, discussionThreadsByReview]) {
-      for (const [reviewId, threads] of source.entries()) {
-        for (const thread of threads) {
-          map.set(thread.root.id, reviewId);
-          for (const reply of thread.replies) {
-            map.set(reply.id, reviewId);
-          }
-        }
-      }
-    }
-    return map;
-  });
+  const commentToReviewId = $derived.by(() =>
+    reviewIdByComment(threadsByReview, discussionThreadsByReview),
+  );
 
-  const commentToRevisionId = $derived.by(() => {
-    // eslint-disable-next-line svelte/prefer-svelte-reactivity
-    const map = new Map<string, string>();
-    for (const rev of revisions) {
-      for (const comment of rev.discussion ?? []) {
-        map.set(comment.id, rev.id);
-      }
-    }
-    return map;
-  });
+  const commentToRevisionId = $derived(revisionIdByComment(revisions));
 
   // Every code-comment mutation dispatches on the owning object; resolving it
   // once here keeps the actions shared.
@@ -473,18 +422,15 @@
   function canResolveComment(commentId: string): boolean {
     const owner = ownerOf(commentId);
     if (owner?.kind !== "review") return false;
-    if (
-      roles.isDelegate(
-        config.publicKey,
-        repoDelegates.map(d => d.did),
-      )
-    ) {
-      return true;
-    }
     const review = (revision.reviews ?? []).find(r => r.id === owner.reviewId);
-    const comment = review?.comments?.find(c => c.id === commentId);
-    return [comment?.author.did, review?.author.did, revision.author.did].some(
-      did => did !== undefined && publicKeyFromDid(did) === config.publicKey,
+    return roles.canResolveReviewComment(
+      config.publicKey,
+      repoDelegates.map(d => d.did),
+      {
+        comment: review?.comments?.find(c => c.id === commentId)?.author.did,
+        review: review?.author.did,
+        revision: revision.author.did,
+      },
     );
   }
 
@@ -538,15 +484,7 @@
   // render on the diff alongside review comments instead of vanishing.
   const revisionCodeCommentThreads: Thread<CodeLocation>[] = $derived.by(() => {
     if (hiddenCommentSources.includes(STANDALONE_COMMENTS)) return [];
-    const discussion = revision.discussion ?? [];
-    return discussion
-      .filter(c => c.location && !c.replyTo)
-      .map(root => ({
-        root,
-        replies: discussion
-          .filter(c => c.replyTo === root.id)
-          .sort((a, b) => a.edits[0].timestamp - b.edits[0].timestamp),
-      })) as Thread<CodeLocation>[];
+    return buildThreads(revision.discussion ?? [], isCodeRoot);
   });
 
   const codeComments: CodeComments | undefined = $derived.by(() => {
@@ -643,25 +581,7 @@
     new Set(revisions.filter(r => r.id !== latestRevisionId).map(r => r.id)),
   );
 
-  const threadsByReview = $derived.by(() => {
-    // eslint-disable-next-line svelte/prefer-svelte-reactivity
-    const map = new Map<string, Thread<CodeLocation>[]>();
-    (revision.reviews ?? []).forEach(review => {
-      const reviewComments = review.comments ?? [];
-      const threads = reviewComments
-        .filter(c => c.location && !c.replyTo)
-        .map(root => {
-          const replies = reviewComments
-            .filter(c => c.replyTo === root.id)
-            .sort((a, b) => a.edits[0].timestamp - b.edits[0].timestamp);
-          return { root, replies } as Thread<CodeLocation>;
-        });
-      if (threads.length > 0) {
-        map.set(review.id, threads);
-      }
-    });
-    return map;
-  });
+  const threadsByReview = $derived(codeThreadsByReview(revision.reviews ?? []));
 
   const activityItems = $derived(
     patchTimeline({
@@ -673,45 +593,11 @@
       expandedRevisionRuns,
     }),
   );
-  const reviewSummaryFingerprints = $derived(
-    new Set(
-      revisions
-        .flatMap(r => r.reviews ?? [])
-        .filter(r => r.summary && r.summary.trim() !== "")
-        .map(r => `${r.author.did} ${r.summary}`),
-    ),
-  );
   // Gather discussion comments from every revision, not just the selected one,
   // so comments left on a revision stay in the timeline after a newer revision
   // is pushed. Discussion.svelte orders the merged timeline by timestamp.
   const commentThreads = $derived(
-    revisions.flatMap(rev => {
-      const discussion = rev.discussion;
-      if (!discussion) return [];
-      return (
-        discussion
-          .filter(
-            comment =>
-              (comment.id !== rev.id && !comment.replyTo) ||
-              comment.replyTo === rev.id,
-          )
-          // Code comments (those with a location) render on the diff, not as
-          // plain entries in the activity timeline.
-          .filter(comment => !comment.location)
-          .filter(comment => {
-            const body = comment.edits[comment.edits.length - 1]?.body ?? "";
-            return !reviewSummaryFingerprints.has(
-              `${comment.author.did} ${body}`,
-            );
-          })
-          .map(thread => ({
-            root: thread,
-            replies: discussion
-              .filter(comment => comment.replyTo === thread.id)
-              .sort((a, b) => a.edits[0].timestamp - b.edits[0].timestamp),
-          }))
-      );
-    }) as Thread[],
+    revisionDiscussionThreads(revisions) as unknown as Thread[],
   );
 
   async function editRevision(

src/lib/roles.ts

@@ -24,3 +24,20 @@ export function isDelegateOrAuthor(
     undefined
   );
 }
+
+/// The protocol lets the comment author, the review author or the revision
+/// author resolve a review comment; delegates may do anything.
+export function canResolveReviewComment(
+  publicKey: string,
+  delegates: string[],
+  authors: {
+    comment: string | undefined;
+    review: string | undefined;
+    revision: string | undefined;
+  },
+): boolean {
+  if (isDelegate(publicKey, delegates)) return true;
+  return [authors.comment, authors.review, authors.revision].some(
+    did => did !== undefined && publicKeyFromDid(did) === publicKey,
+  );
+}

src/lib/threads.ts

@@ -0,0 +1,132 @@
+import type { Review } from "@bindings/cob/patch/Review";
+import type { Revision } from "@bindings/cob/patch/Revision";
+import type { CodeLocation } from "@bindings/cob/thread/CodeLocation";
+import type { Comment } from "@bindings/cob/thread/Comment";
+import type { Thread } from "@bindings/cob/thread/Thread";
+
+/// Threads rooted at the comments `isRoot` picks, each with its replies oldest
+/// first.
+export function buildThreads<T>(
+  comments: Comment<T>[],
+  isRoot: (comment: Comment<T>) => boolean,
+): Thread<T>[] {
+  return comments.filter(isRoot).map(root => ({
+    root,
+    replies: comments
+      .filter(c => c.replyTo === root.id)
+      .sort((a, b) => a.edits[0].timestamp - b.edits[0].timestamp),
+  }));
+}
+
+/// A code comment that starts a thread.
+export function isCodeRoot(comment: Comment<CodeLocation>): boolean {
+  return Boolean(comment.location) && !comment.replyTo;
+}
+
+/// Each review's code threads, for reviews that have any.
+export function codeThreadsByReview(
+  reviews: Review[],
+): Map<string, Thread<CodeLocation>[]> {
+  const map = new Map<string, Thread<CodeLocation>[]>();
+  for (const review of reviews) {
+    const threads = buildThreads(review.comments ?? [], isCodeRoot);
+    if (threads.length > 0) map.set(review.id, threads);
+  }
+  return map;
+}
+
+/// Comments on a review itself rather than on a line. The summary is a field,
+/// not the root of this thread, so a comment replying to the review id starts
+/// a thread of its own.
+export function reviewDiscussionThreads(
+  review: Review,
+): Thread<CodeLocation>[] {
+  return buildThreads(
+    review.comments ?? [],
+    c => (!c.location && !c.replyTo) || c.replyTo === review.id,
+  );
+}
+
+/// Each review's discussion threads, for reviews that have any.
+export function discussionThreadsByReview(
+  reviews: Review[],
+): Map<string, Thread<CodeLocation>[]> {
+  const map = new Map<string, Thread<CodeLocation>[]>();
+  for (const review of reviews) {
+    const threads = reviewDiscussionThreads(review);
+    if (threads.length > 0) map.set(review.id, threads);
+  }
+  return map;
+}
+
+/// Which review each comment in these threads belongs to.
+export function reviewIdByComment(
+  ...sources: Map<string, Thread<CodeLocation>[]>[]
+): Map<string, string> {
+  const map = new Map<string, string>();
+  for (const source of sources) {
+    for (const [reviewId, threads] of source) {
+      for (const thread of threads) {
+        map.set(thread.root.id, reviewId);
+        for (const reply of thread.replies) map.set(reply.id, reviewId);
+      }
+    }
+  }
+  return map;
+}
+
+/// Which revision each discussion comment was left on.
+export function revisionIdByComment(
+  revisions: Revision[],
+): Map<string, string> {
+  const map = new Map<string, string>();
+  for (const revision of revisions) {
+    for (const comment of revision.discussion ?? []) {
+      map.set(comment.id, revision.id);
+    }
+  }
+  return map;
+}
+
+/// One group per file, in order of first appearance, so a file's threads
+/// render under a single header.
+export function groupThreadsByFile(
+  threads: Thread<CodeLocation>[],
+): { path: string; threads: Thread<CodeLocation>[] }[] {
+  const groups = new Map<string, Thread<CodeLocation>[]>();
+  for (const thread of threads) {
+    const path = thread.root.location?.path;
+    if (!path) continue;
+    const group = groups.get(path) ?? [];
+    group.push(thread);
+    groups.set(path, group);
+  }
+  return [...groups.entries()].map(([path, threads]) => ({ path, threads }));
+}
+
+/// The discussion threads of every revision, for the activity timeline.
+///
+/// Code comments render on the diff instead. A comment whose body repeats one
+/// of its author's review summaries is left out too, since the review already
+/// shows that text.
+export function revisionDiscussionThreads(
+  revisions: Revision[],
+): Thread<CodeLocation>[] {
+  const summaries = new Set(
+    revisions
+      .flatMap(r => r.reviews ?? [])
+      .filter(r => r.summary && r.summary.trim() !== "")
+      .map(r => `${r.author.did} ${r.summary}`),
+  );
+  return revisions.flatMap(revision =>
+    buildThreads(revision.discussion ?? [], comment => {
+      const body = comment.edits[comment.edits.length - 1]?.body ?? "";
+      return (
+        ((comment.id !== revision.id && !comment.replyTo) ||
+          comment.replyTo === revision.id) &&
+        !comment.location &&
+        !summaries.has(`${comment.author.did} ${body}`)
+      );
+    }),
+  );
+}

tests/unit/roles.test.ts

@@ -0,0 +1,68 @@
+import { describe, expect, test } from "vitest";
+
+import {
+  canResolveReviewComment,
+  isDelegate,
+  isDelegateOrAuthor,
+} from "@app/lib/roles";
+
+const me = "z6Mkme";
+const delegates = ["did:key:z6Mkdelegate", "did:key:z6Mkother"];
+
+describe("isDelegate", () => {
+  test("matches a delegate's public key", () => {
+    expect(isDelegate("z6Mkdelegate", delegates)).toBe(true);
+  });
+
+  test.each([[me], [undefined]])("is undefined for %j", key => {
+    expect(isDelegate(key, delegates)).toBeUndefined();
+  });
+});
+
+describe("isDelegateOrAuthor", () => {
+  test("allows delegates and the author", () => {
+    expect(isDelegateOrAuthor("z6Mkdelegate", delegates, "did:key:x")).toBe(
+      true,
+    );
+    expect(isDelegateOrAuthor(me, delegates, `did:key:${me}`)).toBe(true);
+  });
+
+  test("is undefined for anyone else", () => {
+    expect(isDelegateOrAuthor(me, delegates, "did:key:x")).toBeUndefined();
+    expect(
+      isDelegateOrAuthor(undefined, delegates, "did:key:x"),
+    ).toBeUndefined();
+  });
+});
+
+describe("canResolveReviewComment", () => {
+  const nobody = { comment: undefined, review: undefined, revision: undefined };
+
+  test("a delegate can resolve any review comment", () => {
+    expect(canResolveReviewComment("z6Mkdelegate", delegates, nobody)).toBe(
+      true,
+    );
+  });
+
+  test.each(["comment", "review", "revision"] as const)(
+    "the %s author can resolve it",
+    role => {
+      expect(
+        canResolveReviewComment(me, delegates, {
+          ...nobody,
+          [role]: `did:key:${me}`,
+        }),
+      ).toBe(true);
+    },
+  );
+
+  test("nobody else can", () => {
+    expect(
+      canResolveReviewComment(me, delegates, {
+        comment: "did:key:a",
+        review: "did:key:b",
+        revision: "did:key:c",
+      }),
+    ).toBe(false);
+  });
+});

tests/unit/threads.test.ts

@@ -0,0 +1,181 @@
+import { describe, expect, test } from "vitest";
+
+import {
+  buildThreads,
+  codeThreadsByReview,
+  discussionThreadsByReview,
+  groupThreadsByFile,
+  isCodeRoot,
+  reviewDiscussionThreads,
+  reviewIdByComment,
+  revisionDiscussionThreads,
+  revisionIdByComment,
+} from "@app/lib/threads";
+
+import {
+  author,
+  comment,
+  location,
+  review,
+  revision,
+  thread,
+} from "./support/cobs";
+
+const alice = author("alice");
+const bob = author("bob");
+const code = location("a.ts", "new", 1);
+
+function ids(threads: { root: { id: string }; replies: { id: string }[] }[]) {
+  return threads.map(t => [t.root.id, ...t.replies.map(r => r.id)]);
+}
+
+describe("buildThreads", () => {
+  test("attaches replies to their root, oldest first", () => {
+    const threads = buildThreads(
+      [
+        comment({ id: "root" }),
+        comment({ id: "late", replyTo: "root", timestamp: 20 }),
+        comment({ id: "early", replyTo: "root", timestamp: 10 }),
+        comment({ id: "other" }),
+      ],
+      c => !c.replyTo,
+    );
+
+    expect(ids(threads)).toEqual([["root", "early", "late"], ["other"]]);
+  });
+
+  test("isCodeRoot picks located comments that aren't replies", () => {
+    expect(isCodeRoot(comment({ location: code }))).toBe(true);
+    expect(isCodeRoot(comment({ location: code, replyTo: "x" }))).toBe(false);
+    expect(isCodeRoot(comment({ location: null }))).toBe(false);
+  });
+});
+
+describe("threads by review", () => {
+  const reviews = [
+    review({
+      id: "r1",
+      comments: [
+        comment({ id: "code", location: code }),
+        comment({ id: "code-reply", location: code, replyTo: "code" }),
+        comment({ id: "general" }),
+        comment({ id: "on-review", location: code, replyTo: "r1" }),
+      ],
+    }),
+    review({ id: "r2", comments: [] }),
+  ];
+
+  test("codeThreadsByReview keeps reviews with code threads", () => {
+    const map = codeThreadsByReview(reviews);
+
+    expect([...map.keys()]).toEqual(["r1"]);
+    expect(ids(map.get("r1") ?? [])).toEqual([["code", "code-reply"]]);
+  });
+
+  test("a reply to the review itself starts a discussion thread", () => {
+    expect(ids(reviewDiscussionThreads(reviews[0]))).toEqual([
+      ["general"],
+      ["on-review"],
+    ]);
+  });
+
+  test("discussionThreadsByReview keeps reviews with discussion threads", () => {
+    expect([...discussionThreadsByReview(reviews).keys()]).toEqual(["r1"]);
+  });
+
+  test("reviewIdByComment maps roots and replies to their review", () => {
+    const map = reviewIdByComment(
+      codeThreadsByReview(reviews),
+      discussionThreadsByReview(reviews),
+    );
+
+    expect(Object.fromEntries(map)).toEqual({
+      code: "r1",
+      "code-reply": "r1",
+      general: "r1",
+      "on-review": "r1",
+    });
+  });
+});
+
+test("revisionIdByComment maps discussion comments to their revision", () => {
+  const map = revisionIdByComment([
+    revision({ id: "v1", discussion: [comment({ id: "a" })] }),
+    revision({ id: "v2", discussion: [comment({ id: "b" })] }),
+  ]);
+
+  expect(Object.fromEntries(map)).toEqual({ a: "v1", b: "v2" });
+});
+
+test("groupThreadsByFile groups by path in order of first appearance", () => {
+  const groups = groupThreadsByFile([
+    thread(comment({ id: "b1", location: location("b.ts", "new", 1) })),
+    thread(comment({ id: "a1", location: location("a.ts", "new", 1) })),
+    thread(comment({ id: "b2", location: location("b.ts", "old", 4) })),
+    thread(comment({ id: "none", location: null })),
+  ]);
+
+  expect(groups.map(g => [g.path, ...g.threads.map(t => t.root.id)])).toEqual([
+    ["b.ts", "b1", "b2"],
+    ["a.ts", "a1"],
+  ]);
+});
+
+describe("revisionDiscussionThreads", () => {
+  test("collects each revision's plain comments and replies to it", () => {
+    const threads = revisionDiscussionThreads([
+      revision({
+        id: "v1",
+        discussion: [
+          comment({ id: "c1" }),
+          comment({ id: "reply", replyTo: "c1" }),
+          comment({ id: "code", location: code }),
+          comment({ id: "on-revision", replyTo: "v1" }),
+        ],
+      }),
+      revision({ id: "v2", discussion: [comment({ id: "c2" })] }),
+    ]);
+
+    expect(ids(threads)).toEqual([["c1", "reply"], ["on-revision"], ["c2"]]);
+  });
+
+  test("skips the revision's own description comment", () => {
+    expect(
+      revisionDiscussionThreads([
+        revision({ id: "v1", discussion: [comment({ id: "v1" })] }),
+      ]),
+    ).toEqual([]);
+  });
+
+  test("skips a comment repeating its author's review summary", () => {
+    const threads = revisionDiscussionThreads([
+      revision({
+        id: "v1",
+        reviews: [review({ author: alice, summary: "LGTM" })],
+        discussion: [
+          comment({
+            id: "repeat",
+            author: alice,
+            edits: [edit(alice, "LGTM")],
+          }),
+          comment({
+            id: "other-author",
+            author: bob,
+            edits: [edit(bob, "LGTM")],
+          }),
+          comment({
+            id: "edited",
+            author: alice,
+            edits: [edit(alice, "LGTM"), edit(alice, "Changed")],
+          }),
+        ],
+      }),
+    ]);
+
+    expect(ids(threads)).toEqual([["other-author"], ["edited"]]);
+  });
+});
+
+function edit(by: typeof alice, body: string) {
+  return { author: by, timestamp: 0, body };
+}