From 75ef4ac4fad4d9c4b94cbc6f77e5eb7a26489102 Mon Sep 17 00:00:00 2001 From: Don Brown Date: Wed, 1 Apr 2026 11:43:40 -0600 Subject: [PATCH 1/2] Add /reject inline, /skip, and hide reviewed PRs - /reject accepts inline text like /comment - /reject and /comment share code via runCommentOrReject - /skip hides PRs persistently via skipped.json cache - Hide open PRs where user already approved or requested changes - Add skip_pr MCP tool so chat can skip PRs - ctrl+a reveals all hidden PRs including skipped --- internal/app/app.go | 2 + internal/cache/skip.go | 88 ++++++++++++++++++++++++++++++ internal/mcp/server.go | 17 ++++++ internal/mcp/tools.go | 16 ++++++ internal/mcp/types.go | 1 + internal/tui/conversation_keys.go | 2 + internal/tui/diff_overlay_scene.go | 22 +++++--- internal/tui/handlers.go | 11 ++++ internal/tui/model_helpers.go | 53 ++++++++++++++++-- internal/tui/perm/perm.go | 7 +++ internal/tui/prcard.go | 13 +++-- internal/tui/slashcmds.go | 82 +++++++++++++++++++++------- internal/tui/tui.go | 2 + 13 files changed, 277 insertions(+), 39 deletions(-) create mode 100644 internal/cache/skip.go diff --git a/internal/app/app.go b/internal/app/app.go index 5ef7056..082757d 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -25,6 +25,7 @@ type App struct { CurrentUser string Config config.Config Cache *cache.Cache + SkipStore *cache.SkipStore Skills []skills.Skill } @@ -100,6 +101,7 @@ func New(repoDirs []string) (*App, error) { CurrentUser: user, Config: cfg, Cache: cache.Load(), + SkipStore: cache.LoadSkipStore(), Skills: discovered, } diff --git a/internal/cache/skip.go b/internal/cache/skip.go new file mode 100644 index 0000000..6e8ecfb --- /dev/null +++ b/internal/cache/skip.go @@ -0,0 +1,88 @@ +package cache + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "sync" + + "github.com/sleuth-io/prx/internal/dirs" + "github.com/sleuth-io/prx/internal/logger" +) + +// SkipStore persists a set of skipped PRs keyed by "repo#number". +type SkipStore struct { + mu sync.Mutex + skipped map[string]bool + path string +} + +func LoadSkipStore() *SkipStore { + path := skipPath() + s := &SkipStore{path: path, skipped: make(map[string]bool)} + data, err := os.ReadFile(path) + if os.IsNotExist(err) { + return s + } + if err != nil { + logger.Error("reading skip store: %v", err) + return s + } + if err := json.Unmarshal(data, &s.skipped); err != nil { + logger.Error("parsing skip store: %v", err) + } + logger.Info("loaded %d skipped PRs", len(s.skipped)) + return s +} + +func SkipKey(repo string, number int) string { + return fmt.Sprintf("%s#%d", repo, number) +} + +func (s *SkipStore) IsSkipped(key string) bool { + s.mu.Lock() + defer s.mu.Unlock() + return s.skipped[key] +} + +func (s *SkipStore) Skip(key string) { + s.mu.Lock() + defer s.mu.Unlock() + s.skipped[key] = true + if err := s.save(); err != nil { + logger.Error("saving skip store: %v", err) + } +} + +func (s *SkipStore) Unskip(key string) { + s.mu.Lock() + defer s.mu.Unlock() + delete(s.skipped, key) + if err := s.save(); err != nil { + logger.Error("saving skip store: %v", err) + } +} + +func (s *SkipStore) save() error { + if err := os.MkdirAll(filepath.Dir(s.path), 0755); err != nil { + return err + } + data, err := json.MarshalIndent(s.skipped, "", " ") + if err != nil { + return err + } + return os.WriteFile(s.path, data, 0644) +} + +func skipPath() string { + dir, err := dirs.GetCacheDir() + if err != nil { + xdg := os.Getenv("XDG_CACHE_HOME") + if xdg == "" { + xdg = filepath.Join(os.Getenv("HOME"), ".cache") + } + dir = filepath.Join(xdg, "prx") + } + return filepath.Join(dir, "skipped.json") +} diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 2966dcd..895aaa1 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -151,6 +151,23 @@ func (s *Server) notifyConfigReload() { }) } +// notifySkip signals the TUI to reload its skip store and update visibility. +func (s *Server) notifySkip() { + if s.socketPath == "" { + return + } + conn, err := net.Dial("unix", s.socketPath) + if err != nil { + logger.Error("mcp: notify skip: %v", err) + return + } + defer func() { _ = conn.Close() }() + _ = json.NewEncoder(conn).Encode(map[string]interface{}{ + "type": "skip", + "pr_number": s.prNumber, + }) +} + // notifyRefresh signals the TUI to re-fetch this PR's data. func (s *Server) notifyRefresh() { if s.socketPath == "" { diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index 0ff48f1..eba8f14 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/sleuth-io/prx/internal/cache" "github.com/sleuth-io/prx/internal/config" "github.com/sleuth-io/prx/internal/github" ) @@ -53,6 +54,14 @@ var toolDefs = []map[string]interface{}{ "required": []string{"body"}, }, }, + { + "name": "skip_pr", + "description": "Skip the pull request — hides it from the review queue until un-skipped", + "inputSchema": map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{}, + }, + }, { "name": "merge_pr", "description": "Merge the pull request", @@ -232,6 +241,8 @@ func (s *Server) toolDescription(name string, args map[string]interface{}) strin return fmt.Sprintf("Post inline comment on PR #%d at %s:%d: %s", s.prNumber, path, int(line), truncate(body, 100)) } return fmt.Sprintf("Post comment on PR #%d: %s", s.prNumber, truncate(body, 100)) + case "skip_pr": + return fmt.Sprintf("Skip PR #%d", s.prNumber) case "merge_pr": return fmt.Sprintf("Merge PR #%d", s.prNumber) case "set_model": @@ -288,6 +299,11 @@ func (s *Server) executeAction(name string, args map[string]interface{}) (string return "", err } return "Comment posted successfully", nil + case "skip_pr": + store := cache.LoadSkipStore() + store.Skip(cache.SkipKey(s.repo, s.prNumber)) + s.notifySkip() + return "PR skipped successfully", nil case "merge_pr": cfg, err := config.Load() if err != nil { diff --git a/internal/mcp/types.go b/internal/mcp/types.go index 936475a..868cdce 100644 --- a/internal/mcp/types.go +++ b/internal/mcp/types.go @@ -34,6 +34,7 @@ var toolMetas = map[string]toolMeta{ "approve_pr": {requiresPermission: true, isMutation: true}, "request_changes": {requiresPermission: true, isMutation: true}, "comment_on_pr": {requiresPermission: true, isMutation: true}, + "skip_pr": {requiresPermission: false, isMutation: false}, "merge_pr": {requiresPermission: true, isMutation: true}, "get_config": {requiresPermission: false, isMutation: false}, "set_model": {requiresPermission: true, isMutation: false}, diff --git a/internal/tui/conversation_keys.go b/internal/tui/conversation_keys.go index 655bbad..d6e1eb8 100644 --- a/internal/tui/conversation_keys.go +++ b/internal/tui/conversation_keys.go @@ -227,8 +227,10 @@ func (s *ConversationScene) handleActionDone(msg actionDoneMsg, m *Model) (Scene } case actionApprove: s.actionStatus = fmt.Sprintf("Approved PR #%d", msg.pr) + m.addLocalReview(msg.repo, msg.pr, "APPROVED") case actionRequestChanges: s.actionStatus = fmt.Sprintf("Requested changes on PR #%d", msg.pr) + m.addLocalReview(msg.repo, msg.pr, "CHANGES_REQUESTED") case actionPostMergeApprove: s.actionStatus = fmt.Sprintf("Approved post-merge PR #%d \U0001f44d", msg.pr) m.markPostMergeReacted(msg.repo, msg.pr, "+1") diff --git a/internal/tui/diff_overlay_scene.go b/internal/tui/diff_overlay_scene.go index 3e5aaeb..1b5cb25 100644 --- a/internal/tui/diff_overlay_scene.go +++ b/internal/tui/diff_overlay_scene.go @@ -49,6 +49,9 @@ func (s *DiffOverlayScene) View(m *Model) string { if s.modal.active { title := " Add comment (Enter submit · Alt+Enter newline · Esc cancel)" + if s.modal.requestChanges { + title = " Request changes (Enter submit · Alt+Enter newline · Esc cancel)" + } if s.modal.isInline { title = fmt.Sprintf(" Comment on %s:%d (Enter submit · Alt+Enter newline · Esc cancel)", s.modal.filePath, s.modal.fileLine) } @@ -153,6 +156,7 @@ func (s *DiffOverlayScene) handleModalKey(msg tea.KeyMsg, m *Model) (Scene, tea. return s, nil } isInline := s.modal.isInline + requestChanges := s.modal.requestChanges filePath := s.modal.filePath fileLine := s.modal.fileLine commitSHA := s.modal.commitSHA @@ -163,6 +167,9 @@ func (s *DiffOverlayScene) handleModalKey(msg tea.KeyMsg, m *Model) (Scene, tea. Line: fileLine, }) s.modal = commentModal{} + if requestChanges { + return s, requestChangesCmd(card.Ctx.Repo, card.PR.Number, body) + } if isInline { return s, postInlineCommentCmd(card.Ctx.Repo, card.PR.Number, commitSHA, filePath, fileLine, body, pendingItem) } @@ -174,18 +181,19 @@ func (s *DiffOverlayScene) handleModalKey(msg tea.KeyMsg, m *Model) (Scene, tea. } } -func (s *DiffOverlayScene) openCommentModal(card *PRCard, isInline bool, path string, line int) { +func (s *DiffOverlayScene) openCommentModal(card *PRCard, isInline bool, path string, line int, requestChanges ...bool) { ta := textarea.New() ta.Placeholder = "Write your comment..." ta.SetWidth(s.width - 4) ta.SetHeight(4) s.modal = commentModal{ - active: true, - isInline: isInline, - filePath: path, - fileLine: line, - commitSHA: card.PR.HeadSHA, - textarea: ta, + active: true, + isInline: isInline, + requestChanges: len(requestChanges) > 0 && requestChanges[0], + filePath: path, + fileLine: line, + commitSHA: card.PR.HeadSHA, + textarea: ta, } } diff --git a/internal/tui/handlers.go b/internal/tui/handlers.go index 51eb475..bb38b75 100644 --- a/internal/tui/handlers.go +++ b/internal/tui/handlers.go @@ -142,6 +142,17 @@ func (m *Model) handlePermRefresh(msg perm.RefreshMsg) (Model, tea.Cmd) { return *m, nil } +func (m *Model) handleSkip(msg perm.SkipMsg) (Model, tea.Cmd) { + // Reload skip store from disk (MCP server wrote it). + m.app.SkipStore = cache.LoadSkipStore() + if card := m.currentCard(); card != nil && !m.isCardVisible(card) { + m.skipToVisibleCard() + m.loadCurrentDiff() + } + m.buildScrollback() + return *m, nil +} + func (m *Model) handleConfigReload(_ perm.ConfigReloadMsg) (Model, tea.Cmd) { var cmds []tea.Cmd oldHash := config.CriteriaHash(m.app.Config.Criteria) diff --git a/internal/tui/model_helpers.go b/internal/tui/model_helpers.go index 3a4c274..8bc6f1d 100644 --- a/internal/tui/model_helpers.go +++ b/internal/tui/model_helpers.go @@ -12,6 +12,8 @@ import ( tea "github.com/charmbracelet/bubbletea" "github.com/sleuth-io/prx/internal/ai" + "github.com/sleuth-io/prx/internal/cache" + "github.com/sleuth-io/prx/internal/github" "github.com/sleuth-io/prx/internal/logger" "github.com/sleuth-io/prx/internal/mcp" "github.com/sleuth-io/prx/internal/reviewstate" @@ -88,21 +90,62 @@ func (m *Model) checkNoPRs() { } // isCardVisible returns whether a card should be shown in the current view. -// Open PRs are always visible. Post-merge PRs are hidden when already reviewed/reacted -// unless showAllMerged is toggled on. +// Open PRs are hidden when the user has reviewed (approved or requested changes) +// and nothing has changed since, or when skipped. Post-merge PRs are hidden when +// already reviewed/reacted. Ctrl+a (showAllMerged) reveals all hidden cards. func (m Model) isCardVisible(card *PRCard) bool { - if !card.PostMerge { - return true - } if m.showAllMerged { return true } + // Skipped PRs are always hidden (unless show-all is on). + key := cache.SkipKey(card.Ctx.Repo, card.PR.Number) + if m.app.SkipStore.IsSkipped(key) { + return false + } + if !card.PostMerge { + if card.HasNewContent || m.isOwnPR(card) { + return true + } + return !m.userHasReviewedPR(card) + } if card.HasNewContent { return true // new comments since last review — keep visible } return !card.UserHasReviewed && !card.UserHasReacted } +// userHasReviewedPR checks if the current user's latest review on an open PR +// is APPROVED or CHANGES_REQUESTED. +func (m Model) userHasReviewedPR(card *PRCard) bool { + if card.PR == nil { + return false + } + // Walk reviews in reverse to find the current user's latest review. + for i := len(card.PR.Reviews) - 1; i >= 0; i-- { + r := card.PR.Reviews[i] + if r.Author == m.app.CurrentUser { + return r.State == "APPROVED" || r.State == "CHANGES_REQUESTED" + } + } + return false +} + +// addLocalReview appends a review to the local PR data so visibility checks +// reflect the user's action without waiting for a refresh from GitHub. +func (m *Model) addLocalReview(repo string, prNumber int, state string) { + if card := m.findCard(repo, prNumber); card != nil && card.PR != nil { + card.PR.Reviews = append(card.PR.Reviews, github.ReviewComment{ + Author: m.app.CurrentUser, + State: state, + }) + // Card may now be hidden — advance to next visible card. + if !m.isCardVisible(card) { + m.skipToVisibleCard() + m.loadCurrentDiff() + } + } +} + // visibleCardCount returns the number of currently visible cards. func (m Model) visibleCardCount() int { n := 0 diff --git a/internal/tui/perm/perm.go b/internal/tui/perm/perm.go index 81059d1..d5e2948 100644 --- a/internal/tui/perm/perm.go +++ b/internal/tui/perm/perm.go @@ -33,6 +33,11 @@ type RefreshMsg struct { PRNumber int } +// SkipMsg is sent to the TUI when a PR has been skipped via MCP. +type SkipMsg struct { + PRNumber int +} + // ConfigReloadMsg is sent to the TUI when config has been updated on disk // and the in-memory state should be refreshed. type ConfigReloadMsg struct{} @@ -91,6 +96,8 @@ func handleConn(conn net.Conn, program *tea.Program) { } case "refresh": program.Send(RefreshMsg{PRNumber: req.PRNumber}) + case "skip": + program.Send(SkipMsg{PRNumber: req.PRNumber}) case "config_reload": program.Send(ConfigReloadMsg{}) default: diff --git a/internal/tui/prcard.go b/internal/tui/prcard.go index e14e916..213b35f 100644 --- a/internal/tui/prcard.go +++ b/internal/tui/prcard.go @@ -37,10 +37,11 @@ type PRCard struct { } type commentModal struct { - active bool - isInline bool - filePath string - fileLine int - commitSHA string - textarea textarea.Model + active bool + isInline bool + requestChanges bool + filePath string + fileLine int + commitSHA string + textarea textarea.Model } diff --git a/internal/tui/slashcmds.go b/internal/tui/slashcmds.go index e34d15b..38a2f67 100644 --- a/internal/tui/slashcmds.go +++ b/internal/tui/slashcmds.go @@ -4,6 +4,7 @@ import ( "fmt" tea "github.com/charmbracelet/bubbletea" + "github.com/sleuth-io/prx/internal/cache" ) // CommandScope defines when a command is available. @@ -166,16 +167,10 @@ func commands() []Command { Scope: ScopePR, Run: func(s *ConversationScene, m *Model, args string) (Scene, tea.Cmd, bool) { card := m.currentCard() - if card == nil || card.Scoring || card.Assessment == nil || m.isOwnPR(card) { + if card == nil || card.Scoring || m.isOwnPR(card) { return s, nil, true } - repo, num, notes := card.Ctx.Repo, card.PR.Number, card.Assessment.ReviewNotes - s.confirm = &confirmDialog{ - description: fmt.Sprintf("Request changes on PR #%d?", num), - actionStatus: "Requesting changes…", - cmd: requestChangesCmd(repo, num, notes), - } - return s, nil, true + return runCommentOrReject(s, m, card, args, true) }, }, { @@ -187,18 +182,43 @@ func commands() []Command { if card == nil { return s, nil, true } - // If args provided, post the comment directly - if args != "" { - repo, num := card.Ctx.Repo, card.PR.Number - s.actionStatus = "Posting comment…" - return s, postGlobalCommentCmd(repo, num, args, nil), true + return runCommentOrReject(s, m, card, args, false) + }, + }, + { + Name: "skip", + Description: "Skip current PR (hide until un-skipped)", + Scope: ScopePR, + Run: func(s *ConversationScene, m *Model, args string) (Scene, tea.Cmd, bool) { + card := m.currentCard() + if card == nil { + return s, nil, true } - // No args — enter diff overlay with comment modal open - s.input.Blur() - m.diffView.Focused = true - ds := newDiffOverlayScene(s, m.width, m.height) - ds.openCommentModal(card, false, "", 0) - return ds, ds.modal.textarea.Focus(), true + key := cache.SkipKey(card.Ctx.Repo, card.PR.Number) + m.app.SkipStore.Skip(key) + s.actionStatus = fmt.Sprintf("Skipped PR #%d", card.PR.Number) + s.actionDone = true + m.skipToVisibleCard() + m.loadCurrentDiff() + s.BuildScrollback(m) + return m.scene, nil, true + }, + }, + { + Name: "unskip", + Description: "Un-skip current PR", + Scope: ScopePR, + Run: func(s *ConversationScene, m *Model, args string) (Scene, tea.Cmd, bool) { + card := m.currentCard() + if card == nil { + return s, nil, true + } + key := cache.SkipKey(card.Ctx.Repo, card.PR.Number) + m.app.SkipStore.Unskip(key) + s.actionStatus = fmt.Sprintf("Un-skipped PR #%d", card.PR.Number) + s.actionDone = true + s.BuildScrollback(m) + return s, nil, true }, }, { @@ -265,11 +285,31 @@ func commandMap() (slashMap map[string]*Command, keyMap map[string]*Command) { return } +// runCommentOrReject handles the shared logic for /comment and /reject. +// With args: posts immediately. Without args: opens the diff overlay comment modal. +func runCommentOrReject(s *ConversationScene, m *Model, card *PRCard, args string, requestChanges bool) (Scene, tea.Cmd, bool) { + repo, num := card.Ctx.Repo, card.PR.Number + if args != "" { + if requestChanges { + s.actionStatus = "Requesting changes…" + return s, requestChangesCmd(repo, num, args), true + } + s.actionStatus = "Posting comment…" + return s, postGlobalCommentCmd(repo, num, args, nil), true + } + // No args — enter diff overlay with comment modal open + s.input.Blur() + m.diffView.Focused = true + ds := newDiffOverlayScene(s, m.width, m.height) + ds.openCommentModal(card, false, "", 0, requestChanges) + return ds, ds.modal.textarea.Focus(), true +} + // ActionToolNames returns the MCP-prefixed action tool names filtered by ownership. // This replaces the hardcoded tool lists in sendChatCmd. func ActionToolNames(isOwnPR bool) []string { if isOwnPR { - return []string{"mcp__prx__comment_on_pr", "mcp__prx__merge_pr"} + return []string{"mcp__prx__comment_on_pr", "mcp__prx__merge_pr", "mcp__prx__skip_pr"} } - return []string{"mcp__prx__comment_on_pr", "mcp__prx__approve_pr", "mcp__prx__request_changes"} + return []string{"mcp__prx__comment_on_pr", "mcp__prx__approve_pr", "mcp__prx__request_changes", "mcp__prx__skip_pr"} } diff --git a/internal/tui/tui.go b/internal/tui/tui.go index 821e33e..06a043f 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -122,6 +122,8 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil case perm.RefreshMsg: return m.handlePermRefresh(msg) + case perm.SkipMsg: + return m.handleSkip(msg) case perm.ConfigReloadMsg: return m.handleConfigReload(msg) case tea.KeyMsg: From 61f9978e5572537692e49ee3e71f60b423763437 Mon Sep 17 00:00:00 2001 From: Don Brown Date: Wed, 1 Apr 2026 12:15:36 -0600 Subject: [PATCH 2/2] Improve PR visibility and navigation - Fetch tracked PRs from review state store - Wait for diff parsing before startup transition - Exclude own comments from new content check - Add dedup guard in handlePRDetails - Extract visiblePosition for consistent counts - Add ctrl+n/p navigation in diff overlay - Show message at end of PR list --- internal/github/fetch.go | 17 ++++++++ internal/reviewstate/reviewstate.go | 11 +++++ internal/tui/bulk_approve_scene.go | 2 +- internal/tui/commands.go | 35 ++++++++++++++++ internal/tui/conversation_scene.go | 10 +---- internal/tui/diff/identity.go | 12 ++++-- internal/tui/diff_overlay_scene.go | 15 ++++++- internal/tui/handlers.go | 64 ++++++++++++++++++++++++++--- internal/tui/messages.go | 6 +++ internal/tui/model_helpers.go | 49 +++++++++++++++++++--- internal/tui/slashcmds.go | 2 +- internal/tui/tui.go | 8 ++-- 12 files changed, 203 insertions(+), 28 deletions(-) diff --git a/internal/github/fetch.go b/internal/github/fetch.go index 71c3866..eabce92 100644 --- a/internal/github/fetch.go +++ b/internal/github/fetch.go @@ -330,6 +330,23 @@ func ListMergedPRsMeta(repo, currentUser string, since time.Time) ([]map[string] return filtered, nil } +// FetchPRMeta returns lightweight metadata for a single PR by number. +func FetchPRMeta(repo string, number int) (map[string]any, error) { + out, err := exec.Command("gh", "pr", "view", + fmt.Sprintf("%d", number), + "--repo", repo, + "--json", "number,title,author,url,createdAt,additions,deletions,files,body,reviewRequests,headRefOid,headRefName,mergeStateStatus,state", + ).Output() + if err != nil { + return nil, fmt.Errorf("gh pr view %d: %w", number, err) + } + var raw map[string]any + if err := json.Unmarshal(out, &raw); err != nil { + return nil, fmt.Errorf("parsing pr view %d: %w", number, err) + } + return raw, nil +} + func getDiff(repo string, number int) (string, error) { out, err := exec.Command("gh", "pr", "diff", fmt.Sprintf("%d", number), "--repo", repo).Output() if err != nil { diff --git a/internal/reviewstate/reviewstate.go b/internal/reviewstate/reviewstate.go index 665559c..559bff6 100644 --- a/internal/reviewstate/reviewstate.go +++ b/internal/reviewstate/reviewstate.go @@ -81,6 +81,17 @@ func (s *Store) Get(key string) *PRState { return s.states[key] } +// Keys returns all store keys (repo#number format). +func (s *Store) Keys() []string { + s.mu.Lock() + defer s.mu.Unlock() + keys := make([]string, 0, len(s.states)) + for k := range s.states { + keys = append(keys, k) + } + return keys +} + // Set stores the review state for a PR and persists to disk. func (s *Store) Set(key string, state *PRState) { s.mu.Lock() diff --git a/internal/tui/bulk_approve_scene.go b/internal/tui/bulk_approve_scene.go index 5e4f12d..2df8e80 100644 --- a/internal/tui/bulk_approve_scene.go +++ b/internal/tui/bulk_approve_scene.go @@ -45,7 +45,7 @@ func (s *BulkApproveScene) Update(msg tea.Msg, m *Model) (Scene, tea.Cmd) { if msg.String() == "ctrl+r" { var cmds []tea.Cmd for _, r := range m.app.Repos { - cmds = append(cmds, fetchPRListCmd(r), fetchMergedPRListCmd(r)) + cmds = append(cmds, fetchPRListCmd(r), fetchMergedPRListCmd(r), fetchTrackedPRListCmd(r, m.reviewStore, nil)) } return s, tea.Batch(cmds...) } diff --git a/internal/tui/commands.go b/internal/tui/commands.go index ea24ea8..9cb24d9 100644 --- a/internal/tui/commands.go +++ b/internal/tui/commands.go @@ -20,6 +20,7 @@ import ( "github.com/sleuth-io/prx/internal/imgrender" "github.com/sleuth-io/prx/internal/logger" "github.com/sleuth-io/prx/internal/mcp" + "github.com/sleuth-io/prx/internal/reviewstate" "github.com/sleuth-io/prx/internal/tui/chat" "github.com/sleuth-io/prx/internal/tui/diff" ) @@ -163,6 +164,40 @@ func fetchMergedPRListCmd(ctx *app.RepoContext) tea.Cmd { } } +// fetchTrackedPRListCmd fetches metadata for PRs we've previously interacted with +// (from the review state store) that aren't already in the card set. +func fetchTrackedPRListCmd(ctx *app.RepoContext, store *reviewstate.Store, existingNums map[int]bool) tea.Cmd { + // Collect PR numbers from the store for this repo that we don't already have. + var toFetch []int + prefix := ctx.Repo + "#" + for _, key := range store.Keys() { + if !strings.HasPrefix(key, prefix) { + continue + } + numStr := key[len(prefix):] + num, err := strconv.Atoi(numStr) + if err != nil { + continue + } + if existingNums[num] { + continue + } + toFetch = append(toFetch, num) + } + return func() tea.Msg { + var rawPRs []map[string]any + for _, num := range toFetch { + raw, err := github.FetchPRMeta(ctx.Repo, num) + if err != nil { + logger.Info("tracked PR #%d fetch failed: %v", num, err) + continue + } + rawPRs = append(rawPRs, raw) + } + return trackedPRListFetchedMsg{ctx: ctx, rawPRs: rawPRs} + } +} + func fetchMergedPRStatusCmd(repo string, number int, currentUser string) tea.Cmd { return func() tea.Msg { hasReview, hasReaction, err := github.FetchPRReviewAndReactionStatus(repo, number, currentUser) diff --git a/internal/tui/conversation_scene.go b/internal/tui/conversation_scene.go index 91b0dfc..2120bfc 100644 --- a/internal/tui/conversation_scene.go +++ b/internal/tui/conversation_scene.go @@ -274,14 +274,8 @@ func (s *ConversationScene) renderFooter(m *Model) string { if width == 0 { width = 80 } - visible := m.visibleCardCount() - visIdx := 0 - for i := 0; i < m.current && i < len(m.cards); i++ { - if m.isCardVisible(m.cards[i]) { - visIdx++ - } - } - status := fmt.Sprintf("prx PR %d/%d", visIdx+1, visible) + visIdx, visible := m.visiblePosition() + status := fmt.Sprintf("prx PR %d/%d", visIdx, visible) if s.actionStatus != "" && s.actionDone { status += fmt.Sprintf(" %s", s.actionStatus) } else if s.actionStatus != "" { diff --git a/internal/tui/diff/identity.go b/internal/tui/diff/identity.go index 46d348d..4bc5631 100644 --- a/internal/tui/diff/identity.go +++ b/internal/tui/diff/identity.go @@ -33,10 +33,16 @@ func CommentBodyHash(body string) string { } // CommentDigestsFromPR produces comment digests from a PR's inline and top-level comments. -func CommentDigestsFromPR(pr *github.PR) []reviewstate.CommentDigest { +// If excludeUser is provided, comments by that user are excluded — this prevents +// the current user's own comments from being tracked or flagged as new. +func CommentDigestsFromPR(pr *github.PR, excludeUser ...string) []reviewstate.CommentDigest { + exclude := "" + if len(excludeUser) > 0 { + exclude = excludeUser[0] + } var digests []reviewstate.CommentDigest for _, c := range pr.InlineComments { - if c.ID != 0 { + if c.ID != 0 && c.Author != exclude { digests = append(digests, reviewstate.CommentDigest{ ID: c.ID, Hash: CommentBodyHash(c.Body), @@ -44,7 +50,7 @@ func CommentDigestsFromPR(pr *github.PR) []reviewstate.CommentDigest { } } for _, c := range pr.Comments { - if c.ID != 0 { + if c.ID != 0 && c.Author != exclude { digests = append(digests, reviewstate.CommentDigest{ ID: c.ID, Hash: CommentBodyHash(c.Body), diff --git a/internal/tui/diff_overlay_scene.go b/internal/tui/diff_overlay_scene.go index 1b5cb25..a70bc57 100644 --- a/internal/tui/diff_overlay_scene.go +++ b/internal/tui/diff_overlay_scene.go @@ -84,6 +84,18 @@ func (s *DiffOverlayScene) handleKey(msg tea.KeyMsg, m *Model) (Scene, tea.Cmd) switch msg.String() { case "ctrl+q": return s, m.cleanupWorktrees() + case "ctrl+n": + m.diffView.Focused = false + m.navigatePR(1, s.conv) + m.diffView.Focused = true + s.conv.BuildScrollback(m) + return s, nil + case "ctrl+p": + m.diffView.Focused = false + m.navigatePR(-1, s.conv) + m.diffView.Focused = true + s.conv.BuildScrollback(m) + return s, nil case "q", "esc", "ctrl+d": logger.Info("[user] diff overlay: exit (%s)", msg.String()) // Snapshot review state on diff exit @@ -206,7 +218,8 @@ func (s *DiffOverlayScene) renderFooter(m *Model) string { if width == 0 { width = 80 } - status := fmt.Sprintf("prx PR %d/%d", m.current+1, len(m.cards)) + visIdx, visible := m.visiblePosition() + status := fmt.Sprintf("prx PR %d/%d", visIdx, visible) hints := "j/k scroll [/] file {/} hunk ←/→ collapse all ? ask c comment q back" gap := width - lipgloss.Width(status) - lipgloss.Width(hints) - 2 if gap < 1 { diff --git a/internal/tui/handlers.go b/internal/tui/handlers.go index bb38b75..23c979d 100644 --- a/internal/tui/handlers.go +++ b/internal/tui/handlers.go @@ -67,6 +67,8 @@ func (m *Model) updateReview(msg tea.Msg) (Model, tea.Cmd) { return m.handlePRRefreshed(msg) case mergedPRListFetchedMsg: return m.handleMergedPRList(msg) + case trackedPRListFetchedMsg: + return m.handleTrackedPRList(msg) case mergedPRStatusMsg: return m.handleMergedPRStatus(msg) case imageFetchedMsg: @@ -191,8 +193,8 @@ func (m *Model) handlePRList(msg prListFetchedMsg) (Model, tea.Cmd) { newRaws = append(newRaws, raw) } } - if len(m.cards) == 0 && m.mergedListsDone < len(m.app.Repos) { - // First open list result — set total but don't finalize yet (merged lists pending). + if len(m.cards) == 0 && (m.mergedListsDone < len(m.app.Repos) || m.trackedListsDone < len(m.app.Repos)) { + // First open list result — set total but don't finalize yet (other lists pending). m.total = len(msg.rawPRs) m.fetching = len(newRaws) } else { @@ -245,6 +247,42 @@ func (m *Model) handleMergedPRList(msg mergedPRListFetchedMsg) (Model, tea.Cmd) return *m, tea.Batch(cmds...) } +func (m *Model) handleTrackedPRList(msg trackedPRListFetchedMsg) (Model, tea.Cmd) { + m.trackedListsDone++ + if msg.err != nil { + logger.Error("fetching tracked PRs: %v", msg.err) + m.checkNoPRs() + return *m, nil + } + if len(msg.rawPRs) == 0 { + m.checkNoPRs() + return *m, nil + } + existing := make(map[cardKey]bool, len(m.cards)) + for _, card := range m.cards { + existing[cardKey{card.Ctx.Repo, card.PR.Number}] = true + } + // Also check in-flight fetches by looking at all pending details. + var newRaws []map[string]any + for _, raw := range msg.rawPRs { + num := int(raw["number"].(float64)) + if !existing[cardKey{msg.ctx.Repo, num}] { + newRaws = append(newRaws, raw) + } + } + if len(newRaws) == 0 { + return *m, nil + } + m.total += len(newRaws) + m.fetching += len(newRaws) + m.logStep(fmt.Sprintf("Found %d tracked PRs in %s", len(newRaws), msg.ctx.Repo)) + cmds := make([]tea.Cmd, len(newRaws)) + for i, raw := range newRaws { + cmds[i] = fetchPRDetailsCmd(raw, msg.ctx) + } + return *m, tea.Batch(cmds...) +} + func (m *Model) handleMergedPRStatus(msg mergedPRStatusMsg) (Model, tea.Cmd) { var targetCard *PRCard for _, card := range m.cards { @@ -306,8 +344,7 @@ func (m *Model) skipToVisibleCard() { return } } - // No visible cards — show bulk approve (fireworks). - m.tryEnterBulkApprove() + // No visible cards — stay on current card. } func (m *Model) handlePRDetails(msg prDetailsFetchedMsg) (Model, tea.Cmd) { @@ -318,6 +355,13 @@ func (m *Model) handlePRDetails(msg prDetailsFetchedMsg) (Model, tea.Cmd) { } pr := msg.pr ctx := msg.ctx + // Dedup: another list handler may have already created this card. + if m.findCard(ctx.Repo, pr.Number) != nil { + if m.fetching == 0 { + m.tryStartupTransition() + } + return *m, nil + } if !m.startupDone { fetched := m.total - m.fetching cached := "" @@ -329,7 +373,7 @@ func (m *Model) handlePRDetails(msg prDetailsFetchedMsg) (Model, tea.Cmd) { } m.logStep(fmt.Sprintf("Loaded PR #%d: %s%s (%d/%d)", pr.Number, truncate(pr.Title, 40), cached, fetched, m.total)) } - isPostMerge := pr.State == "MERGED" + isPostMerge := pr.State == "MERGED" || pr.State == "CLOSED" card := &PRCard{Ctx: ctx, PR: pr, PostMerge: isPostMerge, Chat: &conversation.ChatSession{ Status: "Preparing chat...", }} @@ -340,6 +384,7 @@ func (m *Model) handlePRDetails(msg prDetailsFetchedMsg) (Model, tea.Cmd) { if isPostMerge { // For merged PRs, defer scoring until we know if user already reviewed. // Only parse diff + fetch status for now. + m.parsing++ cmds := []tea.Cmd{parseDiffCmd(ctx.Repo, pr), fetchMergedPRStatusCmd(ctx.Repo, pr.Number, m.app.CurrentUser)} cmds = append(cmds, m.fetchBodyImages(pr, ctx.Repo)...) if m.fetching == 0 { @@ -351,6 +396,7 @@ func (m *Model) handlePRDetails(msg prDetailsFetchedMsg) (Model, tea.Cmd) { // Open PRs: score + create worktree immediately. card.Scoring = true m.scoring++ + m.parsing++ cmds := []tea.Cmd{scorePRCmd(pr, ctx, m.program), parseDiffCmd(ctx.Repo, pr)} cmds = append(cmds, createWorktreeCmd(ctx.RepoDir, pr.HeadSHA, ctx.Repo, pr.Number)) cmds = append(cmds, m.fetchBodyImages(pr, ctx.Repo)...) @@ -365,13 +411,19 @@ func (m *Model) handlePRDetails(msg prDetailsFetchedMsg) (Model, tea.Cmd) { } func (m *Model) handleDiffParsed(msg prDiffParsedMsg) (Model, tea.Cmd) { + m.parsing-- if card := m.findCard(msg.repo, msg.prNumber); card != nil { card.parsedFiles = msg.files applyHunkAnnotations(card) + // Eagerly compute HasNewContent so visibility checks work at startup. + m.computeHasNewContent(card) } if card := m.currentCard(); card != nil && card.Ctx.Repo == msg.repo && card.PR.Number == msg.prNumber { m.loadCurrentDiff() } + if !m.startupDone && m.parsing == 0 { + m.tryStartupTransition() + } return *m, nil } @@ -425,7 +477,7 @@ func (m *Model) handlePRScored(msg prScoredMsg) (Model, tea.Cmd) { // Transition from startup screen once the card list is stable (all details // fetched) and at least one visible PR is scored. Waiting for fetching==0 // ensures no more card inserts will shift m.current and cause flashing. - if !m.startupDone && scoredCard != nil && m.fetching == 0 { + if !m.startupDone && scoredCard != nil && m.fetching == 0 && m.parsing == 0 { // Count truly visible cards: post-merge cards must have their status checked. settled := true visibleSettled := 0 diff --git a/internal/tui/messages.go b/internal/tui/messages.go index 9cf7717..2b2363f 100644 --- a/internal/tui/messages.go +++ b/internal/tui/messages.go @@ -144,6 +144,12 @@ type mergedPRListFetchedMsg struct { err error } +type trackedPRListFetchedMsg struct { + ctx *app.RepoContext + rawPRs []map[string]any + err error +} + type mergedPRStatusMsg struct { repo string prNumber int diff --git a/internal/tui/model_helpers.go b/internal/tui/model_helpers.go index 8bc6f1d..f9eb3a4 100644 --- a/internal/tui/model_helpers.go +++ b/internal/tui/model_helpers.go @@ -43,7 +43,7 @@ func (m Model) currentCard() *PRCard { // tryStartupTransition checks if we can exit the startup screen. // Called when a merged PR status arrives and no scoring was triggered. func (m *Model) tryStartupTransition() { - if m.startupDone || m.fetching > 0 { + if m.startupDone || m.fetching > 0 || m.parsing > 0 { return } // Check if any visible card exists (scored or not — unscored merged PRs are still viewable). @@ -79,7 +79,7 @@ func (m *Model) tryStartupTransition() { // checkNoPRs sets noPRs=true only when all repo lists have returned and there are no cards or fetches pending. func (m *Model) checkNoPRs() { - if m.openListsDone < len(m.app.Repos) || m.mergedListsDone < len(m.app.Repos) { + if m.openListsDone < len(m.app.Repos) || m.mergedListsDone < len(m.app.Repos) || m.trackedListsDone < len(m.app.Repos) { return } if len(m.cards) == 0 && m.fetching == 0 { @@ -146,6 +146,18 @@ func (m *Model) addLocalReview(repo string, prNumber int, state string) { } } +// visiblePosition returns the 1-based index of the current card among visible cards, +// and the total number of visible cards. +func (m Model) visiblePosition() (int, int) { + visIdx := 0 + for i := 0; i < m.current && i < len(m.cards); i++ { + if m.isCardVisible(m.cards[i]) { + visIdx++ + } + } + return visIdx + 1, m.visibleCardCount() +} + // visibleCardCount returns the number of currently visible cards. func (m Model) visibleCardCount() int { n := 0 @@ -175,6 +187,13 @@ func (m *Model) navigatePR(delta int, s *ConversationScene) { next += delta } if next < 0 || next >= len(m.cards) { + if delta > 0 { + s.actionStatus = "No more PRs" + } else { + s.actionStatus = "Already at first PR" + } + s.actionDone = true + s.BuildScrollback(m) return } m.current = next @@ -203,6 +222,26 @@ func (m *Model) loadCurrentDiff() { } } +// computeHasNewContent sets HasNewContent on a card by comparing current state +// against the review state store. Does not update the DiffView. +func (m *Model) computeHasNewContent(card *PRCard) { + if m.reviewStore == nil || card.parsedFiles == nil { + card.HasNewContent = false + return + } + key := reviewstate.Key(card.Ctx.Repo, card.PR.Number) + state := m.reviewStore.Get(key) + if state == nil { + card.HasNewContent = false + return + } + card.ReviewState = state + fileNames, fileHunks := diff.FileHunkInfo(card.parsedFiles) + commentDigests := diff.CommentDigestsFromPR(card.PR, m.app.CurrentUser) + inc := reviewstate.ComputeIncremental(fileNames, fileHunks, commentDigests, state) + card.HasNewContent = inc.HasChanges || inc.HasNewComments +} + // updateIncrementalState computes incremental review state and stores it on the // DiffView (quietly, without triggering a rebuild). The stored state is applied // automatically by rebuildViewport() via applyIncrementalFlags(). @@ -220,7 +259,7 @@ func (m *Model) updateIncrementalState(card *PRCard) { return } fileNames, fileHunks := diff.FileHunkInfo(card.parsedFiles) - commentDigests := diff.CommentDigestsFromPR(card.PR) + commentDigests := diff.CommentDigestsFromPR(card.PR, m.app.CurrentUser) state := reviewstate.ComputeIncremental(fileNames, fileHunks, commentDigests, card.ReviewState) card.HasNewContent = state.HasChanges || state.HasNewComments logger.Info("incremental state for PR #%d: %d new hunks, %d new comments, %d edited, mode=%v", @@ -236,7 +275,7 @@ func (m *Model) snapshotCurrentPR() { return } hunkDigests := diff.DigestsFromFiles(card.parsedFiles) - commentDigests := diff.CommentDigestsFromPR(card.PR) + commentDigests := diff.CommentDigestsFromPR(card.PR, m.app.CurrentUser) key := reviewstate.Key(card.Ctx.Repo, card.PR.Number) // Don't re-snapshot if nothing changed — prevents bouncing between @@ -411,7 +450,7 @@ func (m *Model) hardReset() tea.Cmd { m.startupLog = log cmds = append(cmds, m.spinner.Tick) for _, r := range m.app.Repos { - cmds = append(cmds, fetchPRListCmd(r), fetchMergedPRListCmd(r)) + cmds = append(cmds, fetchPRListCmd(r), fetchMergedPRListCmd(r), fetchTrackedPRListCmd(r, m.reviewStore, nil)) } return tea.Batch(cmds...) } diff --git a/internal/tui/slashcmds.go b/internal/tui/slashcmds.go index 38a2f67..f6342ce 100644 --- a/internal/tui/slashcmds.go +++ b/internal/tui/slashcmds.go @@ -245,7 +245,7 @@ func commands() []Command { s.actionDone = false cmds := []tea.Cmd{refreshPRCmd(card.PR, card.Ctx)} for _, r := range m.app.Repos { - cmds = append(cmds, fetchPRListCmd(r), fetchMergedPRListCmd(r)) + cmds = append(cmds, fetchPRListCmd(r), fetchMergedPRListCmd(r), fetchTrackedPRListCmd(r, m.reviewStore, nil)) } return s, tea.Batch(cmds...), true }, diff --git a/internal/tui/tui.go b/internal/tui/tui.go index 06a043f..8094f08 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -30,6 +30,7 @@ type Model struct { cards []*PRCard total int fetching int // PRs whose details are still being fetched + parsing int // PRs whose diffs are still being parsed scoring int // PRs whose assessments are still in progress current int spinner spinner.Model @@ -62,8 +63,9 @@ type Model struct { // Post-merge review showAllMerged bool // when true, show all merged PRs including already-reviewed/reacted - openListsDone int // count of repos whose open PR list fetch has returned - mergedListsDone int // count of repos whose merged PR list fetch has returned + openListsDone int // count of repos whose open PR list fetch has returned + mergedListsDone int // count of repos whose merged PR list fetch has returned + trackedListsDone int // count of repos whose tracked PR list fetch has returned // Incremental review reviewStore *reviewstate.Store @@ -103,7 +105,7 @@ func New(a *app.App) Model { func (m Model) Init() tea.Cmd { cmds := []tea.Cmd{m.spinner.Tick, m.convScene.FocusInput()} for _, r := range m.app.Repos { - cmds = append(cmds, fetchPRListCmd(r), fetchMergedPRListCmd(r)) + cmds = append(cmds, fetchPRListCmd(r), fetchMergedPRListCmd(r), fetchTrackedPRListCmd(r, m.reviewStore, nil)) } return tea.Batch(cmds...) }