Repository navigation
Navigate adjacent local Review diffs with Previous file and Next file #402
Description
User problem and benefit
When reading several local changes, opening a file replaces the Review list with a diff document. To read the next file, users must go Back, locate the next row, and open it again. This repeated navigation is especially disruptive in large agent-generated change sets.
Add Previous file / Next file controls and a File X of Y position to the single-file local Review document. Reuse the current loader and patch renderer; do not add a new diff engine, persistent review markers, or GitHub requests.
Research evidence
Source-code review only; no applications were launched or visually inspected. Snapshots: Synara 48603e869add604809b10f77461d3a3cd75a6b77, T3 Code bfec2387b8102975c84690f99be0f5f834fd0cbe, Threadlane upstream ab31a98e1329cf67171b554f0cd34f98f6135fc8. Relevant native destination files were compared with the automation checkout and are identical.
- Synara: adjacent-file resolver computes bounded previous/next file paths; tests cover both ends and missing selection. DiffPanel wiring derives file paths and scrolls to the adjacent file. Its controls expose names and disabled boundaries. Despite the source label “Next change,” this is file navigation, not hunk navigation. Borrow the explicit bounded interaction, with more precise labels.
- T3 Code: file reveal resolves an exact file, expands it, requests lazy content if needed, and asks the viewer to reveal it. useCodeViewFileReveal waits for readiness and rejects a request whose scope changed. Borrow stable target identity and scope validation—not its React state, tree, lazy renderer, or an unverified claim that it has the same Previous/Next controls.
Threadlane baseline and reuse
- Single-file and combined loaders already preserve
DiffOptions, load via daemon-awareproject_io, and assign request revisions. Result application rejects stale project/checkout/request results. - Document rendering replaces the list with a document header, whitespace option and diff body; it has Back/Close but no adjacent-file command. filtered_review_files already defines the filtered inventory. Commit checkbox selection is a separate collection; navigation must not mutate it.
ReviewDiffTarget/ReviewDiffRequestprovide typed file-vs-AllChanges scope; never infer scope by parsing the title.- Shared
review_diff_bodyalready handles loading, retry, empty and ready states. Preview Review uses the same kit presentation with captured read-only patches.
Proposed UX and interaction states
- Scope/order: show a compact, always-visible
Previous file · File X of Y · Next filerow only for a single-file local Review diff. Use the existing filtered inventory's order (the flat list order, also when opened from tree mode), with full relative paths as identities. If a filter is active, sayFile X of Y matching filesand expose the filter in accessible help. Include staged/unstaged, untracked, deleted, binary and rename entries as supplied by that inventory; do not silently skip files with empty/whitespace-filtered patches. No wrapping at the ends. All changes, commit/stash previews, editable documents and GitHub PR Code are outside v1. - Navigation: activating a direction loads that exact adjacent file through
open_file_diff, preserves Ignore whitespace, shows the new target title and existing loading state, and starts its patch at the top. Keep focus on the initiating control; if it becomes disabled at a boundary, move focus to the stable navigation group rather than dropping focus. Back/Close retain their current meaning. Navigation never stages, commits, discards, marks Viewed, or sends content to an agent. No automatic advance on load or background updates. - Loading/error: allow moving to another known file while a read is pending or failed; advance from the currently requested path so rapid actions are deterministic. Only the latest request may replace the body. Retry reloads the current file, not the original failed neighbor. Reuse current empty/filtered-empty/binary/error presentation; failure is not a clean tree. If the file inventory itself is unavailable or the checkout is switching, disable navigation with a textual reason while retaining Back/Close. No added network requirement: a local checkout works offline; remote daemon failures retain existing error semantics.
- Inventory changes: compute position by exact path, not stored row index. Revalidate checkout and target membership at activation. If background refresh removes/renames the current file, do not select another file at its former index or silently advance; show
File no longer in the current changes listwith Back to changes and disable adjacent navigation until the user selects a valid file. Clear navigation scope on Back/Close or checkout/branch changes. A list refresh may update the count, but must not change the currently displayed target. - Keyboard/accessibility/layout: standard quiet kit Buttons, Tab/Shift-Tab and Enter/Space suffice for v1—no new global shortcut or interception of editor/composer/terminal keys. Use visible focus, explicit roles for any focus-tracked group, accessible names including adjacent target paths and boundary descriptions, and full-path tooltips. Announce position/loading/error text without color-only cues. Keep Back/Close and whitespace controls usable at minimum width; use a compact second row rather than clipping controls or long-path labels. Follow semantic theme tokens and existing component sizing.
Scoped implementation plan
- Small shared presentation/helper —
crates/threadlane-ui-kit/src/review.rs(export throughsrc/lib.rs): compose the navigation row from existing Buttons and text. Add a pure exact-path adjacency/position helper only if shared between native and preview; no independent navigation store. Inputs describe current path, available ordered paths, enabled state and filtered scope; callbacks request Previous/Next. Preserve existingeditor.rs::panel_document_headerand whitespace control rather than expanding generic document controls with Git-specific state. - Native host —
crates/threadlane-ui-right-panel/src/{view.rs,types.rs}: derive navigation fromReviewDiffTarget::Fileandfiltered_review_files; validate action-time checkout/path against current state and reuseopen_file_diff. Keep existing revision/options/project checks inapply_review_diff_result. Distinguish an absent current path from loading/empty diffs. Ensure scroll-to-top occurs for an explicit new file, not ordinary status updates; inspect/reuse the installed GPUI scroll facilities rather than estimating offsets or replacingTextView. Reconcile focus when a boundary button becomes disabled. No changes to Git diff generation, daemon protocol, credentials, or application shell. - Shared preview —
crates/threadlane-ui-kit/examples/preview/{review.rs,review_tests.rs}: reuse the same control/helper over captured file inventory and patches. Keep the current explicit unavailable-snapshot state when a patch was not captured; do not read live Git or host files. Add representative multi-file, filtered, first/last and missing-target fixtures. - Focused validation — kit helper tests and
crates/threadlane-ui-right-panel/src/{tests.rs,view.rs}existing Review interaction tests: cover target identity, boundary/focus behavior, option preservation and reordered responses. Mount GPUI tests beneath initializedgpui_component::Root. Follow AGENTS.md and the GPUI Kit Design Guides: native commands, restrained hierarchy, explicit focus ownership, standard state treatment and no hover-only essential action.
Acceptance criteria and testing
- Review three changed files consecutively without returning to the list. Previous/Next stop at boundaries; count reflects the current filtered inventory, not commit checkbox selection.
- Exact paths distinguish duplicate basenames, Unicode, spaces, renames and a file literally named
All changes. Zero/one-file, filtered-out/missing target, binary, deleted, untracked and whitespace-empty states are explicit. - Switching files preserves Ignore whitespace and leaves staged state, commit selection, agent state and GitHub Viewed markers unchanged. Navigation is absent from excluded surfaces.
- Rapid Next/Previous, failure followed by navigation, Retry, refresh removing a target, and checkout/branch changes cannot install a stale patch or jump to a different identity. Back/Close invalidate pending work as before.
- Keyboard-only traversal, focus at boundaries, accessible names/position/error text, long paths, narrow layout, increased base font and light/dark/custom themes are usable in a real native window; native/WASM preview uses captured data only.
Planned commands: cargo nextest run -p threadlane-ui-kit -p threadlane-ui-right-panel, cargo check -p threadlane-gpui, git diff --check; run the preview's documented native/WASM checks and focused Review tests. Manually exercise a disposable checkout with 3+ files, delayed daemon responses and a removed file. These implementation tests have not been run in this research-only filing.
Why this is not covered by existing issues
Audited all 98 open and closed issues, their 32 comments, and zero GitHub Discussions, searching concepts/synonyms including adjacent/next/previous file, change navigation, diff/review navigation, hunk, file stepping, filtered changes and review progress. Also inspected current implementation rather than treating closed issues as absent features.
- #311 is the closest overlap: GitHub-backed Viewed state and Next unviewed in PR Code, with skip/wrap semantics. It cites Synara's adjacency as inspiration but does not propose local Review Previous/Next. This proposal has no viewed tracking, remote writes, skip logic or wrap and does not reopen Track PR file review progress with GitHub Viewed markers and Next unviewed #311.
- #333 covers local Ignore whitespace, now implemented; this reuses its loader/state guards and preserves that option, not another filter.
- #156 covers GitHub lifecycle/readiness/comment workflows; #172 covers selected-file discard. Neither covers reading adjacent local patches.
Draft stash, dirty-buffer protection, search, code wrapping and PR review-progress candidates were excluded because implemented or already covered. A combined-diff file tree/hunk navigator would require more renderer work; bounded single-file stepping offers the smallest useful improvement.
Risks / tradeoffs
The existing flat filtered order may differ from tree visual order; make the ordering contract explicit rather than adding a second tree-flattening algorithm. A live working tree can remove the current file, so conservative invalidation is preferable to surprising automatic recovery. Keeping navigation enabled during patch loading depends on request-identity tests; do not weaken existing guards. The feature does not preserve scroll per file, track completed review, add hunk stepping or provide a file picker in All changes—those are intentionally deferred, not hidden requirements.
Activity