Improve rendering performance & reliability

Author: tstromberg

cache.go

@@ -29,14 +29,18 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
 	hash := sha256.Sum256([]byte(key))
 	cacheFile := filepath.Join(app.cacheDir, hex.EncodeToString(hash[:])[:16]+".json")
 
-	// Try to read from cache
-	if data, err := os.ReadFile(cacheFile); err == nil {
+	// Try to read from cache (gracefully handle all cache errors)
+	if data, readErr := os.ReadFile(cacheFile); readErr == nil {
 		var entry cacheEntry
-		if err := json.Unmarshal(data, &entry); err == nil {
-			// Check if cache is still valid (2 hour TTL)
-			if time.Since(entry.CachedAt) < cacheTTL && entry.UpdatedAt.Equal(updatedAt) {
-				return entry.Data, nil
+		if unmarshalErr := json.Unmarshal(data, &entry); unmarshalErr != nil {
+			log.Printf("Failed to unmarshal cache data for %s: %v", url, unmarshalErr)
+			// Remove corrupted cache file
+			if removeErr := os.Remove(cacheFile); removeErr != nil {
+				log.Printf("Failed to remove corrupted cache file: %v", removeErr)
 			}
+		} else if time.Since(entry.CachedAt) < cacheTTL && entry.UpdatedAt.Equal(updatedAt) {
+			// Check if cache is still valid (2 hour TTL)
+			return entry.Data, nil
 		}
 	}
 
@@ -53,16 +57,20 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
 		return nil, err
 	}
 
-	// Save to cache
+	// Save to cache (don't fail if caching fails)
 	entry := cacheEntry{
 		Data:      data,
 		CachedAt:  time.Now(),
 		UpdatedAt: updatedAt,
 	}
-	cacheData, err := json.Marshal(entry)
-	if err == nil {
-		if err := os.WriteFile(cacheFile, cacheData, 0o600); err != nil {
-			log.Printf("Failed to write cache for %s: %v", url, err)
+	if cacheData, marshalErr := json.Marshal(entry); marshalErr != nil {
+		log.Printf("Failed to marshal cache data for %s: %v", url, marshalErr)
+	} else {
+		// Ensure cache directory exists
+		if dirErr := os.MkdirAll(filepath.Dir(cacheFile), 0o700); dirErr != nil {
+			log.Printf("Failed to create cache directory: %v", dirErr)
+		} else if writeErr := os.WriteFile(cacheFile, cacheData, 0o600); writeErr != nil {
+			log.Printf("Failed to write cache file for %s: %v", url, writeErr)
 		}
 	}
 
@@ -73,24 +81,36 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
 func (app *App) cleanupOldCache() {
 	entries, err := os.ReadDir(app.cacheDir)
 	if err != nil {
+		log.Printf("Failed to read cache directory for cleanup: %v", err)
 		return
 	}
 
+	var cleanupCount, errorCount int
 	for _, entry := range entries {
 		if !strings.HasSuffix(entry.Name(), ".json") {
 			continue
 		}
 
 		info, err := entry.Info()
 		if err != nil {
+			log.Printf("Failed to get file info for cache entry %s: %v", entry.Name(), err)
+			errorCount++
 			continue
 		}
 
 		// Remove cache files older than 5 days
 		if time.Since(info.ModTime()) > cacheCleanupInterval {
-			if err := os.Remove(filepath.Join(app.cacheDir, entry.Name())); err != nil {
-				log.Printf("Failed to remove old cache: %v", err)
+			filePath := filepath.Join(app.cacheDir, entry.Name())
+			if removeErr := os.Remove(filePath); removeErr != nil {
+				log.Printf("Failed to remove old cache file %s: %v", filePath, removeErr)
+				errorCount++
+			} else {
+				cleanupCount++
 			}
 		}
 	}
+
+	if cleanupCount > 0 || errorCount > 0 {
+		log.Printf("Cache cleanup completed: %d files removed, %d errors", cleanupCount, errorCount)
+	}
 }

github.go

@@ -11,8 +11,10 @@ import (
 	"path/filepath"
 	"runtime"
 	"strings"
+	"sync"
 	"time"
 
+	"github.com/codeGROOVE-dev/retry"
 	"github.com/google/go-github/v57/github"
 	"github.com/ready-to-review/turnclient/pkg/turn"
 	"golang.org/x/oauth2"
@@ -118,6 +120,7 @@ func (*App) githubToken(ctx context.Context) (string, error) {
 }
 
 // fetchPRs retrieves all PRs involving the current user.
+// It returns GitHub data immediately and starts Turn API queries in the background.
 func (app *App) fetchPRs(ctx context.Context) (incoming []PR, outgoing []PR, err error) {
 	// Use targetUser if specified, otherwise use authenticated user
 	user := app.currentUser.GetLogin()
@@ -138,13 +141,40 @@ func (app *App) fetchPRs(ctx context.Context) (incoming []PR, outgoing []PR, err
 	log.Printf("Searching for PRs with query: %s", query)
 	searchStart := time.Now()
 
-	// Just try once - GitHub API is reliable enough
-	result, resp, err := app.client.Search.Issues(ctx, query, opts)
+	// Create timeout context for GitHub API call
+	githubCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
+	defer cancel()
+
+	result, resp, err := app.client.Search.Issues(githubCtx, query, opts)
 	if err != nil {
-		// Check for rate limit
-		const httpStatusForbidden = 403
-		if resp != nil && resp.StatusCode == httpStatusForbidden {
-			log.Print("GitHub API rate limited")
+		// Enhanced error handling with specific cases
+		if resp != nil {
+			const (
+				httpStatusUnauthorized  = 401
+				httpStatusForbidden     = 403
+				httpStatusUnprocessable = 422
+			)
+			switch resp.StatusCode {
+			case httpStatusForbidden:
+				if resp.Header.Get("X-Ratelimit-Remaining") == "0" {
+					resetTime := resp.Header.Get("X-Ratelimit-Reset")
+					log.Printf("GitHub API rate limited, reset at: %s", resetTime)
+					return nil, nil, fmt.Errorf("github API rate limited, try again later: %w", err)
+				}
+				log.Print("GitHub API access forbidden (check token permissions)")
+				return nil, nil, fmt.Errorf("github API access forbidden: %w", err)
+			case httpStatusUnauthorized:
+				log.Print("GitHub API authentication failed (check token)")
+				return nil, nil, fmt.Errorf("github API authentication failed: %w", err)
+			case httpStatusUnprocessable:
+				log.Printf("GitHub API query invalid: %s", query)
+				return nil, nil, fmt.Errorf("github API query invalid: %w", err)
+			default:
+				log.Printf("GitHub API error (status %d): %v", resp.StatusCode, err)
+			}
+		} else {
+			// Likely network error
+			log.Printf("GitHub API network error: %v", err)
 		}
 		return nil, nil, fmt.Errorf("search PRs: %w", err)
 	}
@@ -157,10 +187,7 @@ func (app *App) fetchPRs(ctx context.Context) (incoming []PR, outgoing []PR, err
 		result.Issues = result.Issues[:maxPRsToProcess]
 	}
 
-	// Process results
-	turnSuccesses := 0
-	turnFailures := 0
-
+	// Process GitHub results immediately
 	for _, issue := range result.Issues {
 		if !issue.IsPullRequest() {
 			continue
@@ -175,28 +202,188 @@ func (app *App) fetchPRs(ctx context.Context) (incoming []PR, outgoing []PR, err
 			UpdatedAt:  issue.GetUpdatedAt().Time,
 		}
 
-		// Get Turn API data with caching
-		turnData, err := app.turnData(ctx, issue.GetHTMLURL(), issue.GetUpdatedAt().Time)
-		if err == nil && turnData != nil && turnData.PRState.UnblockAction != nil {
-			turnSuccesses++
-			if _, exists := turnData.PRState.UnblockAction[user]; exists {
-				pr.NeedsReview = true
-			}
-		} else if err != nil {
-			turnFailures++
-		}
-
 		// Categorize as incoming or outgoing
 		// When viewing another user's PRs, we're looking at it from their perspective
 		if issue.GetUser().GetLogin() == user {
-			pr.IsBlocked = pr.NeedsReview
 			outgoing = append(outgoing, pr)
 		} else {
 			incoming = append(incoming, pr)
 		}
 	}
 
-	log.Printf("Found %d incoming, %d outgoing PRs (Turn API: %d/%d succeeded)",
-		len(incoming), len(outgoing), turnSuccesses, turnSuccesses+turnFailures)
+	log.Printf("[GITHUB] Found %d incoming, %d outgoing PRs from GitHub", len(incoming), len(outgoing))
+	for _, pr := range incoming {
+		log.Printf("[GITHUB] Incoming PR: %s", pr.URL)
+	}
+	for _, pr := range outgoing {
+		log.Printf("[GITHUB] Outgoing PR: %s", pr.URL)
+	}
+
+	// Start Turn API queries in background
+	go app.fetchTurnDataAsync(ctx, result.Issues, user)
+
 	return incoming, outgoing, nil
 }
+
+// updatePRData updates PR data with Turn API results.
+func (app *App) updatePRData(url string, needsReview bool, isOwner bool) *PR {
+	app.mu.Lock()
+	defer app.mu.Unlock()
+
+	if isOwner {
+		// Update outgoing PRs
+		for i := range app.outgoing {
+			if app.outgoing[i].URL == url {
+				app.outgoing[i].NeedsReview = needsReview
+				app.outgoing[i].IsBlocked = needsReview
+				return &app.outgoing[i]
+			}
+		}
+	} else {
+		// Update incoming PRs
+		for i := range app.incoming {
+			if app.incoming[i].URL == url {
+				app.incoming[i].NeedsReview = needsReview
+				return &app.incoming[i]
+			}
+		}
+	}
+	return nil
+}
+
+// fetchTurnDataAsync fetches Turn API data in the background and updates PRs as results arrive.
+func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue, user string) {
+	// Set loading state
+	app.mu.Lock()
+	app.turnDataLoading = true
+	app.mu.Unlock()
+
+	// Update section headers to show loading state
+	log.Print("[TURN] Starting Turn API queries, updating section headers to show loading state")
+	app.updateSectionHeaders()
+
+	turnStart := time.Now()
+	type prResult struct {
+		err      error
+		turnData *turn.CheckResponse
+		url      string
+		isOwner  bool
+	}
+
+	// Create a channel for results
+	results := make(chan prResult, len(issues))
+
+	// Use a WaitGroup to track goroutines
+	var wg sync.WaitGroup
+
+	// Process PRs in parallel
+	for _, issue := range issues {
+		if !issue.IsPullRequest() {
+			continue
+		}
+
+		wg.Add(1)
+		go func(issue *github.Issue) {
+			defer wg.Done()
+
+			// Retry logic for Turn API with exponential backoff and jitter
+			var turnData *turn.CheckResponse
+			var err error
+
+			turnData, err = retry.DoWithData(
+				func() (*turn.CheckResponse, error) {
+					data, apiErr := app.turnData(ctx, issue.GetHTMLURL(), issue.GetUpdatedAt().Time)
+					if apiErr != nil {
+						log.Printf("Turn API attempt failed for %s: %v", issue.GetHTMLURL(), apiErr)
+					}
+					return data, apiErr
+				},
+				retry.Context(ctx),
+				retry.Attempts(5),                             // 5 attempts max
+				retry.Delay(500*time.Millisecond),             // Start with 500ms
+				retry.MaxDelay(30*time.Second),                // Cap at 30 seconds
+				retry.DelayType(retry.FullJitterBackoffDelay), // Exponential backoff with jitter
+				retry.OnRetry(func(attempt uint, err error) {
+					log.Printf("Turn API retry attempt %d for %s: %v", attempt, issue.GetHTMLURL(), err)
+				}),
+			)
+			if err != nil {
+				log.Printf("Turn API failed after all retries for %s: %v", issue.GetHTMLURL(), err)
+			}
+
+			results <- prResult{
+				url:      issue.GetHTMLURL(),
+				turnData: turnData,
+				err:      err,
+				isOwner:  issue.GetUser().GetLogin() == user,
+			}
+		}(issue)
+	}
+
+	// Close the results channel when all goroutines are done
+	go func() {
+		wg.Wait()
+		close(results)
+	}()
+
+	// Collect results and update PRs incrementally
+	turnSuccesses := 0
+	turnFailures := 0
+	updatesApplied := 0
+
+	// Batch updates to reduce menu rebuilds
+	updateBatch := 0
+	const batchSize = 10
+	lastUpdateTime := time.Now()
+	const minUpdateInterval = 500 * time.Millisecond
+
+	for result := range results {
+		if result.err == nil && result.turnData != nil && result.turnData.PRState.UnblockAction != nil {
+			turnSuccesses++
+
+			// Check if user needs to review
+			needsReview := false
+			if _, exists := result.turnData.PRState.UnblockAction[user]; exists {
+				needsReview = true
+			}
+
+			// Update the PR in our lists
+			pr := app.updatePRData(result.url, needsReview, result.isOwner)
+
+			if pr != nil {
+				updatesApplied++
+				updateBatch++
+				log.Printf("[TURN] Turn data received for %s (needsReview=%v)", result.url, needsReview)
+				// Update the specific menu item immediately
+				app.updatePRMenuItem(*pr)
+
+				// Periodically update section headers and tray title
+				if updateBatch >= batchSize || time.Since(lastUpdateTime) >= minUpdateInterval {
+					log.Printf("[TURN] Batch update threshold reached (%d updates), updating headers and title", updateBatch)
+					app.updateSectionHeaders()
+					app.setTrayTitle()
+					updateBatch = 0
+					lastUpdateTime = time.Now()
+				}
+			}
+		} else if result.err != nil {
+			turnFailures++
+		}
+	}
+
+	// Clear loading state
+	app.mu.Lock()
+	app.turnDataLoading = false
+	app.turnDataLoaded = true
+	app.mu.Unlock()
+
+	log.Printf("[TURN] Turn API queries completed in %v (%d/%d succeeded, %d PRs updated)",
+		time.Since(turnStart), turnSuccesses, turnSuccesses+turnFailures, updatesApplied)
+
+	// Update section headers with final counts
+	log.Print("[TURN] Updating section headers and tray title with final counts")
+	app.updateSectionHeaders()
+
+	// Update tray title
+	app.setTrayTitle()
+}

go.mod

@@ -3,6 +3,7 @@ module ready-to-review
 go 1.23.4
 
 require (
+	github.com/codeGROOVE-dev/retry v1.2.0
 	github.com/energye/systray v1.0.2
 	github.com/gen2brain/beeep v0.11.1
 	github.com/google/go-github/v57 v57.0.0

go.sum

@@ -1,5 +1,7 @@
 git.sr.ht/~jackmordaunt/go-toast v1.1.2 h1:/yrfI55LRt1M7H1vkaw+NaH1+L1CDxrqDltwm5euVuE=
 git.sr.ht/~jackmordaunt/go-toast v1.1.2/go.mod h1:jA4OqHKTQ4AFBdwrSnwnskUIIS3HYzlJSgdzCKqfavo=
+github.com/codeGROOVE-dev/retry v1.2.0 h1:xYpYPX2PQZmdHwuiQAGGzsBm392xIMl4nfMEFApQnu8=
+github.com/codeGROOVE-dev/retry v1.2.0/go.mod h1:8OgefgV1XP7lzX2PdKlCXILsYKuz6b4ZpHa/20iLi8E=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=