From 18562c74e8e49fe8402dc1c2cae5b6488406ec20 Mon Sep 17 00:00:00 2001
From: Test User <test@example.com>
Date: Wed, 11 Mar 2026 10:54:52 +0100
Subject: [PATCH] fix: clamp diff scroll offset in Update to prevent j/k
 appearing broken

The down/j handler was incrementing diffScrollOffset without an upper
bound. View() clamped it for rendering (value receiver, local copy only),
so the stored offset could grow past maxScroll. This made j appear stuck
at the bottom and k appear unresponsive until enough presses unwound the
excess offset. Adds the same maxScroll calculation in Update() and tests
for all scroll edge cases.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 pkg/tui/tui_test.go | 66 +++++++++++++++++++++++++++++++++++++++++++++
 pkg/tui/update.go   | 13 ++++++++-
 2 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/pkg/tui/tui_test.go b/pkg/tui/tui_test.go
index 11dc332..bac77e5 100644
--- a/pkg/tui/tui_test.go
+++ b/pkg/tui/tui_test.go
@@ -880,6 +880,72 @@ func TestRenderDiffLine_PlainLine(t *testing.T) {
 	}
 }
 
+// ---------------------------------------------------------------------------
+// Diff scrolling
+// ---------------------------------------------------------------------------
+
+func TestDiffScrolling_JScrollsDown(t *testing.T) {
+	m := newTestModel()
+	// Build diff content with more lines than availHeight (height=40, availHeight=32)
+	var lines []string
+	for i := 0; i < 50; i++ {
+		lines = append(lines, fmt.Sprintf("line %d", i))
+	}
+	m.diffViewing = true
+	m.diffContent = strings.Join(lines, "\n")
+	m.diffScrollOffset = 0
+
+	m, _ = applyUpdate(m, keyMsg("j"))
+	if m.diffScrollOffset != 1 {
+		t.Errorf("diffScrollOffset = %d, want 1", m.diffScrollOffset)
+	}
+}
+
+func TestDiffScrolling_KScrollsUp(t *testing.T) {
+	m := newTestModel()
+	var lines []string
+	for i := 0; i < 50; i++ {
+		lines = append(lines, fmt.Sprintf("line %d", i))
+	}
+	m.diffViewing = true
+	m.diffContent = strings.Join(lines, "\n")
+	m.diffScrollOffset = 5
+
+	m, _ = applyUpdate(m, keyMsg("k"))
+	if m.diffScrollOffset != 4 {
+		t.Errorf("diffScrollOffset = %d, want 4", m.diffScrollOffset)
+	}
+}
+
+func TestDiffScrolling_JClampsAtMaxScroll(t *testing.T) {
+	m := newTestModel() // height=40, availHeight=32
+	var lines []string
+	for i := 0; i < 50; i++ {
+		lines = append(lines, fmt.Sprintf("line %d", i))
+	}
+	m.diffViewing = true
+	m.diffContent = strings.Join(lines, "\n")
+	// maxScroll = 50 - 32 = 18
+	m.diffScrollOffset = 18
+
+	m, _ = applyUpdate(m, keyMsg("j"))
+	if m.diffScrollOffset != 18 {
+		t.Errorf("diffScrollOffset = %d after j at maxScroll, want 18 (clamped)", m.diffScrollOffset)
+	}
+}
+
+func TestDiffScrolling_KClampsAtZero(t *testing.T) {
+	m := newTestModel()
+	m.diffViewing = true
+	m.diffContent = "line 1\nline 2\nline 3"
+	m.diffScrollOffset = 0
+
+	m, _ = applyUpdate(m, keyMsg("k"))
+	if m.diffScrollOffset != 0 {
+		t.Errorf("diffScrollOffset = %d after k at 0, want 0 (clamped)", m.diffScrollOffset)
+	}
+}
+
 func TestView_IssueBadge_MultipleWorkspaces(t *testing.T) {
 	m := newTestModel(
 		testWSWithIssue("issue-branch", 99, "Refactor auth"),
diff --git a/pkg/tui/update.go b/pkg/tui/update.go
index 48aa361..12e8b7f 100644
--- a/pkg/tui/update.go
+++ b/pkg/tui/update.go
@@ -2,6 +2,7 @@ package tui
 
 import (
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/charmbracelet/bubbles/key"
@@ -79,7 +80,17 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 					m.diffScrollOffset--
 				}
 			case "down", "j":
-				m.diffScrollOffset++
+				availHeight := m.height - 8
+				if availHeight < 5 {
+					availHeight = 5
+				}
+				maxScroll := len(strings.Split(m.diffContent, "\n")) - availHeight
+				if maxScroll < 0 {
+					maxScroll = 0
+				}
+				if m.diffScrollOffset < maxScroll {
+					m.diffScrollOffset++
+				}
 			}
 			return m, nil
 		}