Skip to content

Commit 00c6c6e

Browse files
Thomas StrombergThomas Stromberg
Thomas Stromberg
authored and
Thomas Stromberg
committed
add participant_access map
1 parent cb95cd0 commit 00c6c6e

File tree

7 files changed

+717
-7
lines changed

7 files changed

+717
-7
lines changed

‎.claude/API_OPTIMIZATION_GUIDE.md‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
# GitHub API Optimization Guide
2+
3+
## Current API Call Analysis
4+
5+
The current implementation makes **13+ REST API calls** per pull request fetch:
6+
7+
### Synchronous Calls (Required Checks Detection)
8+
1. GraphQL for refUpdateRule
9+
2. Combined status (`/commits/{sha}/status`)
10+
3. Branch protection (`/branches/{branch}/protection`)
11+
4. Branch protection fallback (`/branches/{branch}/protection/required_status_checks`)
12+
5. Repository rulesets (`/rulesets`)
13+
6. Workflows (if PR blocked) - multiple calls
14+
15+
### Parallel Calls (Event Fetching)
16+
7. Commits (`/pulls/{pr}/commits`)
17+
8. Comments (`/issues/{pr}/comments`)
18+
9. Reviews (`/pulls/{pr}/reviews`)
19+
10. Review comments (`/pulls/{pr}/comments`)
20+
11. Timeline (`/issues/{pr}/timeline`)
21+
12. Status checks (`/statuses/{sha}`)
22+
13. Check runs (`/commits/{sha}/check-runs`)
23+
24+
## Optimization Opportunities
25+
26+
### 1. **Single GraphQL Query Solution** (RECOMMENDED)
27+
Replace all 13+ REST calls with **1 GraphQL query** that fetches everything:
28+
- **API Quota Savings**: ~92% reduction (1 call vs 13+)
29+
- **Performance**: Faster due to single round-trip
30+
- **Implementation**: See `fetchers_graphql_optimized.go`
31+
32+
### 2. **Remove Redundant Calls**
33+
Several calls appear redundant or could be eliminated:
34+
35+
#### a. Combined Status API
36+
- **Current**: Fetches `/commits/{sha}/status`
37+
- **Issue**: Only used for logging, not for actual required checks
38+
- **Fix**: Remove this call entirely
39+
40+
#### b. Branch Protection Fallback
41+
- **Current**: Tries main endpoint, then fallback
42+
- **Issue**: Two calls when one would suffice
43+
- **Fix**: Use only the main endpoint or handle in GraphQL
44+
45+
#### c. Workflow Checks
46+
- **Current**: Multiple calls to workflows API when PR is blocked
47+
- **Issue**: Expensive and often unnecessary
48+
- **Fix**: Remove or make optional
49+
50+
### 3. **Batch Timeline Events**
51+
The timeline API already includes many event types. It could potentially replace:
52+
- Comments
53+
- Some review events
54+
- Assignment/label events
55+
56+
### 4. **Cache Optimization**
57+
Improve caching to reduce redundant API calls:
58+
- Cache required checks for longer (they rarely change)
59+
- Cache user permissions
60+
- Use ETags for conditional requests
61+
62+
## Implementation Priority
63+
64+
### High Priority (Quick Wins)
65+
1. **Remove combined status call** - Easy, saves 1 API call
66+
2. **Remove branch protection fallback** - Easy, saves 1 API call
67+
3. **Skip workflow checks unless explicitly needed** - Easy, saves 2-5 API calls
68+
69+
### Medium Priority (More Complex)
70+
1. **Implement single GraphQL query** - Best long-term solution
71+
2. **Use timeline API to replace multiple event calls** - Moderate complexity
72+
73+
### Low Priority (Nice to Have)
74+
1. **Implement ETag caching** - Complex but useful for high-traffic repos
75+
2. **Add permission caching across requests** - Helps with repeated user checks
76+
77+
## GraphQL Query Benefits
78+
79+
The provided GraphQL implementation (`fetchers_graphql_optimized.go`) offers:
80+
81+
1. **Single API Call**: All data in one request
82+
2. **Precise Data Selection**: Only fetch needed fields
83+
3. **Rate Limit Info**: Built-in rate limit tracking
84+
4. **Future-Proof**: Easy to add new fields without new endpoints
85+
86+
## Migration Path
87+
88+
1. **Phase 1**: Remove redundant REST calls (combined status, fallbacks)
89+
2. **Phase 2**: Add GraphQL query as optional alternative
90+
3. **Phase 3**: Migrate to GraphQL by default with REST fallback
91+
4. **Phase 4**: Remove REST implementation
92+
93+
## Expected Savings
94+
95+
- **Current**: 13+ API calls per PR fetch
96+
- **After Quick Wins**: 8-10 API calls (38% reduction)
97+
- **With GraphQL**: 1 API call (92% reduction)
98+
99+
For a user checking 100 PRs:
100+
- **Current**: 1,300+ API calls (likely hitting rate limits)
101+
- **With GraphQL**: 100 API calls (well within limits)

‎.claude/GRAPHQL_MODE.md‎

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
# GraphQL Mode (Experimental)
2+
3+
## Overview
4+
5+
The prx library now includes an experimental hybrid GraphQL+REST mode that significantly reduces GitHub API calls from 13+ to approximately 3-4 calls per pull request. This hybrid approach uses GraphQL for most data and selective REST calls for missing pieces (primarily check runs and rulesets), maintaining data completeness while optimizing API usage.
6+
7+
This is especially useful when working with repositories that have many pull requests or when you're close to GitHub's API rate limits.
8+
9+
## How to Enable
10+
11+
Add the `WithGraphQL()` option when creating a client:
12+
13+
```go
14+
import "github.com/codeGROOVE-dev/prx/pkg/prx"
15+
16+
// REST mode (default)
17+
client := prx.NewClient(token)
18+
19+
// GraphQL mode (experimental)
20+
client := prx.NewClient(token, prx.WithGraphQL())
21+
22+
// GraphQL with other options
23+
client := prx.NewClient(token,
24+
prx.WithGraphQL(),
25+
prx.WithNoCache(),
26+
prx.WithLogger(customLogger),
27+
)
28+
```
29+
30+
## API Call Comparison
31+
32+
### REST Mode (Default)
33+
- **Total API Calls**: 13+ per pull request
34+
- Synchronous calls (5-6):
35+
- Pull request details
36+
- Combined status
37+
- Branch protection
38+
- Branch protection fallback
39+
- Repository rulesets
40+
- Workflow checks (if blocked)
41+
- Parallel calls (7):
42+
- Commits
43+
- Comments
44+
- Reviews
45+
- Review comments
46+
- Timeline events
47+
- Status checks
48+
- Check runs
49+
50+
### Hybrid GraphQL+REST Mode
51+
- **Total API Calls**: 3-4 per pull request
52+
- Main GraphQL query (1 call) fetches:
53+
- Pull request details
54+
- All commits, comments, reviews
55+
- Timeline events (including draft/ready events)
56+
- Branch protection rules
57+
- Required status checks
58+
- Additional REST calls (2-3):
59+
- Check runs (GraphQL's statusCheckRollup is often null)
60+
- Repository rulesets (not available in GraphQL)
61+
- Permission verification for MEMBER users (if needed)
62+
63+
## Benefits
64+
65+
- **70-75% Reduction in API Calls**: From 13+ to 3-4 calls
66+
- **Data Completeness**: Hybrid approach maintains full data parity with REST
67+
- **Faster Performance**: Bulk data fetching via GraphQL
68+
- **Better Rate Limit Management**: Reduced REST API usage
69+
- **Reliability**: REST fallbacks for critical missing data
70+
71+
## Testing & Comparison
72+
73+
You can easily compare REST vs GraphQL output using Unix diff:
74+
75+
```bash
76+
# Fetch with REST (default)
77+
prx https://github.com/owner/repo/pull/123 > rest.json
78+
79+
# Fetch with GraphQL
80+
PRX_USE_GRAPHQL=1 prx https://github.com/owner/repo/pull/123 > graphql.json
81+
82+
# Compare outputs
83+
diff rest.json graphql.json
84+
```
85+
86+
## Known Differences
87+
88+
While we strive for parity, there are some minor differences:
89+
90+
1. **Bot Detection**: GraphQL uses login pattern matching (`-bot`, `[bot]`, `-robot` suffixes) rather than the `type` field
91+
2. **Permissions**: MEMBER association requires an additional REST call for exact permission level
92+
3. **Event Ordering**: Events may be ordered slightly differently but all data is present
93+
94+
## Limitations
95+
96+
- Repository rulesets are not available via GraphQL (requires 1 REST call)
97+
- Some edge cases in bot detection may differ
98+
- Write access detection for MEMBER users is approximated
99+
- **Check Runs**: GraphQL has significant limitations fetching check runs:
100+
- `statusCheckRollup` is often null for many repositories
101+
- Check runs are not included in timeline events
102+
- Fork PRs may have null `headRef`, preventing status checks from being fetched
103+
- Draft PRs may have incomplete status information
104+
- REST API fetches check runs separately and more reliably
105+
- **Old PRs**: Check runs from old PRs (>90 days) may not be available via GraphQL
106+
- GitHub may not expose historical check run data consistently
107+
- REST API may have cached or different retention policies
108+
- For critical historical analysis, REST mode may be more reliable
109+
- **Event Completeness**: GraphQL may miss certain events:
110+
- Check run events are often missing entirely
111+
- Some timeline event types may not be captured
112+
113+
## Stability
114+
115+
This is an **experimental feature**. While it has been tested, you may encounter:
116+
- Minor data differences compared to REST
117+
- Edge cases not yet handled
118+
- Need to fall back to REST for certain operations
119+
120+
Please report any issues or discrepancies you find.
121+
122+
## Environment Variable
123+
124+
You can also enable GraphQL mode via environment variable:
125+
126+
```bash
127+
export PRX_USE_GRAPHQL=1
128+
prx https://github.com/owner/repo/pull/123
129+
```
130+
131+
## Metrics
132+
133+
When GraphQL mode is enabled, the client logs the mode and number of API calls:
134+
135+
```
136+
INFO GraphQL mode enabled owner=golang repo=go pr=123
137+
INFO successfully fetched pull request via GraphQL api_calls_made="2 (vs 13+ with REST)"
138+
```
139+
140+
## Future Improvements
141+
142+
- Full GraphQL implementation without REST fallbacks
143+
- Better caching integration
144+
- Pagination support for very large PRs
145+
- WebSocket subscriptions for real-time updates

‎pkg/prx/client.go‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ func WithHTTPClient(httpClient *http.Client) Option {
6060
func WithNoCache() Option {
6161
return func(c *Client) {
6262
c.cacheDir = ""
63+
// Clear disk path from collaborators cache
64+
c.collaboratorsCache = &collaboratorsCache{
65+
memory: c.collaboratorsCache.memory, // Keep in-memory cache
66+
}
6367
}
6468
}
6569

@@ -93,10 +97,20 @@ func NewClient(token string, opts ...Option) *Client {
9397
api: githubAPI,
9498
},
9599
}
96-
// Initialize in-memory cache (no disk persistence for regular client)
97-
c.collaboratorsCache = &collaboratorsCache{
98-
memory: make(map[string]collaboratorsEntry),
99-
// diskPath is empty, so it won't persist to disk
100+
// Initialize collaborators cache with disk persistence if caching is enabled
101+
if defaultCacheDir != "" {
102+
if err := os.MkdirAll(defaultCacheDir, 0o700); err == nil {
103+
c.collaboratorsCache = newCollaboratorsCache(defaultCacheDir)
104+
} else {
105+
// Fall back to in-memory only cache if directory creation fails
106+
c.collaboratorsCache = &collaboratorsCache{
107+
memory: make(map[string]collaboratorsEntry),
108+
}
109+
}
110+
} else {
111+
c.collaboratorsCache = &collaboratorsCache{
112+
memory: make(map[string]collaboratorsEntry),
113+
}
100114
}
101115

102116
for _, opt := range opts {

0 commit comments

Comments
 (0)