Conversation
…ands Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c808f74b-e7ac-4409-ab29-12db76956413 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…nction Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c808f74b-e7ac-4409-ab29-12db76956413 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot add support in audit diff |
There was a problem hiding this comment.
Pull request overview
Adds GitHub API rate-limit usage analysis by parsing the github_rate_limits.jsonl artifact and surfacing aggregated API call/quota consumption metrics in both gh aw logs and gh aw audit outputs.
Changes:
- Introduces parsing/aggregation logic for
github_rate_limits.jsonland unit tests for key scenarios. - Propagates
GitHubRateLimitUsagethrough logs/audit processing models and download pipelines (including cached-run paths). - Renders the new audit console section and adds a new logs table column + summary total for GitHub API calls.
Show a summary per file
| File | Description |
|---|---|
| pkg/cli/logs_github_rate_limit_usage.go | Implements file discovery + JSONL parsing and aggregates per-resource and core GitHub API usage metrics. |
| pkg/cli/logs_github_rate_limit_usage_test.go | Adds unit tests covering parsing, file discovery, window reset handling, and row sorting. |
| pkg/cli/logs_models.go | Plumbs GitHubRateLimitUsage through ProcessedRun, RunSummary, and DownloadResult. |
| pkg/cli/logs_orchestrator.go | Runs usage analysis after artifact download and propagates into processed/cached run summaries. |
| pkg/cli/logs_report.go | Adds per-run and total “GitHub API Calls” fields/columns to the logs report output. |
| pkg/cli/audit.go | Runs usage analysis during audit processing and stores it on processed run + summary. |
| pkg/cli/audit_report.go | Adds GitHubRateLimitUsage to AuditData and populates it from ProcessedRun. |
| pkg/cli/audit_report_render.go | Renders a new “GitHub API Usage” console section with summary + per-resource breakdown. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 8/8 changed files
- Comments generated: 5
| var consumed int | ||
| if state.requestsMade > 0 { | ||
| diff := state.lastUsed - state.firstUsed | ||
| if diff >= 0 { | ||
| consumed = diff | ||
| } else { | ||
| // Window reset mid-run; use lastUsed as a lower-bound estimate | ||
| consumed = state.lastUsed | ||
| } | ||
| } |
There was a problem hiding this comment.
QuotaConsumed is undercounted: with one response_headers entry, diff := lastUsed - firstUsed yields 0 even though at least 1 request was made. Since x-ratelimit-used reflects the post-request value, the per-window delta should include the first observed request (e.g., max(0, lastUsed-firstUsed)+1 when requestsMade>0), and tests should cover the single-request case.
|
|
||
| // Sort resources for deterministic output | ||
| sort.Slice(usage.Resources, func(i, j int) bool { | ||
| return usage.Resources[i].RequestsMade > usage.Resources[j].RequestsMade |
There was a problem hiding this comment.
The sort comparator only considers RequestsMade. When counts are equal (common for snapshot-only files where all RequestsMade are 0), the resulting order is nondeterministic due to Go map iteration + sort.Slice being unstable, despite the comment about deterministic output. Add a secondary tie-breaker (e.g., Resource name) here, and mirror it in ResourceRows() for consistent console/JSON ordering.
| return usage.Resources[i].RequestsMade > usage.Resources[j].RequestsMade | |
| if usage.Resources[i].RequestsMade != usage.Resources[j].RequestsMade { | |
| return usage.Resources[i].RequestsMade > usage.Resources[j].RequestsMade | |
| } | |
| return usage.Resources[i].Resource < usage.Resources[j].Resource |
| TotalSafeItems int `json:"total_safe_items" console:"header:Total Safe Items"` | ||
| TotalEpisodes int `json:"total_episodes" console:"header:Total Episodes"` | ||
| HighConfidenceEpisodes int `json:"high_confidence_episodes" console:"header:High Confidence Episodes"` | ||
| TotalGitHubAPICalls int `json:"total_github_api_calls,omitempty" console:"header:Total GitHub API Calls,format:number,omitempty"` | ||
| } |
There was a problem hiding this comment.
With omitempty, this field is omitted (and the console column hidden) whenever the total is 0. That makes it indistinguishable from “no rate-limit artifact found”, and it contradicts the PR description for the snapshot-only case (rate-limit data found but TotalRequestsMade == 0). Consider using a pointer/nullable type (or a separate “data present” flag) so you can render 0 when data exists while still omitting when absent.
| AgenticAssessments []AgenticAssessment `json:"agentic_assessments,omitempty" console:"-"` | ||
| AwContext *AwContext `json:"context,omitempty" console:"-"` // aw_context data from aw_info.json | ||
| TokenUsageSummary *TokenUsageSummary `json:"token_usage_summary,omitempty" console:"-"` // Token usage from firewall proxy | ||
| AwContext *AwContext `json:"context,omitempty" console:"-"` // aw_context data from aw_info.json | ||
| TokenUsageSummary *TokenUsageSummary `json:"token_usage_summary,omitempty" console:"-"` // Token usage from firewall proxy | ||
| GitHubAPICalls int `json:"github_api_calls,omitempty" console:"header:GitHub API Calls,format:number,omitempty"` // GitHub API calls made during the run | ||
| } |
There was a problem hiding this comment.
Same issue as the summary: omitempty on an int will hide the per-run column when the value is 0, even if GitHubRateLimitUsage was present but indicated zero calls (snapshot-only). If the intent is “hide only when artifact missing”, consider making this a *int and setting it only when usage is non-nil, or rendering a sentinel/default value (e.g., N/A) for missing data.
| // Per-resource breakdown table (only when there are multiple resources or non-core resources) | ||
| rows := usage.ResourceRows() | ||
| if len(rows) == 0 { | ||
| return | ||
| } |
There was a problem hiding this comment.
The comment says the per-resource breakdown table is shown “only when there are multiple resources or non-core resources”, but the code renders it whenever len(rows) > 0 (including a single core row). Either adjust the condition to match the comment, or update the comment to reflect the current behavior.
|
@copilot review comments |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/ac769c8d-3a8f-4bae-ac01-06ed0846e26a Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in commit
|
- Add git, copilot_spaces, github_support_docs_search toolsets to github-tools.md (from PR #24729, v2.2 toolsets mapping) - Add GitHub API rate limit consumption to audit report sections, cross-run logs report, and audit diff output (from PR #24748) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Parses the
github_rate_limits.jsonlartifact produced by workflow runs to provide a measure of how much GitHub API quota each agentic workflow consumes. This data is now surfaced in thelogs,audit, andaudit diffcommands.Changes
New file:
pkg/cli/logs_github_rate_limit_usage.goGitHubRateLimitEntry(JSONL line structure),GitHubRateLimitResourceUsage(per-resource stats), andGitHubRateLimitUsage(aggregated summary)analyzeGitHubRateLimits(runDir, verbose)– locates and parsesgithub_rate_limits.jsonlfrom a run's artifact directorylastUsed), only API snapshot entries (zeroTotalRequestsMade), entries without aresourcefield (bucketed as"core")pkg/cli/logs_models.goGitHubRateLimitUsage *GitHubRateLimitUsagetoProcessedRun,RunSummary, andDownloadResultpkg/cli/audit.goanalyzeGitHubRateLimits()after artifact download, stores result inProcessedRunandRunSummarypkg/cli/audit_report.goGitHubRateLimitUsage *GitHubRateLimitUsagetoAuditData; populated fromprocessedRun.GitHubRateLimitUsagepkg/cli/audit_report_render.gorenderConsole()(after Token Usage) – shows a summary line (total calls, core consumed/remaining/limit) and a per-resource breakdown tablepkg/cli/logs_orchestrator.goanalyzeGitHubRateLimits()in the download worker; propagates through cached-run path and bothProcessedRunandRunSummaryconstructionpkg/cli/logs_report.goGitHubAPICalls inttoRunData(column:"GitHub API Calls")TotalGitHubAPICalls inttoLogsSummary(column:"Total GitHub API Calls")omitempty– column is hidden for runs/summaries where no rate-limit data was foundpkg/cli/audit_diff.goGitHubRateLimitDiffstruct with per-run API call counts, core quota consumed/remaining/limit, and percentage-change stringsGitHubRateLimitDetails *GitHubRateLimitDifftoRunMetricsDiffalongside the existingTokenUsageDetailscomputeGitHubRateLimitDiff()function; updatescomputeRunMetricsDiff()to call itloadRunSummaryForDiff()to callanalyzeGitHubRateLimits()when building partial summaries for non-cached runspkg/cli/audit_diff_render.gorenderGitHubRateLimitDiffMarkdownSection()– renders a#### GitHub API Usagemarkdown tablerenderGitHubRateLimitDiffPrettySection()– renders a🐙 GitHub API Usageconsole tableNew file:
pkg/cli/logs_github_rate_limit_usage_test.go9 unit tests covering:
resourcefield defaults to"core"lastUsedResourceRows()sort orderpkg/cli/audit_diff_test.go5 new unit tests covering:
computeGitHubRateLimitDiffwith both nil, with data, and run1 nilcomputeRunMetricsDiffwith rate limit data and rate limit data alone triggering non-nil resultExample output
gh aw audit– new section:gh aw logs– new column in run table:gh aw audit diff– new section in run metrics diff: