From 7511311297c154df67fe674c70201253408bdb2a Mon Sep 17 00:00:00 2001 From: Pablu Date: Tue, 28 Jul 2026 15:40:11 +0200 Subject: [PATCH] Fix high prio recommendations, add error / health screen --- README.md | 29 ++- TODO.md | 152 ++++++++++--- cache.go | 187 ++++++++++++++-- cache_test.go | 57 +++++ config.go | 14 +- drafts.go | 208 ++++++++++++++++++ github.go | 229 ++++++++++++++++---- github_test.go | 48 ++++- health.go | 157 ++++++++++++++ health_test.go | 201 +++++++++++++++++ keybindings.go | 12 +- main.go | 6 +- persistence.go | 53 +++++ pr_editor.go | 5 +- text_editor.go | 82 +++++-- text_editor_test.go | 41 ++++ tui.go | 511 ++++++++++++++++++++++++++++++++++++++++++-- tui_test.go | 23 ++ types.go | 18 ++ 19 files changed, 1885 insertions(+), 148 deletions(-) create mode 100644 drafts.go create mode 100644 health.go create mode 100644 health_test.go create mode 100644 persistence.go diff --git a/README.md b/README.md index 855c5bd..7b067c7 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,7 @@ within_status = "file" # "file" or "timestamp" (oldest first) enabled = true # instant stale view plus offline fallback max_age = "168h" # 7 days; 0 means no age limit directory = "" # defaults to the OS user cache directory +max_entries = 200 # bounded oldest-first pruning; 10-10000 [editing] mode = "vim" # "vim" or "standard"; description field only for now @@ -123,6 +124,7 @@ page_up = ["ctrl+u", "pgup"] [keybindings.views] open = ["enter", "l"] dashboard = ["d"] +health = ["H"] edit = ["e"] toggle_list = ["tab"] @@ -196,14 +198,25 @@ can be active together reports the context and both conflicting actions. When cached data exists, the picker and PR details are rendered immediately from that snapshot while a live GitHub refresh runs in the background. Cached screens are labelled with their save time and are replaced automatically when -fresh data arrives. Check annotations are fetched separately only for failed -checks so they do not inflate the initial PR query. +fresh data arrives. Core PR and review data is rendered before check +annotations and conflict-file analysis finish. A failed subsection keeps its +last complete value, is marked partial, and does not discard the rest of a +successful refresh. Check annotations are fetched separately only for failed +checks and are reused by immutable check ID. The cache uses separate JSON files for the picker and each visited PR. Cache content is hashed before writing: unchanged responses do not rewrite their files. Their modification time is touched at most once per day (or half the configured maximum age, when shorter) so recently validated snapshots remain -usable without writing on every poll. Changed files are replaced atomically. +usable without writing on every poll. Changed files are replaced atomically, +and oldest cache entries are pruned at the configured bound. Read state and +recoverable reply/metadata drafts use versioned, atomic files beside the +configuration. + +Polling adapts to GitHub's reported rate-limit budget. It backs off as the +remaining budget gets low, honors server retry windows, and adds jitter to +avoid synchronized clients. Opening another PR or starting another refresh +cancels the superseded request. GitHub's public APIs report whether a PR conflicts but do not expose its conflicting file paths. For conflicting PRs only, `gh-threads` performs a @@ -240,6 +253,7 @@ history and metadata. | `h` / `l` | Focus the thread list / thread detail | | `j` / `k` | Move between items or scroll the dashboard/focused detail | | `?` | Show contextual keybinding help | +| `H` | Open application health and diagnostics | | `d` | Open the current pull request dashboard | | `e` | Edit the current PR title, target branch, and description from its dashboard | | `/` | Fuzzy-search paths and filter with `status:`, `author:`, `updated:true` | @@ -258,6 +272,15 @@ history and metadata. | `r` | Refresh now | | `q` | Quit | +The Health modal reports the interactive loop, configuration, GitHub API, +rate-limit budget and reset/retry time, disk cache, unread-state persistence, +draft recovery, core PR data, and secondary enrichment. Session warnings and +errors are retained there with their component and timestamp. Long diagnostics +wrap to the modal width. Refresh activity occupies a stable informational row +so polling does not reorder the report. Press `H` from the picker, dashboard, +or thread view; `b` or `esc` closes it without changing the underlying scroll +position. + The reply composer appears inline beneath the selected thread so its code and comments remain visible while writing. It supports multiple lines: `enter` inserts a newline, `ctrl-s` opens the rendered confirmation preview, and `esc` diff --git a/TODO.md b/TODO.md index 454ce51..06d6c9b 100644 --- a/TODO.md +++ b/TODO.md @@ -1,42 +1,126 @@ # TODO -The read-only model now includes paginated review threads and comments, PR -conversation comments, submitted reviews, individual checks, branch-protection -requirements, rulesets, merge queues, deployment requirements, viewer -capabilities, head SHA, persistent unread updates, offline cache fallback, -commit/force-push history, and paginated checks and annotations. -The following read-only improvements remain before or alongside write support. +This list reflects the current implementation: paginated review threads, +thread comments, conversation comments, reviews, timeline events, checks, and +annotations; cached read-only snapshots; persistent unread state; contextual +keybindings; thread replies and resolution changes; and pull-request metadata +editing are already implemented. -## Workflow and navigation +## Completed resilience work -- Open the current PR, comment, review, check, or source location in a browser. -- Copy URLs, commit SHAs, file paths, and rendered comment text to the - clipboard. -- Add navigation for the next thread by status or author, not only the next - unread update. -- Remember the selected PR, thread, scroll position, pane width, and folded - state between runs. -- Audit screen-reader behavior beyond the new no-color and high-contrast modes. +- Refreshes render core data before annotations and conflict analysis. A + failed paginated subsection is marked partial and keeps the last complete + value while other sections continue updating. +- Polling uses GitHub rate-limit and retry headers, backs off at low remaining + budgets, and adds jitter. Failed-check annotations are cached by immutable + check ID. +- Superseded list, detail, and enrichment requests are canceled as newer + navigation or refresh work starts. +- Reply and PR-metadata drafts are stored in a versioned, atomic, permission + restricted file and restored after cancellation or restart. +- Unread state and disk cache are versioned and atomically replaced. Corrupt + persistence is reported by the health screen rather than silently trusted. + Cache writes are content-addressed, bounded, oldest-first pruned, and cached + branch recommendations remain available offline. +- Editor character motions, deletion, selection, wrapping, and clipboard + ranges operate on grapheme boundaries, with regression coverage for + combining marks, full-width characters, variation selectors, and joined + emoji. +- The health screen reports component status, rate-limit state, persistence + paths, partial data, and the session's wrapped warning/error history. -## Data completeness and resilience +## High value workflow additions -- Model pending reviews, minimized comments, deleted comments, and explicit - reply relationships. -- Show rate-limit remaining/reset information and distinguish it from network - or permission failures. -- Support retrying individual failed pages while retaining clearly marked - partial data, rather than failing the entire refresh. -- Add fixture coverage for GitHub Enterprise Server schema differences, team - reviewers, deleted users, very large PRs, and mixed legacy status contexts. +- Open the current PR, thread comment, submitted review, check, annotation, + commit, or source location in a browser. +- Copy URLs, commit SHAs, file paths, branch names, rendered comment text, and + raw Markdown through explicit contextual actions. +- Add a dedicated changed-files/check-details view. It should make the complete + PR diff and check annotations inspectable even when no review thread exists + at that location. +- Add navigation to the next thread by status, file, author, or failed check, + not only the next unread update. +- Persist the selected PR/thread, scroll anchors, focused pane, hidden-list + state, folded threads, active filter, and pane width between runs. +- Make the picker scope configurable: assigned PRs, viewer-authored PRs, + review-requested PRs, subscribed PRs, or a union of those scopes. Clearly + label why each PR appears. +- Add saved/named thread filters and search history for repeated review + workflows. +- Distinguish local unread state from GitHub notification state, and optionally + integrate with GitHub notifications without silently marking remote + notifications as read. +- Make health events individually selectable and copyable, and retain + per-subsection last-success timestamps across refreshes. -## Preparation for write support +## Data completeness and compatibility -- Add dashboard selectors for requested reviewers, labels, and assignees. Each - selector must support adding, removing, and clearing values with explicit - confirmation. -- Fetch per-comment update/delete permissions. -- Define head-SHA conflict handling for comments and suggestions composed - against an older revision. -- Design confirmation and optimistic-update behavior for submitting reviews - and applying suggestions. Thread replies and resolution changes now use - confirmed server responses. +- Paginate or explicitly mark truncation for the remaining fixed-size + connections: assignees, labels, review requests, latest reviews, repository + rulesets, and rules within a ruleset. +- Model pending reviews, minimized comments, deleted comments/users, edited + timestamps, and explicit reply relationships. +- Preserve enough team-reviewer identity to distinguish teams with the same + display name and to generate a correct browser target. +- Represent partial permissions per comment and conversation item, not only + aggregate PR/thread capabilities. +- Verify ruleset, branch-protection, merge-queue, deployment, and check behavior + against supported GitHub Enterprise Server versions. Degrade individual + fields when a schema feature is unavailable instead of rejecting the whole + PR query. +- Improve uploaded-image and attachment handling with optional terminal image + protocols or an open/download action while keeping a textual fallback. + +## Write roadmap + +- Add fuzzy multi-select editors for requested reviewers, assignees, labels, + and milestone. Support adding, removing, and clearing values with an explicit + before/after confirmation. +- Add top-level PR conversation replies and editing/deleting the viewer's own + comments. Fetch and enforce per-comment update/delete permissions. +- Add reaction add/remove actions while retaining the current read-only counts. +- Support submitting pending reviews and review summaries, including approve, + comment, and request-changes states. +- Support applying GitHub suggestions only after validating the original + commit/head SHA and showing the exact resulting patch. Define behavior for + multiple suggestions, conflicts, dirty Git/Jujutsu workspaces, and remote + application. +- Add draft/ready-for-review and close/reopen actions. +- Consider merge, auto-merge, and merge-queue actions only after required + checks, review decision, permissions, stale-head protection, and destructive + confirmations are represented accurately. +- Define consistent optimistic-update and rollback behavior for every mutation. + Preserve drafts and server responses when a post-mutation refresh fails. + +## UX and configurability + +- Add diff-view settings for context size, tab width, whitespace visibility, + line-number style, syntax theme, and whether outdated/resolved context starts + collapsed. +- Add an optional command palette so configured actions remain discoverable + even when their key is forgotten or unbound. +- Audit screen-reader behavior beyond no-color/high-contrast themes, including + focus announcements, status symbols, popup ordering, and live refreshes. +- Add optional mouse selection/scrolling without changing keyboard-first + defaults. +- Make relative/absolute timestamp display and timezone configurable. + +## Testing and maintainability + +- Finish consolidating key dispatch, help, compact footers, and contextual + conflict validation into one action registry. Context validation is already + enforced, but declaration order and help descriptions remain separate. +- Coalesce unread-state persistence through the same delayed flush mechanism + used for drafts if future read-state actions make writes frequent. +- Add recorded GraphQL fixtures for GitHub.com and supported GitHub Enterprise + Server versions, including partial errors, rate limits, deleted actors, team + reviewers, mixed legacy statuses, and very large PRs. +- Add golden terminal snapshots across narrow/wide sizes, all themes, + configurable keys, Unicode-heavy content, editor modes, partial-data states, + and cached/live transitions. +- Add end-to-end mutation tests covering permission changes, stale head SHAs, + offline transitions, server success followed by refresh failure, and draft + recovery. +- Split the large GitHub-fetch and TUI update/render modules by data source and + screen once doing so removes duplicated state transitions; keep shared + behavior in small typed helpers rather than introducing a framework. diff --git a/cache.go b/cache.go index e3a760f..56f2f4b 100644 --- a/cache.go +++ b/cache.go @@ -9,19 +9,23 @@ import ( "fmt" "os" "path/filepath" + "sort" "time" ) type cacheEnvelope[T any] struct { + Version int `json:"version,omitempty"` SavedAt time.Time `json:"saved_at"` ContentHash string `json:"content_hash,omitempty"` Value T `json:"value"` } type CachedGitHubService struct { - remote GitHubService - dir string - maxAge time.Duration + remote GitHubService + dir string + maxAge time.Duration + maxEntries int + health healthTracker } type cachedSnapshotService interface { @@ -34,8 +38,18 @@ type liveGitHubService interface { LivePullRequest(context.Context, string, string, int) (PRDetails, error) } -func NewCachedGitHubService(remote GitHubService, dir string, maxAge time.Duration) *CachedGitHubService { - return &CachedGitHubService{remote: remote, dir: dir, maxAge: maxAge} +const cacheSchemaVersion = 1 + +func NewCachedGitHubService( + remote GitHubService, dir string, maxAge time.Duration, configuredMaxEntries ...int, +) *CachedGitHubService { + maxEntries := 200 + if len(configuredMaxEntries) > 0 && configuredMaxEntries[0] > 0 { + maxEntries = configuredMaxEntries[0] + } + return &CachedGitHubService{ + remote: remote, dir: dir, maxAge: maxAge, maxEntries: maxEntries, + } } func (c *CachedGitHubService) CachedPullRequests( @@ -157,7 +171,37 @@ func (c *CachedGitHubService) ListBranches( if !ok { return nil, errors.New("GitHub service does not support listing branches") } - return service.ListBranches(ctx, owner, repo) + branches, err := service.ListBranches(ctx, owner, repo) + if err == nil { + _ = c.write(c.branchesKey(owner, repo), branches) + return branches, nil + } + var cached cacheEnvelope[[]RepositoryBranch] + if _, cacheErr := c.read(c.branchesKey(owner, repo), &cached); cacheErr == nil { + c.health.set(HealthComponent{ + Name: "branch cache", Level: healthWarning, + Summary: "using cached branches", Detail: err.Error(), UpdatedAt: time.Now(), + }) + return cached.Value, nil + } + return nil, err +} + +func (c *CachedGitHubService) EnrichPullRequest( + ctx context.Context, details PRDetails, +) PRDetailsEnrichment { + service, ok := c.remote.(GitHubEnrichmentService) + if !ok { + return PRDetailsEnrichment{ + Owner: details.Owner, Repository: details.Repository, Number: details.Number, + HeadOID: details.HeadOID, + Issues: []DataIssue{{ + Component: "PR enrichment", + Message: "GitHub service does not support secondary PR data", + }}, + } + } + return service.EnrichPullRequest(ctx, details) } func (c *CachedGitHubService) pullRequestsKey(owner, repo string, limit int, showAll bool) string { @@ -168,12 +212,27 @@ func (c *CachedGitHubService) pullRequestKey(owner, repo string, number int) str return fmt.Sprintf("pr:%s/%s:%d", owner, repo, number) } +func (c *CachedGitHubService) branchesKey(owner, repo string) string { + return fmt.Sprintf("branches:%s/%s", owner, repo) +} + func (c *CachedGitHubService) file(key string) string { sum := sha256.Sum256([]byte(key)) return filepath.Join(c.dir, hex.EncodeToString(sum[:])+".json") } -func (c *CachedGitHubService) write(key string, value any) error { +func (c *CachedGitHubService) write(key string, value any) (resultErr error) { + defer func() { + component := HealthComponent{ + Name: "disk cache", Level: healthOK, Summary: "cache write succeeded", + Detail: c.dir, UpdatedAt: time.Now(), + } + if resultErr != nil { + component.Level = healthWarning + component.Summary = resultErr.Error() + } + c.health.set(component) + }() if err := os.MkdirAll(c.dir, 0o700); err != nil { return err } @@ -198,7 +257,8 @@ func (c *CachedGitHubService) write(key string, value any) error { } } data, err := json.Marshal(cacheEnvelope[any]{ - SavedAt: time.Now(), ContentHash: contentHash, Value: value, + Version: cacheSchemaVersion, SavedAt: time.Now(), + ContentHash: contentHash, Value: value, }) if err != nil { return err @@ -220,7 +280,45 @@ func (c *CachedGitHubService) write(key string, value any) error { if err := temp.Close(); err != nil { return err } - return os.Rename(name, target) + if err := os.Rename(name, target); err != nil { + return err + } + return c.prune() +} + +func (c *CachedGitHubService) prune() error { + if c.maxEntries <= 0 { + return nil + } + entries, err := os.ReadDir(c.dir) + if err != nil { + return err + } + type cacheFile struct { + path string + modTime time.Time + } + var files []cacheFile + for _, entry := range entries { + if entry.IsDir() || filepath.Ext(entry.Name()) != ".json" { + continue + } + info, err := entry.Info() + if err != nil { + continue + } + files = append(files, cacheFile{ + path: filepath.Join(c.dir, entry.Name()), modTime: info.ModTime(), + }) + } + sort.Slice(files, func(i, j int) bool { return files[i].modTime.Before(files[j].modTime) }) + for len(files) > c.maxEntries { + if err := os.Remove(files[0].path); err != nil && !errors.Is(err, os.ErrNotExist) { + return err + } + files = files[1:] + } + return nil } func (c *CachedGitHubService) cacheTouchInterval() time.Duration { @@ -231,7 +329,20 @@ func (c *CachedGitHubService) cacheTouchInterval() time.Duration { return interval } -func (c *CachedGitHubService) read(key string, target any) (time.Time, error) { +func (c *CachedGitHubService) read(key string, target any) ( + saved time.Time, resultErr error, +) { + defer func() { + component := HealthComponent{ + Name: "disk cache", Level: healthOK, Summary: "cache read succeeded", + Detail: c.dir, UpdatedAt: time.Now(), + } + if resultErr != nil { + component.Level = healthWarning + component.Summary = resultErr.Error() + } + c.health.set(component) + }() path := c.file(key) data, err := os.ReadFile(path) if err != nil { @@ -241,11 +352,17 @@ func (c *CachedGitHubService) read(key string, target any) (time.Time, error) { return time.Time{}, err } var metadata struct { + Version int `json:"version"` SavedAt time.Time `json:"saved_at"` } if err := json.Unmarshal(data, &metadata); err != nil { return time.Time{}, err } + if metadata.Version != 0 && metadata.Version != cacheSchemaVersion { + return time.Time{}, fmt.Errorf( + "unsupported cache schema version %d", metadata.Version, + ) + } savedAt := metadata.SavedAt if info, statErr := os.Stat(path); statErr == nil && info.ModTime().After(savedAt) { savedAt = info.ModTime() @@ -256,9 +373,32 @@ func (c *CachedGitHubService) read(key string, target any) (time.Time, error) { return savedAt, nil } +func (c *CachedGitHubService) HealthReport() []HealthComponent { + components := c.health.report() + if provider, ok := c.remote.(healthProvider); ok { + components = append(components, provider.HealthReport()...) + } + return components +} + +func (c *CachedGitHubService) RateLimit() RateLimitSnapshot { + if provider, ok := c.remote.(healthProvider); ok { + return provider.RateLimit() + } + return RateLimitSnapshot{} +} + type readStateStore struct { - path string - Data map[string]readPRState `json:"pull_requests"` + path string + Data map[string]readPRState `json:"pull_requests"` + loadErr error +} + +const readStateSchemaVersion = 1 + +type readStateEnvelope struct { + Version int `json:"version"` + PullRequests map[string]readPRState `json:"pull_requests"` } type readPRState struct { @@ -271,7 +411,20 @@ func loadReadState(path string) *readStateStore { store := &readStateStore{path: path, Data: make(map[string]readPRState)} data, err := os.ReadFile(path) if err == nil { - _ = json.Unmarshal(data, &store.Data) + var envelope readStateEnvelope + if json.Unmarshal(data, &envelope) == nil && + envelope.Version == readStateSchemaVersion && + envelope.PullRequests != nil { + store.Data = envelope.PullRequests + } else { + // Backward-compatible migration from the original unversioned map. + if migrationErr := json.Unmarshal(data, &store.Data); migrationErr != nil { + store.Data = make(map[string]readPRState) + store.loadErr = fmt.Errorf("read state is corrupt: %w", migrationErr) + } + } + } else if !errors.Is(err, os.ErrNotExist) { + store.loadErr = err } return store } @@ -283,9 +436,7 @@ func (s *readStateStore) save() error { if err := os.MkdirAll(filepath.Dir(s.path), 0o700); err != nil { return err } - data, err := json.MarshalIndent(s.Data, "", " ") - if err != nil { - return err - } - return os.WriteFile(s.path, data, 0o600) + return atomicWriteJSON(s.path, readStateEnvelope{ + Version: readStateSchemaVersion, PullRequests: s.Data, + }, 0o600) } diff --git a/cache_test.go b/cache_test.go index 37f2431..8604b83 100644 --- a/cache_test.go +++ b/cache_test.go @@ -3,9 +3,11 @@ package main import ( "bytes" "context" + "encoding/json" "errors" "os" "path/filepath" + "strings" "testing" "time" @@ -105,6 +107,17 @@ func TestReadStateSurvivesRestartWithUnreadComment(t *testing.T) { } } +func TestCorruptReadStateIsReportedAndRecoveredEmpty(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.json") + if err := os.WriteFile(path, []byte("{broken"), 0o600); err != nil { + t.Fatal(err) + } + store := loadReadState(path) + if store.loadErr == nil || len(store.Data) != 0 { + t.Fatalf("corrupt state recovery = error %v data %#v", store.loadErr, store.Data) + } +} + func TestCachedPickerSnapshotStaysVisibleWhileLiveRefreshContinues(t *testing.T) { m := NewApp(nil, "", "", false, 50, time.Second) m.loading = true @@ -189,3 +202,47 @@ func TestCacheDoesNotRewriteUnchangedContent(t *testing.T) { t.Fatal("changed cache content was not persisted") } } + +func TestCachePrunesOldestEntriesAtConfiguredBound(t *testing.T) { + remote := &switchService{} + service := NewCachedGitHubService(remote, t.TempDir(), time.Hour, 2) + for number := 1; number <= 3; number++ { + remote.details = PRDetails{PullRequest: PullRequest{ + ID: "pr-" + fmtInt(number), Owner: "o", Repository: "r", Number: number, + }} + if _, err := service.LivePullRequest(context.Background(), "o", "r", number); err != nil { + t.Fatal(err) + } + time.Sleep(time.Millisecond) + } + entries, err := os.ReadDir(service.dir) + if err != nil { + t.Fatal(err) + } + if len(entries) != 2 { + t.Fatalf("cache entries = %d, want 2", len(entries)) + } + if _, err := service.CachedPullRequest("o", "r", 1); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("oldest cache entry error = %v, want not exist", err) + } +} + +func TestCacheRejectsUnknownSchemaVersion(t *testing.T) { + service := NewCachedGitHubService(&switchService{}, t.TempDir(), time.Hour) + path := service.file(service.pullRequestKey("o", "r", 1)) + envelope := map[string]any{ + "version": 999, "saved_at": time.Now(), + "value": PRDetails{PullRequest: PullRequest{ID: "pr"}}, + } + data, err := json.Marshal(envelope) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, data, 0o600); err != nil { + t.Fatal(err) + } + if _, err := service.CachedPullRequest("o", "r", 1); err == nil || + !strings.Contains(err.Error(), "unsupported cache schema") { + t.Fatalf("schema error = %v", err) + } +} diff --git a/config.go b/config.go index aabc9cb..cb8bc24 100644 --- a/config.go +++ b/config.go @@ -57,9 +57,10 @@ type ThreadConfig struct { } type CacheConfig struct { - Enabled bool `toml:"enabled"` - MaxAge configDuration `toml:"max_age"` - Directory string `toml:"directory"` + Enabled bool `toml:"enabled"` + MaxAge configDuration `toml:"max_age"` + Directory string `toml:"directory"` + MaxEntries int `toml:"max_entries"` } type EditingConfig struct { @@ -86,7 +87,9 @@ func defaultConfig() Config { StatusOrder: []string{"unresolved", "outdated", "resolved"}, WithinStatus: "file", }, - Cache: CacheConfig{Enabled: true, MaxAge: configDuration{7 * 24 * time.Hour}}, + Cache: CacheConfig{ + Enabled: true, MaxAge: configDuration{7 * 24 * time.Hour}, MaxEntries: 200, + }, Editing: EditingConfig{Mode: "vim"}, KeyBindings: defaultKeyBindings(), } @@ -178,6 +181,9 @@ func validateConfig(config Config) error { if config.Cache.MaxAge.Duration < 0 { return fmt.Errorf("cache.max_age must not be negative") } + if config.Cache.MaxEntries < 10 || config.Cache.MaxEntries > 10000 { + return fmt.Errorf("cache.max_entries must be between 10 and 10000") + } switch config.Editing.Mode { case "standard", "vim": default: diff --git a/drafts.go b/drafts.go new file mode 100644 index 0000000..49aa634 --- /dev/null +++ b/drafts.go @@ -0,0 +1,208 @@ +package main + +import ( + "encoding/json" + "errors" + "os" + "strconv" + "sync" + "time" + + tea "github.com/charmbracelet/bubbletea" +) + +const draftSchemaVersion = 1 + +type savedDraft struct { + Kind string `json:"kind"` + Owner string `json:"owner"` + Repository string `json:"repository"` + Number int `json:"number"` + ThreadID string `json:"thread_id,omitempty"` + Reply string `json:"reply,omitempty"` + Title string `json:"title,omitempty"` + BaseRef string `json:"base_ref,omitempty"` + Body string `json:"body,omitempty"` + OriginalUpdatedAt time.Time `json:"original_updated_at,omitempty"` + SavedAt time.Time `json:"saved_at"` +} + +type draftEnvelope struct { + Version int `json:"version"` + Drafts map[string]savedDraft `json:"drafts"` +} + +type draftStore struct { + mu sync.Mutex + path string + data map[string]savedDraft + dirty bool + loadErr error +} + +func loadDraftStore(path string) *draftStore { + store := &draftStore{path: path, data: make(map[string]savedDraft)} + data, err := os.ReadFile(path) + if err != nil { + if !os.IsNotExist(err) { + store.loadErr = err + } + return store + } + var envelope draftEnvelope + if json.Unmarshal(data, &envelope) == nil && envelope.Version == draftSchemaVersion { + store.data = envelope.Drafts + if store.data == nil { + store.data = make(map[string]savedDraft) + } + } else { + store.loadErr = errors.New("draft file is corrupt or has an unsupported schema version") + } + return store +} + +func (s *draftStore) get(key string) (savedDraft, bool) { + if s == nil { + return savedDraft{}, false + } + s.mu.Lock() + defer s.mu.Unlock() + draft, ok := s.data[key] + return draft, ok +} + +func (s *draftStore) put(key string, draft savedDraft) { + if s == nil || key == "" { + return + } + s.mu.Lock() + defer s.mu.Unlock() + if existing, ok := s.data[key]; ok { + existing.SavedAt = time.Time{} + candidate := draft + candidate.SavedAt = time.Time{} + if existing == candidate { + return + } + } + draft.SavedAt = time.Now() + s.data[key] = draft + s.dirty = true +} + +func (s *draftStore) delete(key string) error { + if s == nil || key == "" { + return nil + } + s.mu.Lock() + delete(s.data, key) + s.dirty = true + s.mu.Unlock() + return s.flush() +} + +func (s *draftStore) flush() error { + if s == nil || s.path == "" { + return nil + } + s.mu.Lock() + defer s.mu.Unlock() + if !s.dirty { + return nil + } + envelope := draftEnvelope{Version: draftSchemaVersion, Drafts: s.data} + if err := atomicWriteJSON(s.path, envelope, 0o600); err != nil { + return err + } + s.dirty = false + return nil +} + +type draftFlushMsg struct{ err error } + +func flushDraftsAfter(store *draftStore) tea.Cmd { + if store == nil { + return nil + } + return tea.Tick(400*time.Millisecond, func(time.Time) tea.Msg { + return draftFlushMsg{err: store.flush()} + }) +} + +func replyDraftKey(owner, repo string, number int, threadID string) string { + return "reply:" + owner + "/" + repo + ":" + + fmtInt(number) + ":" + threadID +} + +func prMetadataDraftKey(owner, repo string, number int) string { + return "pr:" + owner + "/" + repo + ":" + fmtInt(number) +} + +func fmtInt(value int) string { + return strconv.Itoa(value) +} + +func (m *App) restoreReplyDraft(threadID string) { + key := replyDraftKey(m.details.Owner, m.details.Repository, m.details.Number, threadID) + if draft, ok := m.drafts.get(key); ok && draft.Kind == "reply" && draft.Reply != "" { + m.replyDraft = draft.Reply + m.recordHealth( + "draft recovery", healthWarning, + "restored reply draft saved "+draft.SavedAt.Local().Format("2006-01-02 15:04:05"), + ) + } +} + +func (m *App) queueReplyDraft() tea.Cmd { + if m.drafts == nil || m.writeThreadID == "" { + return nil + } + key := replyDraftKey( + m.details.Owner, m.details.Repository, m.details.Number, m.writeThreadID, + ) + m.drafts.put(key, savedDraft{ + Kind: "reply", Owner: m.details.Owner, Repository: m.details.Repository, + Number: m.details.Number, ThreadID: m.writeThreadID, Reply: m.replyDraft, + }) + return flushDraftsAfter(m.drafts) +} + +func (m *App) restorePREditDraft() { + key := prMetadataDraftKey(m.details.Owner, m.details.Repository, m.details.Number) + draft, ok := m.drafts.get(key) + if !ok || draft.Kind != "pr-metadata" { + return + } + if !draft.OriginalUpdatedAt.Equal(m.details.UpdatedAt) { + m.recordHealth( + "draft recovery", healthWarning, + "saved PR metadata draft was not restored because the pull request changed", + ) + return + } + m.prEditEditors[prEditTitleField] = newTextEditor(draft.Title, false) + m.prEditEditors[prEditBaseField] = newTextEditor(draft.BaseRef, false) + m.prEditEditors[prEditBodyField] = newTextEditor( + normalizeLineEndings(draft.Body), m.editorMode == "vim", + ) + m.prEditEditors[prEditBodyField].highlightMarkdown = true + m.recordHealth( + "draft recovery", healthWarning, + "restored PR metadata draft saved "+draft.SavedAt.Local().Format("2006-01-02 15:04:05"), + ) +} + +func (m *App) queuePREditDraft() tea.Cmd { + if m.drafts == nil { + return nil + } + key := prMetadataDraftKey(m.details.Owner, m.details.Repository, m.details.Number) + m.drafts.put(key, savedDraft{ + Kind: "pr-metadata", Owner: m.details.Owner, Repository: m.details.Repository, + Number: m.details.Number, Title: m.prEditEditors[prEditTitleField].Text, + BaseRef: m.prEditEditors[prEditBaseField].Text, + Body: m.prEditEditors[prEditBodyField].Text, + OriginalUpdatedAt: m.prEditOriginal.UpdatedAt, + }) + return flushDraftsAfter(m.drafts) +} diff --git a/github.go b/github.go index fdfb46a..59730d0 100644 --- a/github.go +++ b/github.go @@ -10,6 +10,7 @@ import ( "net/http" "path" "sort" + "strconv" "strings" "sync" "time" @@ -33,22 +34,30 @@ type GitHubBranchService interface { ListBranches(context.Context, string, string) ([]RepositoryBranch, error) } +type GitHubEnrichmentService interface { + EnrichPullRequest(context.Context, PRDetails) PRDetailsEnrichment +} + type GitHubClient struct { - endpoint string - token string - http *http.Client - conflicts conflictFileLoader - conflictMu sync.Mutex - conflictCache map[string]conflictFileResult + endpoint string + token string + http *http.Client + conflicts conflictFileLoader + conflictMu sync.Mutex + conflictCache map[string]conflictFileResult + health healthTracker + annotationMu sync.Mutex + annotationCache map[string][]CheckAnnotation } func NewGitHubClient(endpoint, token string) *GitHubClient { return &GitHubClient{ - endpoint: endpoint, - token: token, - http: &http.Client{Timeout: 20 * time.Second}, - conflicts: analyzeConflictFiles, - conflictCache: make(map[string]conflictFileResult), + endpoint: endpoint, + token: token, + http: &http.Client{Timeout: 20 * time.Second}, + conflicts: analyzeConflictFiles, + conflictCache: make(map[string]conflictFileResult), + annotationCache: make(map[string][]CheckAnnotation), } } @@ -66,7 +75,21 @@ type graphQLResponse[T any] struct { Errors []graphQLError `json:"errors"` } -func (c *GitHubClient) query(ctx context.Context, query string, variables map[string]any, target any) error { +func (c *GitHubClient) query( + ctx context.Context, query string, variables map[string]any, target any, +) (resultErr error) { + started := time.Now() + defer func() { + component := HealthComponent{ + Name: "GitHub API", Level: healthOK, Summary: "last request succeeded", + Detail: time.Since(started).Round(time.Millisecond).String(), UpdatedAt: time.Now(), + } + if resultErr != nil { + component.Level = healthError + component.Summary = resultErr.Error() + } + c.health.set(component) + }() payload, err := json.Marshal(graphQLRequest{Query: query, Variables: variables}) if err != nil { return fmt.Errorf("encode GraphQL request: %w", err) @@ -84,6 +107,7 @@ func (c *GitHubClient) query(ctx context.Context, query string, variables map[st return fmt.Errorf("GitHub request: %w", err) } defer resp.Body.Close() + c.recordRateLimit(resp) body, err := io.ReadAll(io.LimitReader(resp.Body, 32<<20)) if err != nil { return fmt.Errorf("read GitHub response: %w", err) @@ -109,6 +133,40 @@ func (c *GitHubClient) query(ctx context.Context, query string, variables map[st return nil } +func (c *GitHubClient) recordRateLimit(response *http.Response) { + parseInt := func(name string) int { + value, _ := strconv.Atoi(response.Header.Get(name)) + return value + } + rate := RateLimitSnapshot{ + Limit: parseInt("X-RateLimit-Limit"), Remaining: parseInt("X-RateLimit-Remaining"), + Used: parseInt("X-RateLimit-Used"), UpdatedAt: time.Now(), + } + if reset, err := strconv.ParseInt(response.Header.Get("X-RateLimit-Reset"), 10, 64); err == nil { + rate.ResetAt = time.Unix(reset, 0) + } + if seconds, err := strconv.Atoi(response.Header.Get("Retry-After")); err == nil && seconds > 0 { + rate.RetryAfter = time.Now().Add(time.Duration(seconds) * time.Second) + } + if (response.StatusCode == http.StatusForbidden || + response.StatusCode == http.StatusTooManyRequests) && rate.RetryAfter.IsZero() { + if rate.Remaining == 0 && !rate.ResetAt.IsZero() { + rate.RetryAfter = rate.ResetAt + } else { + rate.RetryAfter = time.Now().Add(time.Minute) + } + } + c.health.setRate(rate) +} + +func (c *GitHubClient) HealthReport() []HealthComponent { + return c.health.report() +} + +func (c *GitHubClient) RateLimit() RateLimitSnapshot { + return c.health.rateLimit() +} + const listPRsQuery = ` query PullRequests($query: String!, $first: Int!, $after: String) { viewer { login } @@ -937,16 +995,6 @@ func (c *GitHubClient) allCheckContexts( connection = data.Node.Contexts nodes = append(nodes, connection.Nodes...) } - for index := range nodes { - if nodes[index].ID == "" || !checkMayHaveUsefulAnnotations(nodes[index]) { - continue - } - annotations, err := c.checkAnnotations(ctx, nodes[index].ID) - if err != nil { - return nil, err - } - nodes[index].Annotations.Nodes = annotations - } return nodes, nil } @@ -1034,8 +1082,6 @@ func (c *GitHubClient) GetPullRequest(ctx context.Context, owner, name string, n reviewErr error timelineErr error checkErr error - conflictFiles []string - conflictFileErr error wait sync.WaitGroup ) wait.Add(4) @@ -1063,24 +1109,25 @@ func (c *GitHubClient) GetPullRequest(ctx context.Context, owner, name string, n checkNodes, checkErr = c.allCheckContexts(ctx, rollup.Contexts, rollup.ID) }() } - if node.Mergeable == "CONFLICTING" && c.conflicts != nil { - wait.Add(1) - go func() { - defer wait.Done() - baseOID := "" - if node.BaseRef != nil && node.BaseRef.Target != nil { - baseOID = node.BaseRef.Target.OID - } - conflictFiles, conflictFileErr = c.loadConflictFiles( - ctx, data.Repository.URL, number, node.BaseRefName, baseOID, node.HeadRefOID, - ) - }() - } wait.Wait() - for _, err := range []error{threadErr, conversationErr, reviewErr, timelineErr, checkErr} { - if err != nil { - return PRDetails{}, err - } + if threadErr != nil { + threadNodes = append([]githubReviewThread(nil), node.ReviewThreads.Nodes...) + } + if conversationErr != nil { + conversationNodes = append([]githubPRComment(nil), node.Comments.Nodes...) + } + if reviewErr != nil { + reviewNodes = append([]githubReviewSummary(nil), node.Reviews.Nodes...) + } + if timelineErr != nil { + timelineNodes = append([]githubTimelineNode(nil), node.TimelineItems.Nodes...) + } + if checkErr != nil && len(node.Commits.Nodes) > 0 && + node.Commits.Nodes[0].Commit.StatusCheckRollup != nil { + checkNodes = append( + []githubCheckContext(nil), + node.Commits.Nodes[0].Commit.StatusCheckRollup.Contexts.Nodes..., + ) } details := PRDetails{ PullRequest: PullRequest{ @@ -1091,7 +1138,7 @@ func (c *GitHubClient) GetPullRequest(ctx context.Context, owner, name string, n }, Body: node.Body, CreatedAt: node.CreatedAt, BaseRef: node.BaseRefName, HeadRef: node.HeadRefName, HeadOID: node.HeadRefOID, Mergeable: node.Mergeable, MergeState: node.MergeStateStatus, - ConflictFiles: conflictFiles, + RepositoryURL: data.Repository.URL, Additions: node.Additions, Deletions: node.Deletions, ChangedFiles: node.ChangedFiles, CommitCount: node.Commits.TotalCount, CommentCount: node.Comments.TotalCount, CheckState: "NONE", ReviewDecision: node.ReviewDecision, @@ -1101,8 +1148,18 @@ func (c *GitHubClient) GetPullRequest(ctx context.Context, owner, name string, n CanSubscribe: node.ViewerCanSubscribe, CanEnableMerge: node.ViewerCanEnableAutoMerge, }, } - if conflictFileErr != nil { - details.ConflictFileError = conflictFileErr.Error() + if node.BaseRef != nil && node.BaseRef.Target != nil { + details.BaseOID = node.BaseRef.Target.OID + } + for component, err := range map[string]error{ + "review threads": threadErr, "conversation": conversationErr, + "submitted reviews": reviewErr, "timeline": timelineErr, "checks": checkErr, + } { + if err != nil { + details.DataIssues = append(details.DataIssues, DataIssue{ + Component: component, Message: err.Error(), + }) + } } if node.BaseRef != nil && node.BaseRef.BranchProtectionRule != nil { rule := node.BaseRef.BranchProtectionRule @@ -1228,6 +1285,90 @@ func (c *GitHubClient) GetPullRequest(ctx context.Context, owner, name string, n return details, nil } +func (c *GitHubClient) EnrichPullRequest( + ctx context.Context, details PRDetails, +) PRDetailsEnrichment { + result := PRDetailsEnrichment{ + Owner: details.Owner, Repository: details.Repository, Number: details.Number, + HeadOID: details.HeadOID, CheckAnnotations: make(map[string][]CheckAnnotation), + } + for _, check := range details.Checks { + if check.ID == "" || !checkStateMayHaveUsefulAnnotations(check) { + continue + } + if annotations, ok := c.cachedAnnotations(check.ID); ok { + result.CheckAnnotations[check.ID] = annotations + continue + } + nodes, err := c.checkAnnotations(ctx, check.ID) + if err != nil { + result.Issues = append(result.Issues, DataIssue{ + Component: "check annotations", Message: err.Error(), + }) + continue + } + annotations := make([]CheckAnnotation, 0, len(nodes)) + for _, annotation := range nodes { + annotations = append(annotations, CheckAnnotation{ + Path: annotation.Path, StartLine: annotation.Location.Start.Line, + EndLine: annotation.Location.End.Line, Level: annotation.AnnotationLevel, + Title: annotation.Title, Message: annotation.Message, + }) + } + c.storeAnnotations(check.ID, annotations) + result.CheckAnnotations[check.ID] = annotations + } + if details.Mergeable == "CONFLICTING" && c.conflicts != nil { + files, err := c.loadConflictFiles( + ctx, details.RepositoryURL, details.Number, details.BaseRef, + details.BaseOID, details.HeadOID, + ) + if err != nil { + result.Issues = append(result.Issues, DataIssue{ + Component: "conflict file scan", Message: err.Error(), + }) + } else { + result.ConflictFiles = files + } + } + level, summary := healthOK, "secondary PR data loaded" + if len(result.Issues) > 0 { + level, summary = healthWarning, fmt.Sprintf( + "%d secondary data source(s) failed", len(result.Issues), + ) + } + c.health.set(HealthComponent{ + Name: "PR enrichment", Level: level, Summary: summary, UpdatedAt: time.Now(), + }) + return result +} + +func checkStateMayHaveUsefulAnnotations(check Check) bool { + state := strings.ToUpper(firstNonEmpty(check.Conclusion, check.State)) + switch state { + case "FAILURE", "ERROR", "CANCELLED", "TIMED_OUT", "ACTION_REQUIRED", "STALE": + return true + default: + return false + } +} + +func (c *GitHubClient) cachedAnnotations(checkID string) ([]CheckAnnotation, bool) { + c.annotationMu.Lock() + defer c.annotationMu.Unlock() + annotations, ok := c.annotationCache[checkID] + return append([]CheckAnnotation(nil), annotations...), ok +} + +func (c *GitHubClient) storeAnnotations(checkID string, annotations []CheckAnnotation) { + c.annotationMu.Lock() + defer c.annotationMu.Unlock() + if len(c.annotationCache) >= 256 { + c.annotationCache = make(map[string][]CheckAnnotation) + } + c.annotationCache[checkID] = append([]CheckAnnotation(nil), annotations...) +} + func (c *GitHubClient) SetThreadResolved( ctx context.Context, threadID string, resolved bool, ) (ReviewThread, error) { diff --git a/github_test.go b/github_test.go index 516d6fe..4f5ba47 100644 --- a/github_test.go +++ b/github_test.go @@ -6,11 +6,41 @@ import ( "net/http" "net/http/httptest" "reflect" + "strconv" "strings" "testing" "time" ) +func TestGraphQLRequestRecordsRateLimitHeaders(t *testing.T) { + reset := time.Now().Add(time.Hour).Unix() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("X-RateLimit-Limit", "5000") + w.Header().Set("X-RateLimit-Remaining", "321") + w.Header().Set("X-RateLimit-Used", "4679") + w.Header().Set("X-RateLimit-Reset", fmtInt64(reset)) + _, _ = w.Write([]byte(`{"data":{"viewer":{"login":"octocat"}}}`)) + })) + defer server.Close() + + client := NewGitHubClient(server.URL, "secret") + var target struct { + Viewer struct{ Login string } + } + if err := client.query(context.Background(), "query { viewer { login } }", nil, &target); err != nil { + t.Fatal(err) + } + rate := client.RateLimit() + if rate.Limit != 5000 || rate.Remaining != 321 || rate.Used != 4679 || + rate.ResetAt.Unix() != reset || rate.UpdatedAt.IsZero() { + t.Fatalf("rate limit = %#v", rate) + } +} + +func fmtInt64(value int64) string { + return strconv.FormatInt(value, 10) +} + func TestListBranchesPaginatesAndMarksTheDefaultBranch(t *testing.T) { requests := 0 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -304,9 +334,13 @@ func TestCheckContextsAndAnnotationsArePaginated(t *testing.T) { if err != nil { t.Fatal(err) } - if len(nodes) != 2 || len(nodes[1].Annotations.Nodes) != 2 || - nodes[1].Annotations.Nodes[0].Location.Start.Line != 4 || - nodes[1].Annotations.Nodes[1].Location.End.Line != 8 { + annotations, err := client.checkAnnotations(context.Background(), nodes[1].ID) + if err != nil { + t.Fatal(err) + } + if len(nodes) != 2 || len(annotations) != 2 || + annotations[0].Location.Start.Line != 4 || + annotations[1].Location.End.Line != 8 { t.Fatalf("paginated checks = %#v", nodes) } } @@ -513,7 +547,11 @@ func TestGetPullRequestLoadsConflictFilesForConflictingPR(t *testing.T) { if err != nil { t.Fatal(err) } - if !reflect.DeepEqual(got.ConflictFiles, []string{"src/conflict.go", "README.md"}) { - t.Fatalf("conflict files = %#v", got.ConflictFiles) + if len(got.ConflictFiles) != 0 { + t.Fatalf("core refresh loaded conflict files eagerly: %#v", got.ConflictFiles) + } + enrichment := client.EnrichPullRequest(context.Background(), got) + if !reflect.DeepEqual(enrichment.ConflictFiles, []string{"src/conflict.go", "README.md"}) { + t.Fatalf("conflict files = %#v", enrichment.ConflictFiles) } } diff --git a/health.go b/health.go new file mode 100644 index 0000000..ab211be --- /dev/null +++ b/health.go @@ -0,0 +1,157 @@ +package main + +import ( + "context" + "fmt" + "sort" + "strings" + "sync" + "time" +) + +type requestCoordinator struct { + mu sync.Mutex + id uint64 + cancel context.CancelFunc +} + +func (r *requestCoordinator) start(timeout time.Duration) (context.Context, context.CancelFunc, uint64) { + r.mu.Lock() + defer r.mu.Unlock() + if r.cancel != nil { + r.cancel() + } + r.id++ + ctx, cancel := context.WithTimeout(context.Background(), timeout) + r.cancel = cancel + return ctx, cancel, r.id +} + +func (r *requestCoordinator) current(id uint64) bool { + if id == 0 { + return true + } + r.mu.Lock() + defer r.mu.Unlock() + return r.id == id +} + +type HealthLevel string + +const ( + healthOK HealthLevel = "ok" + healthInfo HealthLevel = "info" + healthWarning HealthLevel = "warning" + healthError HealthLevel = "error" + healthUnknown HealthLevel = "unknown" +) + +type HealthComponent struct { + Name string + Level HealthLevel + Summary string + Detail string + UpdatedAt time.Time +} + +type HealthEvent struct { + Component string + Level HealthLevel + Message string + At time.Time +} + +type RateLimitSnapshot struct { + Limit int + Remaining int + Used int + ResetAt time.Time + RetryAfter time.Time + UpdatedAt time.Time +} + +type healthProvider interface { + HealthReport() []HealthComponent + RateLimit() RateLimitSnapshot +} + +type healthTracker struct { + mu sync.Mutex + components map[string]HealthComponent + rate RateLimitSnapshot +} + +func (h *healthTracker) set(component HealthComponent) { + h.mu.Lock() + defer h.mu.Unlock() + if h.components == nil { + h.components = make(map[string]HealthComponent) + } + if component.UpdatedAt.IsZero() { + component.UpdatedAt = time.Now() + } + h.components[component.Name] = component +} + +func (h *healthTracker) setRate(rate RateLimitSnapshot) { + h.mu.Lock() + defer h.mu.Unlock() + h.rate = rate +} + +func (h *healthTracker) report() []HealthComponent { + h.mu.Lock() + defer h.mu.Unlock() + components := make([]HealthComponent, 0, len(h.components)) + for _, component := range h.components { + components = append(components, component) + } + sort.Slice(components, func(i, j int) bool { + return strings.ToLower(components[i].Name) < strings.ToLower(components[j].Name) + }) + return components +} + +func (h *healthTracker) rateLimit() RateLimitSnapshot { + h.mu.Lock() + defer h.mu.Unlock() + return h.rate +} + +func (m *App) recordHealth(component string, level HealthLevel, message string) { + if strings.TrimSpace(message) == "" { + return + } + event := HealthEvent{Component: component, Level: level, Message: message, At: time.Now()} + const maximumHealthEvents = 100 + m.healthEvents = append(m.healthEvents, event) + if len(m.healthEvents) > maximumHealthEvents { + m.healthEvents = append([]HealthEvent(nil), m.healthEvents[len(m.healthEvents)-maximumHealthEvents:]...) + } +} + +func healthLevelLabel(level HealthLevel) string { + switch level { + case healthOK: + return "OK" + case healthInfo: + return "INFO" + case healthWarning: + return "WARN" + case healthError: + return "ERROR" + default: + return "UNKNOWN" + } +} + +func healthComponentText(component HealthComponent) string { + text := component.Summary + if component.Detail != "" { + text += " — " + component.Detail + } + if !component.UpdatedAt.IsZero() { + text += " (" + component.UpdatedAt.Local().Format("15:04:05") + ")" + } + return fmt.Sprintf("%-7s %-20s %s", healthLevelLabel(component.Level), component.Name, text) +} diff --git a/health_test.go b/health_test.go new file mode 100644 index 0000000..a881d24 --- /dev/null +++ b/health_test.go @@ -0,0 +1,201 @@ +package main + +import ( + "context" + "errors" + "strings" + "testing" + "time" + + tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/x/ansi" +) + +type healthTestService struct { + components []HealthComponent + rate RateLimitSnapshot +} + +func (s *healthTestService) ListPullRequests( + context.Context, string, string, int, bool, +) ([]PullRequest, error) { + return nil, nil +} + +func (s *healthTestService) GetPullRequest( + context.Context, string, string, int, +) (PRDetails, error) { + return PRDetails{}, nil +} + +func (s *healthTestService) HealthReport() []HealthComponent { + return append([]HealthComponent(nil), s.components...) +} + +func (s *healthTestService) RateLimit() RateLimitSnapshot { return s.rate } + +func TestHealthScreenReportsComponentsRateLimitAndEvents(t *testing.T) { + service := &healthTestService{ + components: []HealthComponent{{ + Name: "GitHub API", Level: healthOK, Summary: "request succeeded", + }}, + rate: RateLimitSnapshot{ + Limit: 5000, Remaining: 42, UpdatedAt: time.Now(), + ResetAt: time.Now().Add(time.Hour), + }, + } + settings := defaultAppSettings() + settings.ReadState = loadReadState(t.TempDir() + "/state.json") + settings.Drafts = loadDraftStore(t.TempDir() + "/drafts.json") + app := NewAppWithSettings(service, "o", "r", false, 50, time.Minute, settings) + app.width, app.height = 64, 30 + app.details = PRDetails{ + PullRequest: PullRequest{ID: "pr", UpdatedAt: time.Now()}, + DataIssues: []DataIssue{{Component: "timeline", Message: "unavailable"}}, + } + app.recordHealth("timeline", healthWarning, "a deliberately long warning that must remain readable") + + lines := app.healthLines() + plain := ansi.Strip(strings.Join(lines, "\n")) + for _, wanted := range []string{ + "configuration", "read state", "draft persistence", "GitHub API", + "rate limit", "42/5000", "PR core data", "timeline", + } { + if !strings.Contains(plain, wanted) { + t.Fatalf("health output missing %q:\n%s", wanted, plain) + } + } + for _, line := range lines { + if ansi.StringWidth(line) > app.width-2 { + t.Fatalf("health line width = %d, want <= %d: %q", ansi.StringWidth(line), app.width-2, line) + } + } +} + +func TestHealthScreenOpensAndReturnsToPreviousScreen(t *testing.T) { + app := NewApp(nil, "", "", false, 50, time.Minute) + app.screen = dashboardScreen + app.scroll = 7 + app.width, app.height = 100, 30 + + updated, _ := app.Update(runeKey("H")) + app = updated.(App) + if app.screen != healthScreen || app.healthReturn != dashboardScreen || app.scroll != 7 { + t.Fatalf("health navigation = screen %v return %v", app.screen, app.healthReturn) + } + view := ansi.Strip(app.View()) + if !strings.Contains(view, "Application health") || + !strings.Contains(view, "╭") || !strings.Contains(view, "╰") { + t.Fatalf("health is not rendered as a modal:\n%s", view) + } + updated, _ = app.Update(tea.KeyMsg{Type: tea.KeyEsc}) + app = updated.(App) + if app.screen != dashboardScreen || app.scroll != 7 { + t.Fatalf("health back returned to screen %v at scroll %d", app.screen, app.scroll) + } +} + +func TestHealthRefreshLineIsStableAndInformational(t *testing.T) { + app := NewApp(nil, "", "", false, 50, time.Minute) + app.width, app.height = 100, 30 + app.loading = false + idle := app.healthLines() + app.loading = true + loading := app.healthLines() + + findRefresh := func(lines []string) (int, string) { + for index, line := range lines { + plain := ansi.Strip(line) + if strings.Contains(plain, "refresh") { + return index, plain + } + } + return -1, "" + } + idleIndex, idleLine := findRefresh(idle) + loadingIndex, loadingLine := findRefresh(loading) + if idleIndex < 0 || idleIndex != loadingIndex { + t.Fatalf("refresh line moved from %d to %d", idleIndex, loadingIndex) + } + if !strings.Contains(idleLine, "OK") || !strings.Contains(loadingLine, "INFO") || + strings.Contains(loadingLine, "WARN") { + t.Fatalf("refresh states: idle=%q loading=%q", idleLine, loadingLine) + } +} + +func TestAdaptivePollingHonorsRateLimitBackoff(t *testing.T) { + now := time.Unix(1000, 0) + service := &healthTestService{rate: RateLimitSnapshot{ + Limit: 5000, Remaining: 100, UpdatedAt: now, + }} + app := NewApp(service, "", "", false, 50, 10*time.Second) + got := app.adaptivePollInterval(now) + if got < 72*time.Second || got > 88*time.Second { + t.Fatalf("low-budget interval = %s, want about 80s with jitter", got) + } + + service.rate.RetryAfter = now.Add(2 * time.Minute) + got = app.adaptivePollInterval(now) + if got < 108*time.Second || got > 132*time.Second { + t.Fatalf("retry-after interval = %s, want about 2m with jitter", got) + } +} + +func TestRequestCoordinatorCancelsSupersededRequest(t *testing.T) { + var coordinator requestCoordinator + first, cancelFirst, firstID := coordinator.start(time.Minute) + defer cancelFirst() + _, cancelSecond, secondID := coordinator.start(time.Minute) + defer cancelSecond() + select { + case <-first.Done(): + default: + t.Fatal("superseded request context was not canceled") + } + if coordinator.current(firstID) || !coordinator.current(secondID) { + t.Fatalf("current request ids: first=%t second=%t", + coordinator.current(firstID), coordinator.current(secondID)) + } + if !errors.Is(first.Err(), context.Canceled) { + t.Fatalf("first context error = %v", first.Err()) + } +} + +func TestPartialRefreshPreservesLastCompleteSubsections(t *testing.T) { + previous := PRDetails{ + PullRequest: PullRequest{Owner: "o", Repository: "r", Number: 1}, + HeadOID: "head", + BaseOID: "base", + Threads: []ReviewThread{{ID: "old-thread"}, {ID: "second-thread"}}, + Conversation: []PRComment{{ID: "old-comment"}, {ID: "second-comment"}}, + ConflictFiles: []string{"conflicted.go"}, + Checks: []Check{{ + ID: "check", Annotations: []CheckAnnotation{{Path: "problem.go"}}, + }}, + } + fresh := PRDetails{ + PullRequest: PullRequest{Owner: "o", Repository: "r", Number: 1}, + HeadOID: "head", + BaseOID: "base", + Threads: []ReviewThread{{ID: "first-page-only"}}, + Conversation: []PRComment{{ID: "first-page-only"}}, + Checks: []Check{{ID: "check"}}, + DataIssues: []DataIssue{ + {Component: "review threads", Message: "page failed"}, + {Component: "conversation", Message: "page failed"}, + }, + } + merged := preservePartialPRData(fresh, previous) + if len(merged.Threads) != 2 || merged.Threads[0].ID != "old-thread" { + t.Fatalf("preserved threads = %#v", merged.Threads) + } + if len(merged.Conversation) != 2 || merged.Conversation[0].ID != "old-comment" { + t.Fatalf("preserved conversation = %#v", merged.Conversation) + } + if len(merged.DataIssues) != 2 { + t.Fatalf("partial markers were lost: %#v", merged.DataIssues) + } + if len(merged.ConflictFiles) != 1 || len(merged.Checks[0].Annotations) != 1 { + t.Fatalf("secondary data flickered during core refresh: %#v", merged) + } +} diff --git a/keybindings.go b/keybindings.go index fb8d54f..c08dde5 100644 --- a/keybindings.go +++ b/keybindings.go @@ -38,6 +38,7 @@ type NavigationKeyBindings struct { type ViewKeyBindings struct { Open []string `toml:"open"` Dashboard []string `toml:"dashboard"` + Health []string `toml:"health"` Edit []string `toml:"edit"` ToggleList []string `toml:"toggle_list"` } @@ -117,7 +118,7 @@ func defaultKeyBindings() KeyBindings { }, Views: ViewKeyBindings{ Open: []string{"enter", "l"}, Dashboard: []string{"d"}, - Edit: []string{"e"}, ToggleList: []string{"tab"}, + Health: []string{"H"}, Edit: []string{"e"}, ToggleList: []string{"tab"}, }, Threads: ThreadKeyBindings{ Search: []string{"/"}, ClearFilter: []string{"F"}, @@ -322,6 +323,8 @@ func (k KeyBindings) canonicalMainKey(key string, current screen) string { switch { case keyMatches(key, k.Views.Dashboard): return "d" + case keyMatches(key, k.Views.Health): + return "H" case keyMatches(key, k.Views.Edit): return "e" default: @@ -383,7 +386,8 @@ func validateKeyBindings(bindings KeyBindings) error { }}, {"keybindings.views", map[string][]string{ "open": bindings.Views.Open, "dashboard": bindings.Views.Dashboard, - "edit": bindings.Views.Edit, "toggle_list": bindings.Views.ToggleList, + "health": bindings.Views.Health, "edit": bindings.Views.Edit, + "toggle_list": bindings.Views.ToggleList, }}, {"keybindings.threads", map[string][]string{ "search": bindings.Threads.Search, "clear_filter": bindings.Threads.ClearFilter, @@ -463,6 +467,7 @@ func validateKeyBindingContexts(bindings KeyBindings) error { {"back", general.Back}, {"down", navigation.Down}, {"up", navigation.Up}, {"first", navigation.First}, {"last", navigation.Last}, {"page_down", navigation.PageDown}, {"page_up", navigation.PageUp}, + {"health", views.Health}, } if err := validateKeyContext("pull request list", append(screenCommon, contextBinding{"open", views.Open}, @@ -476,6 +481,9 @@ func validateKeyBindingContexts(bindings KeyBindings) error { )...); err != nil { return err } + if err := validateKeyContext("health screen", screenCommon...); err != nil { + return err + } if err := validateKeyContext("review threads", append(screenCommon, contextBinding{"left", navigation.Left}, contextBinding{"right", navigation.Right}, diff --git a/main.go b/main.go index 5356405..2d6667f 100644 --- a/main.go +++ b/main.go @@ -123,9 +123,12 @@ func main() { exitf("configuration: %v", err) } } - service = NewCachedGitHubService(client, cacheDir, config.Cache.MaxAge.Duration) + service = NewCachedGitHubService( + client, cacheDir, config.Cache.MaxAge.Duration, config.Cache.MaxEntries, + ) } statePath := filepath.Join(filepath.Dir(*configFile), "state.json") + draftPath := filepath.Join(filepath.Dir(*configFile), "drafts.json") app := NewAppWithSettings( service, owner, name, config.ShowAll, config.Limit, config.RefreshInterval.Duration, AppSettings{ @@ -134,6 +137,7 @@ func main() { DashboardMode: config.Display.DashboardMode, CompactReviews: config.Display.CompactReviews, ReadState: loadReadState(statePath), + Drafts: loadDraftStore(draftPath), PathScroll: config.Paths.Scroll, PathScrollInterval: config.Paths.ScrollInterval.Duration, ThreadStatusOrder: config.Threads.StatusOrder, diff --git a/persistence.go b/persistence.go new file mode 100644 index 0000000..e832083 --- /dev/null +++ b/persistence.go @@ -0,0 +1,53 @@ +package main + +import ( + "encoding/json" + "errors" + "os" + "path/filepath" +) + +func atomicWriteJSON(path string, value any, mode os.FileMode) error { + if path == "" { + return nil + } + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0o700); err != nil { + return err + } + data, err := json.MarshalIndent(value, "", " ") + if err != nil { + return err + } + temp, err := os.CreateTemp(dir, ".gh-threads-*") + if err != nil { + return err + } + name := temp.Name() + defer os.Remove(name) + if err := temp.Chmod(mode); err != nil { + _ = temp.Close() + return err + } + if _, err := temp.Write(data); err != nil { + _ = temp.Close() + return err + } + if err := temp.Sync(); err != nil { + _ = temp.Close() + return err + } + if err := temp.Close(); err != nil { + return err + } + if err := os.Rename(name, path); err != nil { + return err + } + if directory, err := os.Open(dir); err == nil { + defer directory.Close() + if syncErr := directory.Sync(); syncErr != nil && !errors.Is(syncErr, os.ErrInvalid) { + return syncErr + } + } + return nil +} diff --git a/pr_editor.go b/pr_editor.go index 488c2d6..0e987ce 100644 --- a/pr_editor.go +++ b/pr_editor.go @@ -32,11 +32,12 @@ func (m *App) startPREdit() tea.Cmd { m.editorMode == "vim", ) m.prEditEditors[prEditBodyField].highlightMarkdown = true + m.prEditOriginal = m.currentPRMetadata() + m.restorePREditDraft() for index := range m.prEditEditors { m.prEditEditors[index].hardwareCursor = m.cursorOutput != nil m.prEditEditors[index].keys = m.keybindings } - m.prEditOriginal = m.currentPRMetadata() m.prEditBranches = nil m.prEditBranchesLoading = false m.prEditBranchesError = "" @@ -206,7 +207,7 @@ func (m App) updatePREditInput(key tea.KeyMsg) (tea.Model, tea.Cmd) { } m.err = nil m.ensurePREditCursorVisible() - return m, nil + return m, m.queuePREditDraft() } func (m App) prEditEditorWidth() int { diff --git a/text_editor.go b/text_editor.go index b7f958e..ecc1f5d 100644 --- a/text_editor.go +++ b/text_editor.go @@ -3,10 +3,12 @@ package main import ( "strings" "unicode" + "unicode/utf8" tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" "github.com/charmbracelet/x/ansi" + "github.com/rivo/uniseg" ) type textEditorMode string @@ -69,7 +71,7 @@ func (e *textEditor) handleKeyAtWidth(key tea.KeyMsg, multiline bool, wrapWidth e.clearPending() start, _ := editorLineBounds(e.Text, e.Cursor, wrapWidth) if e.Cursor > start { - e.Cursor-- + e.Cursor = max(start, previousGraphemeBoundary(e.Text, e.Cursor)) } return true } @@ -92,10 +94,10 @@ func (e *textEditor) handleStandardKey(key tea.KeyMsg, multiline bool, wrapWidth switch { case key.Type != tea.KeyRunes && key.Type != tea.KeySpace && keyMatches(k, e.keys.Navigation.Left): - e.Cursor = max(0, e.Cursor-1) + e.Cursor = previousGraphemeBoundary(e.Text, e.Cursor) case key.Type != tea.KeyRunes && key.Type != tea.KeySpace && keyMatches(k, e.keys.Navigation.Right): - e.Cursor = min(len([]rune(e.Text)), e.Cursor+1) + e.Cursor = nextGraphemeBoundary(e.Text, e.Cursor) case key.Type != tea.KeyRunes && key.Type != tea.KeySpace && keyMatches(k, e.keys.Navigation.Up): if !multiline { @@ -130,6 +132,7 @@ func (e *textEditor) handleStandardKey(key tea.KeyMsg, multiline bool, wrapWidth return false } } + e.Cursor = previousOrCurrentGraphemeBoundary(e.Text, e.Cursor) return true } @@ -166,7 +169,7 @@ func (e *textEditor) handleNormalKey(key tea.KeyMsg, multiline bool, wrapWidth i e.Mode = textEditorInsert case keyMatches(k, e.keys.Vim.Append): _, end := editorLineBounds(e.Text, e.Cursor, wrapWidth) - e.Cursor = min(end, e.Cursor+1) + e.Cursor = min(end, nextGraphemeBoundary(e.Text, e.Cursor)) e.Mode = textEditorInsert case keyMatches(k, e.keys.Vim.InsertLineStart): e.Cursor = firstNonBlankAtWidth(e.Text, e.Cursor, wrapWidth) @@ -192,9 +195,12 @@ func (e *textEditor) handleNormalKey(key tea.KeyMsg, multiline bool, wrapWidth i e.Mode = textEditorInsert case keyMatches(k, e.keys.Navigation.Left): start, _ := editorLineBounds(e.Text, e.Cursor, wrapWidth) - e.Cursor = max(start, e.Cursor-1) + e.Cursor = max(start, previousGraphemeBoundary(e.Text, e.Cursor)) case keyMatches(k, e.keys.Navigation.Right): - e.Cursor = min(normalEditorLineLast(e.Text, e.Cursor, wrapWidth), e.Cursor+1) + e.Cursor = min( + normalEditorLineLast(e.Text, e.Cursor, wrapWidth), + nextGraphemeBoundary(e.Text, e.Cursor), + ) case keyMatches(k, e.keys.Navigation.Down): if multiline { e.Cursor = moveEditorCursorLine(e.Text, e.Cursor, 1, wrapWidth, true) @@ -266,6 +272,7 @@ func (e *textEditor) handleNormalKey(key tea.KeyMsg, multiline bool, wrapWidth i default: return false } + e.Cursor = previousOrCurrentGraphemeBoundary(e.Text, e.Cursor) return true } @@ -347,7 +354,9 @@ func (e textEditor) selectionBounds(wrapWidth int) (int, int, bool) { anchor := clamp(e.visualAnchor, 0, len(runes)) cursor := clamp(e.Cursor, 0, len(runes)) if !e.visualLine { - start, end := min(anchor, cursor), min(len(runes), max(anchor, cursor)+1) + start := previousOrCurrentGraphemeBoundary(e.Text, min(anchor, cursor)) + end := nextGraphemeBoundary(e.Text, max(anchor, cursor)) + end = min(len(runes), end) return start, end, end > start } anchorStart, anchorEnd := editorLineBounds(e.Text, anchor, wrapWidth) @@ -386,7 +395,7 @@ func (e *textEditor) deleteSelection(wrapWidth int) { e.Text = string(append(runes[:start], runes[end:]...)) e.Cursor = min(start, len([]rune(e.Text))) if e.Cursor == len([]rune(e.Text)) && e.Cursor > 0 { - e.Cursor-- + e.Cursor = previousGraphemeBoundary(e.Text, e.Cursor) } e.stopVisual() } @@ -411,11 +420,11 @@ func (e *textEditor) pasteClipboard(replaceSelection bool, wrapWidth int) { e.stopVisual() } else { _, end := editorLineBounds(e.Text, e.Cursor, 0) - e.Cursor = min(end, e.Cursor+1) + e.Cursor = min(end, nextGraphemeBoundary(e.Text, e.Cursor)) } e.insert(value) if e.Cursor > 0 { - e.Cursor-- + e.Cursor = previousGraphemeBoundary(e.Text, e.Cursor) } } @@ -442,8 +451,9 @@ func (e *textEditor) deleteBefore() { if e.Cursor == 0 { return } - value = append(value[:e.Cursor-1], value[e.Cursor:]...) - e.Cursor-- + start := previousGraphemeBoundary(e.Text, e.Cursor) + value = append(value[:start], value[e.Cursor:]...) + e.Cursor = start e.Text = string(value) } @@ -453,10 +463,56 @@ func (e *textEditor) deleteAt() { if e.Cursor == len(value) { return } - value = append(value[:e.Cursor], value[e.Cursor+1:]...) + end := nextGraphemeBoundary(e.Text, e.Cursor) + value = append(value[:e.Cursor], value[end:]...) e.Text = string(value) } +func graphemeBoundaries(value string) []int { + boundaries := []int{0} + graphemes := uniseg.NewGraphemes(value) + offset := 0 + for graphemes.Next() { + offset += utf8.RuneCountInString(graphemes.Str()) + boundaries = append(boundaries, offset) + } + return boundaries +} + +func previousGraphemeBoundary(value string, cursor int) int { + cursor = clamp(cursor, 0, len([]rune(value))) + previous := 0 + for _, boundary := range graphemeBoundaries(value) { + if boundary >= cursor { + return previous + } + previous = boundary + } + return previous +} + +func previousOrCurrentGraphemeBoundary(value string, cursor int) int { + cursor = clamp(cursor, 0, len([]rune(value))) + previous := 0 + for _, boundary := range graphemeBoundaries(value) { + if boundary > cursor { + return previous + } + previous = boundary + } + return previous +} + +func nextGraphemeBoundary(value string, cursor int) int { + cursor = clamp(cursor, 0, len([]rune(value))) + for _, boundary := range graphemeBoundaries(value) { + if boundary > cursor { + return boundary + } + } + return len([]rune(value)) +} + func (e *textEditor) performFind(command, target rune, remember bool, wrapWidth int) { runes := []rune(e.Text) cursor := clamp(e.Cursor, 0, len(runes)) diff --git a/text_editor_test.go b/text_editor_test.go index a938635..3dedf33 100644 --- a/text_editor_test.go +++ b/text_editor_test.go @@ -170,6 +170,47 @@ func TestVimTextEditorSubstituteDeletesCharacterAndEntersInsert(t *testing.T) { } } +func TestEditorMotionsAndDeletionPreserveGraphemeClusters(t *testing.T) { + tests := []struct { + name string + cluster string + }{ + {name: "combining mark", cluster: "e\u0301"}, + {name: "emoji with variation selector", cluster: "❤️"}, + {name: "multi-code-point emoji", cluster: "👨‍👩‍👧‍👦"}, + {name: "full-width character", cluster: "界"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + editor := newTextEditor(test.cluster+"x", true) + editor.handleKey(runeKey("l"), true) + want := len([]rune(test.cluster)) + if editor.Cursor != want { + t.Fatalf("cursor = %d, want grapheme boundary %d", editor.Cursor, want) + } + editor.handleKey(runeKey("h"), true) + if editor.Cursor != 0 { + t.Fatalf("reverse cursor = %d, want 0", editor.Cursor) + } + editor.handleKey(runeKey("x"), true) + if editor.Text != "x" || editor.Cursor != 0 { + t.Fatalf("delete split grapheme: text=%q cursor=%d", editor.Text, editor.Cursor) + } + }) + } +} + +func TestEditorVisualYankIncludesWholeGrapheme(t *testing.T) { + clipboard := &memoryTextClipboard{} + editor := newTextEditor("e\u0301x", true) + editor.clipboard = clipboard + editor.handleKey(runeKey("v"), true) + editor.handleKey(runeKey("y"), true) + if clipboard.written != "e\u0301" { + t.Fatalf("yanked text = %q, want complete combining grapheme", clipboard.written) + } +} + func TestVimTextEditorNormalMotionsStayOnCharactersWithinLine(t *testing.T) { editor := newTextEditor("ab\n cd\n", true) editor.Cursor = 1 diff --git a/tui.go b/tui.go index ed36f1d..5b27966 100644 --- a/tui.go +++ b/tui.go @@ -21,6 +21,7 @@ const ( prScreen screen = iota dashboardScreen threadScreen + healthScreen ) type pane int @@ -65,17 +66,19 @@ type pullRequestUpdatedMsg struct { } type prsLoadedMsg struct { - prs []PullRequest - err error - cached bool + prs []PullRequest + err error + cached bool + requestID uint64 } type detailsLoadedMsg struct { - owner string - repo string - number int - details PRDetails - err error - cached bool + owner string + repo string + number int + details PRDetails + err error + cached bool + requestID uint64 } type branchesLoadedMsg struct { @@ -85,6 +88,11 @@ type branchesLoadedMsg struct { err error } +type detailsEnrichedMsg struct { + enrichment PRDetailsEnrichment + requestID uint64 +} + type App struct { service GitHubService owner, repo string @@ -93,6 +101,10 @@ type App struct { poll time.Duration screen screen + healthReturn screen + healthScroll int + healthEvents []HealthEvent + requests *requestCoordinator prs []PullRequest prIndex int details PRDetails @@ -103,6 +115,7 @@ type App struct { scroll int width, height int loading bool + secondaryLoading bool err error lastRefresh time.Time pendingZ bool @@ -137,6 +150,7 @@ type App struct { editorMode string keybindings KeyBindings readState *readStateStore + drafts *draftStore knownThreads map[string]bool knownComments map[string]bool initializedPRs map[string]bool @@ -156,6 +170,7 @@ type AppSettings struct { EditorMode string KeyBindings KeyBindings ReadState *readStateStore + Drafts *draftStore } func defaultAppSettings() AppSettings { @@ -196,10 +211,13 @@ func NewAppWithSettings( threadStatusOrder: append([]string(nil), settings.ThreadStatusOrder...), threadWithinStatus: settings.ThreadWithinStatus, dashboardMode: settings.DashboardMode, dashboardReturn: prScreen, + healthReturn: prScreen, + requests: &requestCoordinator{}, compactReviews: settings.CompactReviews, editorMode: settings.EditorMode, keybindings: settings.KeyBindings, readState: state, + drafts: settings.Drafts, knownThreads: make(map[string]bool), knownComments: make(map[string]bool), initializedPRs: make(map[string]bool), unreadThreads: make(map[string]bool), updatedThreads: make(map[string]bool), @@ -215,7 +233,33 @@ func (m App) Init() tea.Cmd { } func (m App) nextTick() tea.Cmd { - return tea.Tick(m.poll, func(t time.Time) tea.Msg { return tickMsg(t) }) + return tea.Tick(m.adaptivePollInterval(time.Now()), func(t time.Time) tea.Msg { return tickMsg(t) }) +} + +func (m App) adaptivePollInterval(now time.Time) time.Duration { + interval := m.poll + if provider, ok := m.service.(healthProvider); ok { + rate := provider.RateLimit() + switch { + case now.Before(rate.RetryAfter): + interval = max(interval, rate.RetryAfter.Sub(now)) + case rate.Limit > 0 && rate.Remaining*20 < rate.Limit: + interval *= 8 + case rate.Limit > 0 && rate.Remaining*100 < rate.Limit*15: + interval *= 4 + case rate.Limit > 0 && rate.Remaining*10 < rate.Limit*3: + interval *= 2 + } + } + interval = min(interval, 15*time.Minute) + // Stable-enough per-call jitter prevents synchronized clients without + // introducing a shared random source into the model. + jitter := interval / 10 + if jitter > 0 { + offset := time.Duration(now.UnixNano()%int64(2*jitter+1)) - jitter + interval += offset + } + return max(interval, 2*time.Second) } func (m App) nextPathTick() tea.Cmd { @@ -235,7 +279,7 @@ func (m App) loadPRs(useCache bool) tea.Cmd { func (m App) loadLivePRs() tea.Cmd { return func() tea.Msg { - ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + ctx, cancel, requestID := m.requests.start(20 * time.Second) defer cancel() var prs []PullRequest var err error @@ -244,7 +288,7 @@ func (m App) loadLivePRs() tea.Cmd { } else { prs, err = m.service.ListPullRequests(ctx, m.owner, m.repo, m.limit, m.showAll) } - return prsLoadedMsg{prs: prs, err: err} + return prsLoadedMsg{prs: prs, err: err, requestID: requestID} } } @@ -264,7 +308,7 @@ func (m App) loadDetails(pr PullRequest, useCache bool) tea.Cmd { func (m App) loadLiveDetails(pr PullRequest) tea.Cmd { return func() tea.Msg { - ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + ctx, cancel, requestID := m.requests.start(60 * time.Second) defer cancel() var details PRDetails var err error @@ -275,7 +319,21 @@ func (m App) loadLiveDetails(pr PullRequest) tea.Cmd { } return detailsLoadedMsg{ owner: pr.Owner, repo: pr.Repository, number: pr.Number, - details: details, err: err, + details: details, err: err, requestID: requestID, + } + } +} + +func (m App) loadDetailsEnrichment(details PRDetails) tea.Cmd { + service, ok := m.service.(GitHubEnrichmentService) + if !ok { + return nil + } + return func() tea.Msg { + ctx, cancel, requestID := m.requests.start(60 * time.Second) + defer cancel() + return detailsEnrichedMsg{ + enrichment: service.EnrichPullRequest(ctx, details), requestID: requestID, } } } @@ -287,6 +345,7 @@ func (m *App) startReply() { return } m.writeMode, m.writeThreadID, m.replyDraft, m.err = writeReply, thread.ID, "", nil + m.restoreReplyDraft(thread.ID) m.folded[thread.ID] = false m.focus = threadDetailPane m.scroll = m.detailMaxScroll() @@ -365,6 +424,7 @@ func (m App) updateWriteInput(key tea.KeyMsg) (tea.Model, tea.Cmd) { } if m.writeMode == writeReply { m.scroll = m.detailMaxScroll() + return m, m.queueReplyDraft() } case writeReplyConfirm: switch k { @@ -418,7 +478,11 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case tickMsg: if !m.loading { m.loading = true - if (m.screen == dashboardScreen || m.screen == threadScreen) && m.details.Number != 0 { + targetScreen := m.screen + if targetScreen == healthScreen { + targetScreen = m.healthReturn + } + if (targetScreen == dashboardScreen || targetScreen == threadScreen) && m.details.Number != 0 { return m, tea.Batch(m.loadDetails(m.details.PullRequest, false), m.nextTick()) } return m, tea.Batch(m.loadPRs(false), m.nextTick()) @@ -433,7 +497,11 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } return m, nil case prsLoadedMsg: - if m.screen != prScreen || msg.cached && !m.loading { + if m.requests != nil && !m.requests.current(msg.requestID) { + return m, nil + } + if (m.screen != prScreen && !(m.screen == healthScreen && m.healthReturn == prScreen)) || + msg.cached && !m.loading { return m, nil } if !msg.cached { @@ -441,9 +509,11 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } if msg.err != nil { if msg.cached { + m.recordHealth("pull request cache", healthWarning, msg.err.Error()) return m, nil } m.err = msg.err + m.recordHealth("pull request list", healthError, msg.err.Error()) return m, nil } selected := "" @@ -466,7 +536,12 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.lastRefresh = time.Now() } case detailsLoadedMsg: - if m.screen != dashboardScreen && m.screen != threadScreen { + if m.requests != nil && !m.requests.current(msg.requestID) { + return m, nil + } + if m.screen != dashboardScreen && m.screen != threadScreen && + !(m.screen == healthScreen && + (m.healthReturn == dashboardScreen || m.healthReturn == threadScreen)) { return m, nil } if msg.cached && !m.loading { @@ -481,9 +556,11 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } if msg.err != nil { if msg.cached { + m.recordHealth("PR cache", healthWarning, msg.err.Error()) return m, nil } m.err = msg.err + m.recordHealth("PR refresh", healthError, msg.err.Error()) return m, nil } selected := "" @@ -492,9 +569,16 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { selected = m.details.Threads[m.threadIndex].ID anchor = m.detailScrollAnchor() } + msg.details = preservePartialPRData(msg.details, m.details) m.trackThreadUpdates(msg.details) sortReviewThreads(msg.details.Threads, m.threadStatusOrder, m.threadWithinStatus) m.details = msg.details + for _, issue := range msg.details.DataIssues { + m.recordHealth(issue.Component, healthWarning, issue.Message) + } + if msg.details.ConflictFileError != "" { + m.recordHealth("conflict file scan", healthWarning, msg.details.ConflictFileError) + } m.threadIndex = indexThread(m.details.Threads, selected) if selected != "" && (len(m.details.Threads) == 0 || m.details.Threads[m.threadIndex].ID != selected) { m.scroll = 0 @@ -518,6 +602,45 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.lastRefresh = msg.details.CachedAt } else { m.lastRefresh = time.Now() + if _, ok := m.service.(GitHubEnrichmentService); ok { + m.secondaryLoading = true + return m, m.loadDetailsEnrichment(msg.details) + } + } + case detailsEnrichedMsg: + if m.requests != nil && !m.requests.current(msg.requestID) { + return m, nil + } + enrichment := msg.enrichment + if enrichment.Owner != m.details.Owner || + enrichment.Repository != m.details.Repository || + enrichment.Number != m.details.Number || + enrichment.HeadOID != m.details.HeadOID { + return m, nil + } + m.secondaryLoading = false + m.details.DataIssues = removeDataIssues( + m.details.DataIssues, "check annotations", "conflict file scan", + ) + for index := range m.details.Checks { + if annotations, ok := enrichment.CheckAnnotations[m.details.Checks[index].ID]; ok { + m.details.Checks[index].Annotations = annotations + } + } + if enrichment.ConflictFiles != nil { + m.details.ConflictFiles = enrichment.ConflictFiles + m.details.ConflictFileError = "" + } + for _, issue := range enrichment.Issues { + m.details.DataIssues = append(m.details.DataIssues, issue) + if issue.Component == "conflict file scan" { + m.details.ConflictFileError = issue.Message + } + m.recordHealth(issue.Component, healthWarning, issue.Message) + } + case draftFlushMsg: + if msg.err != nil { + m.recordHealth("draft persistence", healthWarning, msg.err.Error()) } case branchesLoadedMsg: if m.writeMode != writePREdit || @@ -527,6 +650,7 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.prEditBranchesLoading = false if msg.err != nil { m.prEditBranchesError = msg.err.Error() + m.recordHealth("branch recommendations", healthWarning, msg.err.Error()) m.ensurePREditCursorVisible() return m, nil } @@ -538,6 +662,7 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.writeMode = writeNone if msg.err != nil { m.err = fmt.Errorf("change thread resolution: %w", msg.err) + m.recordHealth("thread resolution", healthError, msg.err.Error()) return m, nil } selected := "" @@ -566,6 +691,7 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if msg.err != nil { m.writeMode = writeReply m.err = fmt.Errorf("reply to review thread: %w", msg.err) + m.recordHealth("thread reply", healthError, msg.err.Error()) m.scroll = m.detailMaxScroll() return m, nil } @@ -578,6 +704,12 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { break } } + draftKey := replyDraftKey( + m.details.Owner, m.details.Repository, m.details.Number, msg.threadID, + ) + if err := m.drafts.delete(draftKey); err != nil { + m.recordHealth("draft persistence", healthWarning, err.Error()) + } m.replyDraft, m.writeThreadID = "", "" m.err = nil m.lastRefresh = time.Now() @@ -585,6 +717,7 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if msg.err != nil { m.writeMode = writePREdit m.err = fmt.Errorf("update pull request: %w", msg.err) + m.recordHealth("PR metadata update", healthError, msg.err.Error()) m.scroll = 0 return m, nil } @@ -604,6 +737,12 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { break } } + draftKey := prMetadataDraftKey( + m.details.Owner, m.details.Repository, m.details.Number, + ) + if err := m.drafts.delete(draftKey); err != nil { + m.recordHealth("draft persistence", healthWarning, err.Error()) + } m.clearPREdit() m.err = nil m.lastRefresh = time.Now() @@ -739,13 +878,19 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil } m.loading = true - if m.screen == dashboardScreen || m.screen == threadScreen { + targetScreen := m.screen + if targetScreen == healthScreen { + targetScreen = m.healthReturn + } + if targetScreen == dashboardScreen || targetScreen == threadScreen { return m, m.loadDetails(m.details.PullRequest, false) } return m, m.loadPRs(false) case "j", "down": if m.screen == dashboardScreen { m.scrollDashboard(1) + } else if m.screen == healthScreen { + m.healthScroll = clamp(m.healthScroll+1, 0, m.healthMaxScroll()) } else if m.screen == threadScreen && m.focus == threadDetailPane { m.scrollDetail(1) } else { @@ -754,6 +899,8 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case "k", "up": if m.screen == dashboardScreen { m.scrollDashboard(-1) + } else if m.screen == healthScreen { + m.healthScroll = clamp(m.healthScroll-1, 0, m.healthMaxScroll()) } else if m.screen == threadScreen && m.focus == threadDetailPane { m.scrollDetail(-1) } else { @@ -793,6 +940,10 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.dashboardReturn, m.screen, m.scroll = threadScreen, dashboardScreen, 0 return m, nil } + case "H": + if m.screen != healthScreen { + m.healthReturn, m.screen, m.healthScroll = m.screen, healthScreen, 0 + } case "l": if m.screen == prScreen && len(m.prs) > 0 { return m, m.openSelectedPR(m.defaultPRTargetScreen()) @@ -825,6 +976,10 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.scroll = 0 } case "b", "esc": + if m.screen == healthScreen { + m.screen = m.healthReturn + return m, nil + } if m.screen == threadScreen { if m.dashboardMode == "intermediate" { m.dashboardReturn, m.screen, m.scroll, m.err = prScreen, dashboardScreen, 0, nil @@ -846,6 +1001,78 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil } +func preservePartialPRData(fresh, previous PRDetails) PRDetails { + if previous.Number == 0 || + fresh.Owner != previous.Owner || + fresh.Repository != previous.Repository || + fresh.Number != previous.Number { + return fresh + } + if fresh.HeadOID != "" && fresh.HeadOID == previous.HeadOID && + fresh.BaseOID == previous.BaseOID { + if fresh.ConflictFiles == nil { + fresh.ConflictFiles = previous.ConflictFiles + fresh.ConflictFileError = previous.ConflictFileError + } + annotations := make(map[string][]CheckAnnotation, len(previous.Checks)) + for _, check := range previous.Checks { + if len(check.Annotations) > 0 { + annotations[check.ID] = check.Annotations + } + } + for index := range fresh.Checks { + if len(fresh.Checks[index].Annotations) == 0 { + fresh.Checks[index].Annotations = annotations[fresh.Checks[index].ID] + } + } + for _, issue := range previous.DataIssues { + if issue.Component == "check annotations" || + issue.Component == "conflict file scan" { + fresh.DataIssues = append(fresh.DataIssues, issue) + } + } + } + for _, issue := range fresh.DataIssues { + switch issue.Component { + case "review threads": + if len(previous.Threads) > len(fresh.Threads) { + fresh.Threads = previous.Threads + } + case "conversation": + if len(previous.Conversation) > len(fresh.Conversation) { + fresh.Conversation = previous.Conversation + } + case "submitted reviews": + if len(previous.Reviews) > len(fresh.Reviews) { + fresh.Reviews = previous.Reviews + } + case "timeline": + if len(previous.Timeline) > len(fresh.Timeline) { + fresh.Timeline = previous.Timeline + } + case "checks": + if len(previous.Checks) > len(fresh.Checks) { + fresh.Checks = previous.Checks + } + } + } + return fresh +} + +func removeDataIssues(issues []DataIssue, components ...string) []DataIssue { + removed := make(map[string]bool, len(components)) + for _, component := range components { + removed[component] = true + } + filtered := issues[:0] + for _, issue := range issues { + if !removed[issue.Component] { + filtered = append(filtered, issue) + } + } + return filtered +} + func (m App) defaultPRTargetScreen() screen { if m.dashboardMode == "hotkey" { return threadScreen @@ -888,7 +1115,9 @@ func (m *App) trackThreadUpdates(details PRDetails) { } state.Initialized = true m.readState.Data[prID] = state - _ = m.readState.save() + if err := m.readState.save(); err != nil { + m.recordHealth("read state", healthWarning, err.Error()) + } return } for _, thread := range details.Threads { @@ -924,7 +1153,9 @@ func (m *App) markCurrentThreadRead() { state.Comments[comment.ID] = true } m.readState.Data[prID] = state - _ = m.readState.save() + if err := m.readState.save(); err != nil { + m.recordHealth("read state", healthWarning, err.Error()) + } } } @@ -1166,6 +1397,8 @@ func compareThreadTimestamps(left, right ReviewThread) (less, decided bool) { func (m *App) toStart() { if m.screen == prScreen { m.prIndex = 0 + } else if m.screen == healthScreen { + m.healthScroll = 0 } else if m.screen == dashboardScreen { m.scroll = 0 } else if m.focus == threadDetailPane { @@ -1179,6 +1412,8 @@ func (m *App) toEnd() { m.prIndex = max(0, len(m.prs)-1) } else if m.screen == dashboardScreen { m.scroll = m.dashboardMaxScroll() + } else if m.screen == healthScreen { + m.healthScroll = m.healthMaxScroll() } else if m.focus == threadDetailPane { m.scroll = m.detailMaxScroll() } else { @@ -1190,6 +1425,14 @@ func (m *App) page(direction int) { m.scrollDashboard(direction * max(3, m.dashboardViewportHeight()/2)) return } + if m.screen == healthScreen { + m.healthScroll = clamp( + m.healthScroll+direction*max(3, m.healthViewportHeight()/2), + 0, + m.healthMaxScroll(), + ) + return + } if m.screen == threadScreen && m.focus == threadDetailPane { m.scrollDetail(direction * max(3, m.detailViewportHeight()/2)) return @@ -1202,7 +1445,8 @@ func (m *App) scrollDetail(delta int) { } func (m *App) scrollDashboard(delta int) { - m.scroll = clamp(m.scroll+delta, 0, m.dashboardMaxScroll()) + maxScroll := m.dashboardMaxScroll() + m.scroll = clamp(m.scroll+delta, 0, maxScroll) } func (m App) dashboardViewportHeight() int { @@ -1266,6 +1510,9 @@ func (m App) View() string { if m.screen == dashboardScreen { return m.viewDashboard() } + if m.screen == healthScreen { + return m.viewHealth() + } return m.viewThreads() } @@ -1449,6 +1696,18 @@ func (m App) helpBindings() []helpBinding { {keyLabel(m.keybindings.General.Quit), "Quit"}, } } + if m.screen == healthScreen { + return []helpBinding{ + {keyLabel(m.keybindings.Navigation.Down), "Scroll health details down"}, + {keyLabel(m.keybindings.Navigation.Up), "Scroll health details up"}, + {combinedKeyLabel(m.keybindings.Navigation.First, m.keybindings.Navigation.Last), "Top / bottom"}, + {combinedKeyLabel(m.keybindings.Navigation.PageDown, m.keybindings.Navigation.PageUp), "Page down / up"}, + {keyLabel(m.keybindings.General.Refresh), "Refresh application data"}, + {keyLabel(m.keybindings.General.Back), "Return to the previous screen"}, + {keyLabel(m.keybindings.General.Help), "Close this help"}, + {keyLabel(m.keybindings.General.Quit), "Quit"}, + } + } if m.screen == dashboardScreen { backAction := "Return to pull requests" if m.dashboardReturn == threadScreen { @@ -1516,6 +1775,8 @@ func (m App) viewHelp() string { title := "Pull request picker keys" if m.writeMode == writePREdit || m.writeMode == writePREditConfirm { title = "Pull request editor keys" + } else if m.screen == healthScreen { + title = "Application health keys" } else if m.screen == dashboardScreen { title = "Pull request dashboard keys" } else if m.screen == threadScreen { @@ -1741,6 +2002,200 @@ func (m App) viewDashboard() string { return view } +func (m App) viewHealth() string { + lines := m.healthLines() + viewportHeight := m.healthViewportHeight() + start := clamp(m.healthScroll, 0, max(0, len(lines)-viewportHeight)) + end := min(len(lines), start+viewportHeight) + footer := fmt.Sprintf( + "%s keys • %s scroll • %s refresh • %s close", + primaryKeyLabel(m.keybindings.General.Help), + primaryCombinedKeyLabel(m.keybindings.Navigation.Down, m.keybindings.Navigation.Up), + primaryKeyLabel(m.keybindings.General.Refresh), + primaryKeyLabel(m.keybindings.General.Back), + ) + contentWidth := m.healthContentWidth() + border := lipgloss.NewStyle().Foreground(paneActiveColor) + boxLines := []string{ + border.Render("╭" + strings.Repeat("─", contentWidth) + "╮"), + border.Render("│") + pad(titleStyle.Render("Application health"), contentWidth) + border.Render("│"), + border.Render("├" + strings.Repeat("─", contentWidth) + "┤"), + } + for _, line := range lines[start:end] { + boxLines = append(boxLines, + border.Render("│")+pad(ansi.Truncate(line, contentWidth, ""), contentWidth)+border.Render("│"), + ) + } + for len(boxLines) < viewportHeight+3 { + boxLines = append(boxLines, + border.Render("│")+strings.Repeat(" ", contentWidth)+border.Render("│"), + ) + } + boxLines = append(boxLines, + border.Render("├"+strings.Repeat("─", contentWidth)+"┤"), + border.Render("│")+pad(dimStyle.Render(ansi.Truncate(footer, contentWidth, "")), contentWidth)+border.Render("│"), + border.Render("╰"+strings.Repeat("─", contentWidth)+"╯"), + ) + popup := strings.Join(boxLines, "\n") + return lipgloss.Place(m.width, m.height, lipgloss.Center, lipgloss.Center, popup) +} + +func (m App) healthMaxScroll() int { + return max(0, len(m.healthLines())-m.healthViewportHeight()) +} + +func (m App) healthContentWidth() int { + return max(18, min(96, m.width-4)) +} + +func (m App) healthViewportHeight() int { + // Border, title, divider, footer divider, footer, and closing border. + return max(1, min(28, m.height-6)) +} + +func (m App) healthLines() []string { + width := m.healthContentWidth() + lines := []string{} + components := []HealthComponent{{ + Name: "application", Level: healthOK, Summary: "interactive loop is running", + UpdatedAt: time.Now(), + }} + refresh := HealthComponent{ + Name: "refresh", Level: healthOK, Summary: "idle", + UpdatedAt: m.lastRefresh, + } + if m.loading { + refresh.Level = healthInfo + refresh.Summary = "core refresh in progress" + } else if m.secondaryLoading { + refresh.Level = healthInfo + refresh.Summary = "core data ready; secondary enrichment in progress" + } + components = append(components, refresh) + components = append(components, HealthComponent{ + Name: "configuration", Level: healthOK, Summary: "configuration loaded and validated", + }) + if m.readState != nil { + component := HealthComponent{ + Name: "read state", Level: healthOK, Summary: "persistent unread state is available", + Detail: m.readState.path, + } + if m.readState.loadErr != nil { + component.Level = healthWarning + component.Summary = "state recovery started with an empty store" + component.Detail = m.readState.loadErr.Error() + } + components = append(components, component) + } + if m.drafts != nil { + component := HealthComponent{ + Name: "draft persistence", Level: healthOK, Summary: "draft recovery is available", + Detail: m.drafts.path, + } + if m.drafts.loadErr != nil { + component.Level = healthWarning + component.Summary = "draft recovery file could not be loaded" + component.Detail = m.drafts.loadErr.Error() + } + components = append(components, component) + } + if m.details.ID != "" { + core := HealthComponent{ + Name: "PR core data", Level: healthOK, + Summary: "pull request and review data loaded", + UpdatedAt: m.details.UpdatedAt, + } + if len(m.details.DataIssues) > 0 { + core.Level = healthWarning + core.Summary = fmt.Sprintf("%d data subsection(s) are partial", len(m.details.DataIssues)) + } + components = append(components, core) + enrichment := HealthComponent{ + Name: "PR enrichment", Level: healthOK, + Summary: "annotations and conflict analysis loaded", + } + if m.secondaryLoading { + enrichment.Level = healthUnknown + enrichment.Summary = "annotations and conflict analysis are still loading" + } + if m.details.ConflictFileError != "" { + enrichment.Level = healthWarning + enrichment.Summary = "conflict-file analysis is partial" + enrichment.Detail = m.details.ConflictFileError + } + components = append(components, enrichment) + } + if m.err != nil { + components[0].Level = healthError + components[0].Summary = m.err.Error() + } + if provider, ok := m.service.(healthProvider); ok { + components = append(components, provider.HealthReport()...) + rate := provider.RateLimit() + if !rate.UpdatedAt.IsZero() { + level := healthOK + if rate.Remaining == 0 || time.Now().Before(rate.RetryAfter) { + level = healthError + } else if rate.Limit > 0 && rate.Remaining*10 < rate.Limit { + level = healthWarning + } + summary := fmt.Sprintf("%d/%d points remaining", rate.Remaining, rate.Limit) + detail := "" + if !rate.ResetAt.IsZero() { + detail = "resets " + rate.ResetAt.Local().Format("15:04:05") + } + if time.Now().Before(rate.RetryAfter) { + detail = "retry after " + rate.RetryAfter.Local().Format("15:04:05") + } + components = append(components, HealthComponent{ + Name: "rate limit", Level: level, Summary: summary, Detail: detail, + UpdatedAt: rate.UpdatedAt, + }) + } + } + sort.SliceStable(components, func(i, j int) bool { + return components[i].Name < components[j].Name + }) + for _, component := range components { + style := okStyle + if component.Level == healthWarning { + style = warnStyle + } else if component.Level == healthError { + style = badStyle + } else if component.Level == healthInfo || component.Level == healthUnknown { + style = dimStyle + } + text := healthComponentText(component) + wrapped := ansi.Hardwrap(ansi.Wordwrap(text, width, ""), width, false) + for _, line := range strings.Split(wrapped, "\n") { + lines = append(lines, style.Render(line)) + } + } + lines = append(lines, "", titleStyle.Render("Warnings and errors")) + if len(m.healthEvents) == 0 { + lines = append(lines, dimStyle.Render("No warnings or errors recorded this session.")) + } + for index := len(m.healthEvents) - 1; index >= 0; index-- { + event := m.healthEvents[index] + text := fmt.Sprintf( + "%s %-7s %s: %s", + event.At.Local().Format("15:04:05"), + healthLevelLabel(event.Level), + event.Component, + event.Message, + ) + wrapped := ansi.Hardwrap(ansi.Wordwrap(text, width, ""), width, false) + style := warnStyle + if event.Level == healthError { + style = badStyle + } + for _, line := range strings.Split(wrapped, "\n") { + lines = append(lines, style.Render(line)) + } + } + return lines +} + func (m App) dashboardLines() []string { if m.writeMode == writePREdit { return m.dashboardEditLines() @@ -1758,6 +2213,12 @@ func (m App) dashboardLines() []string { if pr.FromCache { lines = append(lines, warnStyle.Render("OFFLINE CACHE • saved "+pr.CachedAt.Local().Format("2006-01-02 15:04"))) } + if len(pr.DataIssues) > 0 { + lines = append(lines, warnStyle.Render(fmt.Sprintf( + "PARTIAL DATA • %d subsection(s) unavailable; press %s for details", + len(pr.DataIssues), primaryKeyLabel(m.keybindings.Views.Health), + ))) + } if m.loading && pr.BaseRef == "" { return append(lines, "", "Loading pull request details…") } @@ -2744,8 +3205,14 @@ func (m App) frame(lines []string, help string) string { if m.err != nil { status = badStyle.Render("error: " + truncate(m.err.Error(), max(20, m.width-8))) } - if m.loading { + // Keep an already-rendered screen byte-for-byte stable while a background + // refresh starts. Changing only this footer on a full-height alternate + // screen makes some terminals clear and repaint the entire frame. + initialLoad := m.lastRefresh.IsZero() + if status == "" && m.loading && initialLoad { status = warnStyle.Render("refreshing…") + } else if status == "" && m.secondaryLoading && initialLoad { + status = warnStyle.Render("loading details…") } if status == "" && !m.lastRefresh.IsZero() { status = dimStyle.Render("updated " + m.lastRefresh.Format("15:04:05")) diff --git a/tui_test.go b/tui_test.go index 6a372bf..a821960 100644 --- a/tui_test.go +++ b/tui_test.go @@ -1732,3 +1732,26 @@ func TestPeopleMetadataUsesColoredHandles(t *testing.T) { t.Fatal("people handles do not use the deterministic author color") } } + +func TestDashboardFrameDoesNotChangeWhenBackgroundRefreshStarts(t *testing.T) { + m := NewApp(&recordingService{}, "", "", false, 50, time.Minute) + m.screen, m.loading, m.width, m.height = dashboardScreen, false, 100, 30 + m.lastRefresh = time.Date(2026, 7, 28, 12, 0, 0, 0, time.Local) + m.details = PRDetails{ + PullRequest: PullRequest{ + ID: "pr", Owner: "o", Repository: "r", RepoWithOwner: "o/r", + Number: 1, Title: "Stable dashboard", Author: "alice", + }, + BaseRef: "main", HeadRef: "feature", + } + before := m.View() + updated, _ := m.Update(tickMsg(time.Now())) + m = updated.(App) + after := m.View() + if !m.loading { + t.Fatal("background refresh did not start") + } + if after != before { + t.Fatal("dashboard frame changed solely because a background refresh started") + } +} diff --git a/types.go b/types.go index 35ac84a..942b94d 100644 --- a/types.go +++ b/types.go @@ -39,6 +39,8 @@ type PRDetails struct { CommitCount int CommentCount int HeadOID string + BaseOID string + RepositoryURL string CheckState string Checks []Check ReviewDecision string @@ -49,12 +51,28 @@ type PRDetails struct { Threads []ReviewThread ThreadsTruncated bool Timeline []TimelineEvent + DataIssues []DataIssue Rulesets []Ruleset MergeQueue *MergeQueue FromCache bool CachedAt time.Time } +type DataIssue struct { + Component string + Message string +} + +type PRDetailsEnrichment struct { + Owner string + Repository string + Number int + HeadOID string + CheckAnnotations map[string][]CheckAnnotation + ConflictFiles []string + Issues []DataIssue +} + type PullRequestMetadata struct { Title string Body string