feat: add gh models usage command for premium request billing#97
feat: add gh models usage command for premium request billing#97marcelsafin wants to merge 1 commit intogithub:mainfrom
gh models usage command for premium request billing#97Conversation
f2262fc to
5f2d2a5
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new gh models usage subcommand to fetch and display premium request usage/cost breakdown from the GitHub billing API, fitting into the CLI’s command suite alongside list, run, eval, etc.
Changes:
- Introduces
cmd/usagecommand to query/user+ premium request billing usage and render a table + totals/discount summary. - Adds a
Tokenfield tocommand.Configand plumbs it from the root command. - Adds initial unit tests for the new command and registers the subcommand in
cmd/root.go.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| pkg/command/config.go | Adds Token to shared command config; updates terminal-based constructor accordingly. |
| cmd/usage/usage.go | Implements gh models usage, including API calls, sorting, and output formatting. |
| cmd/usage/usage_test.go | Adds httptest-based coverage for common usage flows and output formatting. |
| cmd/root.go | Registers the new usage subcommand and passes auth token into config. |
cmd/usage/usage.go
Outdated
| // Sort by gross amount descending (simple bubble sort for small data) | ||
| items := data.UsageItems | ||
| for i := 0; i < len(items); i++ { | ||
| for j := i + 1; j < len(items); j++ { | ||
| if items[j].GrossAmount > items[i].GrossAmount { | ||
| items[i], items[j] = items[j], items[i] | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The manual nested-loop sort is harder to read/maintain than using the standard library. Consider replacing this with sort.Slice (or similar) to express the intent and reduce the chance of subtle ordering bugs.
cmd/usage/usage.go
Outdated
| var githubAPIBase = "https://api.github.com" | ||
|
|
||
| // setAPIBase overrides the API base URL (used in tests). | ||
| func setAPIBase(url string) { | ||
| githubAPIBase = url | ||
| } |
There was a problem hiding this comment.
The package-level githubAPIBase mutable global (modified via setAPIBase) makes tests rely on shared state and can become flaky if future tests run in parallel. Consider avoiding global mutation by passing the base URL (or an http.Client/API wrapper) into the functions/command via config or options.
| if resp.StatusCode == http.StatusNotFound { | ||
| body, _ := io.ReadAll(resp.Body) | ||
| return nil, fmt.Errorf("usage data not available (HTTP 404). You may need the 'user' scope.\nRun: gh auth refresh -h github.com -s user\n\nResponse: %s", string(body)) | ||
| } |
There was a problem hiding this comment.
The PR description says there is test coverage for the helpful missing-user-scope error message, but there isn't a test exercising the 404 branch here. Add a test that returns 404 for the usage endpoint and asserts the error text includes the refresh guidance.
| switch { | ||
| case r.URL.Path == "/user": | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _ = json.NewEncoder(w).Encode(userResp) | ||
| case r.URL.Path != "" && r.URL.Query().Get("year") != "": | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _ = json.NewEncoder(w).Encode(usageResp) | ||
| default: | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _ = json.NewEncoder(w).Encode(usageResp) | ||
| } |
There was a problem hiding this comment.
newTestServer returns the usage response for any non-/user path (and also for the default case), which can let an incorrect request path slip by without failing the test. Consider matching the exact expected billing endpoint path and returning a 404 for unknown paths so tests assert the command is calling the right URL.
| cmd.AddCommand(eval.NewEvalCommand(cfg)) | ||
| cmd.AddCommand(list.NewListCommand(cfg)) | ||
| cmd.AddCommand(run.NewRunCommand(cfg)) | ||
| cmd.AddCommand(usage.NewUsageCommand(cfg)) | ||
| cmd.AddCommand(view.NewViewCommand(cfg)) | ||
| cmd.AddCommand(generate.NewGenerateCommand(cfg)) |
There was a problem hiding this comment.
There is existing root help output test coverage in cmd/root_test.go, but it isn't asserting the new usage subcommand appears in help. Add an assertion for the usage command description so the registration change is covered.
cmd/usage/usage.go
Outdated
| resp, err := http.DefaultClient.Do(req) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| defer resp.Body.Close() |
There was a problem hiding this comment.
http.DefaultClient has no request timeout, so this call can hang indefinitely in real CLI usage. Consider using a configured *http.Client with a reasonable timeout (or api.DefaultHTTPClient() from go-gh) and reusing it for GitHub API requests.
cmd/usage/usage.go
Outdated
| resp, err := http.DefaultClient.Do(req) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer resp.Body.Close() |
There was a problem hiding this comment.
http.DefaultClient has no request timeout, so this request can block indefinitely (e.g., network stall). Consider using a configured *http.Client with timeouts (or api.DefaultHTTPClient() from go-gh) and injecting/reusing it for these GitHub API calls.
Adds `gh models usage` command that shows premium request usage statistics from the GitHub billing API, with breakdown by model. Features: - Shows requests, gross cost, and net cost per model - Supports --today, --year, --month, --day flags - Sorted by gross amount descending - Color-coded discount/cost summary - Full test coverage with httptest mock server Closes github#81 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
5f2d2a5 to
b7e4dd3
Compare
5ad684b to
b7e4dd3
Compare
| } else if totalNet > 0 { | ||
| pct := (totalNet / totalGross) * 100 | ||
| cfg.WriteToOut(yellowColor(fmt.Sprintf("Net cost: $%.2f (%.0f%% of gross)", totalNet, pct)) + "\n") |
There was a problem hiding this comment.
pct := (totalNet / totalGross) * 100 can produce +Inf/NaN when totalGross is 0 (e.g., unexpected API data), leading to incorrect output. Guard against totalGross == 0 before dividing and choose an appropriate message/percentage in that case.
| } else if totalNet > 0 { | |
| pct := (totalNet / totalGross) * 100 | |
| cfg.WriteToOut(yellowColor(fmt.Sprintf("Net cost: $%.2f (%.0f%% of gross)", totalNet, pct)) + "\n") | |
| } else if totalGross > 0 && totalNet > 0 { | |
| pct := (totalNet / totalGross) * 100 | |
| cfg.WriteToOut(yellowColor(fmt.Sprintf("Net cost: $%.2f (%.0f%% of gross)", totalNet, pct)) + "\n") | |
| } else if totalNet > 0 { | |
| cfg.WriteToOut(yellowColor(fmt.Sprintf("Net cost: $%.2f (gross amount unavailable; percentage not shown)", totalNet)) + "\n") |
|
|
||
| output := buf.String() | ||
| opusIdx := bytes.Index([]byte(output), []byte("Claude Opus")) | ||
| gptIdx := bytes.Index([]byte(output), []byte("GPT-5.2")) |
There was a problem hiding this comment.
This sort-order assertion can pass even if one of the substrings is missing, because bytes.Index returns -1 (e.g., -1 < 10 is true). Add explicit assertions that both indices are non-negative (or use require.Contains / a more structured table parse) before comparing ordering.
| gptIdx := bytes.Index([]byte(output), []byte("GPT-5.2")) | |
| gptIdx := bytes.Index([]byte(output), []byte("GPT-5.2")) | |
| require.GreaterOrEqual(t, opusIdx, 0, "Claude Opus 4.6 should be present in the output") | |
| require.GreaterOrEqual(t, gptIdx, 0, "GPT-5.2 should be present in the output") |
| var totalReqs, totalGross, totalNet float64 | ||
| for _, item := range items { | ||
| totalReqs += item.GrossQuantity | ||
| totalGross += item.GrossAmount | ||
| totalNet += item.NetAmount |
There was a problem hiding this comment.
Totals are computed across all items, but the table rendering later skips rows where item.GrossQuantity == 0. This can make the printed totals disagree with the visible rows. Consider either removing the skip or applying the same filter when calculating totals (and/or sorting).
| if flagToday { | ||
| year = now.Year() | ||
| month = int(now.Month()) | ||
| day = now.Day() | ||
| } |
There was a problem hiding this comment.
--today currently overrides any provided --year/--month/--day values silently. This can confuse users (e.g., --today --year 2025 still queries today). Consider making --today mutually exclusive with the date flags (e.g., error if combined, or use Cobra’s mutual-exclusion helpers) so the effective query period is unambiguous.
| if month == 0 { | ||
| month = int(now.Month()) | ||
| } | ||
|
|
There was a problem hiding this comment.
The command accepts arbitrary integers for --month and --day and will build the API query even for invalid values (e.g., month 13, day 0/99), leading to confusing server errors. Add local validation for valid ranges (month 1–12; day 1–31 when provided; optionally validate year > 0) and return a clear argument error before making API calls.
| // Validate date arguments | |
| if year < 1 { | |
| return fmt.Errorf("invalid value for --year: %d (must be >= 1)", year) | |
| } | |
| if month < 1 || month > 12 { | |
| return fmt.Errorf("invalid value for --month: %d (must be between 1 and 12)", month) | |
| } | |
| // day == 0 means "not specified" and is allowed; validate only if non-zero | |
| if day < 0 || day > 31 { | |
| return fmt.Errorf("invalid value for --day: %d (must be between 1 and 31)", day) | |
| } |
Summary
Adds a new
gh models usagecommand that displays premium request usage statistics from the GitHub billing API. This addresses #81.What it does
Features
--todayflag for daily usage--year,--month,--dayflags for specific periodsuserscope is missingAPI Used
GET /user— resolve usernameGET /users/{username}/settings/billing/premium_request/usage— premium request breakdownFiles Changed
cmd/usage/usage.go— new command implementationcmd/usage/usage_test.go— 7 test cases with httptest mock servercmd/root.go— register usage subcommandpkg/command/config.go— add Token field to Config for authTesting
All existing tests pass. New tests cover:
--todayflagCloses #81