Wrap token usage step summary in a details section#24375
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/835f9989-d5ae-42b5-846b-8f6fdd4cc6e4 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
|
||
| const lines = []; | ||
| lines.push("### 📊 Token Usage\n"); | ||
| lines.push("<details>"); |
There was a problem hiding this comment.
@copilot remove emoji, use core.summary.* helpers
There was a problem hiding this comment.
Done in 3ce5f64. Removed the emoji from the label and refactored writeStepSummaryWithTokenUsage to use coreObj.summary.addDetails("Token Usage", ...) instead of building raw HTML in generateTokenUsageSummary. The generator now returns just the table markdown content.
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/1d8a78e1-e123-46fd-adb7-4e41cb919cf1 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Wraps the “Token Usage” step summary output in a collapsible section to match the UX of other step summaries.
Changes:
- Refactors
generateTokenUsageSummaryto return only the token-usage table markdown (removes the “### 📊 Token Usage” heading). - Updates
writeStepSummaryWithTokenUsageto wrap the token usage markdown viacore.summary.addDetails("Token Usage", ...). - Adjusts unit tests to match the new
generateTokenUsageSummaryoutput and mocksaddDetailson the summary object.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/parse_mcp_gateway_log.cjs | Removes the token-usage heading from generated markdown and renders token usage inside a step-summary details block via addDetails. |
| actions/setup/js/parse_mcp_gateway_log.test.cjs | Updates assertions to reflect the new token-usage markdown shape and extends the mocked summary API with addDetails. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 2
| coreObj.info(`Found token-usage.jsonl (${content.length} bytes)`); | ||
| const parsedSummary = parseTokenUsageJsonl(content); | ||
| const markdown = generateTokenUsageSummary(parsedSummary); | ||
| if (markdown.length > 0) { | ||
| coreObj.summary.addRaw(markdown); | ||
| coreObj.summary.addDetails("Token Usage", "\n\n" + markdown); | ||
| } |
There was a problem hiding this comment.
The new coreObj.summary.addDetails("Token Usage", ...) behavior isn’t covered by tests. Consider adding a unit test that mocks fs.existsSync/fs.readFileSync for TOKEN_USAGE_PATH and asserts summary.addDetails is called with the expected label and markdown content (and that summary.write() still runs).
| test("renders header and table columns", () => { | ||
| const summary = parseTokenUsageJsonl(JSON.stringify({ model: "claude-sonnet-4-6", provider: "anthropic", input_tokens: 100, output_tokens: 200, cache_read_tokens: 5000, cache_write_tokens: 3000, duration_ms: 2500 })); | ||
| const md = generateTokenUsageSummary(summary); | ||
| expect(md).toContain("### 📊 Token Usage"); | ||
| expect(md).toContain("| Model | Input | Output | Cache Read | Cache Write | ET | Requests | Duration |"); | ||
| expect(md).toContain("claude-sonnet-4-6"); |
There was a problem hiding this comment.
This test name is now misleading: generateTokenUsageSummary no longer renders a header (the assertion for the header was removed). Rename the test to reflect what it actually verifies (e.g., table columns/content).
Summary
Wraps the Token Usage step summary in a collapsible details section, consistent with how other step summaries are rendered (e.g. "MCP Gateway Activity", "DIFC Filtered Events", "Agentic Conversation").
Changes
actions/setup/js/parse_mcp_gateway_log.cjs: RefactoredgenerateTokenUsageSummaryto return only the table markdown content (no raw HTML wrapper).writeStepSummaryWithTokenUsagenow usescoreObj.summary.addDetails("Token Usage", ...)to wrap the content, using thecore.summary.*helper API. The emoji has been removed from the section label.actions/setup/js/parse_mcp_gateway_log.test.cjs: Updated test assertions to reflect the new return value ofgenerateTokenUsageSummary(table content only, no<details>wrapper), and addedaddDetailsto the mock summary object.