Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 38 additions & 14 deletions pkg/cli/logs_github_rate_limit_usage.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,12 @@ type GitHubRateLimitResourceUsage struct {
// by a single workflow run. It is populated by parsing the github_rate_limits.jsonl
// artifact produced during the run.
type GitHubRateLimitUsage struct {
TotalRequestsMade int `json:"total_requests_made" console:"header:Total GitHub API Calls,format:number"`
CoreConsumed int `json:"core_consumed" console:"header:Core Quota Consumed,format:number"`
CoreRemaining int `json:"core_remaining" console:"header:Core Remaining,format:number"`
CoreLimit int `json:"core_limit" console:"header:Core Limit,format:number"`
Resources []*GitHubRateLimitResourceUsage `json:"resources,omitempty"`
TotalRequestsMade int `json:"total_requests_made" console:"header:Total GitHub API Calls,format:number"`
CoreConsumed int `json:"core_consumed" console:"header:Core Quota Consumed,format:number"`
CoreConsumedSource string `json:"core_consumed_source,omitempty" console:"-"`
CoreRemaining int `json:"core_remaining" console:"header:Core Remaining,format:number"`
CoreLimit int `json:"core_limit" console:"header:Core Limit,format:number"`
Resources []*GitHubRateLimitResourceUsage `json:"resources,omitempty"`
}

// ResourceRows returns per-resource rows sorted by total requests made descending,
Expand Down Expand Up @@ -130,13 +131,16 @@ func parseGitHubRateLimitsFile(filePath string) (*GitHubRateLimitUsage, error) {
defer file.Close()

type resourceState struct {
requestsMade int
firstRemaining int
lastRemaining int
firstUsed int
lastUsed int
limit int
firstEntrySet bool
requestsMade int
firstRemaining int
lastRemaining int
firstUsed int
lastUsed int
firstSnapshotUsed int
lastSnapshotUsed int
snapshotCount int
limit int
firstEntrySet bool
}
byResource := make(map[string]*resourceState)

Expand Down Expand Up @@ -187,13 +191,19 @@ func parseGitHubRateLimitsFile(filePath string) (*GitHubRateLimitUsage, error) {
state.limit = entry.Limit
}
case "rate_limit_api":
// Use rate-limit API snapshots to fill in limit and remaining when we
// have no response-header entries for this resource yet.
// Use rate-limit API snapshots to fill in limit and remaining, and to
// derive core_consumed via firstSnapshotUsed/lastSnapshotUsed when no
// response-header entries are present for this resource.
state, ok := byResource[resource]
if !ok {
state = &resourceState{}
byResource[resource] = state
}
state.snapshotCount++
if state.snapshotCount == 1 {
state.firstSnapshotUsed = entry.Used
}
state.lastSnapshotUsed = entry.Used
Comment on lines +202 to +206

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the comment in 0346fa7 to reflect that snapshots are also used to derive core_consumed via firstSnapshotUsed/lastSnapshotUsed.

if entry.Limit > 0 && state.limit == 0 {
state.limit = entry.Limit
}
Expand Down Expand Up @@ -226,6 +236,7 @@ func parseGitHubRateLimitsFile(filePath string) (*GitHubRateLimitUsage, error) {
// If used values suggest a window reset occurred (lastUsed < firstUsed),
// fall back to using the absolute lastUsed value as the consumption metric.
var consumed int
consumedSource := ""
if state.requestsMade > 0 {
diff := state.lastUsed - state.firstUsed
if diff >= 0 {
Expand All @@ -234,6 +245,18 @@ func parseGitHubRateLimitsFile(filePath string) (*GitHubRateLimitUsage, error) {
// Window reset mid-run; use lastUsed as a lower-bound estimate
consumed = state.lastUsed
}
consumedSource = "response_headers_delta"
} else if state.snapshotCount >= 2 {
diff := state.lastSnapshotUsed - state.firstSnapshotUsed
if diff >= 0 {
consumed = diff
} else {
// Window reset across snapshots; use last snapshot used as a lower-bound estimate
consumed = state.lastSnapshotUsed
}
consumedSource = "rate_limit_api_delta"
} else if state.snapshotCount == 1 {
consumedSource = "rate_limit_api_single_snapshot"
}

row := &GitHubRateLimitResourceUsage{
Expand All @@ -247,6 +270,7 @@ func parseGitHubRateLimitsFile(filePath string) (*GitHubRateLimitUsage, error) {

if resource == "core" {
usage.CoreConsumed = consumed
usage.CoreConsumedSource = consumedSource
usage.CoreRemaining = state.lastRemaining
usage.CoreLimit = state.limit
}
Expand Down
43 changes: 43 additions & 0 deletions pkg/cli/logs_github_rate_limit_usage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ func TestParseGitHubRateLimitsFileBasic(t *testing.T) {

// Core resource: 2 calls, consumed = lastUsed(120) - firstUsed(110) = 10
assert.Equal(t, 10, usage.CoreConsumed, "core quota consumed should be 10")
assert.Equal(t, "response_headers_delta", usage.CoreConsumedSource, "core consumed source should come from response headers")
assert.Equal(t, 4880, usage.CoreRemaining, "core remaining should match last entry")
assert.Equal(t, 5000, usage.CoreLimit, "core limit should be 5000")

Expand Down Expand Up @@ -103,6 +104,48 @@ func TestParseGitHubRateLimitsFileOnlyAPISnapshots(t *testing.T) {
require.NotNil(t, usage, "usage should not be nil")

assert.Equal(t, 0, usage.TotalRequestsMade, "should count 0 API calls from response_headers")
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")
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added TestParseGitHubRateLimitsFileOnlyAPISnapshotSingle in 0346fa7, covering the exactly-one-snapshot case: asserts CoreConsumed==0 and CoreConsumedSource=="rate_limit_api_single_snapshot".

// TestParseGitHubRateLimitsFileOnlyAPISnapshotsWindowReset verifies snapshot-only
// runs still produce a lower-bound consumption value when the window resets.
func TestParseGitHubRateLimitsFileOnlyAPISnapshotsWindowReset(t *testing.T) {
content := `{"timestamp":"2026-04-05T08:00:00.000Z","source":"rate_limit_api","operation":"startup","resource":"core","limit":5000,"remaining":100,"used":4900,"reset":"2026-04-05T09:00:00.000Z"}
{"timestamp":"2026-04-05T09:00:10.000Z","source":"rate_limit_api","operation":"shutdown","resource":"core","limit":5000,"remaining":4995,"used":5,"reset":"2026-04-05T10:00:00.000Z"}
`
dir := t.TempDir()
path := filepath.Join(dir, "github_rate_limits.jsonl")
require.NoError(t, os.WriteFile(path, []byte(content), 0600), "should write test JSONL file")

usage, err := parseGitHubRateLimitsFile(path)
require.NoError(t, err, "should not return an error")
require.NotNil(t, usage, "usage should not be nil")

assert.Equal(t, 0, usage.TotalRequestsMade, "should count 0 API calls from response_headers")
assert.Equal(t, 5, usage.CoreConsumed, "window reset should fall back to last snapshot used value")
assert.Equal(t, "rate_limit_api_delta", usage.CoreConsumedSource, "core consumed source should come from rate_limit_api snapshots")
}

// TestParseGitHubRateLimitsFileOnlyAPISnapshotSingle verifies that a file containing
// exactly one rate_limit_api snapshot (no response_headers) produces CoreConsumed==0
// with provenance tagged as rate_limit_api_single_snapshot.
func TestParseGitHubRateLimitsFileOnlyAPISnapshotSingle(t *testing.T) {
content := `{"timestamp":"2026-04-05T08:00:00.000Z","source":"rate_limit_api","operation":"startup","resource":"core","limit":5000,"remaining":4850,"used":150,"reset":"2026-04-05T09:00:00.000Z"}
`
dir := t.TempDir()
path := filepath.Join(dir, "github_rate_limits.jsonl")
require.NoError(t, os.WriteFile(path, []byte(content), 0600), "should write test JSONL file")

usage, err := parseGitHubRateLimitsFile(path)
require.NoError(t, err, "should not return an error")
require.NotNil(t, usage, "usage should not be nil")

assert.Equal(t, 0, usage.TotalRequestsMade, "should count 0 API calls from response_headers")
assert.Equal(t, 0, usage.CoreConsumed, "single snapshot cannot compute a delta; consumed should be 0")
assert.Equal(t, "rate_limit_api_single_snapshot", usage.CoreConsumedSource, "provenance should be rate_limit_api_single_snapshot")
assert.Equal(t, 4850, usage.CoreRemaining, "core remaining should match snapshot value")
assert.Equal(t, 5000, usage.CoreLimit, "core limit should match snapshot value")
}

// TestFindGitHubRateLimitsFileAbsent verifies that findGitHubRateLimitsFile returns
Expand Down
Loading