refactor: extract TokenCoreMetrics to unify token-usage quartet#46059
Conversation
Extract a shared TokenCoreMetrics struct as the single source of truth
for the {InputTokens, OutputTokens, CacheReadTokens, CacheWriteTokens}
quartet (plus ReasoningTokens and EffectiveTokens).
Embed TokenCoreMetrics in TokenUsageEntry, ModelTokenUsage, and
ModelTokenUsageRow, collapsing the ModelTokenUsage/ModelTokenUsageRow
near-dup and eliminating the duplicated field declarations.
Update ModelRows() to copy the embedded struct directly instead of
field-by-field to automatically include future additions.
Add a comment to copilotUsageMetrics explaining it intentionally uses
camelCase json tags to match the external Copilot events.jsonl format,
distinguishing it from the snake_case convention used by TokenCoreMetrics.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
🤖 PR Triage
Rationale: Small refactor (3 files, +69/-64) extracting Run §29524501227
|
There was a problem hiding this comment.
Pull request overview
Refactors token metrics into a shared structure while preserving Copilot’s external wire format.
Changes:
- Adds and embeds
TokenCoreMetrics. - Simplifies model-row construction.
- Clarifies Copilot’s camelCase JSON requirement.
Show a summary per file
| File | Description |
|---|---|
pkg/cli/token_usage.go |
Consolidates token metrics and row mapping. |
pkg/cli/token_usage_test.go |
Updates fixtures for embedding. |
pkg/cli/copilot_events_jsonl.go |
Documents wire-format differences. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Medium
| AvgDuration string `json:"avg_duration" console:"header:Avg Duration"` | ||
| Model string `json:"model" console:"header:Model"` | ||
| Provider string `json:"provider" console:"header:Provider"` | ||
| TokenCoreMetrics |
There was a problem hiding this comment.
Implemented in cb19855. ModelTokenUsageRow now explicitly carries only the token quartet (no embedded TokenCoreMetrics), and ModelRows() maps only those four fields. I also added a regression in TestTokenUsageSummaryMethods to assert row JSON/rendered output do not include reasoning_tokens or effective_tokens columns.
|
@copilot run pr-finisher skill |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
🎉 This pull request is included in a new release. Release: |
The
{InputTokens, OutputTokens, CacheReadTokens, CacheWriteTokens}quartet (plusReasoningTokens/EffectiveTokens) was duplicated across four structs with a camelCase/snake_case json tag inconsistency betweencopilotUsageMetricsand the rest.Changes
New
TokenCoreMetricsstruct — single source of truth for the token quartet with snake_case json tags and console display tags.ReasoningTokensandEffectiveTokensareomitempty.Embed in
TokenUsageEntry,ModelTokenUsage,ModelTokenUsageRow— removes 5–6 duplicated field declarations per struct, collapses theModelTokenUsage/ModelTokenUsageRownear-dup.ModelRows()— copiesusage.TokenCoreMetricsdirectly rather than field-by-field, so future additions to the core struct propagate automatically.copilotUsageMetrics— kept as-is (must use camelCase to match the external Copilot events.jsonl wire format); now has an explicit comment distinguishing it fromTokenCoreMetrics.