feat(panel): review threads move onto lines a gap opens in Files changed

Merged
DA
11d ago
+528-22

Summary

The orchestrator asked for a review thread whose line sits in a collapsed gap to move from the "Comments on lines this diff does not show" list onto its line as soon as the 20-line arrows (or Expand all) draw that line. It should move back if the gap closes again, and outdated threads should stay above.

What Pierre exposes: CodeView and FileDiff have no expand callback and no public expanded ranges. The expansion map lives on each file's drawing, in the protected hunksRenderer. What is public is FileDiff.isLineRenderable(newLine). It answers correctly once the file has been read whole, but says true for every line of a partial (patch-only) file. So placement asks Pierre about the specific lines that matter, every time Pierre draws the file.

  • Contract (diff-view/contract.ts): each DiffListFile may carry hidden, the new-side lines between its hunks that a note is waiting on. DiffListProps.onDrawn(path, lines) tells the view which of those lines the file draws now, each time that changes.
  • Bundle (diff-view/index.ts, diff-view/drawn-lines.ts): on every onPostRender of a diff item, drawnAmong filters hidden through isLineRenderable, treating a partial file as drawing none of its gap lines, and reports only on change. A file whose patch is read anew, which closes its gaps, reports [] at once. A file whose hidden changes while its item stands is read again after the draw: from Pierre where it is mounted, and from what was last told where it is scrolled away (no gap can open off screen).
  • Model (tasks/changes-model.ts): gapLines(threads, file) names the new-side lines a file's non-outdated threads wait on. A LEFT-side line is counted onto the new side past the hunks before it. threadsOn(..., opened) places a thread on a line an opened gap draws.
  • View (tasks/agent-changes-tab.tsx): useOpenedLines holds what the list reported per file. Each entry asks for its hidden lines only while its gaps can open and it isn't drawn whole. The deep-link reveal (revealedPlace) and the minimized chip use the same threadsOn, so a moved thread is revealed on its line, lit there, and minimized into its line's chip.
  • Layout and virtualization: a move is an annotation change, so the item gets a new version and Pierre measures it again. No layout code changed.

Screenshots

The real AgentPullRequestTab with the real diff bundle and fixture reads. One thread is on line 22, inside a 52-line gap, and one is outdated.

lightdark
Gap collapsed: both threads above the diff
One press of the arrow: the thread moves under line 22, the outdated one stays above

Scrolled back to the file's top after the press, the outdated thread still stands above the diff (dark):

Evidence

  • pnpm check:affected ran out of memory in this sandbox (the panel's tsc was killed with 137), so I ran the per-package checks instead:
    • tsc --noEmit for panel and desktop: clean
    • @luke/web typecheck with a 5.5 GB heap: clean
    • ./scripts/oxlint.sh: 0 errors
    • biome check .: clean
    • pnpm knip: clean
    • ./scripts/repository-checks.sh: passed
    • pnpm discarded-effect: clean
  • vitest run --project panel: 232 files, 2504 tests passed.
  • cd apps/desktop && node scripts/build.mjs: built within the bundle budget.
  • New tests (6):
    • drawn-lines.test.ts (3): two run real Pierre. A line in a gap is drawn after expandHunk up, then down, then all. A patch-only file draws none of its gap lines, even though Pierre says they stand. Plus drawnTeller: a file the list lets go, or a list taken down (a filter that keeps no file), is told it draws none (Devin's finding).
    • changes-model.test.ts (2): gapLines counts an old-side line onto the new side. A thread moves onto an opened line, one opened line leaves the other thread above, and an outdated thread stays above.
    • agent-changes-tab.test.ts (1): the list is asked for the thread's line. When the list reports it drawn, the thread moves onto line 30 and its minimized chip is on that line; when the report goes back to [], the thread returns above. The outdated thread stays above throughout.

Invariants

  • ./scripts/check.sh green. (CI)
  • JSON Schema goldens unchanged.
  • Every behavior change has, in this PR, the test that fails without it; no change-detector test added.
  • Gateway envelope goldens unchanged.
  • No new as type assertion.
  • No Effect.runPromise/runSync/runFork outside the runtime edges.
  • No new setTimeout/setInterval/new Promise/AbortController/fs.watch.
  • No Effect, Stream, or Layer written as a statement and dropped.
  • Test count: +6 (listed above).
  • No hand-rolled file replaced.
  • No CLAUDE.md/AGENTS.md sentence names a changed part.
  • PRIVACY.md unchanged.
  • Package deps unchanged.
  • Barrel/door rule unaffected.

Notes

  • Pierre keeps a file's expansions across a Refresh whose patch is unchanged, so its threads stay on their lines. A Refresh that changes the patch closes the file's gaps, and its threads go back above. Placement follows what Pierre actually draws in both cases.
  • A thread revealed by deep link while its gap is still closed scrolls to the file's header, as before. Opening the gap for it is out of scope here.

Made with Luke