Skip to content

Show review threads under their lines on the Code tab - #68

Merged
krishhgg merged 11 commits into
masterfrom
t3term/pull-request-code-threads
Oct 18, 2026
Merged

krishhgg merged 11 commits into
masterfrom
t3term/pull-request-code-threads

Conversation

@krishhgg

@krishhgg krishhgg commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

Shows the pull request's review threads on the Code tab, each under the line it is on, as the desktop's Code tab does.

  • A thread on a line of an unfolded file shows under that line, behind a bar. Its first row says Open or Resolved (green), how many comments the host counts and outdated when the host says so.
  • It starts open unless resolved. Enter or a click on its first row opens and closes it, and it stays that way across reads, by its id.
  • Open, it shows each comment as the Summary shows a remark, with a blank row between them, then 2 more comments on the host. when the host counts more than T3 sent.
  • On a row with both numbers, a thread on the old side comes before one on the new, as the desktop's diff library orders them.
  • Threads on no line the tab read go to a list above the files: › 1 conversation not on the current diff, or not on the diff read so far while slices are left. It starts closed. Open, it lists each path once, then Line 12 and the thread.
  • docs/pull-request-panel.md gains Review threads and Off the diff rows, plan items 3 and 7 move on, and Known limits names what threads leave out. The README's keys and clicks mention both controls.
  • The Code recipe now reads Map every nightly feature against t3term, in a suggested order #7 with review threads on its diff: the resolved one under removed line 26 and the open one under added line 28 with the host's extra comments, a click on each, both kept across a read again, the list of threads off the diff, a folded file hiding its threads, and schema.rs's outdated thread (87bbf96).

Stacked on #67.

The Code tab with src/config/reader.rs unfolded. Under removed line 26, a picked row reads › Resolved · 1 comment in green. Under added line 28, an open thread behind a bar reads ⌄ Open · 4 comments, then fixture-reviewer 1h ago asking whether reload should check the schema too with a Rust code block, fixture-dev answering that it does now, and 2 more comments on the host.

The same view after Enter on the resolved thread: ⌄ Resolved · 1 comment, then mona 3h ago: This parsed the file twice. The open thread follows under line 28.

Above the files, ⌄ 1 conversation not on the current diff is open and lists src/config/watch.rs, Line 12 and mona's open thread. Further down, src/config/schema.rs is unfolded and a picked thread under added line 45 reads ⌄ Open · 1 comment · outdated, with lee asking whether a config with no version should read as 0.

All three come from the fixture server, with invented threads on #7's invented diff at 120x40.

After Alt+Shift+A twice to Light with the open thread in view, at 100x30. Before ad1590c the comment kept Dark's near-white text. Now it takes Light's.

Before: the Light theme with the open thread under added line 28. fixture-reviewer's comment, Should reload check the schema too?, is near-white on a white background and hard to read, and its code block keeps a black background.

After: the same view. The comment reads in dark grey on white and its code block has a light grey background.

Decision log

What I noticed. The Code tab showed the diff but none of the review threads on it. Plan item 7 in docs/pull-request-panel.md named them as next, and plan item 3 left whole threads to the Code tab, since the Summary only joins each remark to its file and line.

How I confirmed it. In the pinned T3 source (f570bd2):

  • PullRequestCodeTab.tsx:452-472 places a thread where its path is a shown file's path, it has a line, and isLineInFileDiff (pullRequestDiff.logic.ts:12-22) finds that line inside one of the file's hunks on its side.
  • 1376-1440 puts the rest in a collapsible list above the code, closed at first, labelled by whether slices are left, grouped by path with Line N before each.
  • ReviewThreadCard (PullRequestReviewAnnotation.tsx:89-358) starts open unless resolved, and its header has the state, the comment count and outdated.
  • @pierre/diffs 1.5.2, dist/renderers/DiffHunksRenderer.js:1051-1075, puts a row's old-side annotations before its new-side ones in the stacked view.

Options.

  • Chosen: place a thread only on a line the parser read with that number on that side. Rejected: the desktop's range check alone, which would put a thread under no row when a hunk is shorter than its header counts. Such a thread goes to the off-diff list instead.
  • Chosen: keep each file's laid-out rows from Add a Code tab that shows a pull request's diff, file by file #66's cache and splice the threads in at each line's end, so opening a thread lays out no file again. Rejected: caching rows with the threads in them, which would rebuild a file whenever a thread opens, closes or changes.
  • Chosen: a chevron and a word for the state. Rejected: the desktop's circle and checked circle, which read as list bullets in a terminal and need a font that has them.
  • Chosen: N more comments on the host. where the desktop has Load more comments. Reading the rest through pullRequests.threadComments is its own slice.

What review changed. Greptile asked for the off-diff list on a diff with no files, where a force-push left only older threads. Nothing changed: the desktop returns This pull request has no file changes. before it lists any thread (PullRequestCodeTab.tsx:1366-1376), and the reply says so.

Greptile also found that a thread's comments kept the old theme's colors after Alt+Shift+A, as restyle cleared the Summary's remarks but not the threads'. It now clears both (ad1590c), and a_new_theme_restyles_the_threads fails without it. c5cd706 drops a mut clippy flagged in that test.

8184aa0 merges #67 at 4195b13, which brings #66's fix for a hunk that removes nothing at the highest line number (8073497) and its test, with the same patch.

Known limits.

  • No Load more comments, Resolve, Reply, Edit, reactions or Fix in thread.
  • The off-diff list scrolls with the files. The desktop's stays above them, at most 256 pixels high.
  • A thread under a folded file doesn't show, as on the desktop, and the file header doesn't count its threads.

Tests

  • code/tests.rs: threads under their lines, the old side before the new, a thread after a line's \ No newline note, a thread on a cut-short hunk going off the diff, the list's two labels, its grouping and Line N, what the reader opened following a thread across reads and dropping threads that are gone, and hostile paths and text cleaned at every width from 1 to 80 columns. Each row fits widths of 10 and up. Below 10 a remark's rows stay within 10 columns, since the Markdown renderer keeps 8 for text, as the conversation tests allow.
  • pull_requests.rs: Enter and the picks on the list and a thread through the panel, the action names, a folded file's thread taking no Enter, and the list going once the next slice places its thread.
  • 30 mutations of the new code each fail a test. Six survived the first run, so the tests gained the checks above for the list's presence, unshown files, pruning, a wrapped header and the blank row between remarks.
  • code/tests.rs: a thread laid out in Dark, then restyled, takes Light's text color.
  • On c5cd706, fmt, clippy and all 829 tests pass (748 lib, 2 main, 31 CLI, 48 RPC). The Code recipe passes on the builds of ad1590c and 8184aa0, and fails on Show each slice's files in the desktop's reading order #67's (4195b13) at recipe line 111, the first wait for the threads off the diff.

🤖 Generated with Claude Code

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge; no blocking issues remain.

Summary

The Code tab shows review threads beneath their lines and groups threads outside the shown diff above the files.

  • The Code tab shows review threads beside matching lines and in a list.

Reviews (7) · Last reviewed commit: "Drop a needless mut in the theme test" · Reviewed by Greptile

Each review thread on a line of an unfolded file shows under that line, open unless resolved, and Enter or a click opens and closes it. Threads on no line the tab read go to a list above the files, closed at first, as the desktop's Conversations not on the current diff.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@krishhgg
krishhgg marked this pull request as ready for review October 15, 2026 10:28
Comment thread src/tui/pull_requests/code.rs
krishhgg and others added 3 commits October 17, 2026 14:03
…50cd

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…gain after a theme change

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…age recipe

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread src/tui/pull_requests/conversation.rs
krishhgg and others added 5 commits October 17, 2026 16:07
…ile order in its recipe

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The recipe now reads #7 with review threads on its diff: the resolved one under removed
line 26 and the open one under added line 28 with the host's extra comments, a click on
each, both kept across a read again, the list of threads off the diff, a folded file
hiding its threads, and schema.rs's outdated thread.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ast u64::MAX and reads the Code's diff again each time it shows a minute after it came

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…3b63, the environment switcher

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e count at the highest line number

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@krishhgg

Copy link
Copy Markdown
Owner Author

FAIL for head 8184aa0cde78f2cc18209312faed616a5cec3610, patch id ef3deb95c077c5a0eeef4fb6d8f12c3dfb3e0b28.
Verifier: GPT-6.1 Sol (xhigh), not the author. Base 4195b1340f3b2d32722d74383c1d75c31ff971da. Round 1. Scenario links, 100x30 and 80x24, truecolor. This covers this layer against its parent's pinned head.

Blocking finding at src/tui/pull_requests/conversation.rs:660: restyle drops the Summary remarks' layouts but skips the thread remarks. Open reader.rs's review thread, then press Alt+Shift+A to reach Light. Its comment bodies retain dark text and code backgrounds. The near-white body text is difficult to read on the light background. This reproduced at 100x30 and 80x24; switching to a real different text width rebuilds the rows with the right colors. Clear the thread remark layouts when the theme changes and add a regression test. This independently confirms Greptile's new unanswered finding.

Claim Base Head
Threads appear under their lines and off-diff threads have a list No threads; head recipe fails at line 111 Correct removed/added lines, default folds, outdated thread, off-diff grouping
Keys, clicks, folds and a read again work at both widths Controls absent Pass; local controls trigger no host read
Appearance changes redraw thread text No thread text FAIL; cached dark colors remain in Light

The placement and default expansion match the reachable desktop source at f570bd2. The earlier empty-diff review thread is answered and resolved; the desktop also returns its empty message before listing off-diff threads.

Regression sweep: all 13 checkout recipes pass on each build, including the Code rerun with that checkout's fixture; all 12 master recipes pass on both builds. Parent results from the preceding layer are reused by exact SHA. Unchanged captures differ only in run addresses, ages, spinners and the transient reconnect indicator and transport error wording. Shared Code steps have identical rpcRequestsByTag and diff cursors. The linked-list recipe can end with one or two refresh subscriptions on either build; paired repeats that wait for the first read to finish have identical counters. Details are in comparison-final.json.

Rules: doctor and live/after-stop checks pass on valid drives. No added control reads the host without the reader asking. The feature file is updated. Local cargo test --locked: 828 tests passed. CI on this head: macOS, E2E smoke, Linux build and Greptile checks passed.

Greptile: one resolved thread and one new unanswered thread, confirmed above.

Binaries driven, with LC_UUID:

  • /tmp/t3v-bin/4195b1340f3b2d32722d74383c1d75c31ff971da/t3term: AD48C4F5-18BE-3936-8B68-B9395BD61029.
  • /tmp/t3v-bin/8184aa0cde78f2cc18209312faed616a5cec3610/t3term: F5DDCB22-D8EE-3628-8AFF-E4E8DD841F16.

Evidence: /tmp/t3v-verdict/68/round1/. code-head/pull-request-code/ holds the passing recipe captures, code-negative-parent/ the negative control, code-common-prefix.json the matching request counts, and run-index.json the run paths. manual-head-100x30-v4/light-stale.txt, its inspected PNG, and light-reflow.png show the failure and corrected reflow; manual-head-80x24-v4/ repeats it narrowly.

krishhgg and others added 2 commits October 18, 2026 05:40
Conversation::restyle dropped the laid-out text of the Summary's
remarks but not of the review threads' remarks, and Remark::lay_out
keeps its rows while the width is the same. After Alt+Shift+A to
Light, a thread's comments kept Dark's near-white text. The test lays
a thread out in Dark, restyles, and checks its text takes Light's fg.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
krishhgg added a commit that referenced this pull request Oct 18, 2026
… lays review threads out again when the theme changes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
krishhgg added a commit that referenced this pull request Oct 18, 2026
…hat lays review threads out again when the theme changes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
krishhgg added a commit that referenced this pull request Oct 18, 2026
… lays review threads out again when the theme changes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
krishhgg added a commit that referenced this pull request Oct 18, 2026
…hat lays review threads out again when the theme changes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
krishhgg added a commit that referenced this pull request Oct 18, 2026
…run while the file's text is read and brings #68's fix that lays review threads out again when the theme changes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
krishhgg added a commit that referenced this pull request Oct 18, 2026
…a run while the file's text is read and brings #68's fix that lays review threads out again when the theme changes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@krishhgg

Copy link
Copy Markdown
Owner Author

PASS+NOTES for head c5cd706c0f095b6e88931385bbed5b10f2663a41, patch id dadc1e1346d12354102a917be33fac97c22c8628.
Verifier: GPT-6.1 Sol (xhigh), not the author. Base 4195b1340f3b2d32722d74383c1d75c31ff971da. Round 2. Scenario links, 100x30 and 80x24, truecolor.

The round 1 finding at src/tui/pull_requests/conversation.rs:660 is fixed. I reproduced it on 8184aa0 twice from fresh runs at each size, then repeated those steps twice per size on this head. Already open inline and off-diff comments now take Light's 39,39,42 text color, then return to Dark's 245,245,245. Their code blocks change to Light's 236,236,238 background and back to Dark's 19,19,19, without resizing. Rendered captures were inspected.

Claim Base Head
Threads appear under their lines, with a separate off-diff list Controls absent, as recorded in round 1 Correct line placement, default folds, outdated label and off-diff grouping
Keys, clicks and refresh preserve the reader's choices Controls absent Updated Code recipe passes, including retained folds and host comment counts
Appearance changes restyle cached comments and code blocks No Code thread text; old head reproduces stale colors Inline and off-diff comments follow Light and Dark at both sizes, twice each
Local controls add no host reads Shared Code prefix has the old counts Same RPC counts and diff cursors; theme and list controls send zero RPC requests

Regression sweep: all 13 head recipes pass. The pinned parent's 13 passing recipes are reused from round 1 by exact SHA. All shared Code steps through the TypeScript capture have identical rpcRequestsByTag and diff cursors. Unchanged captures match after run addresses, ages and spinners are normalized, apart from the documented transient reconnect indicator and transport wording. Fresh paired master captures match. The linked-list recipe's early cancellation can change its subscription and stack counts; a repeat that waits for the first read to finish has identical counts on both builds. Today's master sweep is 11 of 13 on both builds, with the same two baseline mismatches listed below. There is no new regression.

Rules: doctor and live/after-stop checks pass on the valid drives. All runs were stopped with t3v stop and cleaned up. The feature file is updated. Local cargo test --locked: 829 tests passed.

Step 6: in a separate scratch worktree at this head, I reverted only ad1590c's change to conversation.rs. a_new_theme_restyles_the_threads, at src/tui/pull_requests/code/tests.rs:964, failed at line 998 with Some(Rgb(245, 245, 245)) instead of Some(Rgb(39, 39, 42)). The same test passes with the fix. The scratch worktree was removed.

CI on this exact head: macOS, E2E smoke, Linux build and Greptile passed. Greptile: both threads are answered and resolved; no open thread or new finding. The theme finding's author reply identifies the fix and regression test. The earlier empty-diff finding remains resolved as desktop parity.

Notes, shared with the pinned base:

Binaries driven, with LC_UUID:

  • /tmp/t3v-bin/4195b1340f3b2d32722d74383c1d75c31ff971da/t3term: AD48C4F5-18BE-3936-8B68-B9395BD61029.
  • /tmp/t3v-bin/8184aa0cde78f2cc18209312faed616a5cec3610/t3term: F5DDCB22-D8EE-3628-8AFF-E4E8DD841F16.
  • /tmp/t3v-bin/c5cd706c0f095b6e88931385bbed5b10f2663a41/t3term: FA6C5A32-E6EC-364D-8CEF-565D04CA0413. The build log confirms Compiling t3term.

Evidence: /tmp/t3v-verdict/68/round2/. The manual-head-* and manual-old-* runs hold the repeated captures; theme-colors.json records their exact colors and zero added requests. See reverted-fix-test.log, head-tests.log, code-common-prefix.json, comparison-final.json and run-index.json. The unchanged parent's round 1 sweep and absent-thread negative control remain valid at /tmp/t3v-verdict/67/round1/ and /tmp/t3v-verdict/68/round1/code-negative-parent/.

No new code defect found. No pull request or branch was edited, pushed, merged or closed.

krishhgg added a commit that referenced this pull request Oct 18, 2026
…n while the file's text is read and brings #68's fix that lays review threads out again when the theme changes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
krishhgg added a commit that referenced this pull request Oct 18, 2026
…run while the file's text is read, tests that no hunk is read once the whitespace steps run out and brings #68's fix that lays review threads out again when the theme changes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
krishhgg added a commit that referenced this pull request Oct 18, 2026
…ile the file's text is read, tests that no hunk is read once the whitespace steps run out and brings #68's fix that lays review threads out again when the theme changes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
krishhgg added a commit that referenced this pull request Oct 18, 2026
…hile the file's text is read, tests that no hunk is read once the whitespace steps run out and brings #68's fix that lays review threads out again when the theme changes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
krishhgg added a commit that referenced this pull request Oct 18, 2026
…un while the file's text is read, tests that no hunk is read once the whitespace steps run out and brings #68's fix that lays review threads out again when the theme changes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
krishhgg added a commit that referenced this pull request Oct 18, 2026
…un while the file's text is read, tests that no hunk is read once the whitespace steps run out and brings #68's fix that lays review threads out again when the theme changes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@krishhgg
krishhgg changed the base branch from t3term/pull-request-file-order to master October 18, 2026 17:41
@krishhgg
krishhgg merged commit c91e3e7 into master Oct 18, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant