Add more QoL and high prio features

tui_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"slices"
 	"strings"
 	"testing"
 	"time"
@@ -30,6 +31,7 @@ func (s *recordingService) GetPullRequest(_ context.Context, owner, repo string,
 
 func TestResolvedThreadsStartFolded(t *testing.T) {
 	m := NewApp(nil, "o", "r", false, 50, 10)
+	m.screen = threadScreen
 	m.details = PRDetails{PullRequest: PullRequest{Number: 7}}
 	updated, _ := m.Update(detailsLoadedMsg{number: 7, details: PRDetails{
 		PullRequest: PullRequest{Number: 7},
@@ -48,6 +50,7 @@ func TestResolvedThreadFoldingCanBeDisabled(t *testing.T) {
 	settings := defaultAppSettings()
 	settings.FoldResolved = false
 	m := NewAppWithSettings(nil, "o", "r", false, 50, 10*time.Second, settings)
+	m.screen = threadScreen
 	m.details = PRDetails{PullRequest: PullRequest{Number: 7}}
 	updated, _ := m.Update(detailsLoadedMsg{number: 7, details: PRDetails{
 		PullRequest: PullRequest{Number: 7},
@@ -75,6 +78,7 @@ func TestResolvedThreadIconTakesPrecedenceOverOutdated(t *testing.T) {
 
 func TestSelectionSurvivesRefresh(t *testing.T) {
 	m := NewApp(nil, "o", "r", false, 50, 10)
+	m.screen = threadScreen
 	m.details = PRDetails{
 		PullRequest: PullRequest{Number: 7},
 		Threads:     []ReviewThread{{ID: "a"}, {ID: "b"}},
@@ -266,18 +270,36 @@ func TestDashboardRendersDescriptionAndMetadata(t *testing.T) {
 		ChangedFiles:   3,
 		CommitCount:    2,
 		CommentCount:   5,
+		HeadOID:        "0123456789abcdef",
+		Checks:         []Check{{Name: "unit tests", State: "SUCCESS", URL: "https://checks/1"}},
+		Reviews: []ReviewSummary{{
+			ID: "review", Author: "dave", State: "APPROVED", Body: "Looks **good**.",
+			SubmittedAt: time.Date(2026, 7, 1, 11, 0, 0, 0, time.UTC),
+		}},
+		Conversation: []PRComment{{
+			ID: "comment", Author: "erin", Body: "Please retain `compatibility`.",
+			CreatedAt: time.Date(2026, 7, 1, 10, 0, 0, 0, time.UTC),
+		}},
+		Permissions: ViewerPermissions{Repository: "WRITE", CanUpdatePR: true, CanResolveAny: true},
+		Requirements: MergeRequirements{
+			ApprovalsRequired: 1, RequiresApprovals: true,
+			RequiresStatusChecks: true, RequiresConversation: true,
+		},
 		Threads: []ReviewThread{
 			{ID: "open"},
 			{ID: "old", IsOutdated: true},
 			{ID: "done", IsResolved: true, IsOutdated: true},
 		},
 	}
-	plain := ansi.Strip(m.View())
+	plain := ansi.Strip(strings.Join(m.dashboardLines(), "\n"))
 	for _, wanted := range []string{
 		"Improve dashboard", "@alice", "feature → main", "@bob", "@carol",
 		"ui, review", "v2", "+20", "-4", "3 files", "2 commits",
 		"1 open", "1 outdated", "1 resolved", "Description",
-		"Existing behavior", "new_behavior",
+		"Existing behavior", "new_behavior", "unit tests", "Submitted reviews",
+		"@dave", "Looks good", "Conversation", "@erin", "compatibility",
+		"1 approval", "status checks", "resolved conversations", "write, update, resolve",
+		"0123456",
 	} {
 		if !strings.Contains(plain, wanted) {
 			t.Fatalf("dashboard is missing %q:\n%s", wanted, plain)
@@ -285,6 +307,183 @@ func TestDashboardRendersDescriptionAndMetadata(t *testing.T) {
 	}
 }
 
+func TestDashboardCompactsSubmittedReviewsByDefault(t *testing.T) {
+	m := NewApp(nil, "o", "r", false, 50, 10*time.Second)
+	m.width = 100
+	m.details = PRDetails{
+		PullRequest: PullRequest{RepoWithOwner: "o/r", Number: 1, Title: "PR"},
+		BaseRef:     "main",
+		Reviews: []ReviewSummary{
+			{
+				Author: "alice", State: "COMMENTED",
+				Body:        "First line\n\nSecond **line**",
+				SubmittedAt: time.Date(2099, 1, 2, 3, 4, 0, 0, time.UTC),
+				CommitOID:   "abcdef0123456789",
+			},
+			{Author: "bob", State: "APPROVED", Body: "Ship it."},
+		},
+	}
+
+	lines := m.dashboardLines()
+	plain := make([]string, len(lines))
+	for index, line := range lines {
+		plain[index] = ansi.Strip(line)
+	}
+	alice := slices.IndexFunc(plain, func(line string) bool {
+		return strings.Contains(line, "@alice") && strings.Contains(line, "First line Second line")
+	})
+	bob := slices.IndexFunc(plain, func(line string) bool {
+		return strings.Contains(line, "@bob") && strings.Contains(line, "Ship it.")
+	})
+	if alice < 0 || bob != alice+1 {
+		t.Fatalf("compact reviews are not adjacent one-line entries:\n%s", strings.Join(plain, "\n"))
+	}
+	joined := strings.Join(plain, "\n")
+	if strings.Contains(joined, "2099-01-02") || strings.Contains(joined, "abcdef0") {
+		t.Fatalf("compact reviews include timestamp or commit SHA:\n%s", joined)
+	}
+}
+
+func TestDashboardAggregatesBodylessSubmittedReviews(t *testing.T) {
+	m := NewApp(nil, "o", "r", false, 50, 10*time.Second)
+	m.width = 100
+	reviews := make([]ReviewSummary, 0, 48)
+	for range 40 {
+		reviews = append(reviews, ReviewSummary{Author: "mhoff", State: "COMMENTED"})
+	}
+	for range 8 {
+		reviews = append(reviews, ReviewSummary{Author: "Pablu23", State: "COMMENTED"})
+	}
+	m.details = PRDetails{
+		PullRequest: PullRequest{RepoWithOwner: "o/r", Number: 1, Title: "PR"},
+		BaseRef:     "main",
+		Reviews:     reviews,
+	}
+
+	plain := ansi.Strip(strings.Join(m.dashboardLines(), "\n"))
+	for _, wanted := range []string{"Submitted reviews (48)", "COMMENTED ×48", "@mhoff ×40", "@Pablu23 ×8"} {
+		if !strings.Contains(plain, wanted) {
+			t.Fatalf("compact review aggregate is missing %q:\n%s", wanted, plain)
+		}
+	}
+	if strings.Count(plain, "@mhoff") != 1 || strings.Count(plain, "@Pablu23") != 1 ||
+		strings.Count(plain, "COMMENTED") != 1 {
+		t.Fatalf("body-less reviews were rendered individually:\n%s", plain)
+	}
+}
+
+func TestDashboardCanExpandSubmittedReviews(t *testing.T) {
+	settings := defaultAppSettings()
+	settings.CompactReviews = false
+	m := NewAppWithSettings(nil, "o", "r", false, 50, 10*time.Second, settings)
+	m.width = 100
+	m.details = PRDetails{
+		PullRequest: PullRequest{RepoWithOwner: "o/r", Number: 1, Title: "PR"},
+		BaseRef:     "main",
+		Reviews: []ReviewSummary{{
+			Author: "alice", State: "COMMENTED", Body: "First line\n\nSecond line",
+			SubmittedAt: time.Date(2099, 1, 2, 3, 4, 0, 0, time.UTC),
+			CommitOID:   "abcdef0123456789",
+		}},
+	}
+
+	plain := ansi.Strip(strings.Join(m.dashboardLines(), "\n"))
+	if !strings.Contains(plain, "2099-01-02") || !strings.Contains(plain, "abcdef0") ||
+		!strings.Contains(plain, "First line") || !strings.Contains(plain, "Second line") {
+		t.Fatalf("expanded review metadata or body missing:\n%s", plain)
+	}
+}
+
+func TestThreadFilterCombinesPathStatusAuthorAndUpdates(t *testing.T) {
+	m := NewApp(nil, "o", "r", false, 50, time.Second)
+	m.details.Threads = []ReviewThread{
+		{ID: "match", Path: "src/generic_adder/rule.py", Comments: []ReviewComment{{Author: "Alice"}}},
+		{ID: "wrong-status", Path: "src/generic_adder/rule.py", IsResolved: true, Comments: []ReviewComment{{Author: "Alice"}}},
+		{ID: "wrong-author", Path: "src/generic_adder/rule.py", Comments: []ReviewComment{{Author: "Bob"}}},
+	}
+	m.updatedThreads["match"] = true
+	m.searchQuery = "generic rule status:open author:ali updated:true"
+	if got := m.matchingThreadIndices(); !slices.Equal(got, []int{0}) {
+		t.Fatalf("combined filter matches = %v", got)
+	}
+}
+
+func TestDetailRefreshKeepsLogicalCommentAnchored(t *testing.T) {
+	m := NewApp(nil, "o", "r", false, 50, time.Second)
+	m.screen, m.width, m.height = threadScreen, 80, 6
+	m.listHidden = true
+	m.details = PRDetails{
+		PullRequest: PullRequest{ID: "pr", Owner: "o", Repository: "r", Number: 1},
+		Threads: []ReviewThread{{ID: "thread", Comments: []ReviewComment{
+			{ID: "first", Author: "a", Body: "one"},
+			{ID: "anchor", Author: "b", Body: "two"},
+		}}},
+	}
+	for index, line := range m.detailLines(m.width) {
+		if line.anchor == "comment:anchor:header" {
+			m.scroll = index
+			break
+		}
+	}
+	refreshed := m.details
+	refreshed.Threads = append([]ReviewThread(nil), m.details.Threads...)
+	refreshed.Threads[0].Comments = append([]ReviewComment{{ID: "inserted", Author: "c", Body: "new"}}, refreshed.Threads[0].Comments...)
+	updated, _ := m.Update(detailsLoadedMsg{owner: "o", repo: "r", number: 1, details: refreshed})
+	m = updated.(App)
+	if got := m.detailScrollAnchor(); got != "comment:anchor:header" {
+		t.Fatalf("detail anchor after refresh = %q", got)
+	}
+}
+
+func TestWriteCapabilityGateExplainsCachedAndPermissionStates(t *testing.T) {
+	cached := writeCapabilities(PRDetails{FromCache: true}, nil)
+	if cached[0].reason != "offline cached snapshot" || cached[0].authorized {
+		t.Fatalf("cached capability = %#v", cached[0])
+	}
+	thread := &ReviewThread{ViewerCanReply: true}
+	live := writeCapabilities(PRDetails{Permissions: ViewerPermissions{CanReact: true}}, thread)
+	if !live[0].authorized || live[1].authorized || !live[2].authorized {
+		t.Fatalf("live capabilities = %#v", live)
+	}
+}
+
+func TestPollingMarksNewThreadCommentsUnread(t *testing.T) {
+	m := NewApp(nil, "o", "r", false, 50, 10*time.Second)
+	m.screen = threadScreen
+	initial := PRDetails{
+		PullRequest: PullRequest{ID: "pr", Number: 1},
+		Threads: []ReviewThread{{
+			ID: "thread", Path: "a.go", Comments: []ReviewComment{{ID: "comment-1"}},
+		}},
+	}
+	updated, _ := m.Update(detailsLoadedMsg{number: 1, details: initial})
+	m = updated.(App)
+	if m.unreadThreads["thread"] {
+		t.Fatal("initial data was marked unread")
+	}
+
+	refreshed := initial
+	refreshed.Threads = []ReviewThread{{
+		ID: "thread", Path: "a.go",
+		Comments: []ReviewComment{{ID: "comment-1"}, {ID: "comment-2"}},
+	}}
+	updated, _ = m.Update(detailsLoadedMsg{number: 1, details: refreshed})
+	m = updated.(App)
+	if !m.unreadThreads["thread"] {
+		t.Fatal("new review comment was not marked unread")
+	}
+	plain := ansi.Strip(m.threadList(48, 10))
+	if !strings.Contains(plain, "NEW") {
+		t.Fatalf("thread list does not indicate unread update:\n%s", plain)
+	}
+
+	updated, _ = m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("n")})
+	m = updated.(App)
+	if m.unreadThreads["thread"] {
+		t.Fatal("visiting unread thread did not mark it read")
+	}
+}
+
 func TestDashboardDescriptionScrolls(t *testing.T) {
 	m := NewApp(nil, "o", "r", false, 50, 10*time.Second)
 	m.screen = dashboardScreen
@@ -347,6 +546,35 @@ func TestHelpCanScrollInShortTerminal(t *testing.T) {
 	}
 }
 
+func TestHelpWrapsLongActionsAndExplainsFilterClear(t *testing.T) {
+	m := NewApp(nil, "o", "r", false, 50, 10*time.Second)
+	m.screen, m.width, m.height = threadScreen, 46, 20
+	rows := m.helpRows(m.helpContentWidth())
+	plain := strings.Join(strings.Fields(ansi.Strip(strings.Join(rows, "\n"))), " ")
+	if !strings.Contains(plain, "Clear the active thread filter and show every thread") {
+		t.Fatalf("F binding is not explained clearly:\n%s", plain)
+	}
+	if !strings.Contains(plain, "GitHub did not grant update permission") {
+		t.Fatalf("long capability explanation was cut off:\n%s", plain)
+	}
+	for index, row := range rows {
+		if width := ansi.StringWidth(row); width > m.helpContentWidth() {
+			t.Fatalf("wrapped help row %d width = %d, content width = %d", index, width, m.helpContentWidth())
+		}
+	}
+}
+
+func TestUppercaseFClearsAppliedThreadFilter(t *testing.T) {
+	m := NewApp(nil, "o", "r", false, 50, 10*time.Second)
+	m.screen = threadScreen
+	m.searchQuery = "status:resolved author:alice"
+	updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("F")})
+	m = updated.(App)
+	if m.searchQuery != "" {
+		t.Fatalf("F left filter active: %q", m.searchQuery)
+	}
+}
+
 func TestViewNeverExceedsTerminalWidth(t *testing.T) {
 	m := NewApp(nil, "o", "r", false, 50, 10*time.Second)
 	m.screen = threadScreen
@@ -484,6 +712,21 @@ func TestMergeReadyRequiresApproval(t *testing.T) {
 	if !strings.Contains(approved, "merge: ready") {
 		t.Fatalf("approved PR not shown as ready: %q", approved)
 	}
+	unresolved := reviewAndMergeState(PRDetails{
+		Mergeable: "MERGEABLE", ReviewDecision: "APPROVED",
+		Requirements: MergeRequirements{RequiresConversation: true},
+		Threads:      []ReviewThread{{ID: "open"}},
+	})
+	if !strings.Contains(unresolved, "unresolved conversations") {
+		t.Fatalf("required unresolved conversation did not block merge: %q", unresolved)
+	}
+	failing := reviewAndMergeState(PRDetails{
+		Mergeable: "MERGEABLE", ReviewDecision: "APPROVED", CheckState: "FAILURE",
+		Requirements: MergeRequirements{RequiresStatusChecks: true},
+	})
+	if !strings.Contains(failing, "checks failing") {
+		t.Fatalf("required failing checks did not block merge: %q", failing)
+	}
 }
 
 func TestThreadCommentCountsUseSameColumn(t *testing.T) {
@@ -670,7 +913,7 @@ func TestDetailsLoadUsesSelectedPRRepository(t *testing.T) {
 	service := &recordingService{}
 	m := NewApp(service, "", "", false, 50, 10*time.Second)
 	pr := PullRequest{Owner: "other-owner", Repository: "other-repo", Number: 17}
-	msg := m.loadDetails(pr)().(detailsLoadedMsg)
+	msg := m.loadDetails(pr, false)().(detailsLoadedMsg)
 
 	if service.owner != pr.Owner || service.repo != pr.Repository || service.number != pr.Number {
 		t.Fatalf("detail request used %s/%s#%d", service.owner, service.repo, service.number)