Skip to content

Render WSRF in step-summary token usage with progressive disclosure #54591

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
Aug 21, 2026
+75 −5
Merged

Render WSRF in step-summary token usage with progressive disclosure #54591

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
ffe6977
Render WSRF in token usage step summary
Copilot Aug 21, 2026
2f2e666
Polish WSRF summary rendering
Copilot Aug 21, 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
46 changes: 41 additions & 5 deletions actions/setup/js/parse_token_usage.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const fs = require("fs");
const { getErrorMessage } = require("./error_helpers.cjs");
const { ERR_PARSE } = require("./error_codes.cjs");
const { parseTokenUsageJsonl, generateTokenUsageSummary } = require("./parse_mcp_gateway_log.cjs");
const { calculateWorkingSetFromJSONL } = require("./generate_usage_activity_summary.cjs");

/**
* Parses the firewall proxy token-usage.jsonl and appends a collapsible markdown
Expand Down Expand Up @@ -99,10 +100,42 @@ function getSummaryTitle() {
* Builds the token usage section for the GitHub step summary.
* @param {string} title
* @param {string} markdown
* @param {ReturnType<typeof calculateWorkingSetFromJSONL>["workingSet"] | null} workingSet
* @returns {string}
*/
function buildStepSummarySection(title, markdown) {
return `<details>\n<summary>${title}</summary>\n\nPer-request AI credits and token totals\n\n${markdown}</details>\n\n`;
function buildStepSummarySection(title, markdown, workingSet = null) {
const workingSetSection = buildWorkingSetDetailsSection(workingSet);

@github-actions github-actions Bot Aug 21, 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.

[/codebase-design] The WSRF details block is rendered before the token table (${workingSetSection}${markdown}), but the PR description says "WSRF visible at a glance alongside the token table." Placing WSRF above the table inverts the information hierarchy — users land on WSRF first, then scroll to credits/totals.

Consider reversing the order (${markdown}${workingSetSection}) or adding a blank line between them to keep the token table as the primary surface.

@copilot please address this.

return `<details>\n<summary>${title}</summary>\n\nPer-request AI credits and token totals\n\n${workingSetSection}${markdown}</details>\n\n`;
}

/**
* Builds a progressive-disclosure block for the Working-Set Rebuild Factor.
* @param {ReturnType<typeof calculateWorkingSetFromJSONL>["workingSet"] | null} workingSet
* @returns {string}
*/
function buildWorkingSetDetailsSection(workingSet) {
if (!workingSet || typeof workingSet !== "object") return "";
const measurementState = workingSet.measurement_state || "unavailable";
const rebuildFactor = typeof workingSet.rebuild_factor === "number" && Number.isFinite(workingSet.rebuild_factor) ? workingSet.rebuild_factor : null;
const displayFactor = rebuildFactor === null ? "unavailable" : `${rebuildFactor.toFixed(2)}×`;
const displayInvocations = Number.isFinite(workingSet.invocations) ? workingSet.invocations.toLocaleString() : "0";
const displayCumulative = Number.isFinite(workingSet.cumulative_input_tokens) ? workingSet.cumulative_input_tokens.toLocaleString() : "0";
const displayPeak = Number.isFinite(workingSet.peak_input_tokens) ? workingSet.peak_input_tokens.toLocaleString() : "0";
const displayExcess = Number.isFinite(workingSet.rebuild_excess_tokens) ? workingSet.rebuild_excess_tokens.toLocaleString() : "0";

return [
"<details>",
`<summary>Working-Set Rebuild Factor (WSRF): ${displayFactor} (${measurementState})</summary>`,

@github-actions github-actions Bot Aug 21, 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 nests a <details> block inside another <details> block in the step summary, and GitHub’s markdown renderer does not handle nested disclosure widgets reliably, so the WSRF section can render as broken or permanently expanded instead of being progressively hidden.

💡 Why this blocks

The outer token-usage section is already a <details> element. buildWorkingSetDetailsSection() now injects a second <details> inside it, which is exactly the kind of nesting GitHub markdown tends to flatten or render inconsistently across summary surfaces. That means the new UX goal—“collapsed WSRF details inside collapsed token usage”—is not actually guaranteed, and in the worst case the summary becomes malformed.

Use a non-nested disclosure pattern here instead, e.g. keep the outer <details> and render the WSRF line plus bullets directly inside it, or split WSRF into a sibling section rather than a child <details>.

return [
  `**Working-Set Rebuild Factor (WSRF):** ${displayFactor} (${measurementState})`,
  "",
  `- State: \`${measurementState}\``,
  ...
].join("\n");

That preserves valid markdown structure and avoids renderer-dependent breakage.

"",
`- State: \`${measurementState}\``,
`- Invocations: ${displayInvocations}`,
`- Cumulative input tokens: ${displayCumulative}`,
`- Peak invocation input tokens: ${displayPeak}`,
`- Rebuild excess tokens: ${displayExcess}`,
"",
"</details>",
"",
].join("\n");
Comment on lines +137 to +138
}

/**
Expand Down Expand Up @@ -130,10 +163,11 @@ function renderTokenTableAsPlainText(title, markdown) {
* Falls back to the Actions summary API when the summary path is unavailable.
* @param {string} title
* @param {string} markdown
* @param {ReturnType<typeof calculateWorkingSetFromJSONL>["workingSet"] | null} workingSet
* @returns {Promise<void>}
*/
async function appendStepSummarySection(title, markdown) {
const section = buildStepSummarySection(title, markdown);
async function appendStepSummarySection(title, markdown, workingSet = null) {
const section = buildStepSummarySection(title, markdown, workingSet);
const summaryPath = process.env.GITHUB_STEP_SUMMARY;
if (summaryPath) {
try {
Expand Down Expand Up @@ -168,9 +202,10 @@ async function main() {
return;

@github-actions github-actions Bot Aug 21, 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.

[/diagnosing-bugs] calculateWorkingSetFromJSONL is called before the if (markdown.length > 0) guard — if it throws, the token-usage summary is silently lost.

💡 Suggested fix

Move the call inside the guard so a WSRF failure cannot suppress the token table:

const markdown = generateTokenUsageSummary(summary);
if (markdown.length > 0) {
  const workingSet = calculateWorkingSetFromJSONL(content).workingSet;
  core.info(renderTokenTableAsPlainText(getSummaryTitle(), markdown));
  await appendStepSummarySection(getSummaryTitle(), markdown, workingSet);
}

Alternatively, wrap the call in a try/catch and fall back to null so WSRF failures are non-fatal.

@copilot please address this.

}
const markdown = generateTokenUsageSummary(summary);
const workingSet = calculateWorkingSetFromJSONL(content).workingSet;
if (markdown.length > 0) {
core.info(renderTokenTableAsPlainText(getSummaryTitle(), markdown));
await appendStepSummarySection(getSummaryTitle(), markdown);
await appendStepSummarySection(getSummaryTitle(), markdown, workingSet);
}

core.info("Token usage summary appended to step summary");
Expand Down Expand Up @@ -231,6 +266,7 @@ if (typeof module !== "undefined" && module.exports) {
readDedupedTokenUsage,
getSummaryTitle,
buildStepSummarySection,
buildWorkingSetDetailsSection,
appendStepSummarySection,
renderTokenTableAsPlainText,
TOKEN_USAGE_AUDIT_PATH,
Expand Down
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 @@ -12,6 +12,7 @@ const {
readDedupedTokenUsage,
getSummaryTitle,
buildStepSummarySection,
buildWorkingSetDetailsSection,
renderTokenTableAsPlainText,
TOKEN_USAGE_AUDIT_PATH,
TOKEN_USAGE_PATH,
Expand Down Expand Up @@ -255,6 +256,8 @@ describe("parse_token_usage", () => {
const stepSummary = originalReadFileSync(stepSummaryPath, "utf8");
expect(stepSummary).toContain("<summary>Token Usage</summary>");
expect(stepSummary).toContain("Per-request AI credits and token totals");
expect(stepSummary).toContain("Working-Set Rebuild Factor (WSRF): 1.00× (measured)");
expect(stepSummary).toContain("- Cumulative input tokens: 100");
expect(stepSummary).toContain("| ΔAI Credits | AI Credits |");
expect(fs.appendFileSync).toHaveBeenCalledWith(stepSummaryPath, expect.any(String), "utf8");
expect(mockCore.summary.addRaw).not.toHaveBeenCalled();
Expand Down Expand Up @@ -549,6 +552,37 @@ describe("parse_token_usage", () => {
expect(section).toContain("Per-request AI credits and token totals");

@github-actions github-actions Bot Aug 21, 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] buildWorkingSetDetailsSection(null) and buildWorkingSetDetailsSection(undefined) are not explicitly tested — only the guard's early-return (return "") is implied. A test for falsy input would pin the contract.

💡 Suggested test 
test("buildWorkingSetDetailsSection returns empty string for null or undefined", () => {
  expect(buildWorkingSetDetailsSection(null)).toBe("");
  expect(buildWorkingSetDetailsSection(undefined)).toBe("");
  expect(buildWorkingSetDetailsSection("string")).toBe("");
});

@copilot please address this.

});

test("buildWorkingSetDetailsSection renders measured WSRF details", () => {
const section = buildWorkingSetDetailsSection({
measurement_state: "measured",
rebuild_factor: 3.9017857142857144,
cumulative_input_tokens: 874000,
peak_input_tokens: 224000,
rebuild_excess_tokens: 650000,
invocations: 5,
});

expect(section).toContain("Working-Set Rebuild Factor (WSRF): 3.90× (measured)");
expect(section).toContain("- State: `measured`");
expect(section).toContain("- Cumulative input tokens: 874,000");
expect(section).toContain("- Peak invocation input tokens: 224,000");
expect(section).toContain("- Rebuild excess tokens: 650,000");
expect(section).toContain("- Invocations: 5");
});

test("buildWorkingSetDetailsSection renders unavailable state when no factor exists", () => {
const section = buildWorkingSetDetailsSection({
measurement_state: "unavailable",
cumulative_input_tokens: 0,
peak_input_tokens: 0,
rebuild_excess_tokens: 0,
invocations: 0,
});

expect(section).toContain("Working-Set Rebuild Factor (WSRF): unavailable (unavailable)");
expect(section).toContain("- State: `unavailable`");
});

test("renderTokenTableAsPlainText strips table separator lines and pipes", () => {
const markdown = ["| # | Alias | Input | Output |", "|--:|-------|------:|-------:|", "| 1 | sonnet46 | 100 | 200 |", "| **Total** | | **100** | **200** |", "", "Legend: `Alias` is the model shorthand.", ""].join("\n");

@github-actions github-actions Bot Aug 21, 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 "partial" measurement state is mentioned in the PR body but has no dedicated test. Both buildWorkingSetDetailsSection tests cover "measured" and "unavailable"; a "partial" case would complete the contract.

💡 Suggested test 
test("buildWorkingSetDetailsSection renders partial state", () => {
  const section = buildWorkingSetDetailsSection({
    measurement_state: "partial",
    rebuild_factor: 2.0,
    cumulative_input_tokens: 200000,
    peak_input_tokens: 100000,
    rebuild_excess_tokens: 100000,
    invocations: 2,
  });
  expect(section).toContain("Working-Set Rebuild Factor (WSRF): 2.00× (partial)");
  expect(section).toContain("- State: `partial`");
});

@copilot please address this.


Expand Down
Loading