Skip to content

Fix AIC extraction and computation from token usage JSONL files #40085

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

Closed
Copilot wants to merge 3 commits into from
+295 −45
Closed

Fix AIC extraction and computation from token usage JSONL files #40085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7f13654
Fix AIC extraction from token usage JSONL files
Copilot Jun 18, 2026
576dd8f
Address code review: remove redundant import, document request_id regex
Copilot Jun 18, 2026
0da261a
Merge branch 'main' into copilot/fix-ai-credits-extraction
github-actions[bot] Jun 18, 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
6 changes: 6 additions & 0 deletions actions/setup/js/daily_aic_workflow_helpers.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,12 @@ function sumAICFromUsageJSONLFiles(filePaths) {
total += explicitAIC;
continue;
}
// Prefer proxy-emitted per-request AIC over locally computed when present.

@github-actions github-actions Bot Jun 18, 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 new ai_credits_this_response branch in sumAICFromUsageJSONLFiles has no test coverage. The existing test in check_daily_aic_workflow_guardrail.test.cjs exercises ai_credits, aiCredits, and aic aliases but not this new field — so a future regression here would go undetected.

💡 Suggested test addition (check_daily_aic_workflow_guardrail.test.cjs) 
it("sums ai_credits_this_response when present", () => {
  const tmpDir = fs.mkdtempSync(...);
  const filePath = path.join(tmpDir, "usage.jsonl");
  fs.writeFileSync(
    filePath,
    [
      JSON.stringify({ ai_credits_this_response: 3.5 }),
      JSON.stringify({ ai_credits_this_response: 5.25 }),
    ].join("\n")
  );
  expect(exports.sumAICFromUsageJSONLFiles([filePath])).toBeCloseTo(8.75);
});

const explicitPerRequest = getNumericAliasField(usage, parsed, ["ai_credits_this_response"]);

@github-actions github-actions Bot Jun 18, 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.

[/diagnose] The inline comment says "Prefer proxy-emitted per-request AIC over locally computed" but ai_credits_this_response is checked last — after ai_credits, aiCredits, and aic. If a record contains both ai_credits and ai_credits_this_response (api-proxy v0.27.2 emits both), the older ai_credits field wins here, defeating the fix for daily aggregation.

💡 Suggested ordering fix

Move ai_credits_this_response to be checked before the legacy aliases, consistent with how parseTokenUsageJsonl in parse_mcp_gateway_log.cjs already treats it as the highest-priority field:

// Highest fidelity: proxy-emitted per-request AIC.
const explicitPerRequest = getNumericAliasField(usage, parsed, ["ai_credits_this_response"]);
if (explicitPerRequest > 0) {
  total += explicitPerRequest;
  continue;
}
// Legacy aggregate aliases (lower priority).
const explicitAICredits = getNumericAliasField(usage, parsed, ["ai_credits", "aiCredits"]);

This keeps daily totals consistent with the step-summary values already reported by parseTokenUsageJsonl.

@github-actions github-actions Bot Jun 18, 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.

Misleading comment: ai_credits_this_response is not preferred over locally-computed — it is only checked when ALL legacy explicit fields are absent.

💡 Details

The comment on line 217 says "Prefer proxy-emitted per-request AIC over locally computed when present", which implies ai_credits_this_response takes priority. It doesn't. The actual priority chain is:

  1. ai_credits / aiCredits — if > 0, use it and continue
  2. aic — if > 0, use it and continue
  3. ai_credits_this_response — only reached if the two checks above both returned 0
  4. computeInferenceAIC fallback

If a transitional proxy record happens to emit both ai_credits and ai_credits_this_response, the legacy field wins and the more-authoritative per-request value from ai_credits_this_response is silently discarded. This is inconsistent with how parse_mcp_gateway_log.cjs handles the same data (it checks ai_credits_this_response directly and ignores ai_credits entirely).

If the intent is backward compatibility (old proxy → ai_credits, new proxy → ai_credits_this_response), the comment should say so. If ai_credits_this_response should truly take precedence over ai_credits, move it before the legacy checks.

if (explicitPerRequest > 0) {

@github-actions github-actions Bot Jun 18, 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.

[/diagnose] Same > 0 guard issue as in parse_mcp_gateway_log.cjs: an explicit ai_credits_this_response: 0 from the proxy is silently discarded and falls through to token-based computation. Should use a presence check (!== undefined && !== null) rather than a magnitude check.

total += explicitPerRequest;
continue;
}

const computed = computeInferenceAIC({
provider: getStringField(usage, parsed, "provider", "provider"),
Expand Down
104 changes: 66 additions & 38 deletions actions/setup/js/parse_mcp_gateway_log.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@ const { parseUnknownModelAICreditsFromAuditLog } = require("./ai_credits_context
* - /tmp/gh-aw/mcp-logs/gateway.log (main gateway log, fallback)
* - /tmp/gh-aw/mcp-logs/stderr.log (stderr output, fallback)
* - /tmp/gh-aw/sandbox/firewall/logs/api-proxy-logs/token-usage.jsonl (token usage from firewall proxy)
* - /tmp/gh-aw/sandbox/firewall-audit-logs/api-proxy-logs/token-usage.jsonl (audit copy, checked as fallback)
*/

const TOKEN_USAGE_PATH = "/tmp/gh-aw/sandbox/firewall/logs/api-proxy-logs/token-usage.jsonl";
const TOKEN_USAGE_AUDIT_PATH = "/tmp/gh-aw/sandbox/firewall-audit-logs/api-proxy-logs/token-usage.jsonl";
const MAX_RPC_SUMMARY_DETAILS_LENGTH = 120;
const MAX_RPC_SUMMARY_GENERIC_LENGTH = 160;
const MAX_RPC_MESSAGE_LABEL_LENGTH = 80;
Expand Down Expand Up @@ -66,7 +68,7 @@ function parseTokenUsageJsonl(jsonlContent) {
totalAIC: 0,
ambientContextTokens: undefined,
byModel: {},
/** @type {{ model: string, provider: string, inputTokens: number, outputTokens: number, cacheReadTokens: number, cacheWriteTokens: number, reasoningTokens: number, durationMs: number, deltaAIC: number }[]} */
/** @type {{ model: string, provider: string, inputTokens: number, outputTokens: number, cacheReadTokens: number, cacheWriteTokens: number, reasoningTokens: number, durationMs: number, deltaAIC: number, explicitDeltaAIC: number | null }[]} */
entries: [],
};

Expand All @@ -84,6 +86,9 @@ function parseTokenUsageJsonl(jsonlContent) {
const cacheWriteTokens = entry.cache_write_tokens || 0;
const reasoningTokens = entry.reasoning_tokens || 0;
const durationMs = entry.duration_ms || 0;
// When the proxy emits an explicit per-request AIC value, prefer it over
// the locally-computed value so that proxy-side pricing updates take effect.
const explicitDeltaAIC = typeof entry.ai_credits_this_response === "number" && entry.ai_credits_this_response > 0 ? entry.ai_credits_this_response : null;

@github-actions github-actions Bot Jun 18, 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.

[/diagnose] The > 0 guard silently discards an explicit ai_credits_this_response: 0 emitted by the proxy, falling through to locally-computed AIC instead. A value of 0 could be a legitimate proxy assertion (e.g., a cached or free-tier call), and overriding it with a computed cost produces an incorrect result.

💡 Suggested fix

Drop the > 0 guard — treat any numeric value from the proxy as authoritative, including zero:

const explicitDeltaAIC = typeof entry.ai_credits_this_response === "number"
  ? entry.ai_credits_this_response
  : null;

The ?? computed fallback already handles null correctly, so this change only affects the explicit-zero case.

@github-actions github-actions Bot Jun 18, 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.

ai_credits_this_response: 0 is treated as absent, causing AIC to be overcounted for zero-cost responses.

💡 Details 
const explicitDeltaAIC =
  typeof entry.ai_credits_this_response === "number" && entry.ai_credits_this_response > 0
    ? entry.ai_credits_this_response
    : null;

The > 0 guard cannot distinguish "field not present" from "field is present and explicitly zero". When a proxy emits "ai_credits_this_response": 0 (e.g. a fully-cached, free-tier, or failed request), explicitDeltaAIC is null and the fallback computeInferenceAIC runs, returning a positive value based on token counts. The proxy's explicit assertion that the request cost nothing is ignored and AIC is overcounted.

Fix: separate presence detection from value filtering:

const rawExplicit = entry.ai_credits_this_response;
const explicitDeltaAIC =
  typeof rawExplicit === "number" ? rawExplicit : null;

Then in the aggregation pass:

entry.deltaAIC = entry.explicitDeltaAIC !== null ? entry.explicitDeltaAIC : computed;

The same > 0 pattern appears in daily_aic_workflow_helpers.cjs line 219 for the same field and should be fixed there too.


summary.totalInputTokens += inputTokens;
summary.totalOutputTokens += outputTokens;
Expand Down Expand Up @@ -117,33 +122,19 @@ function parseTokenUsageJsonl(jsonlContent) {
m.requests++;
m.durationMs += durationMs;

summary.entries.push({ model, provider: m.provider, inputTokens, outputTokens, cacheReadTokens, cacheWriteTokens, reasoningTokens, durationMs, deltaAIC: 0 });
summary.entries.push({ model, provider: m.provider, inputTokens, outputTokens, cacheReadTokens, cacheWriteTokens, reasoningTokens, durationMs, deltaAIC: 0, explicitDeltaAIC });
} catch {
// skip malformed lines
}
}

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

let totalAIC = 0;
for (const [model, usage] of Object.entries(summary.byModel)) {
const aic = computeInferenceAIC({
provider: usage.provider || "",
model,
inputTokens: usage.inputTokens,
outputTokens: usage.outputTokens,
cacheReadTokens: usage.cacheReadTokens,
cacheWriteTokens: usage.cacheWriteTokens,
reasoningTokens: usage.reasoningTokens || 0,
});
usage.aic = aic;
totalAIC += aic;
}
summary.totalAIC = totalAIC;

// Compute per-request AI credits.
// Prefer the proxy-emitted explicit value when available; fall back to
// computing from token counts and the local pricing catalog.
for (const entry of summary.entries) {
entry.deltaAIC = computeInferenceAIC({
const computed = computeInferenceAIC({
provider: entry.provider || "",
model: entry.model,
inputTokens: entry.inputTokens,
Expand All @@ -152,7 +143,18 @@ function parseTokenUsageJsonl(jsonlContent) {
cacheWriteTokens: entry.cacheWriteTokens,
reasoningTokens: entry.reasoningTokens || 0,
});
entry.deltaAIC = entry.explicitDeltaAIC ?? computed;
}

// Aggregate per-model AIC and overall total by summing per-entry deltaAIC.
// This keeps model totals consistent with the per-entry view regardless of
// whether explicit or computed AIC is used.
let totalAIC = 0;
for (const entry of summary.entries) {
summary.byModel[entry.model].aic += entry.deltaAIC;
totalAIC += entry.deltaAIC;
}
summary.totalAIC = totalAIC;

return summary;
}
Expand Down Expand Up @@ -202,25 +204,49 @@ function generateTokenUsageSummary(summary) {
* @param {typeof import('@actions/core')} coreObj - The GitHub Actions core object
*/
function writeStepSummaryWithTokenUsage(coreObj) {
if (!fs.existsSync(TOKEN_USAGE_PATH)) {
coreObj.debug(`No token-usage.jsonl found at: ${TOKEN_USAGE_PATH}`);
} else {
const content = fs.readFileSync(TOKEN_USAGE_PATH, "utf8");
if (content?.trim()) {
coreObj.info(`Found token-usage.jsonl (${content.length} bytes)`);
const parsedSummary = parseTokenUsageJsonl(content);
if (parsedSummary && parsedSummary.totalAIC > 0) {
const roundedAIC = parsedSummary.totalAIC.toFixed(3);
coreObj.exportVariable("GH_AW_AIC", roundedAIC);
coreObj.setOutput("aic", roundedAIC);
coreObj.info(`AI Credits: ${roundedAIC}`);
}
if (parsedSummary && typeof parsedSummary.ambientContextTokens === "number" && parsedSummary.ambientContextTokens > 0) {
const roundedAmbientContext = String(Math.round(parsedSummary.ambientContextTokens));
coreObj.exportVariable("GH_AW_AMBIENT_CONTEXT", roundedAmbientContext);
coreObj.setOutput("ambient_context", roundedAmbientContext);
coreObj.info(`Ambient context: ${roundedAmbientContext}`);
}
// Read from both the primary path and the audit path, deduplicating by request_id.
// The audit path may contain additional entries when the primary path is absent or
// partially written (e.g. the proxy was restarted mid-run).
const paths = [TOKEN_USAGE_AUDIT_PATH, TOKEN_USAGE_PATH];

@github-actions github-actions Bot Jun 18, 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.

Dedup gives audit-path data priority over the primary path — first-seen wins, so audit entries shadow primary entries when the same request_id appears in both.

💡 Details 
const paths = [TOKEN_USAGE_AUDIT_PATH, TOKEN_USAGE_PATH];

The docstring calls TOKEN_USAGE_AUDIT_PATH a fallback (checked

@github-actions github-actions Bot Jun 18, 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.

Dedup gives audit-path data priority over the primary path — first-seen wins, so the audit copy shadows the primary copy when the same request_id appears in both files.

💡 Details 
const paths = [TOKEN_USAGE_AUDIT_PATH, TOKEN_USAGE_PATH];

The file-list docstring calls TOKEN_USAGE_AUDIT_PATH "checked as fallback", but the dedup loop processes it first. The first occurrence of a request_id wins, so if both files carry the same entry the audit version is the one kept.

In normal operation both files should be byte-identical for the same request, so there is no practical difference. But if the primary path ever has a corrected/re-emitted record for a request_id that already appeared in the audit log (e.g. a retry with updated credits), the primary version is silently discarded.

Either:

  1. Swap the order to [TOKEN_USAGE_PATH, TOKEN_USAGE_AUDIT_PATH] so the primary is authoritative and the audit is the true fallback, or
  2. Keep the current order but update the comment to explain why audit wins (e.g. "audit is written first and is more complete when the primary is partially flushed").

parse_token_usage.cjs uses the same [AUDIT, PRIMARY] order, so if this is intentional the rationale should be documented in both files.

const seenRequestIds = new Set();
const dedupedLines = [];

for (const filePath of paths) {
if (!fs.existsSync(filePath)) {
coreObj.debug(`No token-usage.jsonl found at: ${filePath}`);
continue;
}
const raw = fs.readFileSync(filePath, "utf8");
if (!raw?.trim()) continue;
coreObj.info(`Found token-usage.jsonl at ${filePath} (${raw.length} bytes)`);
for (const rawLine of raw.split("\n")) {
const line = rawLine.trim();
if (!line) continue;
// Lightweight request_id extraction for deduplication — mirrors extractRequestId() in
// parse_token_usage.cjs. The pattern covers standard JSON escaping (\\, \") but not
// exotic cases; api-proxy request IDs are UUIDs so this is sufficient in practice.
const idMatch = line.match(/"request_id"\s*:\s*"((?:\\.|[^"\\])*)"/);
const dedupeKey = idMatch ? `request_id:${idMatch[1]}` : line;

@github-actions github-actions Bot Jun 18, 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.

[/diagnose] When request_id is absent, the full raw line is used as the dedupe key. If the proxy serializes the same logical record to the primary and audit paths with different JSON field ordering (which is valid), both copies will have different keys and both will be counted — producing a doubled AIC total.

The comment acknowledges "api-proxy request IDs are UUIDs so this is sufficient in practice", which is reasonable given current behavior. Worth adding an explicit // TODO noting the field-order risk so it's easy to find if double-counting is ever reported.

if (seenRequestIds.has(dedupeKey)) continue;
seenRequestIds.add(dedupeKey);
dedupedLines.push(line);
}
}

if (dedupedLines.length > 0) {
const content = dedupedLines.join("\n");
const parsedSummary = parseTokenUsageJsonl(content);
if (parsedSummary && parsedSummary.totalAIC > 0) {
const roundedAIC = parsedSummary.totalAIC.toFixed(3);
coreObj.exportVariable("GH_AW_AIC", roundedAIC);
coreObj.setOutput("aic", roundedAIC);
coreObj.info(`AI Credits: ${roundedAIC}`);
}
if (parsedSummary && typeof parsedSummary.ambientContextTokens === "number" && parsedSummary.ambientContextTokens > 0) {
const roundedAmbientContext = String(Math.round(parsedSummary.ambientContextTokens));
coreObj.exportVariable("GH_AW_AMBIENT_CONTEXT", roundedAmbientContext);
coreObj.setOutput("ambient_context", roundedAmbientContext);
coreObj.info(`Ambient context: ${roundedAmbientContext}`);
}
}

Expand Down Expand Up @@ -1126,6 +1152,8 @@ if (typeof module !== "undefined" && module.exports) {
hasAICreditsRateLimitError,
hasUnknownModelAICreditsError,
setUnknownModelAICreditsOutput,
TOKEN_USAGE_PATH,
TOKEN_USAGE_AUDIT_PATH,
};
}

Expand Down
Loading