Skip to content

Commit 8659366

Browse files
authored
Merge pull request #47 from codeGROOVE-dev/disappear-faster
Fix buggy honk state tracking for menu items, reduce visibility to 25m
2 parents 8807805 + 6b8d08d commit 8659366

File tree

5 files changed

+138
-365
lines changed

5 files changed

+138
-365
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ We don't yet persist fine-grained tokens to disk - PR's welcome!
7676

7777
## Pricing
7878

79-
- Review Goose is free forever for public repositories ❤️
79+
- Free forever for public repositories ❤️
8080
- Private repo access will soon be a supporter-only feature to ensure the goose is fed. ($2.56/mo is our recommendation)
8181

8282
## Privacy

cmd/goose/cache.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,10 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (
7676
return nil
7777
},
7878
retry.Attempts(maxRetries),
79-
retry.DelayType(retry.BackOffDelay),
79+
retry.DelayType(retry.CombineDelay(retry.BackOffDelay, retry.RandomDelay)), // Add jitter for better backoff distribution
8080
retry.MaxDelay(maxRetryDelay),
8181
retry.OnRetry(func(n uint, err error) {
82-
log.Printf("Turn API retry %d/%d for %s: %v", n+1, maxRetries, url, err)
82+
log.Printf("[TURN] API retry %d/%d for %s: %v", n+1, maxRetries, url, err)
8383
}),
8484
retry.Context(ctx),
8585
)

cmd/goose/github.go

Lines changed: 6 additions & 207 deletions
Original file line numberDiff line numberDiff line change
@@ -199,10 +199,10 @@ func (app *App) executeGitHubQuery(ctx context.Context, query string, opts *gith
199199
return nil
200200
},
201201
retry.Attempts(maxRetries),
202-
retry.DelayType(retry.BackOffDelay),
202+
retry.DelayType(retry.CombineDelay(retry.BackOffDelay, retry.RandomDelay)), // Add jitter for better backoff distribution
203203
retry.MaxDelay(maxRetryDelay),
204204
retry.OnRetry(func(n uint, err error) {
205-
log.Printf("GitHub Search.Issues retry %d/%d: %v", n+1, maxRetries, err)
205+
log.Printf("[GITHUB] Search.Issues retry %d/%d: %v", n+1, maxRetries, err)
206206
}),
207207
retry.Context(ctx),
208208
)
@@ -221,10 +221,8 @@ type prResult struct {
221221
wasFromCache bool
222222
}
223223

224-
// fetchPRsInternal is the implementation for PR fetching.
225-
// It returns GitHub data immediately and starts Turn API queries in the background (when waitForTurn=false),
226-
// or waits for Turn data to complete (when waitForTurn=true).
227-
func (app *App) fetchPRsInternal(ctx context.Context, waitForTurn bool) (incoming []PR, outgoing []PR, _ error) {
224+
// fetchPRsInternal fetches PRs and Turn data synchronously for simplicity.
225+
func (app *App) fetchPRsInternal(ctx context.Context) (incoming []PR, outgoing []PR, _ error) {
228226
// Check if we have a client
229227
if app.client == nil {
230228
return nil, nil, fmt.Errorf("no GitHub client available: %s", app.authError)
@@ -362,71 +360,12 @@ func (app *App) fetchPRsInternal(ctx context.Context, waitForTurn bool) (incomin
362360
log.Printf("[GITHUB] Found %d incoming, %d outgoing PRs from GitHub", len(incoming), len(outgoing))
363361

364362
// Fetch Turn API data
365-
if waitForTurn {
366-
// Synchronous - wait for Turn data
367-
// Fetch Turn API data synchronously before building menu
368-
app.fetchTurnDataSync(ctx, allIssues, user, &incoming, &outgoing)
369-
} else {
370-
// Asynchronous - start in background
371-
app.mu.Lock()
372-
app.loadingTurnData = true
373-
app.pendingTurnResults = make([]TurnResult, 0) // Reset buffer
374-
app.mu.Unlock()
375-
go app.fetchTurnDataAsync(ctx, allIssues, user)
376-
}
363+
// Always synchronous now for simplicity - Turn API calls are fast with caching
364+
app.fetchTurnDataSync(ctx, allIssues, user, &incoming, &outgoing)
377365

378366
return incoming, outgoing, nil
379367
}
380368

381-
// updatePRData updates PR data with Turn API results.
382-
func (app *App) updatePRData(url string, needsReview bool, isOwner bool, actionReason string) (*PR, bool) {
383-
app.mu.Lock()
384-
defer app.mu.Unlock()
385-
386-
if isOwner {
387-
// Update outgoing PRs
388-
for i := range app.outgoing {
389-
if app.outgoing[i].URL != url {
390-
continue
391-
}
392-
// Check if Turn data was already applied for this UpdatedAt
393-
now := time.Now()
394-
if app.outgoing[i].TurnDataAppliedAt.After(app.outgoing[i].UpdatedAt) {
395-
// Turn data already applied for this PR version, no change
396-
return &app.outgoing[i], false
397-
}
398-
changed := app.outgoing[i].NeedsReview != needsReview ||
399-
app.outgoing[i].IsBlocked != needsReview ||
400-
app.outgoing[i].ActionReason != actionReason
401-
app.outgoing[i].NeedsReview = needsReview
402-
app.outgoing[i].IsBlocked = needsReview
403-
app.outgoing[i].ActionReason = actionReason
404-
app.outgoing[i].TurnDataAppliedAt = now
405-
return &app.outgoing[i], changed
406-
}
407-
} else {
408-
// Update incoming PRs
409-
for i := range app.incoming {
410-
if app.incoming[i].URL != url {
411-
continue
412-
}
413-
// Check if Turn data was already applied for this UpdatedAt
414-
now := time.Now()
415-
if app.incoming[i].TurnDataAppliedAt.After(app.incoming[i].UpdatedAt) {
416-
// Turn data already applied for this PR version, no change
417-
return &app.incoming[i], false
418-
}
419-
changed := app.incoming[i].NeedsReview != needsReview ||
420-
app.incoming[i].ActionReason != actionReason
421-
app.incoming[i].NeedsReview = needsReview
422-
app.incoming[i].ActionReason = actionReason
423-
app.incoming[i].TurnDataAppliedAt = now
424-
return &app.incoming[i], changed
425-
}
426-
}
427-
return nil, false
428-
}
429-
430369
// fetchTurnDataSync fetches Turn API data synchronously and updates PRs directly.
431370
func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, user string, incoming *[]PR, outgoing *[]PR) {
432371
turnStart := time.Now()
@@ -523,143 +462,3 @@ func (app *App) fetchTurnDataSync(ctx context.Context, issues []*github.Issue, u
523462
log.Printf("[TURN] Turn API queries completed in %v (%d/%d succeeded)",
524463
time.Since(turnStart), turnSuccesses, turnSuccesses+turnFailures)
525464
}
526-
527-
// fetchTurnDataAsync fetches Turn API data in the background and updates PRs as results arrive.
528-
func (app *App) fetchTurnDataAsync(ctx context.Context, issues []*github.Issue, user string) {
529-
turnStart := time.Now()
530-
531-
// Create a channel for results
532-
results := make(chan prResult, len(issues))
533-
534-
// Use a WaitGroup to track goroutines
535-
var wg sync.WaitGroup
536-
537-
// Create semaphore to limit concurrent Turn API calls
538-
sem := make(chan struct{}, maxConcurrentTurnAPICalls)
539-
540-
// Process PRs in parallel with concurrency limit
541-
for _, issue := range issues {
542-
if !issue.IsPullRequest() {
543-
continue
544-
}
545-
546-
wg.Add(1)
547-
go func(issue *github.Issue) {
548-
defer wg.Done()
549-
550-
// Acquire semaphore
551-
sem <- struct{}{}
552-
defer func() { <-sem }()
553-
554-
url := issue.GetHTMLURL()
555-
updatedAt := issue.GetUpdatedAt().Time
556-
557-
// Call turnData - it now has proper exponential backoff with jitter
558-
turnData, wasFromCache, err := app.turnData(ctx, url, updatedAt)
559-
560-
results <- prResult{
561-
url: issue.GetHTMLURL(),
562-
turnData: turnData,
563-
err: err,
564-
isOwner: issue.GetUser().GetLogin() == user,
565-
wasFromCache: wasFromCache,
566-
}
567-
}(issue)
568-
}
569-
570-
// Close the results channel when all goroutines are done
571-
go func() {
572-
wg.Wait()
573-
close(results)
574-
}()
575-
576-
// Collect results and update PRs incrementally
577-
turnSuccesses := 0
578-
turnFailures := 0
579-
updatesApplied := 0
580-
581-
// Process results as they arrive and buffer them
582-
583-
for result := range results {
584-
if result.err == nil && result.turnData != nil && result.turnData.PRState.UnblockAction != nil {
585-
turnSuccesses++
586-
587-
// Check if user needs to review and get action reason
588-
needsReview := false
589-
actionReason := ""
590-
if action, exists := result.turnData.PRState.UnblockAction[user]; exists {
591-
needsReview = true
592-
actionReason = action.Reason
593-
// Only log blocked PRs from fresh API calls
594-
if !result.wasFromCache {
595-
log.Printf("[TURN] UnblockAction for %s: Reason=%q, Kind=%q", result.url, action.Reason, action.Kind)
596-
}
597-
}
598-
599-
// Buffer the Turn result instead of applying immediately
600-
turnResult := TurnResult{
601-
URL: result.url,
602-
NeedsReview: needsReview,
603-
IsOwner: result.isOwner,
604-
ActionReason: actionReason,
605-
WasFromCache: result.wasFromCache,
606-
}
607-
608-
app.mu.Lock()
609-
app.pendingTurnResults = append(app.pendingTurnResults, turnResult)
610-
app.mu.Unlock()
611-
612-
updatesApplied++
613-
// Only log fresh API calls (not cached)
614-
if !result.wasFromCache {
615-
log.Printf("[TURN] Fresh API data for %s (needsReview=%v)", result.url, needsReview)
616-
}
617-
} else if result.err != nil {
618-
turnFailures++
619-
}
620-
}
621-
622-
log.Printf("[TURN] Turn API queries completed in %v (%d/%d succeeded, %d PRs updated)",
623-
time.Since(turnStart), turnSuccesses, turnSuccesses+turnFailures, updatesApplied)
624-
625-
// Apply all buffered Turn results at once
626-
app.mu.Lock()
627-
pendingResults := app.pendingTurnResults
628-
app.pendingTurnResults = nil
629-
app.loadingTurnData = false
630-
app.mu.Unlock()
631-
632-
// Check if any results came from fresh API calls (not cache)
633-
var cacheHits, freshResults int
634-
for _, result := range pendingResults {
635-
if result.WasFromCache {
636-
cacheHits++
637-
} else {
638-
freshResults++
639-
}
640-
}
641-
642-
// Only log if we have fresh results
643-
if freshResults > 0 {
644-
log.Printf("[TURN] Applying %d buffered Turn results (%d from cache, %d fresh)", len(pendingResults), cacheHits, freshResults)
645-
}
646-
647-
// Track how many PRs actually changed
648-
var actualChanges int
649-
for _, result := range pendingResults {
650-
_, changed := app.updatePRData(result.URL, result.NeedsReview, result.IsOwner, result.ActionReason)
651-
if changed {
652-
actualChanges++
653-
}
654-
}
655-
656-
// Only check for newly blocked PRs if there were actual changes
657-
// checkForNewlyBlockedPRs will handle UI updates internally if needed
658-
if actualChanges > 0 {
659-
app.checkForNewlyBlockedPRs(ctx)
660-
// UI updates are handled inside checkForNewlyBlockedPRs
661-
} else {
662-
// No changes, but still update tray title in case of initial load
663-
app.setTrayTitle()
664-
}
665-
}

0 commit comments

Comments
 (0)