Skip to content

Add regression coverage for Z.AI input and cache token totals #75383

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 2 commits into from
Nov 6, 2026
Merged

Add regression coverage for Z.AI input and cache token totals #75383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf85ab6
Initial plan
Copilot Nov 6, 2026
2d2968b
test: preserve Z.AI Anthropic-compatible token usage totals
Copilot Nov 6, 2026
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
34 changes: 34 additions & 0 deletions actions/setup/js/parse_token_usage.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,40 @@ describe("parse_token_usage", () => {
});
});

test("preserves Z.AI Anthropic-compatible usage in the detection summary and artifact", async () => {
process.env.GH_AW_TMP_DIR = tmpDir;
process.env.GH_AW_PHASE = "detection";
const tokenUsagePath = path.join(tmpDir, "sandbox/firewall/logs/api-proxy-logs/token-usage.jsonl");
fs.mkdirSync(path.dirname(tokenUsagePath), { recursive: true });
// Synthetic normalized AWF record using the session totals from gh-aw#75271.
const record = {
event: "token_usage",
provider: "anthropic",
model: "glm-5.3-flash",
streaming: true,
input_tokens: 38719,
output_tokens: 4058,
cache_read_tokens: 112640,
cache_write_tokens: 0,
};
const content = JSON.stringify(record) + "\n";
fs.writeFileSync(tokenUsagePath, content);

await main(path.join(tmpDir, "missing-session-state"));

expect(mockCore.setFailed).not.toHaveBeenCalled();
expect(JSON.parse(originalReadFileSync(path.join(tmpDir, "agent_usage.json"), "utf8"))).toMatchObject({
primary_model: "glm-5.3-flash",
input_tokens: 38719,
output_tokens: 4058,
cache_read_tokens: 112640,
cache_write_tokens: 0,
});
const markdown = mockCore.summary.addRaw.mock.calls[0][0];

@github-actions github-actions Bot Nov 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/tdd] The row check reads mock.calls[0][0], which depends on no earlier addRaw call and couples the test to call order. Use the toHaveBeenCalledWith(expect.stringContaining(...), true) style that other tests in this file already use, so the assertion reads as a spec.

💡 Suggested change 
expect(mockCore.summary.addRaw).toHaveBeenCalledWith(expect.stringMatching(/\|\s*38,719\s*\|\s*4,058\s*\|\s*112,640\s*\|\s*0\s*\|/), true);

This also removes the local markdown variable.

@copilot please address this.

Copilot AI Nov 6, 2026

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.

Changed the assertion to toHaveBeenCalledWith and made it target the glm53 row using locale-aware values. Commit: 36744f1.

expect(markdown).toMatch(/\|\s*38,719\s*\|\s*4,058\s*\|\s*112,640\s*\|\s*0\s*\|/);

@github-actions github-actions Bot Nov 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This assertion makes the regression test locale-dependent, so CI on a non-US locale can fail even when token aggregation is correct.

💡 Make the expected formatting deterministic

toLocaleString() uses the runner locale, while this regex hard-codes comma grouping (38,719). Pass an explicit locale in the production/test formatter or assert the parsed table row using a locale-independent representation; also scope the match to the intended model row so another row cannot satisfy it accidentally.

Copilot AI Nov 6, 2026

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 row check to use the runtime locale’s number formatting and scope the match to the glm53 row. Commit: 36744f1.

expect(originalReadFileSync(tokenUsagePath, "utf8")).toBe(content);
});

test.each([
["a malformed provider", "claude", "not a provider"],
["another native provider", "claude", "openai"],
Expand Down
24 changes: 24 additions & 0 deletions pkg/cli/token_usage_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,30 @@ func TestParseTokenUsageFile(t *testing.T) {
assert.Equal(t, 200, model.OutputTokens, "model output tokens")
})

t.Run("Z.AI Anthropic-compatible usage", func(t *testing.T) {
filePath := filepath.Join(testutil.TempDir(t, "token-usage-zai"), "token-usage.jsonl")
// Synthetic normalized AWF record using the session totals from gh-aw#75271.
content := `{"event":"token_usage","provider":"anthropic","model":"glm-5.3-flash","streaming":true,"input_tokens":38719,"output_tokens":4058,"cache_read_tokens":112640,"cache_write_tokens":0}`
require.NoError(t, os.WriteFile(filePath, []byte(content+"\n"), 0o644))

summary, err := parseTokenUsageFile(filePath)
require.NoError(t, err)
require.NotNil(t, summary)

assert.Equal(t, 38719, summary.TotalInputTokens)
assert.Equal(t, 4058, summary.TotalOutputTokens)
assert.Equal(t, 112640, summary.TotalCacheReadTokens)
assert.Zero(t, summary.TotalCacheWriteTokens)
assert.Equal(t, 155417, summary.TotalInputTokens+summary.TotalOutputTokens+summary.TotalCacheReadTokens)

@github-actions github-actions Bot Nov 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What this is: This assertion recomputes a value from the three token totals already asserted above. Problem: It cannot catch a production regression independently; when those assertions pass, the sum is necessarily 155417, so it only adds another literal to maintain. Fix: Delete this assertion. If we skip it: The test carries a redundant check and a second place to update when the fixture totals change.

Copilot AI Nov 6, 2026

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.

Removed the redundant derived-total assertion. Commit: 36744f1.

@github-actions github-actions Bot Nov 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/tdd] This sum check repeats the three field assertions above it and hard-codes a derived total (155417). It adds no coverage and goes stale if the fixture changes. Consider removing it.

@copilot please address this.

Copilot AI Nov 6, 2026

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.

Removed the redundant derived-total assertion. Commit: 36744f1.

require.Contains(t, summary.ByModel, "glm-5.3-flash")
model := summary.ByModel["glm-5.3-flash"]
assert.Equal(t, "anthropic", model.Provider)
assert.Equal(t, 38719, model.InputTokens)
assert.Equal(t, 4058, model.OutputTokens)
assert.Equal(t, 112640, model.CacheReadTokens)
assert.Zero(t, model.CacheWriteTokens)
})

t.Run("multiple entries with multiple models", func(t *testing.T) {
tmpDir := testutil.TempDir(t, "token-usage")
filePath := filepath.Join(tmpDir, "token-usage.jsonl")
Expand Down
Loading