Clarify github_api_calls provenance and recover snapshot-only core_consumed accounting#39623
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
github_api_calls provenance and recover snapshot-only core_consumed accounting
There was a problem hiding this comment.
Pull request overview
This PR improves the reliability and interpretability of GitHub API quota accounting in gh-aw run/audit reporting by making core_consumed provenance explicit and by restoring meaningful core_consumed estimates for runs where only rate_limit_api snapshots are present (no per-call response headers).
Changes:
- Adds
core_consumed_sourcetoGitHubRateLimitUsageto tag howcore_consumedwas derived. - Extends parsing logic to derive
core_consumedfromrate_limit_apisnapshot deltas (including a reset-safe fallback). - Updates/extends unit tests to cover snapshot-derived accounting and provenance.
Show a summary per file
| File | Description |
|---|---|
| pkg/cli/logs_github_rate_limit_usage.go | Adds provenance tagging and snapshot-based core_consumed derivation logic. |
| pkg/cli/logs_github_rate_limit_usage_test.go | Adds regression coverage for snapshot-only runs and reset behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 2
| state.snapshotCount++ | ||
| if state.snapshotCount == 1 { | ||
| state.firstSnapshotUsed = entry.Used | ||
| } | ||
| state.lastSnapshotUsed = entry.Used |
There was a problem hiding this comment.
Updated the comment in 0346fa7 to reflect that snapshots are also used to derive core_consumed via firstSnapshotUsed/lastSnapshotUsed.
| assert.Equal(t, 10, usage.CoreConsumed, "core consumed should be derived from rate_limit_api snapshot delta") | ||
| assert.Equal(t, "rate_limit_api_delta", usage.CoreConsumedSource, "core consumed source should come from rate_limit_api snapshots") | ||
| } | ||
|
|
There was a problem hiding this comment.
Added TestParseGitHubRateLimitsFileOnlyAPISnapshotSingle in 0346fa7, covering the exactly-one-snapshot case: asserts CoreConsumed==0 and CoreConsumedSource=="rate_limit_api_single_snapshot".
|
@copilot run pr-finisher skill |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
The API-consumption report showed a sharp 06-13 → 06-14
github_api_callsdiscontinuity that looked non-organic and likely instrumentation-related. This change makes the underlyingcore_consumedderivation explicit and robust when log shape shifts from per-call headers to rate-limit snapshots.Metric provenance surfaced in run summaries
core_consumed_sourcetoGitHubRateLimitUsageso downstream analysis can distinguish howcore_consumedwas derived (response_headers_delta,rate_limit_api_delta,rate_limit_api_single_snapshot).Snapshot-only runs now produce usable
core_consumedcore_consumedfromrate_limit_apisnapshot deltas whenresponse_headersentries are absent.usedas lower-bound estimate), mirroring existing resilience for header-based deltas.Targeted parser regression coverage