finally fix the excessive menu rebuilds

Author: tstromberg

cache.go

@@ -24,7 +24,7 @@ type cacheEntry struct {
 }
 
 // turnData fetches Turn API data with caching.
-func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (*turn.CheckResponse, error) {
+func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (*turn.CheckResponse, bool, error) {
 	// Create cache key from URL and updated timestamp
 	key := fmt.Sprintf("%s-%s", url, updatedAt.Format(time.RFC3339))
 	hash := sha256.Sum256([]byte(key))
@@ -43,7 +43,7 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
 				}
 			} else if time.Since(entry.CachedAt) < cacheTTL && entry.UpdatedAt.Equal(updatedAt) {
 				// Check if cache is still valid (2 hour TTL)
-				return entry.Data, nil
+				return entry.Data, true, nil
 			}
 		}
 	}
@@ -80,7 +80,7 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
 	)
 	if err != nil {
 		log.Printf("Turn API error after %d retries (will use PR without metadata): %v", maxRetries, err)
-		return nil, err
+		return nil, false, err
 	}
 
 	// Save to cache (don't fail if caching fails) - skip if --no-cache is set
@@ -102,7 +102,7 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
 		}
 	}
 
-	return data, nil
+	return data, false, nil
 }
 
 // cleanupOldCache removes cache files older than 5 days.

github.go

@@ -230,11 +230,11 @@ func (app *App) fetchPRsInternal(ctx context.Context, waitForTurn bool) (incomin
 	}
 
 	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 i := range incoming {
+		log.Printf("[GITHUB] Incoming PR: %s", incoming[i].URL)
 	}
-	for _, pr := range outgoing {
-		log.Printf("[GITHUB] Outgoing PR: %s", pr.URL)
+	for i := range outgoing {
+		log.Printf("[GITHUB] Outgoing PR: %s", outgoing[i].URL)
 	}
 
 	// Fetch Turn API data
@@ -255,41 +255,63 @@ func (app *App) fetchPRsInternal(ctx context.Context, waitForTurn bool) (incomin
 }
 
 // updatePRData updates PR data with Turn API results.
-func (app *App) updatePRData(url string, needsReview bool, isOwner bool, actionReason string) *PR {
+func (app *App) updatePRData(url string, needsReview bool, isOwner bool, actionReason string) (*PR, bool) {
 	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
-				app.outgoing[i].ActionReason = actionReason
-				return &app.outgoing[i]
+			if app.outgoing[i].URL != url {
+				continue
 			}
+			// Check if Turn data was already applied for this UpdatedAt
+			now := time.Now()
+			if app.outgoing[i].TurnDataAppliedAt.After(app.outgoing[i].UpdatedAt) {
+				// Turn data already applied for this PR version, no change
+				return &app.outgoing[i], false
+			}
+			changed := app.outgoing[i].NeedsReview != needsReview ||
+				app.outgoing[i].IsBlocked != needsReview ||
+				app.outgoing[i].ActionReason != actionReason
+			app.outgoing[i].NeedsReview = needsReview
+			app.outgoing[i].IsBlocked = needsReview
+			app.outgoing[i].ActionReason = actionReason
+			app.outgoing[i].TurnDataAppliedAt = now
+			return &app.outgoing[i], changed
 		}
 	} else {
 		// Update incoming PRs
 		for i := range app.incoming {
-			if app.incoming[i].URL == url {
-				app.incoming[i].NeedsReview = needsReview
-				app.incoming[i].ActionReason = actionReason
-				return &app.incoming[i]
+			if app.incoming[i].URL != url {
+				continue
+			}
+			// Check if Turn data was already applied for this UpdatedAt
+			now := time.Now()
+			if app.incoming[i].TurnDataAppliedAt.After(app.incoming[i].UpdatedAt) {
+				// Turn data already applied for this PR version, no change
+				return &app.incoming[i], false
 			}
+			changed := app.incoming[i].NeedsReview != needsReview ||
+				app.incoming[i].ActionReason != actionReason
+			app.incoming[i].NeedsReview = needsReview
+			app.incoming[i].ActionReason = actionReason
+			app.incoming[i].TurnDataAppliedAt = now
+			return &app.incoming[i], changed
 		}
 	}
-	return nil
+	return nil, false
 }
 
 // fetchTurnDataSync fetches Turn API data synchronously and updates PRs directly.
 func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, user string, incoming *[]PR, outgoing *[]PR) {
 	turnStart := time.Now()
 	type prResult struct {
-		err      error
-		turnData *turn.CheckResponse
-		url      string
-		isOwner  bool
+		err          error
+		turnData     *turn.CheckResponse
+		url          string
+		isOwner      bool
+		wasFromCache bool
 	}
 
 	// Create a channel for results
@@ -312,13 +334,14 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
 			updatedAt := issue.GetUpdatedAt().Time
 
 			// Call turnData - it now has proper exponential backoff with jitter
-			turnData, err := app.turnData(ctx, url, updatedAt)
+			turnData, wasFromCache, err := app.turnData(ctx, url, updatedAt)
 
 			results <- prResult{
-				url:      issue.GetHTMLURL(),
-				turnData: turnData,
-				err:      err,
-				isOwner:  issue.GetUser().GetLogin() == user,
+				url:          issue.GetHTMLURL(),
+				turnData:     turnData,
+				err:          err,
+				isOwner:      issue.GetUser().GetLogin() == user,
+				wasFromCache: wasFromCache,
 			}
 		}(issue)
 	}
@@ -381,10 +404,11 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue,
 
 	turnStart := time.Now()
 	type prResult struct {
-		err      error
-		turnData *turn.CheckResponse
-		url      string
-		isOwner  bool
+		err          error
+		turnData     *turn.CheckResponse
+		url          string
+		isOwner      bool
+		wasFromCache bool
 	}
 
 	// Create a channel for results
@@ -407,13 +431,14 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue,
 			updatedAt := issue.GetUpdatedAt().Time
 
 			// Call turnData - it now has proper exponential backoff with jitter
-			turnData, err := app.turnData(ctx, url, updatedAt)
+			turnData, wasFromCache, err := app.turnData(ctx, url, updatedAt)
 
 			results <- prResult{
-				url:      issue.GetHTMLURL(),
-				turnData: turnData,
-				err:      err,
-				isOwner:  issue.GetUser().GetLogin() == user,
+				url:          issue.GetHTMLURL(),
+				turnData:     turnData,
+				err:          err,
+				isOwner:      issue.GetUser().GetLogin() == user,
+				wasFromCache: wasFromCache,
 			}
 		}(issue)
 	}
@@ -442,7 +467,8 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue,
 				needsReview = true
 				actionReason = action.Reason
 				log.Printf("[TURN] UnblockAction for %s: Reason=%q, Kind=%q", result.url, action.Reason, action.Kind)
-			} else {
+			} else if !result.wasFromCache {
+				// Only log "no action" for fresh API results, not cached ones
 				log.Printf("[TURN] No UnblockAction found for user %s on %s", user, result.url)
 			}
 
@@ -452,14 +478,22 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue,
 				NeedsReview:  needsReview,
 				IsOwner:      result.isOwner,
 				ActionReason: actionReason,
+				WasFromCache: result.wasFromCache,
 			}
 
 			app.mu.Lock()
 			app.pendingTurnResults = append(app.pendingTurnResults, turnResult)
 			app.mu.Unlock()
 
 			updatesApplied++
-			log.Printf("[TURN] Turn data received for %s (needsReview=%v, actionReason=%q) - buffered", result.url, needsReview, actionReason)
+			// Reduce verbosity - only log if not from cache or if blocked
+			if !result.wasFromCache || needsReview {
+				cacheStatus := "fresh"
+				if result.wasFromCache {
+					cacheStatus = "cached"
+				}
+				log.Printf("[TURN] %s data for %s (needsReview=%v, actionReason=%q)", cacheStatus, result.url, needsReview, actionReason)
+			}
 		} else if result.err != nil {
 			turnFailures++
 		}
@@ -475,15 +509,36 @@ func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue,
 	app.loadingTurnData = false
 	app.mu.Unlock()
 
-	log.Printf("[TURN] Applying %d buffered Turn results", len(pendingResults))
+	// Check if any results came from fresh API calls (not cache)
+	var cacheHits, freshResults int
 	for _, result := range pendingResults {
-		app.updatePRData(result.URL, result.NeedsReview, result.IsOwner, result.ActionReason)
+		if result.WasFromCache {
+			cacheHits++
+		} else {
+			freshResults++
+		}
+	}
+
+	log.Printf("[TURN] Applying %d buffered Turn results (%d from cache, %d fresh)", len(pendingResults), cacheHits, freshResults)
+
+	// Track how many PRs actually changed
+	var actualChanges int
+	for _, result := range pendingResults {
+		_, changed := app.updatePRData(result.URL, result.NeedsReview, result.IsOwner, result.ActionReason)
+		if changed {
+			actualChanges++
+		}
 	}
 
 	// Update tray title and menu with final Turn data if menu is already initialized
 	app.setTrayTitle()
 	if app.menuInitialized {
-		log.Print("[TURN] Turn data loaded, checking if menu needs update")
-		app.updateMenuIfChanged(ctx)
+		// Only trigger menu update if PR data actually changed
+		if actualChanges > 0 {
+			log.Printf("[TURN] Turn data applied - %d PRs actually changed, checking if menu needs update", actualChanges)
+			app.updateMenuIfChanged(ctx)
+		} else {
+			log.Print("[TURN] Turn data applied - no PR changes detected (cached data unchanged), skipping menu update")
+		}
 	}
 }

main.go

@@ -51,14 +51,15 @@ const (
 
 // PR represents a pull request with metadata.
 type PR struct {
-	UpdatedAt    time.Time
-	Title        string
-	URL          string
-	Repository   string
-	ActionReason string // Action reason from Turn API when blocked
-	Number       int
-	IsBlocked    bool
-	NeedsReview  bool
+	UpdatedAt         time.Time
+	TurnDataAppliedAt time.Time
+	Title             string
+	URL               string
+	Repository        string
+	ActionReason      string
+	Number            int
+	IsBlocked         bool
+	NeedsReview       bool
 }
 
 // MenuState represents the current state of menu items for comparison.
@@ -85,6 +86,7 @@ type TurnResult struct {
 	ActionReason string
 	NeedsReview  bool
 	IsOwner      bool
+	WasFromCache bool // Track if this result came from cache
 }
 
 // App holds the application state.
@@ -427,15 +429,15 @@ func (app *App) buildCurrentMenuState() *MenuState {
 	now := time.Now()
 	staleThreshold := now.Add(-stalePRThreshold)
 
-	for _, pr := range app.incoming {
-		if !app.hideStaleIncoming || pr.UpdatedAt.After(staleThreshold) {
-			filteredIncoming = append(filteredIncoming, pr)
+	for i := range app.incoming {
+		if !app.hideStaleIncoming || app.incoming[i].UpdatedAt.After(staleThreshold) {
+			filteredIncoming = append(filteredIncoming, app.incoming[i])
 		}
 	}
 
-	for _, pr := range app.outgoing {
-		if !app.hideStaleIncoming || pr.UpdatedAt.After(staleThreshold) {
-			filteredOutgoing = append(filteredOutgoing, pr)
+	for i := range app.outgoing {
+		if !app.hideStaleIncoming || app.outgoing[i].UpdatedAt.After(staleThreshold) {
+			filteredOutgoing = append(filteredOutgoing, app.outgoing[i])
 		}
 	}
 
@@ -445,28 +447,28 @@ func (app *App) buildCurrentMenuState() *MenuState {
 
 	// Build menu item states
 	incomingItems := make([]MenuItemState, len(incomingSorted))
-	for i, pr := range incomingSorted {
+	for i := range incomingSorted {
 		incomingItems[i] = MenuItemState{
-			URL:          pr.URL,
-			Title:        pr.Title,
-			Repository:   pr.Repository,
-			Number:       pr.Number,
-			NeedsReview:  pr.NeedsReview,
+			URL:          incomingSorted[i].URL,
+			Title:        incomingSorted[i].Title,
+			Repository:   incomingSorted[i].Repository,
+			Number:       incomingSorted[i].Number,
+			NeedsReview:  incomingSorted[i].NeedsReview,
 			IsBlocked:    false, // incoming PRs don't use IsBlocked
-			ActionReason: pr.ActionReason,
+			ActionReason: incomingSorted[i].ActionReason,
 		}
 	}
 
 	outgoingItems := make([]MenuItemState, len(outgoingSorted))
-	for i, pr := range outgoingSorted {
+	for i := range outgoingSorted {
 		outgoingItems[i] = MenuItemState{
-			URL:          pr.URL,
-			Title:        pr.Title,
-			Repository:   pr.Repository,
-			Number:       pr.Number,
-			NeedsReview:  pr.NeedsReview,
-			IsBlocked:    pr.IsBlocked,
-			ActionReason: pr.ActionReason,
+			URL:          outgoingSorted[i].URL,
+			Title:        outgoingSorted[i].Title,
+			Repository:   outgoingSorted[i].Repository,
+			Number:       outgoingSorted[i].Number,
+			NeedsReview:  outgoingSorted[i].NeedsReview,
+			IsBlocked:    outgoingSorted[i].IsBlocked,
+			ActionReason: outgoingSorted[i].ActionReason,
 		}
 	}
 
@@ -482,18 +484,8 @@ func (app *App) updateMenuIfChanged(ctx context.Context) {
 	app.mu.RLock()
 	// Skip menu updates while Turn data is still loading to avoid excessive rebuilds
 	if app.loadingTurnData {
-		// But still save the current state for future comparisons
-		currentMenuState := app.buildCurrentMenuState()
-		if app.lastMenuState == nil {
-			log.Print("[MENU] Skipping menu update - Turn data still loading, but saving state for future comparison")
-			app.mu.RUnlock()
-			app.mu.Lock()
-			app.lastMenuState = currentMenuState
-			app.mu.Unlock()
-		} else {
-			app.mu.RUnlock()
-			log.Print("[MENU] Skipping menu update - Turn data still loading")
-		}
+		app.mu.RUnlock()
+		log.Print("[MENU] Skipping menu update - Turn data still loading")
 		return
 	}
 	log.Print("[MENU] *** updateMenuIfChanged called - calculating diff ***")