Skip to content

Pick a commit on the Code tab to read its own diff - #70

Merged
krishhgg merged 17 commits into
masterfrom
t3term/pull-request-commit-scope
Oct 18, 2026
Merged

krishhgg merged 17 commits into
masterfrom
t3term/pull-request-commit-scope

Conversation

@krishhgg

@krishhgg krishhgg commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

Adds the desktop's commit picker to the Code tab. A row above the files says which diff shows, All commits or a commit's headline, and opens the pull request's commits, newest first. Picking one reads that commit's own diff.

  • › All commits sits above the file count. Enter or a click opens the list under it: ✓ All commits, then each commit's headline cut to its row with the first seven characters of its id at the end, then Show more (13 left) while more than 10 are left. ↑ and ↓ choose an entry.
  • Picking a commit closes the list, keeps its row chosen and reads that commit's diff from its first slice, with its files folded and the list of conversations off the diff closed. The row then reads › Reuse the parsed config on reload · 9dc565a, and This commit has no file changes. shows for an empty commit.
  • Under a commit's diff every review thread goes to the list off the diff, since a commit's line numbers can mean other code than the thread's.
  • A read of the conversation that no longer lists the commit, as after a force-push, goes back to the whole change.
  • pull_request::diff takes an optional commit and sends it in PullRequestDiffInput.
  • docs/pull-request-panel.md gains a Commits row, docs/pull-request-reads.md describes a commit's diff, and the README's keys and clicks mention the list.
  • The Code recipe takes one more Down to reach a file now that the commits row sits above the count. It opens the commit list, checks its order, picks each commit by key and by click, checks each read's commit and goes back to All commits (b37df5c).

Stacked on #69.

The Code tab of #7 with the list of commits open: ⌄ All commits, then ✓ All commits, a chosen row reading Reuse the parsed config on reload with 9dc565a at its end, and Split the config reader with 1f3e2a9. The file count and the folded files follow below a blank row. The status line reads Enter show commit 9dc565a.

After the pick: › Reuse the parsed config on reload · 9dc565a, then 2 files, › 4 conversations not on the current diff, and src/config/reader.rs unfolded to the commit's own hunk, where self.load_unchecked() becomes a comment and self.load(). tests/config_reload.rs follows, folded.

The same commit with ⌄ 4 conversations not on the current diff open: src/config/reader.rs with Line 26 and a resolved thread, Line 28 and an open thread with two comments and Load more comments, then src/config/schema.rs with Line 45 and an outdated thread.

All three come from the fixture server, with two invented commits on #7's invented diff at 120x40. The fixture now answers a diff read that names a commit with that commit's own patch.

Decision log

What I noticed. The Code tab showed only the whole change's diff. Plan item 7 in docs/pull-request-panel.md named the commit picker as next, and a Timeline commit couldn't open anything because the tab had no way to show one commit.

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

  • PullRequestCodeTab.tsx:1063-1119 draws the menu: the headline or All commits on the button, a radio group with All commits and each commit's headline and oid.slice(0, 7), 10 at a time with Show more (N left). No menu shows without commits (1067-1069).
  • 683-692 sorts the commits newest first by Date.parse(committedDate).
  • 262-290 scopes the diff query by commit, and 268-277 resets the draft, the folds, the list's length and the list off the diff when the scope changes.
  • 454-458 places no thread on a commit's lines, and 1050-1062 goes back to All commits when the commits no longer list the one picked.
  • On the server, /api/pull-requests/diff takes commit (packages/contracts/src/pullRequest.ts:950-963). GitHub's provider reads a commit's files from the files API and needs a SHA (GitHubPullRequestCli.ts:2216-2230). PullRequestService.ts keeps a commit's slices for 10 minutes under the commit (165-166, 3189, 3195-3201).

Options.

  • Chosen: the list opens in the tab's rows under its toggle, as the list off the diff does. Rejected: a menu drawn over the files, which would need its own layer and click routing in the details, where every other control is a row.
  • Chosen: the toggle shows the headline and the seven-character id, since headlines like Fix tests repeat and the row has room. The desktop's button shows the headline alone.
  • Chosen: an empty headline reads Untitled commit, as the Timeline already says. The desktop's menu shows it empty.
  • Chosen: commits at the same instant keep T3's order, and one whose date t3term can't read goes last. The desktop's comparator gets NaN for such a date, which leaves its place to the sort.
  • Chosen: picking another diff drops the read under way and reads the new one at once. Rejected: waiting for the old read, which would show a diff nobody asked for first.
  • Chosen: after a pick the toggle stays chosen, as a menu gives its button the focus back, and the first slice's arrival keeps it chosen. Once Show more lists the last commits, the first one it listed takes the choice.
  • Chosen: going back to a diff read before reads its first slice from T3 again. Rejected: keeping each diff's slices in t3term, which would hold every picked commit's files in memory while T3 already keeps them for a minute, or ten for a commit.

What review changed.

  • Running the fixture showed that the commit's first slice cleared the toggle's choice, so Enter did nothing until an arrow key chose it again. The pick now survives a first slice when it is on the commit list, and the panel test checks it.
  • Greptile found that a pick left the read before it running, though t3term drops its answer. The app's reader now aborts the read under way when the next one starts, since the details only want the newest. A test against a fake T3 that holds the first read back fails without the abort.
  • Greptile asked for the list off the diff under an empty commit. The desktop returns This commit has no file changes. before that list too (PullRequestCodeTab.tsx:1366-1376), so it stays, and the doc's Empty row says so.

b7d2adc merges #69 at 1f268d5, which brings #66's fix for a hunk that removes nothing at the highest line number (8073497) and its test. On this branch act also takes the pull request's commits, so cebb4a4 gives that test's call an empty list.

f14b610 and 4eb3fbe merge #69 at 25d975e and 942dad5, which bring the fixes that lay a thread's comments and its loaded pages out again when the theme changes. f14b610 gives the new theme test's act and lay_out calls an empty list of commits, as for the other tests.

Known limits.

  • The list pushes the files down, where the desktop's menu opens over them. A headline too long for its row is cut, with no tooltip.
  • A commit on the Timeline doesn't open its diff yet (PullRequestDetailPanel.tsx:516-519). That's the next pull request.
  • Going back to All commits or a commit read before reads its first slice from T3 again.
  • An empty commit shows no review thread, as on the desktop.
  • A diff read whose details closed runs until it ends or times out, since only the next diff read stops it.

Tests

  • code/tests.rs: the order by instant across time zones, ties and a bad date. The controls and their actions. Picking the diff shown reads nothing, and a read under way is dropped. The row stays above Loading and a failure, threads go off a commit's diff, files fold, an empty commit says so, and a failure goes with its diff. 23 commits page 10 at a time and reset to 10 on a pick, a rebase goes back to All commits, and no commits means no row. A hostile headline and id are cleaned at every width from 1 to 80, the raw id is sent, and the id shows beside at least four columns of headline.
  • pull_requests.rs: the panel's picks and actions, Show more's place, the asks naming each commit, a closed list's entry picking nothing, the list off the diff closing, and a reread going back to the whole change.
  • mod.rs: keys and clicks through the app to a read in the link's project, the app's own reader sending cursor and commit to a fake T3, and its next read stopping the one under way.
  • pull_request/tests.rs: the payload with a commit, alone and with a cursor.
  • 68 mutations of the new code: 10 survived the first run, and the tests above now fail 7 of them. The 3 left each remove a reset in scope that a later step repeats: the withheld counts and laid-out files go when the first slice comes, and due gives way to again.
  • On 4eb3fbe, fmt, clippy and all 842 tests pass (761 lib, 2 main, 31 CLI, 48 RPC), and the Code recipe passes on its build. On cebb4a4 the Code recipe passed and failed on Load more comments on a review thread #69's 1f268d5 at recipe line 19, the first wait for the commits row, › All commits.

🤖 Generated with Claude Code

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge; no new blocking issue was found.

Summary

Adds a commit picker above the Code tab’s files.

  • The Code tab can show the diff for a selected commit.
  • Diff requests can ask for one commit’s changes.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Pick a commit or All commits] --> B[Clear the previous diff and folds]
  B --> C[Request the first slice with commit scope]
  C --> D[Cancel the previous read]
  D --> E[Accept only the current ticket]
  E --> F[Show the selected diff]
Loading

Reviews (9) · Last reviewed commit: "Merge the thread-comments branch at 942d..." · Reviewed by Greptile

krishhgg and others added 5 commits October 15, 2026 04:15
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

# Conflicts:
#	docs/pull-request-panel.md
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 11:34
Comment thread src/tui/pull_requests/code.rs
Comment thread src/tui/pull_requests/code.rs
The details want only the answer to their newest diff ask, so a pick of
another commit made the read before it download a patch t3term then
dropped. The app's reader now aborts the read before each new one. The
docs note that an empty diff shows no review thread, as the desktop's
does.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
krishhgg added a commit that referenced this pull request Oct 15, 2026
…anch

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
krishhgg and others added 7 commits October 17, 2026 14:04
…47e50cd

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

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

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ments in the Code recipe

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The commits row now sits above the count, so the recipe takes one more Down to reach a file
and checks the deleted file from the end of the files. It opens the list, checks its order,
picks each commit by key and by click and checks each read's commit, and goes back to All
commits.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…red past 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>
…f043b63, the environment switcher

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
krishhgg added a commit that referenced this pull request Oct 18, 2026
The second read of #70's marks answered 1 s after it was asked, about when the
recipe touched viewed-fail, so the reopen showed either the first-read failure
or the stale note. Touching viewed-fail with diff-fail means the details never
hold marks for #70.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
krishhgg added a commit that referenced this pull request Oct 18, 2026
The row under the count pushes each file down one row and adds a stop for ↓,
so the recipe counts the toolbar's rows in TB. The new section folds and
unfolds every file with Enter and with a click, keeps a file folded after
through a refresh, has the second slice follow the row, and checks that a
picked commit starts folded. It also takes #70's viewed-fail race fix from
the viewed-files branch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
krishhgg added a commit that referenced this pull request Oct 18, 2026
…ds fail from its first open in the Code recipe

The fold-all branch already held the same change, so the tree stays as it was.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
krishhgg added a commit that referenced this pull request Oct 18, 2026
Changes to whitespace start hidden, so the recipe checks reader.rs at +5 -3
with its reindented and repeated lines unchanged, shows them with the row and
checks that nothing is read, then runs the rest with them shown. #70's Code
starts hidden again. TB counts the second toolbar row.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
krishhgg added a commit that referenced this pull request Oct 18, 2026
The recipe unwraps reader.rs's long line, scrolls it with L, H and the shifted
arrows to its end and back, and checks that Esc with Alt+B and a refresh keep
the scroll while the details opened again from the list start at the left. A
picked commit keeps the lines unwrapped and #70 starts wrapped. TB counts the
third toolbar row.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
krishhgg and others added 2 commits October 18, 2026 03:46
…d-line count at the highest line number

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

Copy link
Copy Markdown
Owner Author

PASS for head cebb4a497fec1af5594de3cf155861de0b52ffd5, patch id 9f8921c89c9fd6a548bfb3511498dd0bb72f8330.
Verifier: GPT-6.1 Sol (xhigh), not the author. Base 1f268d58d8d09489990862172dc4ba8fb80a30fb, the #69 parent. Scenario links, 100x30 and 80x24, truecolor.

Claim Base Head
The Code tab offers a commit picker, newest first Head recipe fails at line 19 because › All commits is absent Keys and clicks open the ordered list at both sizes
Each commit opens its own diff, and All commits restores the whole change Only the whole change is available Both commits work by key and click; requests name the correct commit and start without a cursor; selecting the current scope reads nothing
A commit starts with folded files and moves review threads off its lines No commit scope Four threads move to the off-diff list; switching scope closes that list and resets folds
Paging, empty commits and a disappearing selection behave as documented No picker Separate fixture copy with 23 commits passes both Show more actions, reset to ten entries, empty-commit text and return to the whole change after the selected commit disappears

Regression sweep: 13/13 own recipes on base and head. Master's 12/12 recipes pass on both. The head's Code recipe passes; the same recipe fails on the parent at its first new control. No regression outside the change. Matched 84 capture pairs from the own sweeps and 70 from master's sweeps, with expected Code changes and transient loading, reconnect, clock, spinner and local-address differences accounted for. Settled stack and reviewer captures match.

Rules: every completed run passes check before and after stop. No unsolicited diff or file-contents read. Same-step rpcRequestsByTag match outside the changed Code flow. The linked-list recipe can close details before its pending reads finish; repeating its steps with identical settling waits gives identical counters on all four stack builds. Diff requests are HTTP, so their cursor and commit assertions use pr.diff events.

Tests: local cargo test --locked passes all 841 tests. In a scratch worktree, removing only the abort makes the_apps_next_read_of_a_diff_stops_the_one_under_way fail with ticket 7 instead of 8. Removing only the selection-preservation condition makes the_code_tab_reads_the_commit_picked_from_its_list fail with no selected control. The inherited highest-line-number test passes.

CI: all four checks completed successfully on this exact head, including macOS and E2E smoke. Greptile: two threads, both resolved and answered. No new unanswered finding. The feature file is updated. Compared the implementation with the cited desktop Code tab at f570bd2; the documented departures are intentional.

Binary identities, LC_UUID from otool -l:

  • Base /tmp/t3v-bin/1f268d58d8d09489990862172dc4ba8fb80a30fb/t3term: D36BF187-6DE7-3162-B537-6D43F7B7061C, reused cache.
  • Head /tmp/t3v-bin/cebb4a497fec1af5594de3cf155861de0b52ffd5/t3term: 10A90647-7A88-38E0-96DA-CAC5F594508C, build log confirms Compiling t3term.

Evidence: /tmp/t3v-verdict/70/round1/, especially smoke-base, smoke-head, master-smoke-base, master-smoke-head, head-recipe-on-base, focused-80x24-v3, extended-100x30-v2, the comparison JSON files and the two *-reverted.log files. Rendered captures were inspected. All servers and data were invented and local.

Notes: no product finding. No pull request or branch was changed.

krishhgg and others added 2 commits October 18, 2026 05:51
…hat lays review threads out again when the theme changes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…of comments 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 4eb3fbe989fd3533d08b73addc63cd31c876ab88, patch id 74a548f5af282a816f9f30b17b2b0fa9f746f71c.
Verifier: GPT-6.1 Sol (xhigh), not the author. Base 942dad5c91767547bec0df7d45d15b55fded6fac. Scenario links, truecolor. The Code recipe runs at 100x30 and includes 80x24; other recipes use their documented sizes. This verdict covers this layer against the named parent.

Claim Base Head
Open the newest-first commit picker and read a selected commit's own diff 942dad5 has no All commits control Enter and click open the picker; keys select 9dc565a0, a click selects 1f3e2a90, and All commits restores the whole diff
Reset the selected diff's files and move review threads off its lines Only the whole-change diff exists The selected commit starts with folded files and all four conversations off the diff; recorded HTTP requests carry the selected commit and a null first-slice cursor
Preserve round 1's implementation after merging the parent Round 1 production patch id c5194b112856bcba04efa861051df994534c4685 The production patch id is identical; documentation and feature patches are also identical. The sole new test adaptation passes empty commits to act and lay_out in the inherited theme test

Regression sweep: all 13 checkout recipes pass on 942dad5 and 4eb3fbe after the timed refresh rerun. Every current-master recipe was attempted on both binaries; 11 of 13 pass after that rerun, with the two shared baseline limits below. Every unchanged feature has matching settled rpcRequestsByTag. The Code recipes have identical RPC counts, including two comment-page reads. Head has 32 HTTP diff reads against base's 28; the four added reads are the documented commit picks and return to All commits.

Rules: doctor passed. Successful drives and recipes passed check before and after stop. No scope, unknown-RPC, fixture, secret or session-lifecycle failure.
CI: macOS, E2E smoke (macOS), Linux build and Greptile Review all passed on 4eb3fbe. Greptile's two threads are resolved and answered, including the explicitly withdrawn empty-commit parity finding. No new finding on this head.

Evidence: /tmp/t3v-verdict/70/round2/. Direct comparisons are claim-base-proof/ and claim-head/; smoke-head/pull-request-code/ records both commit selections and All commits. I inspected commits-100x30.png and commit-100x30.png, rendered from those captures. Base checkout and master sweeps reuse the identical 942dad5 evidence in /tmp/t3v-verdict/69/round2/{smoke-head,master-smoke-head,live-refresh-awake-head}/; head sweeps are {smoke-head,master-smoke-head,live-refresh-awake-head}/ under #70. comparison.json, summary-comparison.json and runs.json retain counters, screen comparisons and run paths.

Binaries driven, recorded LC_UUID values:

Commit LC_UUID
942dad5c91767547bec0df7d45d15b55fded6fac 616B5F05-21B7-3DEA-A6A6-B288CB320D6C
4eb3fbe989fd3533d08b73addc63cd31c876ab88 01712FC3-BF10-372D-A7D2-C9D4CECAC49C

All newly built binaries showed Compiling t3term. The cached parent binary was reused. t3v and smoke came from origin/master at 90d4de7; each checkout supplied its own feature recipes. No pull request code, branch or commit was changed.

Notes, shared baseline limits:

The first timed refresh attempts crossed a wall-clock gap from 13:34 UTC to 17:10 UTC and timed out on all builds. Fresh runs with idle sleep prevented pass the unchanged recipe. The original failures and successful reruns are retained. Screens differ only in fixture ports, ages, elapsed times, spinner frames and intermediate network/read states outside the intended change. Settled Summary and reviewer captures at 200x69 and 80x24 match, and settled requests match.

@krishhgg
krishhgg changed the base branch from t3term/pull-request-thread-comments to master October 18, 2026 18:17
@krishhgg
krishhgg merged commit a4d54ac 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