Centralize PR data fetching to prevent duplicate API calls

Summary

Multiple event handlers in the GitHub provider independently fetch pull request details, commit information, and changed files. This results in duplicate API calls when processing a single webhook event that triggers multiple handlers.

Problem

Location: pkg/provider/github/parse_payload.go

Multiple functions fetch the same PR data:

// In handleReRequestEvent() - line ~442
pr, _, err := wrapAPI(v, "get_pull_request", func() (*github.PullRequest, *github.Response, error) {
    return v.Client().PullRequests.Get(ctx, runevent.Organization, runevent.Repository, prNumber)
})

// In handleCheckSuites() - line ~474
pr, _, err := wrapAPI(v, "get_pull_request", func() (*github.PullRequest, *github.Response, error) {
    return v.Client().PullRequests.Get(ctx, runevent.Organization, runevent.Repository, prNumber)
})

// In handleIssueCommentEvent() - line ~516
pr, _, err := wrapAPI(v, "get_pull_request", func() (*github.PullRequest, *github.Response, error) {
    return v.Client().PullRequests.Get(ctx, runevent.Organization, runevent.Repository, issueNumber)
})

Similarly, list_pull_request_files is called in multiple places.

Evidence from E2E Instrumentation

Example 1: Single Comment Strategy Webhook (15 calls)

2x /repos/.../pulls/5237           # get_pull_request
2x /repos/.../pulls/5237/files     # list_pull_request_files

Different rate limits (6795 vs 4959) indicate calls from different auth contexts (GitHub App vs OAuth2 webhook).

Example 2: Github PullRequest (21 calls)

2x /repos/.../pulls/40613          # get_pull_request
2x /repos/.../pulls/40613/files    # list_pull_request_files
2x /repos/.../commits/.../check-runs  # list_check_runs_for_ref

Total impact:

• get_pull_request: 3 duplicate calls (12.5% waste)
• list_pull_request_files: 5 duplicate calls (21.7% waste)
• list_check_runs_for_ref: 3 duplicate calls (8.8% waste)
• get_commit_files: 7 duplicate calls (20.6% waste)
• Combined: ~18 wasted calls

Root Cause

1. Independent handlers: Each event handler (handleReRequestEvent, handleCheckSuites, handleIssueCommentEvent) fetches PR data independently.

2. Multiple auth contexts: Some flows use GitHub App auth, others use OAuth2. Each may fetch the same data.

3. No shared state: Handlers don't share fetched data, leading to redundant fetches.

4. Partial caching: While GetFiles() has cachedChangedFiles, this only works within a single Provider instance. If multiple Provider instances are created or if data is fetched before caching occurs, duplicates happen.

Proposed Solution

Option A: Add PR Cache to Provider (Recommended)

Add a cache for PR-related data in the Provider struct:

type Provider struct {
    // ... existing fields

    // Caches for PR-related data
    prCache           map[int]*github.PullRequest
    prCacheMutex      sync.RWMutex

    checkRunsCache      map[string]*github.ListCheckRunsResults
    checkRunsCacheMutex sync.RWMutex
}

func (v *Provider) GetPullRequest(ctx context.Context, org, repo string, prNumber int) (*github.PullRequest, error) {
    v.prCacheMutex.RLock()
    if cached, ok := v.prCache[prNumber]; ok {
        v.prCacheMutex.RUnlock()
        return cached, nil
    }
    v.prCacheMutex.RUnlock()

    pr, _, err := wrapAPI(v, "get_pull_request", func() (*github.PullRequest, *github.Response, error) {
        return v.Client().PullRequests.Get(ctx, org, repo, prNumber)
    })
    if err != nil {
        return nil, err
    }

    v.prCacheMutex.Lock()
    v.prCache[prNumber] = pr
    v.prCacheMutex.Unlock()

    return pr, nil
}

Then update all handlers to use v.GetPullRequest() instead of direct API calls.

Option B: Fetch Once, Pass Through (More Invasive)

Restructure event handling to fetch PR data once at the entry point and pass it to handlers:

type eventContext struct {
    PR           *github.PullRequest
    ChangedFiles []string
    CheckRuns    *github.ListCheckRunsResults
    // ... other shared data
}

func (v *Provider) processEvent(ctx context.Context, ...) error {
    eventCtx := &eventContext{}

    // Fetch common data once
    if isPREvent {
        eventCtx.PR, _ = v.fetchPR(ctx, prNumber)
        eventCtx.ChangedFiles, _ = v.fetchChangedFiles(ctx, prNumber)
    }

    // Pass to handlers
    return v.handleEvent(ctx, eventCtx, ...)
}

Recommendation: Option A is less invasive and can be implemented incrementally.

Files to Modify

For Option A (Recommended)

1. pkg/provider/github/github.go

   • Add prCache, checkRunsCache fields to Provider struct
   • Add GetPullRequest(), GetCheckRunsForRef() helper methods

2. pkg/provider/github/parse_payload.go

   • Update handleReRequestEvent() to use v.GetPullRequest()
   • Update handleCheckSuites() to use v.GetPullRequest()
   • Update handleIssueCommentEvent() to use v.GetPullRequest()
   • Update check runs fetching to use cached helper

3. pkg/provider/github/status.go (if applicable)

   • Update any direct PR/check-runs API calls to use cached helpers

Testing

1. Run existing unit tests
2. Run e2e tests with PAC_API_INSTRUMENTATION_DIR set
3. Verify no duplicate get_pull_request or list_check_runs_for_ref calls

Expected Impact

• Calls saved: ~18 per full e2e test suite
• Per-event savings: 2-4 calls for PR-related events
• Complexity: Medium - requires updating multiple call sites

Acceptance Criteria

• PR details are cached by PR number within a Provider instance
• Check runs are cached by SHA within a Provider instance
• All event handlers use cached helpers instead of direct API calls
• No duplicate get_pull_request calls in instrumentation output
• All existing tests pass

Considerations

Cache Invalidation

Not needed. The Provider instance is short-lived (one webhook event), and PR data doesn't change during processing.

Auth Context Differences

If different auth contexts (App vs OAuth2) are used within the same event, they may have different permissions. The cache should be per-Provider, and each auth context should have its own Provider instance. If this is the current design, the duplication might be necessary. Investigation needed to confirm.

Thread Safety

Use sync.RWMutex for all caches to handle concurrent access.

/label ~enhancement ~performance ~github-provider