fix(ui): keep cursor on same screen row when paging

app/ui/diffnav.go

@@ -93,81 +93,77 @@ func (m *Model) moveDiffCursorUp() {
 }
 
 // moveDiffCursorPageDown moves the diff cursor down by one visual page.
-// accounts for divider lines and annotation rows that occupy rendered space.
-// scrolls the viewport so cursor appears near the top of the new page.
+// keeps the cursor's relative screen position stable by scrolling both
+// cursor and viewport by the same amount.
 func (m *Model) moveDiffCursorPageDown() {
-	startY := m.cursorViewportY()
-	for {
-		prev := m.nav.diffCursor
-		m.moveDiffCursorDown()
-		if m.nav.diffCursor == prev {
-			break
-		}
-		if m.cursorViewportY()-startY >= m.layout.viewport.Height {
-			break
-		}
-	}
-	// place cursor at the top of the viewport for a true page-scroll feel
-	m.layout.viewport.SetYOffset(m.cursorViewportY())
-	m.layout.viewport.SetContent(m.renderDiff())
+	m.moveDiffCursorDownBy(m.layout.viewport.Height)
 }
 
 // moveDiffCursorPageUp moves the diff cursor up by one visual page.
-// accounts for divider lines and annotation rows that occupy rendered space.
-// scrolls the viewport so cursor appears near the bottom of the new page.
+// keeps the cursor's relative screen position stable by scrolling both
+// cursor and viewport by the same amount.
 func (m *Model) moveDiffCursorPageUp() {
-	startY := m.cursorViewportY()
-	for {
-		prev := m.nav.diffCursor
-		m.moveDiffCursorUp()
-		if m.nav.diffCursor == prev {
-			break
-		}
-		if startY-m.cursorViewportY() >= m.layout.viewport.Height {
-			break
-		}
-	}
-	// place cursor at the bottom of the viewport for a true page-scroll feel
-	m.layout.viewport.SetYOffset(max(0, m.cursorViewportY()-m.layout.viewport.Height+1))
-	m.layout.viewport.SetContent(m.renderDiff())
+	m.moveDiffCursorUpBy(m.layout.viewport.Height)
 }
 
 // moveDiffCursorHalfPageDown moves the diff cursor down by half a visual page.
 // scrolls viewport by half page explicitly, matching vim/less ctrl+d behavior.
 func (m *Model) moveDiffCursorHalfPageDown() {
-	halfPage := max(1, m.layout.viewport.Height/2)
+	m.moveDiffCursorDownBy(max(1, m.layout.viewport.Height/2))
+}
+
+// moveDiffCursorHalfPageUp moves the diff cursor up by half a visual page.
+// scrolls viewport by half page explicitly, matching vim/less ctrl+u behavior.
+func (m *Model) moveDiffCursorHalfPageUp() {
+	m.moveDiffCursorUpBy(max(1, m.layout.viewport.Height/2))
+}
+
+// moveDiffCursorDownBy advances the cursor down by up to rows visual rows
+// and scrolls the viewport by the cursor's actual visual delta so the on-screen
+// row stays stable and the cursor never drops below the viewport.
+// accounts for divider lines, wrap continuations, and annotation rows that occupy rendered space.
+// transitions onto an annotation sub-row (cursorOnAnnotation flip with no diffCursor change)
+// count as real progress so the loop does not terminate early on annotated lines.
+func (m *Model) moveDiffCursorDownBy(rows int) {
 	startY := m.cursorViewportY()
 	for {
-		prev := m.nav.diffCursor
+		prevCursor := m.nav.diffCursor
+		prevAnnot := m.annot.cursorOnAnnotation
 		m.moveDiffCursorDown()
-		if m.nav.diffCursor == prev {
-			break
+		if m.nav.diffCursor == prevCursor && m.annot.cursorOnAnnotation == prevAnnot {
+			break // no more movement possible (end of content)
 		}
-		if m.cursorViewportY()-startY >= halfPage {
+		if m.cursorViewportY()-startY >= rows {
 			break
 		}
 	}
+	actualDelta := m.cursorViewportY() - startY
 	maxOffset := max(0, m.layout.viewport.TotalLineCount()-m.layout.viewport.Height)
-	m.layout.viewport.SetYOffset(min(m.layout.viewport.YOffset+halfPage, maxOffset))
+	m.layout.viewport.SetYOffset(min(m.layout.viewport.YOffset+actualDelta, maxOffset))
 	m.layout.viewport.SetContent(m.renderDiff())
 }
 
-// moveDiffCursorHalfPageUp moves the diff cursor up by half a visual page.
-// scrolls viewport by half page explicitly, matching vim/less ctrl+u behavior.
-func (m *Model) moveDiffCursorHalfPageUp() {
-	halfPage := max(1, m.layout.viewport.Height/2)
+// moveDiffCursorUpBy moves the cursor up by up to rows visual rows
+// and scrolls the viewport by the cursor's actual visual delta so the on-screen
+// row stays stable and the cursor never rises above the viewport.
+// accounts for divider lines, wrap continuations, and annotation rows that occupy rendered space.
+// transitions off an annotation sub-row count as real progress so the loop does not
+// terminate early on annotated lines.
+func (m *Model) moveDiffCursorUpBy(rows int) {
 	startY := m.cursorViewportY()
 	for {
-		prev := m.nav.diffCursor
+		prevCursor := m.nav.diffCursor
+		prevAnnot := m.annot.cursorOnAnnotation
 		m.moveDiffCursorUp()
-		if m.nav.diffCursor == prev {
-			break
+		if m.nav.diffCursor == prevCursor && m.annot.cursorOnAnnotation == prevAnnot {
+			break // no more movement possible (start of content)
 		}
-		if startY-m.cursorViewportY() >= halfPage {
+		if startY-m.cursorViewportY() >= rows {
 			break
 		}
 	}
-	m.layout.viewport.SetYOffset(max(0, m.layout.viewport.YOffset-halfPage))
+	actualDelta := startY - m.cursorViewportY()
+	m.layout.viewport.SetYOffset(max(0, m.layout.viewport.YOffset-actualDelta))
 	m.layout.viewport.SetContent(m.renderDiff())
 }
 

app/ui/diffnav_test.go

@@ -395,6 +395,165 @@ func TestModel_CtrlUMovesHalfPageUp(t *testing.T) {
 	model = result.(Model)
 	assert.Equal(t, 80-pageHeight, model.nav.diffCursor, "PgUp should move cursor up by full viewport height")
 }
+
+// pgdown/pgup must keep the cursor's on-screen row stable — same relative position
+// before and after. symmetric to ctrl+d/ctrl+u. regression test for issue #124.
+func TestModel_PgDownPgUpPreservesRelativeCursorPosition(t *testing.T) {
+	makeModel := func() Model {
+		lines := make([]diff.DiffLine, 200)
+		for i := range lines {
+			lines[i] = diff.DiffLine{NewNum: i + 1, Content: "line", ChangeType: diff.ChangeContext}
+		}
+		m := testModel([]string{"a.go"}, map[string][]diff.DiffLine{"a.go": lines})
+		result, _ := m.Update(tea.WindowSizeMsg{Width: 120, Height: 40})
+		model := result.(Model)
+		result, _ = model.Update(fileLoadedMsg{file: "a.go", lines: lines})
+		model = result.(Model)
+		model.layout.focus = paneDiff
+		return model
+	}
+
+	t.Run("from top, pgdown then pgup is reversible", func(t *testing.T) {
+		model := makeModel()
+		pageHeight := model.layout.viewport.Height
+		require.Positive(t, pageHeight, "page height must be positive")
+		require.Equal(t, 0, model.nav.diffCursor, "cursor starts at 0")
+		require.Equal(t, 0, model.layout.viewport.YOffset, "viewport starts at 0")
+
+		result, _ := model.Update(tea.KeyMsg{Type: tea.KeyPgDown})
+		model = result.(Model)
+		assert.Equal(t, 0, model.cursorViewportY()-model.layout.viewport.YOffset,
+			"pgdown from top should keep cursor at screen row 0")
+		assert.Equal(t, pageHeight, model.nav.diffCursor, "pgdown should advance cursor by page height")
+		assert.Equal(t, pageHeight, model.layout.viewport.YOffset, "pgdown should scroll viewport by page height")
+
+		result, _ = model.Update(tea.KeyMsg{Type: tea.KeyPgUp})
+		model = result.(Model)
+		assert.Equal(t, 0, model.nav.diffCursor, "pgup should reverse pgdown exactly")
+		assert.Equal(t, 0, model.layout.viewport.YOffset, "pgup should restore viewport offset")
+	})
+
+	t.Run("cursor at mid-screen row stays at mid-screen after pgdown", func(t *testing.T) {
+		model := makeModel()
+		// move cursor down 5 rows so it is NOT at screen row 0
+		for range 5 {
+			model.moveDiffCursorDown()
+		}
+		midScreenRow := model.cursorViewportY() - model.layout.viewport.YOffset
+		require.Equal(t, 5, midScreenRow, "setup: cursor should be at screen row 5")
+
+		result, _ := model.Update(tea.KeyMsg{Type: tea.KeyPgDown})
+		model = result.(Model)
+		assert.Equal(t, midScreenRow, model.cursorViewportY()-model.layout.viewport.YOffset,
+			"pgdown should preserve cursor's on-screen row (not snap to top)")
+
+		result, _ = model.Update(tea.KeyMsg{Type: tea.KeyPgUp})
+		model = result.(Model)
+		assert.Equal(t, midScreenRow, model.cursorViewportY()-model.layout.viewport.YOffset,
+			"pgup should preserve cursor's on-screen row (not snap to bottom)")
+	})
+
+	t.Run("pgdown+pgdown+pgup returns to after-first-pgdown state", func(t *testing.T) {
+		model := makeModel()
+
+		result, _ := model.Update(tea.KeyMsg{Type: tea.KeyPgDown})
+		model = result.(Model)
+		afterFirstPgDownCursor := model.nav.diffCursor
+		afterFirstPgDownOffset := model.layout.viewport.YOffset
+
+		result, _ = model.Update(tea.KeyMsg{Type: tea.KeyPgDown})
+		model = result.(Model)
+
+		result, _ = model.Update(tea.KeyMsg{Type: tea.KeyPgUp})
+		model = result.(Model)
+		assert.Equal(t, afterFirstPgDownCursor, model.nav.diffCursor,
+			"pgdown+pgdown+pgup should return cursor to after-first-pgdown state")
+		assert.Equal(t, afterFirstPgDownOffset, model.layout.viewport.YOffset,
+			"pgdown+pgdown+pgup should return viewport to after-first-pgdown state")
+	})
+
+	t.Run("wrap mode with long lines preserves cursor on-screen row", func(t *testing.T) {
+		// mix short and long lines so some wrap and cursorViewportY math spans multiple rows per line
+		longLine := strings.Repeat("abcdefghij ", 20) // ~220 chars, forces wrap
+		lines := make([]diff.DiffLine, 100)
+		for i := range lines {
+			content := "short"
+			if i%3 == 0 {
+				content = longLine
+			}
+			lines[i] = diff.DiffLine{NewNum: i + 1, Content: content, ChangeType: diff.ChangeContext}
+		}
+		m := testModel([]string{"a.go"}, map[string][]diff.DiffLine{"a.go": lines})
+		result, _ := m.Update(tea.WindowSizeMsg{Width: 120, Height: 40})
+		model := result.(Model)
+		result, _ = model.Update(fileLoadedMsg{file: "a.go", lines: lines})
+		model = result.(Model)
+		model.layout.focus = paneDiff
+		model.modes.wrap = true
+		model.layout.viewport.SetContent(model.renderDiff())
+
+		// move cursor down 3 rows so it is mid-screen (over wrapped and short lines)
+		for range 3 {
+			model.moveDiffCursorDown()
+		}
+		midScreenRow := model.cursorViewportY() - model.layout.viewport.YOffset
+
+		result, _ = model.Update(tea.KeyMsg{Type: tea.KeyPgDown})
+		model = result.(Model)
+		assert.Equal(t, midScreenRow, model.cursorViewportY()-model.layout.viewport.YOffset,
+			"wrap-mode pgdown should preserve cursor's on-screen row")
+
+		result, _ = model.Update(tea.KeyMsg{Type: tea.KeyPgUp})
+		model = result.(Model)
+		assert.Equal(t, midScreenRow, model.cursorViewportY()-model.layout.viewport.YOffset,
+			"wrap-mode pgup should preserve cursor's on-screen row")
+	})
+}
+
+// on annotated lines the cursor must stay visible within the viewport after pgdown/pgup,
+// even when moveDiffCursorDown only flips cursorOnAnnotation without advancing diffCursor.
+func TestModel_PgDownKeepsCursorVisibleOnAnnotatedLine(t *testing.T) {
+	lines := make([]diff.DiffLine, 100)
+	for i := range lines {
+		lines[i] = diff.DiffLine{NewNum: i + 1, Content: "line", ChangeType: diff.ChangeAdd}
+	}
+
+	m := testModel([]string{"a.go"}, map[string][]diff.DiffLine{"a.go": lines})
+	result, _ := m.Update(tea.WindowSizeMsg{Width: 120, Height: 24})
+	model := result.(Model)
+	result, _ = model.Update(fileLoadedMsg{file: "a.go", lines: lines})
+	model = result.(Model)
+	model.layout.focus = paneDiff
+
+	// annotate the first few lines so that moveDiffCursorDown will stall on cursorOnAnnotation
+	for i := range 5 {
+		model.store.Add(annotation.Annotation{File: "a.go", Line: i + 1, Type: string(diff.ChangeAdd), Comment: "note"})
+	}
+
+	pageHeight := model.layout.viewport.Height
+	require.Positive(t, pageHeight)
+
+	result, _ = model.Update(tea.KeyMsg{Type: tea.KeyPgDown})
+	model = result.(Model)
+
+	cursorY := model.cursorViewportY()
+	yOffset := model.layout.viewport.YOffset
+	assert.GreaterOrEqual(t, cursorY, yOffset,
+		"cursor must stay on or below the top of the viewport after pgdown on annotated line")
+	assert.Less(t, cursorY, yOffset+pageHeight,
+		"cursor must stay above the bottom of the viewport after pgdown on annotated line")
+
+	// pgup must also keep cursor visible
+	result, _ = model.Update(tea.KeyMsg{Type: tea.KeyPgUp})
+	model = result.(Model)
+	cursorY = model.cursorViewportY()
+	yOffset = model.layout.viewport.YOffset
+	assert.GreaterOrEqual(t, cursorY, yOffset,
+		"cursor must stay visible after pgup on annotated line")
+	assert.Less(t, cursorY, yOffset+pageHeight,
+		"cursor must stay visible after pgup on annotated line")
+}
+
 func TestModel_TreeCtrlDUMovesHalfPage(t *testing.T) {
 	files := make([]string, 50)
 	for i := range files {