Skip to content

Commit 6b8d08d

Browse files
committed
refactor data flow for simplicity
1 parent 75e6b84 commit 6b8d08d

File tree

3 files changed

+52
-363
lines changed

3 files changed

+52
-363
lines changed

cmd/goose/github.go

Lines changed: 4 additions & 206 deletions
Original file line numberDiff line numberDiff line change
@@ -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,144 +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 PR data changes, but still need to update menu in case icons expired
663-
// (e.g., party popper changing to bullet after 25 minutes)
664-
app.updateMenu(ctx)
665-
}
666-
}

0 commit comments

Comments
 (0)