docs/internal/REVIEW_DIFF_HUNK_PARITY_FINDINGS.md
Status: open issues found by driving the in-panel hunk-parity Review Diff
interactively in tmux (unified + side-by-side, single- and multi-file
changesets, with capture-pane -e). Companion to
REVIEW_DIFF_HUNK_PARITY_UX_DESIGN.md. None of these are fixed yet.
Read CONTRIBUTING.md before starting. The plugin lives in
crates/fresh-editor/plugins/audit_mode.ts; composite-buffer host code in
crates/fresh-editor/src/app/composite_buffer_actions.rs,
crates/fresh-editor/src/app/input_helpers.rs, and the composite renderer
in crates/fresh-editor/src/view/ui/split_rendering/orchestration/mod.rs.
6093f61): composite handlers resolved the panel leaf via
active_split() (outer group leaf) instead of effective_active_pair()
(focused inner leaf), so the composite_view_states lookup missed and all
keyboard movement / hunk nav was dropped. Side-by-side now scrolls
(Up/Down/PageUp/PageDown) and n/p navigate hunks.2c397c4): n/p weren't "doing nothing" —
they moved the highlight + scrolled, but (a) the status-bar Ln lagged and
(b) in focus-only mode they used the global state.hunks index, so when
the focused file wasn't first the jump targeted an unrendered file and
no-op'd. Now navigate by the rendered hunkHeaderRows (advancing files at
boundaries); set_buffer_cursor_in_splits updates primary_cursor_line_number
so Ln is correct immediately.260316c): the stash review feature works; the e2e
test asserted a title string ("Review Diff (stash") that is never
rendered (panels are labelled with the stash ref), so it timed out at 180s
on every platform. Assertion corrected to the actually-rendered strings.f876884): the [⚠] bump came from the composite fold on
Tab; Tab is now focus-switch, so it no longer fires.2c397c4): navigating files (,/.) scrolls the focused
file into position and puts the cursor on its header.HEAD): the FILES panel scrolls to keep the selected file
visible when the changeset is taller than the sidebar.f876884): Tab/Shift-Tab cycle focus
files → diff → comments; arrows/PageUp/PageDown act on the focused panel; a
▸ marker on the focused panel header shows where input lands.HEAD): toolbar legend now leads with the nav keys
(Tab/,/./n/p/1/2/↑↓).f876884): help text updated to match (Tab = focus, not
fold).Remaining: B3 (side-by-side already scrolls to the first hunk via the
composite's focus-hunk logic — largely covered by A1; revisit only if a
specific case misbehaves) and D2 (? help opens in a new tab and steals
focus — should be an overlay or restore focus on close).
Up/Down/j/k/PageUp/PageDown do nothing in split view — the
cursor stays at Ln 1, Col 1 and the composite never scrolls, so anything
below the first viewport is unreachable. Unified mode moves fine.
Path is confirmed correct up to the host: Down → review_nav_down → editor.executeAction("move_down") → handle_action → catch-all → apply_action_as_events, which checks is_composite_buffer(active_buffer())
(true — status bar shows *Review: <file>*) and calls
handle_composite_action(MoveDown). That function bails early at:
let split_id = …active_split();
let _view_state = self.active_window()
.composite_view_states.get(&(split_id, buffer_id))?; // <-- likely None
The render path lazily creates the view-state keyed by (split_id, buffer_id) for the leaf the composite renders in
(orchestration/mod.rs ~L429). Strong suspicion: active_split() returns a
different leaf than the diff-panel leaf the composite is rendered under, so
the lookup misses and movement is dropped. Next step: add tracing in
handle_composite_action for active_split vs the keys present in
composite_view_states, confirm the mismatch, and resolve the correct leaf
(or create/sync the view-state under active_split).
n / p does nothing (unified focus mode)Cursor never moves between hunks regardless of position (pressed repeatedly
from Ln 6, stayed at Ln 6). Likely the hunk-row model is computed
against the full multi-file stream while the focus-only buffer renders only
the focused file's body, so the target rows don't line up with the rendered
buffer. Verify review_next_hunk/review_prev_hunk row math against the
focus-only buildDiffLines output.
Review Stash → start_review_stash → bootstrapRangeReview with
git stash show -p times out (180s). Reproduced locally on Linux and on all
three CI runners (macOS / Ubuntu / Windows) via
e2e::plugins::review_diff_hunk_parity::test_review_stash_shows_stashed_diff
— this single bug is what is currently reddening PR #2271's CI. Likely hangs
interactively too, not just under test.
[⚠] warning counter increments on composite TabIn side-by-side, pressing Tab bumps the editor warning/error indicator
([⚠ 1] → [⚠ 2] …), suggesting the fold handler throws in composite
context. Investigate review_toggle_file_collapse when the diff panel hosts
a composite buffer.
, / . collapses the other files and expands the target, but the view
stays anchored at the UNSTAGED header — the focused file's diff appears
several rows down and the cursor stays at Ln 1. Expectation: moving to a
file should bring that file to the top (best-fit) of the diff viewport and
place the cursor there.
With a changeset larger than the sidebar height (verified at 27 files),
navigating past the visible window leaves the selection off-screen with no
highlight — the FILES panel never scrolls to follow filesCurrentKey.
The best-fit-to-top behavior from B1 should also apply when jumping hunks
(n/p) and in side-by-side. Currently unmet (compounded by A1/A2).
Tab focus cycling between panelsExpectation: Tab cycles file list → OLD pane → NEW pane → back to file
list (and the diff pane in stacked view). Currently Tab = fold; in
side-by-side the review-mode Tab binding shadows the composite's native
InsertTab → composite_focus_next, so pane focus never switches.
Up/Down/PageUp/PageDown should drive whichever panel has focus.
Today arrows only ever target the diff (and in side-by-side not even that —
see A1); the file list is reachable only via ,/. and comments only via
backtick. There is no "focused panel receives cursor keys" concept.
Nothing shows which pane currently holds keyboard focus, so even the existing backtick→comments focus switch is invisible.
The two persistent toolbar rows show n/p, s/u/d, Tab, Enter, etc.,
but not ,/. (file nav) or 1/2/0 (layout toggle) — exactly the keys
a newcomer needs. They appear only in the ? help screen.
? help hijacks the viewHelp opens as a new *Review Keys* tab and steals focus; review-mode keys
stop working there and there is no "press q/Esc to return" affordance.
Should be an overlay, or restore focus to the review on close.
Help says "Tab — fold the file under the cursor," but Tab actually folds
the hunk under the cursor.
,/. updates the sticky header + sidebar highlight in both
layouts; clamps at the top boundary (no wrap).[No Name] tab churn and no flicker across repeated side-by-side
file switches (the earlier regression is fixed).Tab focus model + focus indicator.A1, A2, and B1 likely share one underlying fix in how the cursor/scroll target is computed and applied per panel.