feat: read and write pull request reviews inside the diff

CHANGELOG.md

@@ -9,18 +9,33 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- **Review a pull request without leaving lich.** Pulls only ever showed the
+  parts that travel one way — the checks, the commits, the diff — so a review
+  that asked for a change was invisible here and reading it meant the browser.
+  The diff now carries the conversation: a thread opens inline under the line it
+  is about, with its replies, and can be answered and resolved where it sits.
+  Right-click a selection and *Comment on the pull request* starts one of your
+  own; each comment waits with the others until **Submit review** sends them
+  together as a single review — approving, commenting, or requesting changes,
+  with a summary of your own above them. Comments waiting to be sent are marked
+  as pending and survive a refresh, a collapsed file and a restart, so a review
+  written over an afternoon is still there. A new **Conversation** tab holds the
+  whole exchange in the order it happened — verdicts, comments on the pull
+  request itself, and the threads whose lines the branch has since rewritten,
+  which no diff can show. Settled threads fold themselves away behind a count.
 - **Review comments, handed to the session as one prompt.** A diff could give
   the session a *reference* — a file, a line range — but never an instruction,
   so anything you actually wanted changed had to be typed out by hand while
   looking straight at it. Right-clicking a selection, in the Review dock or in a
-  pull request's Files changed, now also offers *Comment on lines*: a box opens
-  between the lines themselves, and the note is held against them instead of
-  being written out. Comments collect at the foot of the panel in the order you
+  pull request's Files changed, now also offers *Comment for the session*: a box
+  opens between the lines themselves, and the note is held against them instead
+  of being written out. Comments collect at the foot of the panel in the order you
   read the diff, each removable on its own,
   and one *Send* hands the whole batch over as a single prompt — pasted into the
   session unsent, so the last word before the agent starts is still yours. They
   outlive leaving the tab but not a restart: a line number written before an
-  edit points somewhere else after it.
+  edit points somewhere else after it. On a pull request's diff both destinations
+  are offered side by side, named for where the note goes.
 - **Every worktree gets a dev-server port of its own.** Two worktrees of one
   project both ran `pnpm dev` and both wanted the same port, so the second one
   lost — and the setup script that had just installed the dependencies had no
@@ -52,13 +67,31 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - **The Merge button says no before the click, not after.** Merging a pull
-  request GitHub would not take — a required review missing, a required check
-  still running, the base branch moved on, or GitHub simply still working out
-  whether the branch merges — offered the button anyway and answered with a
-  toast that named none of those reasons. The button is now disabled whenever
-  GitHub reports it cannot merge, and hovering it says which of them it is. A
-  failing check that no rule requires still merges: the checks readout shows it
-  red, and the call stays yours.
+  request GitHub would not take — a review missing, a reviewer asking for
+  changes, a conflict, the base branch moved on — offered the button anyway and
+  answered with a toast that named none of those reasons. The button is now
+  disabled for each of them, and hovering it says which. Everything else it
+  still offers: a failing check no rule requires merges fine, a base branch
+  governed by a ruleset merges fine, and GitHub still deciding is not a no. The
+  call stays yours, and where GitHub does refuse, it refuses out loud.
+- **The Merge menu offers what your repository actually accepts.** A repository
+  can pin a branch to one merge method — squash only is the common one — and
+  nothing in a pull request says so, so the menu offered all three and GitHub
+  refused two of them after the click. lich now reads the base branch's rules
+  and offers only the methods that branch takes. A branch nothing governs, or
+  an answer lich cannot read, still offers all three: the menu never comes back
+  narrower than the truth.
+- **Merging over red CI says so first.** A failing check that no rule requires
+  does not block a merge on GitHub, and lich offers the merge for the same
+  reason — the call is yours. It just used to offer it in silence, with the red
+  a line away in the status row. The Merge button now carries the number of
+  failing checks, and hovering it says GitHub will merge anyway.
+- **Dropdown menus are as wide as what they say.** Every menu was pinned to the
+  width of the button that opened it, so the ones hanging off a small control —
+  New Session, the pull request filter, Merge — wrapped their items over two and
+  three lines. They size to their own text now, on one line each, with a little
+  more room around it. A select still matches its field, which is the one place
+  the old behaviour was right.
 - **Settings › Font lists your installed fonts on Windows.** The picker read the
   font list from fontconfig, which Windows does not have, so it offered nothing
   beyond the bundled default and whatever was already selected. It now reads the

frontend/src/components/common/SkeletonLines.tsx

@@ -17,8 +17,12 @@ interface SkeletonLinesProps {
 export function SkeletonLines({ widths, height = "h-3", className }: SkeletonLinesProps) {
   return (
     <>
-      {widths.map((width) => (
-        <Skeleton key={width} className={cn(height, width, className)} />
+      {/* Keyed by position, not by width: a ragged stack repeats widths by
+          design, and two bars keyed the same are two bars React folds into one.
+          The list is static, so the index is the identity. */}
+      {widths.map((width, line) => (
+        // biome-ignore lint/suspicious/noArrayIndexKey: a fixed stack of bars, never reordered.
+        <Skeleton key={line} className={cn(height, width, className)} />
       ))}
     </>
   )

frontend/src/components/diff/FileDiff.tsx

@@ -1,8 +1,10 @@
-import { ChevronDown, ChevronRight, Paperclip, Undo2 } from "lucide-react"
+import { ChevronDown, ChevronRight, MessageSquare, Paperclip, Undo2 } from "lucide-react"
 import { useEffect, useMemo, useRef, useState } from "react"
+import { createPortal } from "react-dom"
 import { IconAction } from "@/components/common/IconAction"
 import { DiffStat } from "@/components/DiffStat"
 import { Checkbox } from "@/components/ui/checkbox"
+import { threadSlots, type SlotElements, type ThreadSlot } from "@/lib/codemirror-threads"
 import {
   buildFileDoc,
   formatLineRef,
@@ -11,10 +13,12 @@ import {
   type FileDoc,
 } from "@/lib/git/diff"
 import { languageAbbr, splitPath } from "@/lib/git/lang-badge"
+import { COMPOSER_KEY, isAnchored, reviewSlots } from "@/lib/pulls/review-slots"
 import { cn } from "@/lib/utils"
 import type { DiffBulk } from "./diff-bulk"
 import { InjectMenu } from "./InjectMenu"
 import { useDiffEditor } from "./useDiffEditor"
+import { type Composer, type DiffReview, type DiffSelection, ReviewSlot } from "./ReviewSlots"
 
 // Files whose rendered diff exceeds this many lines start collapsed, so one
 // giant lockfile doesn't swamp the panel (expanding is one click away).
@@ -32,9 +36,10 @@ const MOUNT_MARGIN = "600px 0px"
 interface FileDiffProps {
   file: DiffFile
   onInject: (text: string) => void
-  /** Hold a comment on these lines for the panel's batch. Absent = no comment
-   * action, the shape every surface had before review comments existed. */
-  onComment?: (path: string, lines: string, text: string) => void
+  /** Hold a comment on these lines for the panel's batch, which the session's
+   * next prompt carries. Absent = no session comment, the shape every surface
+   * had before review comments existed. */
+  onSessionComment?: (path: string, lines: string, text: string) => void
   /** Ask the panel to confirm and revert this file's changes. Omitted for a
    * read-only diff (a PR's changes), where discarding makes no sense. */
   onDiscard?: () => void
@@ -45,18 +50,23 @@ interface FileDiffProps {
   /** Handle the tick. Absent = no Viewed control (the dock's working diff,
    * where every file is the current one). */
   onViewed?: (next: boolean) => void
+  /** The pull request review over this file: threads to render inline, drafts
+   * waiting to be sent, and what a new comment does. Absent for the working
+   * diff, which has no pull request behind it. */
+  review?: DiffReview
 }
 
 // The card must not clip overflow — a clipping ancestor would break the
 // sticky header.
 export function FileDiff({
   file,
   onInject,
-  onComment,
+  onSessionComment,
   onDiscard,
   bulk,
   viewed,
   onViewed,
+  review,
 }: FileDiffProps) {
   const doc = useMemo(() => buildFileDoc(file), [file])
   const [expanded, setExpanded] = useState(!file.binary && doc.lineMeta.length <= LARGE_FILE_LINES)
@@ -72,6 +82,12 @@ export function FileDiff({
   const Chevron = expanded ? ChevronDown : ChevronRight
   const badge = languageAbbr(file.newPath)
   const { dir, base } = splitPath(file.newPath)
+  // Only what opening the file would actually reveal: a settled thread is not
+  // outstanding, and one the diff cannot anchor lives in the Conversation tab.
+  const talking = review
+    ? review.threads.filter((thread) => !thread.isResolved && isAnchored(thread, doc.lineMeta))
+        .length + review.drafts.length
+    : 0
 
   return (
     <section>
@@ -104,6 +120,15 @@ export function FileDiff({
           </span>
           {dir && <span className="truncate text-muted-foreground">{dir}</span>}
         </button>
+        {/* A collapsed file must still say it is being talked about — the
+            threads inside it are invisible until it is opened. Settled threads
+            are left out of the count: they are not what is outstanding. */}
+        {talking > 0 && (
+          <span className="flex shrink-0 items-center gap-1 text-muted-foreground">
+            <MessageSquare className="size-3.5" />
+            <span className="tabular-nums">{talking}</span>
+          </span>
+        )}
         <span className="flex shrink-0 items-center gap-1.5">
           <DiffStat added={file.added} deleted={file.deleted} />
         </span>
@@ -138,7 +163,13 @@ export function FileDiff({
         (file.binary ? (
           <p className="px-9 py-2 text-xs text-muted-foreground">Binary file</p>
         ) : (
-          <LazyDiffBody doc={doc} path={file.newPath} onInject={onInject} onComment={onComment} />
+          <LazyDiffBody
+            doc={doc}
+            path={file.newPath}
+            onInject={onInject}
+            onSessionComment={onSessionComment}
+            review={review}
+          />
         ))}
     </section>
   )
@@ -148,15 +179,16 @@ interface DiffBodyProps {
   doc: FileDoc
   path: string
   onInject: (text: string) => void
-  onComment?: (path: string, lines: string, text: string) => void
+  onSessionComment?: (path: string, lines: string, text: string) => void
+  review?: DiffReview
 }
 
 // A panel holds one CodeMirror view per expanded file, and a wide diff (a PR
 // touching 191 files) built them all in a single commit — enough to lock the
 // page. LazyDiffBody defers each one until its card comes near the viewport;
 // once built, the editor stays, so scrolling back keeps its selection and
 // highlighting. Collapsing the file still destroys it.
-function LazyDiffBody({ doc, path, onInject, onComment }: DiffBodyProps) {
+function LazyDiffBody({ doc, path, onInject, onSessionComment, review }: DiffBodyProps) {
   const placeholder = useRef<HTMLDivElement>(null)
   const [mounted, setMounted] = useState(false)
 
@@ -178,49 +210,142 @@ function LazyDiffBody({ doc, path, onInject, onComment }: DiffBodyProps) {
   }, [])
 
   if (mounted) {
-    return <DiffBody doc={doc} path={path} onInject={onInject} onComment={onComment} />
+    return (
+      <DiffBody
+        doc={doc}
+        path={path}
+        onInject={onInject}
+        onSessionComment={onSessionComment}
+        review={review}
+      />
+    )
   }
   // The placeholder has to stand in for the file's height: zero-height cards
   // would all sit in the viewport at once and nothing would be deferred.
   return <div ref={placeholder} style={{ height: doc.lineMeta.length * LINE_HEIGHT_PX }} />
 }
 
+const NO_SLOTS: SlotElements = new Map()
+
 // DiffBody exists as its own component so collapsing the file unmounts it,
 // destroying the CodeMirror view instead of keeping it alive off-screen.
 //
 // A diff's document is not the file: the selection lands on doc lines, which
-// newLineRange maps back to the line numbers the agent needs to be told.
-function DiffBody({ doc, path, onInject, onComment }: DiffBodyProps) {
-  const { containerRef, getSelectedDocLines, openComment } = useDiffEditor(
+// newLineRange maps back to the line numbers an inject writes, a session comment
+// anchors to, and GitHub files a review comment against.
+function DiffBody({ doc, path, onInject, onSessionComment, review }: DiffBodyProps) {
+  // One slot extension per view. It is memoised because it *is* part of the
+  // view's identity: a new one would rebuild the editor on every render.
+  const [elements, setElements] = useState<SlotElements>(NO_SLOTS)
+  const slots = useMemo(() => threadSlots(setElements), [])
+  // The gaps exist wherever a comment can be written, which on the dock's
+  // working diff is the session's alone.
+  const gapped = Boolean(review || onSessionComment)
+  const { containerRef, getSelectedDocLines, view } = useDiffEditor(
     doc,
     path,
-    onComment && ((lines, text) => onComment(path, lines, text)),
+    gapped ? slots.extension : undefined,
   )
-  // Both halves of the resolved selection: the file lines an inject or a
-  // comment is filed against, and the doc line the composer hangs under.
-  const [selection, setSelection] = useState<{ lines: string; docLine: number } | null>(null)
+  // Both halves of the resolved selection: the file lines a comment is filed
+  // against, and the doc line a composer hangs under. They differ — a selection
+  // ending on a deleted line has a doc line but no new-file one.
+  const [selection, setSelection] = useState<DiffSelection | null>(null)
+  const [composer, setComposer] = useState<Composer | null>(null)
+
+  // Where the gaps go, never what is in them: keyed off the composer's line
+  // rather than the composer, so typing into it does not re-dispatch the whole
+  // slot set on every keystroke.
+  const composerLine = composer?.docLine ?? 0
+  const wanted = useMemo((): ThreadSlot[] => {
+    const built = review
+      ? reviewSlots({
+          lineMeta: doc.lineMeta,
+          threads: review.threads,
+          drafts: review.drafts.map(({ comment }) => comment),
+        })
+      : []
+    // The composer is placed by doc line, not by file line: it belongs under
+    // the last line the selection covered, whichever side that line came from.
+    return composerLine > 0 ? [...built, { key: COMPOSER_KEY, docLine: composerLine }] : built
+  }, [review, doc.lineMeta, composerLine])
+
+  useEffect(() => {
+    if (view && gapped) {
+      slots.update(view, wanted)
+    }
+  }, [view, gapped, slots, wanted])
+
+  const openComposer = (kind: Composer["kind"]) => (): void => {
+    if (selection) {
+      setComposer({ ...selection, kind, body: "" })
+    }
+  }
+
+  // Where a written comment goes, which is the whole difference between the two
+  // menu items: the batch the session's next prompt carries, or the review
+  // GitHub is waiting for.
+  const fileComment = (): void => {
+    const body = composer?.body.trim() ?? ""
+    if (!composer || body === "") {
+      return
+    }
+    if (composer.kind === "session") {
+      onSessionComment?.(path, composer.lines, body)
+    } else {
+      review?.onAdd({
+        path,
+        line: composer.range.end,
+        startLine: composer.range.start === composer.range.end ? 0 : composer.range.start,
+        side: "RIGHT",
+        body,
+      })
+    }
+    setComposer(null)
+  }
 
   return (
-    <InjectMenu
-      path={path}
-      containerRef={containerRef}
-      lineRef={selection?.lines ?? null}
-      // Resolve the selection when the menu opens, not on every change.
-      onOpenChange={(open) => {
-        if (!open) {
-          return
-        }
-        const selected = getSelectedDocLines()
-        const range = selected ? newLineRange(doc.lineMeta, selected.from, selected.to) : null
-        setSelection(
-          range && selected ? { lines: formatLineRef(range), docLine: selected.to } : null,
-        )
-      }}
-      onInject={onInject}
-      onComment={
-        onComment &&
-        (() => selection && openComment({ line: selection.docLine, lines: selection.lines }))
-      }
-    />
+    <>
+      <InjectMenu
+        path={path}
+        containerRef={containerRef}
+        lineRef={selection?.lines ?? null}
+        // Resolve the selection when the menu opens, not on every change.
+        onOpenChange={(open) => {
+          if (!open) {
+            return
+          }
+          const selected = getSelectedDocLines()
+          const range = selected ? newLineRange(doc.lineMeta, selected.from, selected.to) : null
+          setSelection(
+            selected && range ? { lines: formatLineRef(range), range, docLine: selected.to } : null,
+          )
+        }}
+        onInject={onInject}
+        // Both items are offered whenever their destination exists, not only
+        // once a range is resolved: the selection is read as the menu opens, so
+        // gating them on it would hide them on the very open that produced it.
+        // With nothing selected the menu disables them, like Inject lines.
+        //
+        // Deliberately new-file lines only: a comment on a deleted line is a
+        // thread GitHub anchors on the other side, and that needs its own gesture.
+        onSessionComment={onSessionComment && openComposer("session")}
+        onReviewComment={review && openComposer("review")}
+      />
+      {gapped &&
+        [...elements].map(([key, element]) =>
+          createPortal(
+            <ReviewSlot
+              slotKey={key}
+              review={review}
+              composer={composer}
+              onComposerChange={(body) => setComposer((held) => held && { ...held, body })}
+              onComposerSubmit={fileComment}
+              onComposerCancel={() => setComposer(null)}
+            />,
+            element,
+            key,
+          ),
+        )}
+    </>
   )
 }