Skip to content

Commit 21926a0

Browse files
Copilotpelikhan
andauthored
Fix firewall token usage reporting to preserve raw counts and remove cache-rate transforms (#31581)
* Initial plan * Plan: fix token usage cache accounting Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * Report raw token usage without cache-rate transforms Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Co-authored-by: Peli de Halleux <pelikhan@users.noreply.github.com>
1 parent bec1e7e commit 21926a0

7 files changed

Lines changed: 53 additions & 72 deletions

File tree

.github/workflows/issue-arborist.lock.yml

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

actions/setup/js/parse_mcp_gateway_log.cjs

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ function formatDurationMs(ms) {
5050
* Parses token-usage.jsonl content and returns an aggregated summary.
5151
* Computes effective tokens (ET) per model using the GH_AW_MODEL_MULTIPLIERS env var.
5252
* @param {string} jsonlContent - The token-usage.jsonl file content
53-
* @returns {{totalInputTokens: number, totalOutputTokens: number, totalCacheReadTokens: number, totalCacheWriteTokens: number, totalRequests: number, totalDurationMs: number, cacheEfficiency: number, totalEffectiveTokens: number, byModel: Object} | null}
53+
* @returns {{totalInputTokens: number, totalOutputTokens: number, totalCacheReadTokens: number, totalCacheWriteTokens: number, totalRequests: number, totalDurationMs: number, totalEffectiveTokens: number, byModel: Object} | null}
5454
*/
5555
function parseTokenUsageJsonl(jsonlContent) {
5656
const summary = {
@@ -60,7 +60,6 @@ function parseTokenUsageJsonl(jsonlContent) {
6060
totalCacheWriteTokens: 0,
6161
totalRequests: 0,
6262
totalDurationMs: 0,
63-
cacheEfficiency: 0,
6463
totalEffectiveTokens: 0,
6564
byModel: {},
6665
};
@@ -110,11 +109,6 @@ function parseTokenUsageJsonl(jsonlContent) {
110109

111110
if (summary.totalRequests === 0) return null;
112111

113-
const totalInputPlusCacheRead = summary.totalInputTokens + summary.totalCacheReadTokens;
114-
if (totalInputPlusCacheRead > 0) {
115-
summary.cacheEfficiency = summary.totalCacheReadTokens / totalInputPlusCacheRead;
116-
}
117-
118112
// Compute effective tokens per model and aggregate total
119113
let totalEffectiveTokens = 0;
120114
for (const [model, usage] of Object.entries(summary.byModel)) {
@@ -130,7 +124,7 @@ function parseTokenUsageJsonl(jsonlContent) {
130124
/**
131125
* Generates a markdown summary section for token usage data.
132126
* Includes an Effective Tokens (ET) column per model and a ● ET summary line.
133-
* @param {{totalInputTokens: number, totalOutputTokens: number, totalCacheReadTokens: number, totalCacheWriteTokens: number, totalRequests: number, totalDurationMs: number, cacheEfficiency: number, totalEffectiveTokens: number, byModel: Object} | null} summary
127+
* @param {{totalInputTokens: number, totalOutputTokens: number, totalCacheReadTokens: number, totalCacheWriteTokens: number, totalRequests: number, totalDurationMs: number, totalEffectiveTokens: number, byModel: Object} | null} summary
134128
* @returns {string} Markdown section, or empty string if no data
135129
*/
136130
function generateTokenUsageSummary(summary) {
@@ -159,14 +153,11 @@ function generateTokenUsageSummary(summary) {
159153
`| **Total** | **${summary.totalInputTokens.toLocaleString()}** | **${summary.totalOutputTokens.toLocaleString()}** | **${summary.totalCacheReadTokens.toLocaleString()}** | **${summary.totalCacheWriteTokens.toLocaleString()}** | **${totalET}** | **${summary.totalRequests}** | **${formatDurationMs(summary.totalDurationMs)}** |`
160154
);
161155

162-
// Footer line with ET summary using ● symbol and optional cache efficiency
156+
// Footer line with ET summary using ● symbol
163157
const footerParts = [];
164158
if (summary.totalEffectiveTokens > 0) {
165159
footerParts.push(`● ${formatET(Math.round(summary.totalEffectiveTokens))}`);
166160
}
167-
if (summary.cacheEfficiency > 0) {
168-
footerParts.push(`Cache efficiency: ${(summary.cacheEfficiency * 100).toFixed(1)}%`);
169-
}
170161
if (footerParts.length > 0) {
171162
lines.push(`\n_${footerParts.join(" · ")}_`);
172163
// Disclose the token class weights used to compute ET (required by the ET spec)

actions/setup/js/parse_mcp_gateway_log.test.cjs

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1092,12 +1092,11 @@ not-json
10921092
expect(summary.byModel["unknown"]).toBeDefined();
10931093
});
10941094

1095-
test("computes cache efficiency", () => {
1095+
test("does not compute cache efficiency", () => {
10961096
const content = JSON.stringify({ model: "m", input_tokens: 100, output_tokens: 10, cache_read_tokens: 900, cache_write_tokens: 0, duration_ms: 100 });
10971097
const summary = parseTokenUsageJsonl(content);
10981098
expect(summary).not.toBeNull();
1099-
// cache_read / (input + cache_read) = 900 / 1000 = 0.9
1100-
expect(summary.cacheEfficiency).toBeCloseTo(0.9);
1099+
expect(summary).not.toHaveProperty("cacheEfficiency");
11011100
});
11021101
});
11031102

@@ -1120,17 +1119,10 @@ not-json
11201119
expect(md).toContain("**Total**");
11211120
});
11221121

1123-
test("includes cache efficiency when non-zero", () => {
1122+
test("does not include cache efficiency", () => {
11241123
const content = JSON.stringify({ model: "m", input_tokens: 100, output_tokens: 10, cache_read_tokens: 900, cache_write_tokens: 0, duration_ms: 100 });
11251124
const summary = parseTokenUsageJsonl(content);
11261125
const md = generateTokenUsageSummary(summary);
1127-
expect(md).toContain("Cache efficiency: 90.0%");
1128-
});
1129-
1130-
test("omits cache efficiency line when zero", () => {
1131-
const content = JSON.stringify({ model: "m", input_tokens: 100, output_tokens: 10, cache_read_tokens: 0, cache_write_tokens: 0, duration_ms: 100 });
1132-
const summary = parseTokenUsageJsonl(content);
1133-
const md = generateTokenUsageSummary(summary);
11341126
expect(md).not.toContain("Cache efficiency");
11351127
});
11361128

@@ -1163,16 +1155,12 @@ not-json
11631155
expect(md).toContain("●");
11641156
});
11651157

1166-
test("includes cache efficiency after ● ET in footer line", () => {
1158+
test("includes ● ET in footer line without cache efficiency", () => {
11671159
const content = JSON.stringify({ model: "m", input_tokens: 100, output_tokens: 10, cache_read_tokens: 900, cache_write_tokens: 0, duration_ms: 100 });
11681160
const summary = parseTokenUsageJsonl(content);
11691161
const md = generateTokenUsageSummary(summary);
11701162
expect(md).toContain("●");
1171-
expect(md).toContain("Cache efficiency: 90.0%");
1172-
// ET should appear before cache efficiency
1173-
const etIdx = md.indexOf("●");
1174-
const ceIdx = md.indexOf("Cache efficiency");
1175-
expect(etIdx).toBeLessThan(ceIdx);
1163+
expect(md).not.toContain("Cache efficiency");
11761164
});
11771165
});
11781166

pkg/cli/audit_report_render_findings.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -170,19 +170,13 @@ func renderSafeOutputSummary(summary *SafeOutputSummary) {
170170

171171
// renderTokenUsage displays token usage data from the firewall proxy
172172
func renderTokenUsage(summary *TokenUsageSummary) {
173-
totalTokens := summary.TotalTokens()
174-
cacheTokens := summary.TotalCacheReadTokens + summary.TotalCacheWriteTokens
175-
176-
fmt.Fprintf(os.Stderr, " Total: %s tokens (%s input, %s output, %s cache)\n",
177-
console.FormatNumber(totalTokens),
173+
fmt.Fprintf(os.Stderr, " Tokens: %s input, %s output, %s cache read, %s cache write\n",
178174
console.FormatNumber(summary.TotalInputTokens),
179175
console.FormatNumber(summary.TotalOutputTokens),
180-
console.FormatNumber(cacheTokens))
176+
console.FormatNumber(summary.TotalCacheReadTokens),
177+
console.FormatNumber(summary.TotalCacheWriteTokens))
181178
fmt.Fprintf(os.Stderr, " Requests: %d (avg %s)\n",
182179
summary.TotalRequests, timeutil.FormatDurationMs(summary.AvgDurationMs()))
183-
if summary.CacheEfficiency > 0 {
184-
fmt.Fprintf(os.Stderr, " Cache hit: %.1f%%\n", summary.CacheEfficiency*100)
185-
}
186180
fmt.Fprintln(os.Stderr)
187181

188182
rows := summary.ModelRows()

pkg/cli/audit_report_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1045,6 +1045,39 @@ func TestToolUsageAggregation(t *testing.T) {
10451045
"Bash should be present in tool usage")
10461046
}
10471047

1048+
func TestRenderTokenUsageDisplaysRawCountsOnly(t *testing.T) {
1049+
summary := &TokenUsageSummary{
1050+
TotalInputTokens: 100,
1051+
TotalOutputTokens: 200,
1052+
TotalCacheReadTokens: 5000,
1053+
TotalCacheWriteTokens: 3000,
1054+
TotalRequests: 2,
1055+
TotalDurationMs: 3000,
1056+
ByModel: map[string]*ModelTokenUsage{},
1057+
}
1058+
1059+
oldStderr := os.Stderr
1060+
r, w, err := os.Pipe()
1061+
require.NoError(t, err)
1062+
os.Stderr = w
1063+
1064+
renderTokenUsage(summary)
1065+
require.NoError(t, w.Close())
1066+
os.Stderr = oldStderr
1067+
1068+
var buf bytes.Buffer
1069+
_, copyErr := io.Copy(&buf, r)
1070+
require.NoError(t, copyErr)
1071+
1072+
output := buf.String()
1073+
assert.Contains(t, output, "Tokens:")
1074+
assert.Contains(t, output, "100 input")
1075+
assert.Contains(t, output, "cache read")
1076+
assert.Contains(t, output, "cache write")
1077+
assert.NotContains(t, output, "Total:")
1078+
assert.NotContains(t, output, "Cache hit:")
1079+
}
1080+
10481081
func TestExtractDownloadedFilesEmpty(t *testing.T) {
10491082
// Test with nonexistent directory
10501083
files := extractDownloadedFiles("/nonexistent/path")

pkg/cli/token_usage.go

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -168,12 +168,6 @@ func parseTokenUsageFile(filePath string, customWeights *types.TokenWeights) (*T
168168
m.ResponseBytes += entry.ResponseBytes
169169
}
170170

171-
// Compute cache efficiency: cache_read / (input + cache_read)
172-
totalInputPlusCacheRead := summary.TotalInputTokens + summary.TotalCacheReadTokens
173-
if totalInputPlusCacheRead > 0 {
174-
summary.CacheEfficiency = float64(summary.TotalCacheReadTokens) / float64(totalInputPlusCacheRead)
175-
}
176-
177171
tokenUsageLog.Printf("Parsed %d entries: %d input, %d output, %d cache_read, %d cache_write, %d requests",
178172
lineNum, summary.TotalInputTokens, summary.TotalOutputTokens,
179173
summary.TotalCacheReadTokens, summary.TotalCacheWriteTokens, summary.TotalRequests)
@@ -349,11 +343,6 @@ func parseAgentUsageFile(filePath string, customWeights *types.TokenWeights) (*T
349343
ByModel: make(map[string]*ModelTokenUsage),
350344
}
351345

352-
totalInputPlusCacheRead := summary.TotalInputTokens + summary.TotalCacheReadTokens
353-
if totalInputPlusCacheRead > 0 {
354-
summary.CacheEfficiency = float64(summary.TotalCacheReadTokens) / float64(totalInputPlusCacheRead)
355-
}
356-
357346
hasTokenData := summary.TotalInputTokens > 0 ||
358347
summary.TotalOutputTokens > 0 ||
359348
summary.TotalCacheReadTokens > 0 ||

pkg/cli/token_usage_test.go

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,7 @@ func TestParseTokenUsageFile(t *testing.T) {
6666
assert.Equal(t, 2, summary.ByModel["claude-sonnet-4-6"].Requests, "sonnet requests")
6767
assert.Equal(t, 1, summary.ByModel["claude-haiku-4-5"].Requests, "haiku requests")
6868

69-
// Check cache efficiency
70-
expectedEfficiency := float64(55028) / float64(775+55028)
71-
assert.InDelta(t, expectedEfficiency, summary.CacheEfficiency, 0.001, "cache efficiency")
69+
assert.InDelta(t, 0.0, summary.CacheEfficiency, 0.001, "cache efficiency is not computed from raw token counts")
7270
})
7371

7472
t.Run("extracts ambient context from first chronological invocation", func(t *testing.T) {
@@ -327,19 +325,7 @@ func TestAnalyzeTokenUsage(t *testing.T) {
327325
}
328326

329327
func TestCacheEfficiency(t *testing.T) {
330-
t.Run("zero when no cache reads", func(t *testing.T) {
331-
tmpDir := testutil.TempDir(t, "cache-eff")
332-
filePath := filepath.Join(tmpDir, "token-usage.jsonl")
333-
content := `{"provider":"anthropic","model":"sonnet","input_tokens":100,"output_tokens":50,"cache_read_tokens":0,"cache_write_tokens":0,"duration_ms":100}`
334-
require.NoError(t, os.WriteFile(filePath, []byte(content+"\n"), 0o644))
335-
336-
summary, err := parseTokenUsageFile(filePath, nil)
337-
require.NoError(t, err)
338-
require.NotNil(t, summary)
339-
assert.InDelta(t, 0.0, summary.CacheEfficiency, 0.001, "cache efficiency should be 0 with no cache reads")
340-
})
341-
342-
t.Run("high efficiency with mostly cache reads", func(t *testing.T) {
328+
t.Run("remains zero to avoid transforming raw token counts", func(t *testing.T) {
343329
tmpDir := testutil.TempDir(t, "cache-eff")
344330
filePath := filepath.Join(tmpDir, "token-usage.jsonl")
345331
content := `{"provider":"anthropic","model":"sonnet","input_tokens":100,"output_tokens":50,"cache_read_tokens":9900,"cache_write_tokens":0,"duration_ms":100}`
@@ -348,6 +334,6 @@ func TestCacheEfficiency(t *testing.T) {
348334
summary, err := parseTokenUsageFile(filePath, nil)
349335
require.NoError(t, err)
350336
require.NotNil(t, summary)
351-
assert.InDelta(t, 0.99, summary.CacheEfficiency, 0.001, "cache efficiency should be ~99%")
337+
assert.InDelta(t, 0.0, summary.CacheEfficiency, 0.001, "cache efficiency should remain unset")
352338
})
353339
}

0 commit comments

Comments
 (0)