Fix high prio recommendations, add error / health screen

This commit is contained in:
2026-07-28 15:40:11 +02:00
parent 948f3e1a79
commit 7511311297
19 changed files with 1885 additions and 148 deletions

View File

@@ -97,6 +97,7 @@ within_status = "file" # "file" or "timestamp" (oldest first)
enabled = true # instant stale view plus offline fallback enabled = true # instant stale view plus offline fallback
max_age = "168h" # 7 days; 0 means no age limit max_age = "168h" # 7 days; 0 means no age limit
directory = "" # defaults to the OS user cache directory directory = "" # defaults to the OS user cache directory
max_entries = 200 # bounded oldest-first pruning; 10-10000
[editing] [editing]
mode = "vim" # "vim" or "standard"; description field only for now mode = "vim" # "vim" or "standard"; description field only for now
@@ -123,6 +124,7 @@ page_up = ["ctrl+u", "pgup"]
[keybindings.views] [keybindings.views]
open = ["enter", "l"] open = ["enter", "l"]
dashboard = ["d"] dashboard = ["d"]
health = ["H"]
edit = ["e"] edit = ["e"]
toggle_list = ["tab"] 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 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 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 screens are labelled with their save time and are replaced automatically when
fresh data arrives. Check annotations are fetched separately only for failed fresh data arrives. Core PR and review data is rendered before check
checks so they do not inflate the initial PR query. 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 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 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 files. Their modification time is touched at most once per day (or half the
configured maximum age, when shorter) so recently validated snapshots remain 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 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 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 | | `h` / `l` | Focus the thread list / thread detail |
| `j` / `k` | Move between items or scroll the dashboard/focused detail | | `j` / `k` | Move between items or scroll the dashboard/focused detail |
| `?` | Show contextual keybinding help | | `?` | Show contextual keybinding help |
| `H` | Open application health and diagnostics |
| `d` | Open the current pull request dashboard | | `d` | Open the current pull request dashboard |
| `e` | Edit the current PR title, target branch, and description from its dashboard | | `e` | Edit the current PR title, target branch, and description from its dashboard |
| `/` | Fuzzy-search paths and filter with `status:`, `author:`, `updated:true` | | `/` | Fuzzy-search paths and filter with `status:`, `author:`, `updated:true` |
@@ -258,6 +272,15 @@ history and metadata.
| `r` | Refresh now | | `r` | Refresh now |
| `q` | Quit | | `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 The reply composer appears inline beneath the selected thread so its code and
comments remain visible while writing. It supports multiple lines: `enter` comments remain visible while writing. It supports multiple lines: `enter`
inserts a newline, `ctrl-s` opens the rendered confirmation preview, and `esc` inserts a newline, `ctrl-s` opens the rendered confirmation preview, and `esc`

152
TODO.md
View File

@@ -1,42 +1,126 @@
# TODO # TODO
The read-only model now includes paginated review threads and comments, PR This list reflects the current implementation: paginated review threads,
conversation comments, submitted reviews, individual checks, branch-protection thread comments, conversation comments, reviews, timeline events, checks, and
requirements, rulesets, merge queues, deployment requirements, viewer annotations; cached read-only snapshots; persistent unread state; contextual
capabilities, head SHA, persistent unread updates, offline cache fallback, keybindings; thread replies and resolution changes; and pull-request metadata
commit/force-push history, and paginated checks and annotations. editing are already implemented.
The following read-only improvements remain before or alongside write support.
## Workflow and navigation ## Completed resilience work
- Open the current PR, comment, review, check, or source location in a browser. - Refreshes render core data before annotations and conflict analysis. A
- Copy URLs, commit SHAs, file paths, and rendered comment text to the failed paginated subsection is marked partial and keeps the last complete
clipboard. value while other sections continue updating.
- Add navigation for the next thread by status or author, not only the next - Polling uses GitHub rate-limit and retry headers, backs off at low remaining
unread update. budgets, and adds jitter. Failed-check annotations are cached by immutable
- Remember the selected PR, thread, scroll position, pane width, and folded check ID.
state between runs. - Superseded list, detail, and enrichment requests are canceled as newer
- Audit screen-reader behavior beyond the new no-color and high-contrast modes. 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 - Open the current PR, thread comment, submitted review, check, annotation,
reply relationships. commit, or source location in a browser.
- Show rate-limit remaining/reset information and distinguish it from network - Copy URLs, commit SHAs, file paths, branch names, rendered comment text, and
or permission failures. raw Markdown through explicit contextual actions.
- Support retrying individual failed pages while retaining clearly marked - Add a dedicated changed-files/check-details view. It should make the complete
partial data, rather than failing the entire refresh. PR diff and check annotations inspectable even when no review thread exists
- Add fixture coverage for GitHub Enterprise Server schema differences, team at that location.
reviewers, deleted users, very large PRs, and mixed legacy status contexts. - 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 - Paginate or explicitly mark truncation for the remaining fixed-size
selector must support adding, removing, and clearing values with explicit connections: assignees, labels, review requests, latest reviews, repository
confirmation. rulesets, and rules within a ruleset.
- Fetch per-comment update/delete permissions. - Model pending reviews, minimized comments, deleted comments/users, edited
- Define head-SHA conflict handling for comments and suggestions composed timestamps, and explicit reply relationships.
against an older revision. - Preserve enough team-reviewer identity to distinguish teams with the same
- Design confirmation and optimistic-update behavior for submitting reviews display name and to generate a correct browser target.
and applying suggestions. Thread replies and resolution changes now use - Represent partial permissions per comment and conversation item, not only
confirmed server responses. 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.

187
cache.go
View File

@@ -9,19 +9,23 @@ import (
"fmt" "fmt"
"os" "os"
"path/filepath" "path/filepath"
"sort"
"time" "time"
) )
type cacheEnvelope[T any] struct { type cacheEnvelope[T any] struct {
Version int `json:"version,omitempty"`
SavedAt time.Time `json:"saved_at"` SavedAt time.Time `json:"saved_at"`
ContentHash string `json:"content_hash,omitempty"` ContentHash string `json:"content_hash,omitempty"`
Value T `json:"value"` Value T `json:"value"`
} }
type CachedGitHubService struct { type CachedGitHubService struct {
remote GitHubService remote GitHubService
dir string dir string
maxAge time.Duration maxAge time.Duration
maxEntries int
health healthTracker
} }
type cachedSnapshotService interface { type cachedSnapshotService interface {
@@ -34,8 +38,18 @@ type liveGitHubService interface {
LivePullRequest(context.Context, string, string, int) (PRDetails, error) LivePullRequest(context.Context, string, string, int) (PRDetails, error)
} }
func NewCachedGitHubService(remote GitHubService, dir string, maxAge time.Duration) *CachedGitHubService { const cacheSchemaVersion = 1
return &CachedGitHubService{remote: remote, dir: dir, maxAge: maxAge}
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( func (c *CachedGitHubService) CachedPullRequests(
@@ -157,7 +171,37 @@ func (c *CachedGitHubService) ListBranches(
if !ok { if !ok {
return nil, errors.New("GitHub service does not support listing branches") 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 { 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) 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 { func (c *CachedGitHubService) file(key string) string {
sum := sha256.Sum256([]byte(key)) sum := sha256.Sum256([]byte(key))
return filepath.Join(c.dir, hex.EncodeToString(sum[:])+".json") 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 { if err := os.MkdirAll(c.dir, 0o700); err != nil {
return err return err
} }
@@ -198,7 +257,8 @@ func (c *CachedGitHubService) write(key string, value any) error {
} }
} }
data, err := json.Marshal(cacheEnvelope[any]{ 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 { if err != nil {
return err return err
@@ -220,7 +280,45 @@ func (c *CachedGitHubService) write(key string, value any) error {
if err := temp.Close(); err != nil { if err := temp.Close(); err != nil {
return err 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 { func (c *CachedGitHubService) cacheTouchInterval() time.Duration {
@@ -231,7 +329,20 @@ func (c *CachedGitHubService) cacheTouchInterval() time.Duration {
return interval 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) path := c.file(key)
data, err := os.ReadFile(path) data, err := os.ReadFile(path)
if err != nil { if err != nil {
@@ -241,11 +352,17 @@ func (c *CachedGitHubService) read(key string, target any) (time.Time, error) {
return time.Time{}, err return time.Time{}, err
} }
var metadata struct { var metadata struct {
Version int `json:"version"`
SavedAt time.Time `json:"saved_at"` SavedAt time.Time `json:"saved_at"`
} }
if err := json.Unmarshal(data, &metadata); err != nil { if err := json.Unmarshal(data, &metadata); err != nil {
return time.Time{}, err 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 savedAt := metadata.SavedAt
if info, statErr := os.Stat(path); statErr == nil && info.ModTime().After(savedAt) { if info, statErr := os.Stat(path); statErr == nil && info.ModTime().After(savedAt) {
savedAt = info.ModTime() savedAt = info.ModTime()
@@ -256,9 +373,32 @@ func (c *CachedGitHubService) read(key string, target any) (time.Time, error) {
return savedAt, nil 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 { type readStateStore struct {
path string path string
Data map[string]readPRState `json:"pull_requests"` 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 { type readPRState struct {
@@ -271,7 +411,20 @@ func loadReadState(path string) *readStateStore {
store := &readStateStore{path: path, Data: make(map[string]readPRState)} store := &readStateStore{path: path, Data: make(map[string]readPRState)}
data, err := os.ReadFile(path) data, err := os.ReadFile(path)
if err == nil { 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 return store
} }
@@ -283,9 +436,7 @@ func (s *readStateStore) save() error {
if err := os.MkdirAll(filepath.Dir(s.path), 0o700); err != nil { if err := os.MkdirAll(filepath.Dir(s.path), 0o700); err != nil {
return err return err
} }
data, err := json.MarshalIndent(s.Data, "", " ") return atomicWriteJSON(s.path, readStateEnvelope{
if err != nil { Version: readStateSchemaVersion, PullRequests: s.Data,
return err }, 0o600)
}
return os.WriteFile(s.path, data, 0o600)
} }

View File

@@ -3,9 +3,11 @@ package main
import ( import (
"bytes" "bytes"
"context" "context"
"encoding/json"
"errors" "errors"
"os" "os"
"path/filepath" "path/filepath"
"strings"
"testing" "testing"
"time" "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) { func TestCachedPickerSnapshotStaysVisibleWhileLiveRefreshContinues(t *testing.T) {
m := NewApp(nil, "", "", false, 50, time.Second) m := NewApp(nil, "", "", false, 50, time.Second)
m.loading = true m.loading = true
@@ -189,3 +202,47 @@ func TestCacheDoesNotRewriteUnchangedContent(t *testing.T) {
t.Fatal("changed cache content was not persisted") 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)
}
}

View File

@@ -57,9 +57,10 @@ type ThreadConfig struct {
} }
type CacheConfig struct { type CacheConfig struct {
Enabled bool `toml:"enabled"` Enabled bool `toml:"enabled"`
MaxAge configDuration `toml:"max_age"` MaxAge configDuration `toml:"max_age"`
Directory string `toml:"directory"` Directory string `toml:"directory"`
MaxEntries int `toml:"max_entries"`
} }
type EditingConfig struct { type EditingConfig struct {
@@ -86,7 +87,9 @@ func defaultConfig() Config {
StatusOrder: []string{"unresolved", "outdated", "resolved"}, StatusOrder: []string{"unresolved", "outdated", "resolved"},
WithinStatus: "file", 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"}, Editing: EditingConfig{Mode: "vim"},
KeyBindings: defaultKeyBindings(), KeyBindings: defaultKeyBindings(),
} }
@@ -178,6 +181,9 @@ func validateConfig(config Config) error {
if config.Cache.MaxAge.Duration < 0 { if config.Cache.MaxAge.Duration < 0 {
return fmt.Errorf("cache.max_age must not be negative") 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 { switch config.Editing.Mode {
case "standard", "vim": case "standard", "vim":
default: default:

208
drafts.go Normal file
View File

@@ -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)
}

229
github.go
View File

@@ -10,6 +10,7 @@ import (
"net/http" "net/http"
"path" "path"
"sort" "sort"
"strconv"
"strings" "strings"
"sync" "sync"
"time" "time"
@@ -33,22 +34,30 @@ type GitHubBranchService interface {
ListBranches(context.Context, string, string) ([]RepositoryBranch, error) ListBranches(context.Context, string, string) ([]RepositoryBranch, error)
} }
type GitHubEnrichmentService interface {
EnrichPullRequest(context.Context, PRDetails) PRDetailsEnrichment
}
type GitHubClient struct { type GitHubClient struct {
endpoint string endpoint string
token string token string
http *http.Client http *http.Client
conflicts conflictFileLoader conflicts conflictFileLoader
conflictMu sync.Mutex conflictMu sync.Mutex
conflictCache map[string]conflictFileResult conflictCache map[string]conflictFileResult
health healthTracker
annotationMu sync.Mutex
annotationCache map[string][]CheckAnnotation
} }
func NewGitHubClient(endpoint, token string) *GitHubClient { func NewGitHubClient(endpoint, token string) *GitHubClient {
return &GitHubClient{ return &GitHubClient{
endpoint: endpoint, endpoint: endpoint,
token: token, token: token,
http: &http.Client{Timeout: 20 * time.Second}, http: &http.Client{Timeout: 20 * time.Second},
conflicts: analyzeConflictFiles, conflicts: analyzeConflictFiles,
conflictCache: make(map[string]conflictFileResult), conflictCache: make(map[string]conflictFileResult),
annotationCache: make(map[string][]CheckAnnotation),
} }
} }
@@ -66,7 +75,21 @@ type graphQLResponse[T any] struct {
Errors []graphQLError `json:"errors"` 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}) payload, err := json.Marshal(graphQLRequest{Query: query, Variables: variables})
if err != nil { if err != nil {
return fmt.Errorf("encode GraphQL request: %w", err) 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) return fmt.Errorf("GitHub request: %w", err)
} }
defer resp.Body.Close() defer resp.Body.Close()
c.recordRateLimit(resp)
body, err := io.ReadAll(io.LimitReader(resp.Body, 32<<20)) body, err := io.ReadAll(io.LimitReader(resp.Body, 32<<20))
if err != nil { if err != nil {
return fmt.Errorf("read GitHub response: %w", err) 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 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 = ` const listPRsQuery = `
query PullRequests($query: String!, $first: Int!, $after: String) { query PullRequests($query: String!, $first: Int!, $after: String) {
viewer { login } viewer { login }
@@ -937,16 +995,6 @@ func (c *GitHubClient) allCheckContexts(
connection = data.Node.Contexts connection = data.Node.Contexts
nodes = append(nodes, connection.Nodes...) 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 return nodes, nil
} }
@@ -1034,8 +1082,6 @@ func (c *GitHubClient) GetPullRequest(ctx context.Context, owner, name string, n
reviewErr error reviewErr error
timelineErr error timelineErr error
checkErr error checkErr error
conflictFiles []string
conflictFileErr error
wait sync.WaitGroup wait sync.WaitGroup
) )
wait.Add(4) 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) 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() wait.Wait()
for _, err := range []error{threadErr, conversationErr, reviewErr, timelineErr, checkErr} { if threadErr != nil {
if err != nil { threadNodes = append([]githubReviewThread(nil), node.ReviewThreads.Nodes...)
return PRDetails{}, err }
} 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{ details := PRDetails{
PullRequest: PullRequest{ 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, Body: node.Body, CreatedAt: node.CreatedAt, BaseRef: node.BaseRefName, HeadRef: node.HeadRefName,
HeadOID: node.HeadRefOID, Mergeable: node.Mergeable, MergeState: node.MergeStateStatus, HeadOID: node.HeadRefOID, Mergeable: node.Mergeable, MergeState: node.MergeStateStatus,
ConflictFiles: conflictFiles, RepositoryURL: data.Repository.URL,
Additions: node.Additions, Deletions: node.Deletions, ChangedFiles: node.ChangedFiles, Additions: node.Additions, Deletions: node.Deletions, ChangedFiles: node.ChangedFiles,
CommitCount: node.Commits.TotalCount, CommentCount: node.Comments.TotalCount, CommitCount: node.Commits.TotalCount, CommentCount: node.Comments.TotalCount,
CheckState: "NONE", ReviewDecision: node.ReviewDecision, 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, CanSubscribe: node.ViewerCanSubscribe, CanEnableMerge: node.ViewerCanEnableAutoMerge,
}, },
} }
if conflictFileErr != nil { if node.BaseRef != nil && node.BaseRef.Target != nil {
details.ConflictFileError = conflictFileErr.Error() 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 { if node.BaseRef != nil && node.BaseRef.BranchProtectionRule != nil {
rule := node.BaseRef.BranchProtectionRule rule := node.BaseRef.BranchProtectionRule
@@ -1228,6 +1285,90 @@ func (c *GitHubClient) GetPullRequest(ctx context.Context, owner, name string, n
return details, nil 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( func (c *GitHubClient) SetThreadResolved(
ctx context.Context, threadID string, resolved bool, ctx context.Context, threadID string, resolved bool,
) (ReviewThread, error) { ) (ReviewThread, error) {

View File

@@ -6,11 +6,41 @@ import (
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"reflect" "reflect"
"strconv"
"strings" "strings"
"testing" "testing"
"time" "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) { func TestListBranchesPaginatesAndMarksTheDefaultBranch(t *testing.T) {
requests := 0 requests := 0
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -304,9 +334,13 @@ func TestCheckContextsAndAnnotationsArePaginated(t *testing.T) {
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
if len(nodes) != 2 || len(nodes[1].Annotations.Nodes) != 2 || annotations, err := client.checkAnnotations(context.Background(), nodes[1].ID)
nodes[1].Annotations.Nodes[0].Location.Start.Line != 4 || if err != nil {
nodes[1].Annotations.Nodes[1].Location.End.Line != 8 { 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) t.Fatalf("paginated checks = %#v", nodes)
} }
} }
@@ -513,7 +547,11 @@ func TestGetPullRequestLoadsConflictFilesForConflictingPR(t *testing.T) {
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
if !reflect.DeepEqual(got.ConflictFiles, []string{"src/conflict.go", "README.md"}) { if len(got.ConflictFiles) != 0 {
t.Fatalf("conflict files = %#v", got.ConflictFiles) 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)
} }
} }

157
health.go Normal file
View File

@@ -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)
}

201
health_test.go Normal file
View File

@@ -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)
}
}

View File

@@ -38,6 +38,7 @@ type NavigationKeyBindings struct {
type ViewKeyBindings struct { type ViewKeyBindings struct {
Open []string `toml:"open"` Open []string `toml:"open"`
Dashboard []string `toml:"dashboard"` Dashboard []string `toml:"dashboard"`
Health []string `toml:"health"`
Edit []string `toml:"edit"` Edit []string `toml:"edit"`
ToggleList []string `toml:"toggle_list"` ToggleList []string `toml:"toggle_list"`
} }
@@ -117,7 +118,7 @@ func defaultKeyBindings() KeyBindings {
}, },
Views: ViewKeyBindings{ Views: ViewKeyBindings{
Open: []string{"enter", "l"}, Dashboard: []string{"d"}, Open: []string{"enter", "l"}, Dashboard: []string{"d"},
Edit: []string{"e"}, ToggleList: []string{"tab"}, Health: []string{"H"}, Edit: []string{"e"}, ToggleList: []string{"tab"},
}, },
Threads: ThreadKeyBindings{ Threads: ThreadKeyBindings{
Search: []string{"/"}, ClearFilter: []string{"F"}, Search: []string{"/"}, ClearFilter: []string{"F"},
@@ -322,6 +323,8 @@ func (k KeyBindings) canonicalMainKey(key string, current screen) string {
switch { switch {
case keyMatches(key, k.Views.Dashboard): case keyMatches(key, k.Views.Dashboard):
return "d" return "d"
case keyMatches(key, k.Views.Health):
return "H"
case keyMatches(key, k.Views.Edit): case keyMatches(key, k.Views.Edit):
return "e" return "e"
default: default:
@@ -383,7 +386,8 @@ func validateKeyBindings(bindings KeyBindings) error {
}}, }},
{"keybindings.views", map[string][]string{ {"keybindings.views", map[string][]string{
"open": bindings.Views.Open, "dashboard": bindings.Views.Dashboard, "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{ {"keybindings.threads", map[string][]string{
"search": bindings.Threads.Search, "clear_filter": bindings.Threads.ClearFilter, "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}, {"back", general.Back}, {"down", navigation.Down}, {"up", navigation.Up},
{"first", navigation.First}, {"last", navigation.Last}, {"first", navigation.First}, {"last", navigation.Last},
{"page_down", navigation.PageDown}, {"page_up", navigation.PageUp}, {"page_down", navigation.PageDown}, {"page_up", navigation.PageUp},
{"health", views.Health},
} }
if err := validateKeyContext("pull request list", append(screenCommon, if err := validateKeyContext("pull request list", append(screenCommon,
contextBinding{"open", views.Open}, contextBinding{"open", views.Open},
@@ -476,6 +481,9 @@ func validateKeyBindingContexts(bindings KeyBindings) error {
)...); err != nil { )...); err != nil {
return err return err
} }
if err := validateKeyContext("health screen", screenCommon...); err != nil {
return err
}
if err := validateKeyContext("review threads", append(screenCommon, if err := validateKeyContext("review threads", append(screenCommon,
contextBinding{"left", navigation.Left}, contextBinding{"left", navigation.Left},
contextBinding{"right", navigation.Right}, contextBinding{"right", navigation.Right},

View File

@@ -123,9 +123,12 @@ func main() {
exitf("configuration: %v", err) 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") statePath := filepath.Join(filepath.Dir(*configFile), "state.json")
draftPath := filepath.Join(filepath.Dir(*configFile), "drafts.json")
app := NewAppWithSettings( app := NewAppWithSettings(
service, owner, name, config.ShowAll, config.Limit, config.RefreshInterval.Duration, service, owner, name, config.ShowAll, config.Limit, config.RefreshInterval.Duration,
AppSettings{ AppSettings{
@@ -134,6 +137,7 @@ func main() {
DashboardMode: config.Display.DashboardMode, DashboardMode: config.Display.DashboardMode,
CompactReviews: config.Display.CompactReviews, CompactReviews: config.Display.CompactReviews,
ReadState: loadReadState(statePath), ReadState: loadReadState(statePath),
Drafts: loadDraftStore(draftPath),
PathScroll: config.Paths.Scroll, PathScroll: config.Paths.Scroll,
PathScrollInterval: config.Paths.ScrollInterval.Duration, PathScrollInterval: config.Paths.ScrollInterval.Duration,
ThreadStatusOrder: config.Threads.StatusOrder, ThreadStatusOrder: config.Threads.StatusOrder,

53
persistence.go Normal file
View File

@@ -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
}

View File

@@ -32,11 +32,12 @@ func (m *App) startPREdit() tea.Cmd {
m.editorMode == "vim", m.editorMode == "vim",
) )
m.prEditEditors[prEditBodyField].highlightMarkdown = true m.prEditEditors[prEditBodyField].highlightMarkdown = true
m.prEditOriginal = m.currentPRMetadata()
m.restorePREditDraft()
for index := range m.prEditEditors { for index := range m.prEditEditors {
m.prEditEditors[index].hardwareCursor = m.cursorOutput != nil m.prEditEditors[index].hardwareCursor = m.cursorOutput != nil
m.prEditEditors[index].keys = m.keybindings m.prEditEditors[index].keys = m.keybindings
} }
m.prEditOriginal = m.currentPRMetadata()
m.prEditBranches = nil m.prEditBranches = nil
m.prEditBranchesLoading = false m.prEditBranchesLoading = false
m.prEditBranchesError = "" m.prEditBranchesError = ""
@@ -206,7 +207,7 @@ func (m App) updatePREditInput(key tea.KeyMsg) (tea.Model, tea.Cmd) {
} }
m.err = nil m.err = nil
m.ensurePREditCursorVisible() m.ensurePREditCursorVisible()
return m, nil return m, m.queuePREditDraft()
} }
func (m App) prEditEditorWidth() int { func (m App) prEditEditorWidth() int {

View File

@@ -3,10 +3,12 @@ package main
import ( import (
"strings" "strings"
"unicode" "unicode"
"unicode/utf8"
tea "github.com/charmbracelet/bubbletea" tea "github.com/charmbracelet/bubbletea"
"github.com/charmbracelet/lipgloss" "github.com/charmbracelet/lipgloss"
"github.com/charmbracelet/x/ansi" "github.com/charmbracelet/x/ansi"
"github.com/rivo/uniseg"
) )
type textEditorMode string type textEditorMode string
@@ -69,7 +71,7 @@ func (e *textEditor) handleKeyAtWidth(key tea.KeyMsg, multiline bool, wrapWidth
e.clearPending() e.clearPending()
start, _ := editorLineBounds(e.Text, e.Cursor, wrapWidth) start, _ := editorLineBounds(e.Text, e.Cursor, wrapWidth)
if e.Cursor > start { if e.Cursor > start {
e.Cursor-- e.Cursor = max(start, previousGraphemeBoundary(e.Text, e.Cursor))
} }
return true return true
} }
@@ -92,10 +94,10 @@ func (e *textEditor) handleStandardKey(key tea.KeyMsg, multiline bool, wrapWidth
switch { switch {
case key.Type != tea.KeyRunes && key.Type != tea.KeySpace && case key.Type != tea.KeyRunes && key.Type != tea.KeySpace &&
keyMatches(k, e.keys.Navigation.Left): 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 && case key.Type != tea.KeyRunes && key.Type != tea.KeySpace &&
keyMatches(k, e.keys.Navigation.Right): 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 && case key.Type != tea.KeyRunes && key.Type != tea.KeySpace &&
keyMatches(k, e.keys.Navigation.Up): keyMatches(k, e.keys.Navigation.Up):
if !multiline { if !multiline {
@@ -130,6 +132,7 @@ func (e *textEditor) handleStandardKey(key tea.KeyMsg, multiline bool, wrapWidth
return false return false
} }
} }
e.Cursor = previousOrCurrentGraphemeBoundary(e.Text, e.Cursor)
return true return true
} }
@@ -166,7 +169,7 @@ func (e *textEditor) handleNormalKey(key tea.KeyMsg, multiline bool, wrapWidth i
e.Mode = textEditorInsert e.Mode = textEditorInsert
case keyMatches(k, e.keys.Vim.Append): case keyMatches(k, e.keys.Vim.Append):
_, end := editorLineBounds(e.Text, e.Cursor, wrapWidth) _, 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 e.Mode = textEditorInsert
case keyMatches(k, e.keys.Vim.InsertLineStart): case keyMatches(k, e.keys.Vim.InsertLineStart):
e.Cursor = firstNonBlankAtWidth(e.Text, e.Cursor, wrapWidth) 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 e.Mode = textEditorInsert
case keyMatches(k, e.keys.Navigation.Left): case keyMatches(k, e.keys.Navigation.Left):
start, _ := editorLineBounds(e.Text, e.Cursor, wrapWidth) 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): 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): case keyMatches(k, e.keys.Navigation.Down):
if multiline { if multiline {
e.Cursor = moveEditorCursorLine(e.Text, e.Cursor, 1, wrapWidth, true) 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: default:
return false return false
} }
e.Cursor = previousOrCurrentGraphemeBoundary(e.Text, e.Cursor)
return true return true
} }
@@ -347,7 +354,9 @@ func (e textEditor) selectionBounds(wrapWidth int) (int, int, bool) {
anchor := clamp(e.visualAnchor, 0, len(runes)) anchor := clamp(e.visualAnchor, 0, len(runes))
cursor := clamp(e.Cursor, 0, len(runes)) cursor := clamp(e.Cursor, 0, len(runes))
if !e.visualLine { 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 return start, end, end > start
} }
anchorStart, anchorEnd := editorLineBounds(e.Text, anchor, wrapWidth) 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.Text = string(append(runes[:start], runes[end:]...))
e.Cursor = min(start, len([]rune(e.Text))) e.Cursor = min(start, len([]rune(e.Text)))
if e.Cursor == len([]rune(e.Text)) && e.Cursor > 0 { if e.Cursor == len([]rune(e.Text)) && e.Cursor > 0 {
e.Cursor-- e.Cursor = previousGraphemeBoundary(e.Text, e.Cursor)
} }
e.stopVisual() e.stopVisual()
} }
@@ -411,11 +420,11 @@ func (e *textEditor) pasteClipboard(replaceSelection bool, wrapWidth int) {
e.stopVisual() e.stopVisual()
} else { } else {
_, end := editorLineBounds(e.Text, e.Cursor, 0) _, 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) e.insert(value)
if e.Cursor > 0 { 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 { if e.Cursor == 0 {
return return
} }
value = append(value[:e.Cursor-1], value[e.Cursor:]...) start := previousGraphemeBoundary(e.Text, e.Cursor)
e.Cursor-- value = append(value[:start], value[e.Cursor:]...)
e.Cursor = start
e.Text = string(value) e.Text = string(value)
} }
@@ -453,10 +463,56 @@ func (e *textEditor) deleteAt() {
if e.Cursor == len(value) { if e.Cursor == len(value) {
return 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) 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) { func (e *textEditor) performFind(command, target rune, remember bool, wrapWidth int) {
runes := []rune(e.Text) runes := []rune(e.Text)
cursor := clamp(e.Cursor, 0, len(runes)) cursor := clamp(e.Cursor, 0, len(runes))

View File

@@ -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) { func TestVimTextEditorNormalMotionsStayOnCharactersWithinLine(t *testing.T) {
editor := newTextEditor("ab\n cd\n", true) editor := newTextEditor("ab\n cd\n", true)
editor.Cursor = 1 editor.Cursor = 1

511
tui.go
View File

@@ -21,6 +21,7 @@ const (
prScreen screen = iota prScreen screen = iota
dashboardScreen dashboardScreen
threadScreen threadScreen
healthScreen
) )
type pane int type pane int
@@ -65,17 +66,19 @@ type pullRequestUpdatedMsg struct {
} }
type prsLoadedMsg struct { type prsLoadedMsg struct {
prs []PullRequest prs []PullRequest
err error err error
cached bool cached bool
requestID uint64
} }
type detailsLoadedMsg struct { type detailsLoadedMsg struct {
owner string owner string
repo string repo string
number int number int
details PRDetails details PRDetails
err error err error
cached bool cached bool
requestID uint64
} }
type branchesLoadedMsg struct { type branchesLoadedMsg struct {
@@ -85,6 +88,11 @@ type branchesLoadedMsg struct {
err error err error
} }
type detailsEnrichedMsg struct {
enrichment PRDetailsEnrichment
requestID uint64
}
type App struct { type App struct {
service GitHubService service GitHubService
owner, repo string owner, repo string
@@ -93,6 +101,10 @@ type App struct {
poll time.Duration poll time.Duration
screen screen screen screen
healthReturn screen
healthScroll int
healthEvents []HealthEvent
requests *requestCoordinator
prs []PullRequest prs []PullRequest
prIndex int prIndex int
details PRDetails details PRDetails
@@ -103,6 +115,7 @@ type App struct {
scroll int scroll int
width, height int width, height int
loading bool loading bool
secondaryLoading bool
err error err error
lastRefresh time.Time lastRefresh time.Time
pendingZ bool pendingZ bool
@@ -137,6 +150,7 @@ type App struct {
editorMode string editorMode string
keybindings KeyBindings keybindings KeyBindings
readState *readStateStore readState *readStateStore
drafts *draftStore
knownThreads map[string]bool knownThreads map[string]bool
knownComments map[string]bool knownComments map[string]bool
initializedPRs map[string]bool initializedPRs map[string]bool
@@ -156,6 +170,7 @@ type AppSettings struct {
EditorMode string EditorMode string
KeyBindings KeyBindings KeyBindings KeyBindings
ReadState *readStateStore ReadState *readStateStore
Drafts *draftStore
} }
func defaultAppSettings() AppSettings { func defaultAppSettings() AppSettings {
@@ -196,10 +211,13 @@ func NewAppWithSettings(
threadStatusOrder: append([]string(nil), settings.ThreadStatusOrder...), threadStatusOrder: append([]string(nil), settings.ThreadStatusOrder...),
threadWithinStatus: settings.ThreadWithinStatus, threadWithinStatus: settings.ThreadWithinStatus,
dashboardMode: settings.DashboardMode, dashboardReturn: prScreen, dashboardMode: settings.DashboardMode, dashboardReturn: prScreen,
healthReturn: prScreen,
requests: &requestCoordinator{},
compactReviews: settings.CompactReviews, compactReviews: settings.CompactReviews,
editorMode: settings.EditorMode, editorMode: settings.EditorMode,
keybindings: settings.KeyBindings, keybindings: settings.KeyBindings,
readState: state, readState: state,
drafts: settings.Drafts,
knownThreads: make(map[string]bool), knownComments: make(map[string]bool), knownThreads: make(map[string]bool), knownComments: make(map[string]bool),
initializedPRs: make(map[string]bool), unreadThreads: make(map[string]bool), initializedPRs: make(map[string]bool), unreadThreads: make(map[string]bool),
updatedThreads: 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 { 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 { func (m App) nextPathTick() tea.Cmd {
@@ -235,7 +279,7 @@ func (m App) loadPRs(useCache bool) tea.Cmd {
func (m App) loadLivePRs() tea.Cmd { func (m App) loadLivePRs() tea.Cmd {
return func() tea.Msg { return func() tea.Msg {
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) ctx, cancel, requestID := m.requests.start(20 * time.Second)
defer cancel() defer cancel()
var prs []PullRequest var prs []PullRequest
var err error var err error
@@ -244,7 +288,7 @@ func (m App) loadLivePRs() tea.Cmd {
} else { } else {
prs, err = m.service.ListPullRequests(ctx, m.owner, m.repo, m.limit, m.showAll) 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 { func (m App) loadLiveDetails(pr PullRequest) tea.Cmd {
return func() tea.Msg { return func() tea.Msg {
ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) ctx, cancel, requestID := m.requests.start(60 * time.Second)
defer cancel() defer cancel()
var details PRDetails var details PRDetails
var err error var err error
@@ -275,7 +319,21 @@ func (m App) loadLiveDetails(pr PullRequest) tea.Cmd {
} }
return detailsLoadedMsg{ return detailsLoadedMsg{
owner: pr.Owner, repo: pr.Repository, number: pr.Number, 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 return
} }
m.writeMode, m.writeThreadID, m.replyDraft, m.err = writeReply, thread.ID, "", nil m.writeMode, m.writeThreadID, m.replyDraft, m.err = writeReply, thread.ID, "", nil
m.restoreReplyDraft(thread.ID)
m.folded[thread.ID] = false m.folded[thread.ID] = false
m.focus = threadDetailPane m.focus = threadDetailPane
m.scroll = m.detailMaxScroll() m.scroll = m.detailMaxScroll()
@@ -365,6 +424,7 @@ func (m App) updateWriteInput(key tea.KeyMsg) (tea.Model, tea.Cmd) {
} }
if m.writeMode == writeReply { if m.writeMode == writeReply {
m.scroll = m.detailMaxScroll() m.scroll = m.detailMaxScroll()
return m, m.queueReplyDraft()
} }
case writeReplyConfirm: case writeReplyConfirm:
switch k { switch k {
@@ -418,7 +478,11 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
case tickMsg: case tickMsg:
if !m.loading { if !m.loading {
m.loading = true 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.loadDetails(m.details.PullRequest, false), m.nextTick())
} }
return m, tea.Batch(m.loadPRs(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 return m, nil
case prsLoadedMsg: 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 return m, nil
} }
if !msg.cached { if !msg.cached {
@@ -441,9 +509,11 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
} }
if msg.err != nil { if msg.err != nil {
if msg.cached { if msg.cached {
m.recordHealth("pull request cache", healthWarning, msg.err.Error())
return m, nil return m, nil
} }
m.err = msg.err m.err = msg.err
m.recordHealth("pull request list", healthError, msg.err.Error())
return m, nil return m, nil
} }
selected := "" selected := ""
@@ -466,7 +536,12 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
m.lastRefresh = time.Now() m.lastRefresh = time.Now()
} }
case detailsLoadedMsg: 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 return m, nil
} }
if msg.cached && !m.loading { 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.err != nil {
if msg.cached { if msg.cached {
m.recordHealth("PR cache", healthWarning, msg.err.Error())
return m, nil return m, nil
} }
m.err = msg.err m.err = msg.err
m.recordHealth("PR refresh", healthError, msg.err.Error())
return m, nil return m, nil
} }
selected := "" selected := ""
@@ -492,9 +569,16 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
selected = m.details.Threads[m.threadIndex].ID selected = m.details.Threads[m.threadIndex].ID
anchor = m.detailScrollAnchor() anchor = m.detailScrollAnchor()
} }
msg.details = preservePartialPRData(msg.details, m.details)
m.trackThreadUpdates(msg.details) m.trackThreadUpdates(msg.details)
sortReviewThreads(msg.details.Threads, m.threadStatusOrder, m.threadWithinStatus) sortReviewThreads(msg.details.Threads, m.threadStatusOrder, m.threadWithinStatus)
m.details = msg.details 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) m.threadIndex = indexThread(m.details.Threads, selected)
if selected != "" && (len(m.details.Threads) == 0 || m.details.Threads[m.threadIndex].ID != selected) { if selected != "" && (len(m.details.Threads) == 0 || m.details.Threads[m.threadIndex].ID != selected) {
m.scroll = 0 m.scroll = 0
@@ -518,6 +602,45 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
m.lastRefresh = msg.details.CachedAt m.lastRefresh = msg.details.CachedAt
} else { } else {
m.lastRefresh = time.Now() 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: case branchesLoadedMsg:
if m.writeMode != writePREdit || if m.writeMode != writePREdit ||
@@ -527,6 +650,7 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
m.prEditBranchesLoading = false m.prEditBranchesLoading = false
if msg.err != nil { if msg.err != nil {
m.prEditBranchesError = msg.err.Error() m.prEditBranchesError = msg.err.Error()
m.recordHealth("branch recommendations", healthWarning, msg.err.Error())
m.ensurePREditCursorVisible() m.ensurePREditCursorVisible()
return m, nil return m, nil
} }
@@ -538,6 +662,7 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
m.writeMode = writeNone m.writeMode = writeNone
if msg.err != nil { if msg.err != nil {
m.err = fmt.Errorf("change thread resolution: %w", msg.err) m.err = fmt.Errorf("change thread resolution: %w", msg.err)
m.recordHealth("thread resolution", healthError, msg.err.Error())
return m, nil return m, nil
} }
selected := "" selected := ""
@@ -566,6 +691,7 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
if msg.err != nil { if msg.err != nil {
m.writeMode = writeReply m.writeMode = writeReply
m.err = fmt.Errorf("reply to review thread: %w", msg.err) m.err = fmt.Errorf("reply to review thread: %w", msg.err)
m.recordHealth("thread reply", healthError, msg.err.Error())
m.scroll = m.detailMaxScroll() m.scroll = m.detailMaxScroll()
return m, nil return m, nil
} }
@@ -578,6 +704,12 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
break 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.replyDraft, m.writeThreadID = "", ""
m.err = nil m.err = nil
m.lastRefresh = time.Now() m.lastRefresh = time.Now()
@@ -585,6 +717,7 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
if msg.err != nil { if msg.err != nil {
m.writeMode = writePREdit m.writeMode = writePREdit
m.err = fmt.Errorf("update pull request: %w", msg.err) m.err = fmt.Errorf("update pull request: %w", msg.err)
m.recordHealth("PR metadata update", healthError, msg.err.Error())
m.scroll = 0 m.scroll = 0
return m, nil return m, nil
} }
@@ -604,6 +737,12 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
break 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.clearPREdit()
m.err = nil m.err = nil
m.lastRefresh = time.Now() m.lastRefresh = time.Now()
@@ -739,13 +878,19 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
return m, nil return m, nil
} }
m.loading = true 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.loadDetails(m.details.PullRequest, false)
} }
return m, m.loadPRs(false) return m, m.loadPRs(false)
case "j", "down": case "j", "down":
if m.screen == dashboardScreen { if m.screen == dashboardScreen {
m.scrollDashboard(1) 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 { } else if m.screen == threadScreen && m.focus == threadDetailPane {
m.scrollDetail(1) m.scrollDetail(1)
} else { } else {
@@ -754,6 +899,8 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
case "k", "up": case "k", "up":
if m.screen == dashboardScreen { if m.screen == dashboardScreen {
m.scrollDashboard(-1) 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 { } else if m.screen == threadScreen && m.focus == threadDetailPane {
m.scrollDetail(-1) m.scrollDetail(-1)
} else { } 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 m.dashboardReturn, m.screen, m.scroll = threadScreen, dashboardScreen, 0
return m, nil return m, nil
} }
case "H":
if m.screen != healthScreen {
m.healthReturn, m.screen, m.healthScroll = m.screen, healthScreen, 0
}
case "l": case "l":
if m.screen == prScreen && len(m.prs) > 0 { if m.screen == prScreen && len(m.prs) > 0 {
return m, m.openSelectedPR(m.defaultPRTargetScreen()) return m, m.openSelectedPR(m.defaultPRTargetScreen())
@@ -825,6 +976,10 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
m.scroll = 0 m.scroll = 0
} }
case "b", "esc": case "b", "esc":
if m.screen == healthScreen {
m.screen = m.healthReturn
return m, nil
}
if m.screen == threadScreen { if m.screen == threadScreen {
if m.dashboardMode == "intermediate" { if m.dashboardMode == "intermediate" {
m.dashboardReturn, m.screen, m.scroll, m.err = prScreen, dashboardScreen, 0, nil 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 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 { func (m App) defaultPRTargetScreen() screen {
if m.dashboardMode == "hotkey" { if m.dashboardMode == "hotkey" {
return threadScreen return threadScreen
@@ -888,7 +1115,9 @@ func (m *App) trackThreadUpdates(details PRDetails) {
} }
state.Initialized = true state.Initialized = true
m.readState.Data[prID] = state m.readState.Data[prID] = state
_ = m.readState.save() if err := m.readState.save(); err != nil {
m.recordHealth("read state", healthWarning, err.Error())
}
return return
} }
for _, thread := range details.Threads { for _, thread := range details.Threads {
@@ -924,7 +1153,9 @@ func (m *App) markCurrentThreadRead() {
state.Comments[comment.ID] = true state.Comments[comment.ID] = true
} }
m.readState.Data[prID] = state 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() { func (m *App) toStart() {
if m.screen == prScreen { if m.screen == prScreen {
m.prIndex = 0 m.prIndex = 0
} else if m.screen == healthScreen {
m.healthScroll = 0
} else if m.screen == dashboardScreen { } else if m.screen == dashboardScreen {
m.scroll = 0 m.scroll = 0
} else if m.focus == threadDetailPane { } else if m.focus == threadDetailPane {
@@ -1179,6 +1412,8 @@ func (m *App) toEnd() {
m.prIndex = max(0, len(m.prs)-1) m.prIndex = max(0, len(m.prs)-1)
} else if m.screen == dashboardScreen { } else if m.screen == dashboardScreen {
m.scroll = m.dashboardMaxScroll() m.scroll = m.dashboardMaxScroll()
} else if m.screen == healthScreen {
m.healthScroll = m.healthMaxScroll()
} else if m.focus == threadDetailPane { } else if m.focus == threadDetailPane {
m.scroll = m.detailMaxScroll() m.scroll = m.detailMaxScroll()
} else { } else {
@@ -1190,6 +1425,14 @@ func (m *App) page(direction int) {
m.scrollDashboard(direction * max(3, m.dashboardViewportHeight()/2)) m.scrollDashboard(direction * max(3, m.dashboardViewportHeight()/2))
return 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 { if m.screen == threadScreen && m.focus == threadDetailPane {
m.scrollDetail(direction * max(3, m.detailViewportHeight()/2)) m.scrollDetail(direction * max(3, m.detailViewportHeight()/2))
return return
@@ -1202,7 +1445,8 @@ func (m *App) scrollDetail(delta int) {
} }
func (m *App) scrollDashboard(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 { func (m App) dashboardViewportHeight() int {
@@ -1266,6 +1510,9 @@ func (m App) View() string {
if m.screen == dashboardScreen { if m.screen == dashboardScreen {
return m.viewDashboard() return m.viewDashboard()
} }
if m.screen == healthScreen {
return m.viewHealth()
}
return m.viewThreads() return m.viewThreads()
} }
@@ -1449,6 +1696,18 @@ func (m App) helpBindings() []helpBinding {
{keyLabel(m.keybindings.General.Quit), "Quit"}, {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 { if m.screen == dashboardScreen {
backAction := "Return to pull requests" backAction := "Return to pull requests"
if m.dashboardReturn == threadScreen { if m.dashboardReturn == threadScreen {
@@ -1516,6 +1775,8 @@ func (m App) viewHelp() string {
title := "Pull request picker keys" title := "Pull request picker keys"
if m.writeMode == writePREdit || m.writeMode == writePREditConfirm { if m.writeMode == writePREdit || m.writeMode == writePREditConfirm {
title = "Pull request editor keys" title = "Pull request editor keys"
} else if m.screen == healthScreen {
title = "Application health keys"
} else if m.screen == dashboardScreen { } else if m.screen == dashboardScreen {
title = "Pull request dashboard keys" title = "Pull request dashboard keys"
} else if m.screen == threadScreen { } else if m.screen == threadScreen {
@@ -1741,6 +2002,200 @@ func (m App) viewDashboard() string {
return view 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 { func (m App) dashboardLines() []string {
if m.writeMode == writePREdit { if m.writeMode == writePREdit {
return m.dashboardEditLines() return m.dashboardEditLines()
@@ -1758,6 +2213,12 @@ func (m App) dashboardLines() []string {
if pr.FromCache { if pr.FromCache {
lines = append(lines, warnStyle.Render("OFFLINE CACHE • saved "+pr.CachedAt.Local().Format("2006-01-02 15:04"))) 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 == "" { if m.loading && pr.BaseRef == "" {
return append(lines, "", "Loading pull request details…") return append(lines, "", "Loading pull request details…")
} }
@@ -2744,8 +3205,14 @@ func (m App) frame(lines []string, help string) string {
if m.err != nil { if m.err != nil {
status = badStyle.Render("error: " + truncate(m.err.Error(), max(20, m.width-8))) 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…") status = warnStyle.Render("refreshing…")
} else if status == "" && m.secondaryLoading && initialLoad {
status = warnStyle.Render("loading details…")
} }
if status == "" && !m.lastRefresh.IsZero() { if status == "" && !m.lastRefresh.IsZero() {
status = dimStyle.Render("updated " + m.lastRefresh.Format("15:04:05")) status = dimStyle.Render("updated " + m.lastRefresh.Format("15:04:05"))

View File

@@ -1732,3 +1732,26 @@ func TestPeopleMetadataUsesColoredHandles(t *testing.T) {
t.Fatal("people handles do not use the deterministic author color") 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")
}
}

View File

@@ -39,6 +39,8 @@ type PRDetails struct {
CommitCount int CommitCount int
CommentCount int CommentCount int
HeadOID string HeadOID string
BaseOID string
RepositoryURL string
CheckState string CheckState string
Checks []Check Checks []Check
ReviewDecision string ReviewDecision string
@@ -49,12 +51,28 @@ type PRDetails struct {
Threads []ReviewThread Threads []ReviewThread
ThreadsTruncated bool ThreadsTruncated bool
Timeline []TimelineEvent Timeline []TimelineEvent
DataIssues []DataIssue
Rulesets []Ruleset Rulesets []Ruleset
MergeQueue *MergeQueue MergeQueue *MergeQueue
FromCache bool FromCache bool
CachedAt time.Time 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 { type PullRequestMetadata struct {
Title string Title string
Body string Body string