Conversation
Greptile SummaryThis PR adds comment-to-comment navigation — Confidence Score: 3/5Safe to merge for the navigation mechanics, but two behavioural gaps vs. the PR description need addressing before the feature is complete as documented The core navigation logic (
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant KeyHandler as App.tsx key handler
participant Bridge as useHunkSessionBridge
participant DiffPane
Note over User,DiffPane: Keyboard path (scroll-to-note works)
User->>KeyHandler: press { or }
KeyHandler->>KeyHandler: moveAnnotatedHunk(±1)
KeyHandler->>KeyHandler: setScrollToNote(true)
KeyHandler->>DiffPane: selectedHunkIndex + scrollToNote=true
DiffPane->>DiffPane: selectedNoteBounds → scroll to note row
Note over User,DiffPane: CLI path (scroll-to-note missing)
User->>Bridge: --next-comment / --prev-comment
Bridge->>Bridge: navigateToHunkSelection(commentDirection)
Bridge->>Bridge: buildAnnotatedHunkCursors(filesWithLiveComments())
Bridge->>KeyHandler: jumpToFile(fileId, hunkIndex)
KeyHandler->>KeyHandler: setScrollToNote(false) resets flag
KeyHandler->>DiffPane: selectedHunkIndex + scrollToNote=false
DiffPane->>DiffPane: falls back to hunk-top scroll
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (message.input.commentDirection) { | ||
| const delta = message.input.commentDirection === "next" ? 1 : -1; | ||
| const merged = filesWithLiveComments(); | ||
| const annotatedCursors = buildAnnotatedHunkCursors(merged); | ||
| const nextCursor = findNextHunkCursor( | ||
| annotatedCursors, | ||
| selectedFile?.id, | ||
| selectedHunkIndex, | ||
| delta, | ||
| ); | ||
|
|
||
| if (!nextCursor) { | ||
| throw new Error("No annotated hunks found in the current review."); | ||
| } | ||
|
|
||
| const targetFile = merged.find((f) => f.id === nextCursor.fileId); | ||
| if (!targetFile) { | ||
| throw new Error("Resolved annotated hunk references an unknown file."); | ||
| } | ||
|
|
||
| jumpToFile(targetFile.id, nextCursor.hunkIndex); | ||
| return { | ||
| fileId: targetFile.id, | ||
| filePath: targetFile.path, | ||
| hunkIndex: nextCursor.hunkIndex, | ||
| selectedHunk: buildSelectedHunkSummary(targetFile, nextCursor.hunkIndex), | ||
| }; | ||
| } |
There was a problem hiding this comment.
navigateToHunkSelection calls filesWithLiveComments() which iterates files — and files is wired to bootstrap.changeset.files (all files, see App.tsx line 151).
The keyboard path (moveAnnotatedHunk in App.tsx) builds its cursor list from annotatedHunkCursors = buildAnnotatedHunkCursors(filteredFiles).
When the user has an active filter, pressing {/} navigates only among the visible (filtered) annotated hunks, but --next-comment/--prev-comment can navigate to annotated hunks in filtered-out files. This divergence can silently jump the TUI to a file the user deliberately filtered away.
Consider plumbing the filtered file list into the bridge (e.g., by adding a filteredFiles prop or a getFilteredFiles callback) so both navigation paths operate on the same cursor space.
There was a problem hiding this comment.
Fixed. The bridge now applies the same filter query as the App before building annotated cursors, so {/} and --next-comment / --prev-comment navigate within the same visible review stream. I also added an App interaction test that filters to beta and verifies CLI comment navigation no longer jumps to a hidden annotated file.
Responded by pi-coding-agent using unknown-model.
| const noteRow = metrics.rowMetrics.find( | ||
| (row) => | ||
| row.key.startsWith("inline-note:") && | ||
| row.offset >= selectedEstimatedHunkBounds.top - selectedEstimatedHunkBounds.sectionTop, | ||
| ); |
There was a problem hiding this comment.
The find picks the first inline-note: row whose offset is at or after the selected hunk's section-relative top, but never checks that the row is still within the selected hunk's bottom. If the selected hunk has no inline note rendered yet but a later hunk does, selectedNoteBounds will point to that next hunk's note and the viewport will scroll past the intended position.
In practice scrollToNote is only true after navigating to an annotated hunk (which should always produce a note row), so this is latent rather than immediately broken. Adding an upper-bound guard makes the logic self-documenting and more defensive:
const noteRow = metrics.rowMetrics.find(
(row) =>
row.key.startsWith("inline-note:") &&
row.offset >= selectedEstimatedHunkBounds.top - selectedEstimatedHunkBounds.sectionTop &&
row.offset < selectedEstimatedHunkBounds.top - selectedEstimatedHunkBounds.sectionTop + selectedEstimatedHunkBounds.height,
);There was a problem hiding this comment.
Added the upper-bound guard and switched the note lookup to explicit section-relative hunk top/bottom bounds, so the scroll-to-note path cannot latch onto a later hunk's inline note.
Responded by pi-coding-agent using unknown-model.
benvinegar
force-pushed
the
feat/annotated-hunk-navigation
branch
from
d655330 to
89651dc
Compare
|
Also addressed the review summary's CLI parity note: Responded by pi-coding-agent using unknown-model. |
Navigate directly to the next or previous hunk that has an annotation
(static sidecar or live comment), skipping unannotated hunks. This
mirrors the existing [/] hunk navigation but filtered to only the
hunks a reviewer has commented on.
- Add buildAnnotatedHunkCursors() that filters the review stream to
hunks overlapping any annotation, reusing getAnnotatedHunkIndices
- Wire {/} keybindings in the App keyboard handler
- Add "Previous comment" / "Next comment" entries to the Navigate menu
- Add { / } row to the keyboard help dialog
- Add unit tests for annotated cursor building and navigation
Agents can now jump between annotated hunks without manually resolving
comment positions. The TUI bridge resolves the next or previous
annotated hunk from the current selection using the same cursor logic
as the {/} keyboard shortcuts.
hunk session navigate --repo . --next-comment
hunk session navigate <session-id> --prev-comment --json
The comment direction flags are mutually exclusive with the existing
--file/--hunk/--line targeting, which continues to work unchanged.
- Extend SessionNavigateCommandInput with optional commentDirection
- Thread commentDirection through protocol, daemon, and HTTP client
- Resolve relative comment navigation in useHunkSessionBridge using
buildAnnotatedHunkCursors and findNextHunkCursor
- Make --file optional in the CLI parser when using comment direction
- Add CLI parse tests for both flags and rejection cases
The session bridge receives raw changeset files without live comments merged into file.agent.annotations. When --next-comment or --prev-comment was requested, buildAnnotatedHunkCursors saw no annotations and always returned "No annotated hunks found." Merge liveCommentsByFileId into the file list before building the annotated cursor list, mirroring the allFiles useMemo in App.tsx.
When navigating via {/} or --next-comment/--prev-comment, scroll the
viewport so the inline note card is positioned near the top (with 25%
padding) rather than scrolling to the hunk top. This keeps the review
note visible without requiring the user to scroll down through large
hunks to find it.
- Add scrollToNote prop to DiffPane that switches the scroll anchor
from hunk bounds to the first inline-note row offset
- Set scrollToNote in moveAnnotatedHunk, clear it on other navigation
- Find the note row offset via section metrics row key prefix match
- Add TDD-style component test verifying note visibility with and
without the flag on a large hunk with a deep annotation
benvinegar
force-pushed
the
feat/annotated-hunk-navigation
branch
from
89651dc to
de7de15
Compare
2 participants
Summary
Jump directly between annotated hunks — skipping unannotated ones — via keyboard, menu, or CLI. This makes it easy for both humans and agents to step through review comments without scrolling through the full diff stream.
What's new
Keyboard:
{/}Navigate to the previous/next hunk that has an annotation (static sidecar or live comment). Mirrors
[/]for all-hunk navigation but filtered to only commented hunks.Navigate menu
"Previous comment" and "Next comment" entries with
{/}hints, alongside the existing hunk navigation entries.CLI:
--next-comment/--prev-commentAgents can jump between comments without manually resolving positions:
The TUI resolves the next annotated hunk from the current selection and responds with the navigation result. These flags are mutually exclusive with the existing
--file/--hunk/--linetargeting.Scroll-to-note
When navigating via
{/}or--next-comment/--prev-comment, the viewport scrolls to position the inline note card near the top rather than the hunk top. This keeps the review note visible without requiring extra scrolling through large hunks.Help dialog
The
?keyboard help now includes{ / } previous / next comment.