Address AIC review feedback

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Author: Copilot

.github/workflows/pr-code-quality-reviewer.lock.yml

@@ -16,9 +16,8 @@ on:
     name: review
     events: [pull_request_comment, pull_request_review_comment]
 engine:
-  id: pi
-  model-provider: openai
-model: openai/gpt-5.4
+  id: copilot
+model: copilot/gpt-5.4
 permissions:
   contents: read
   issues: read

.github/workflows/pr-code-quality-reviewer.md

@@ -87,10 +87,10 @@ function parseNonNegativeFiniteNumber(value) {
  * @returns {string}
  */
 function formatAICForOutput(value, source) {
-  if (!Number.isFinite(value) || value <= 0) return "";
+  if (!Number.isFinite(value) || value < 0) return "";
   if (source !== "awf_reported") return value.toFixed(3);
   const rounded = Number(value.toFixed(6));
-  return String(rounded > 0 ? rounded : value);
+  return String(rounded);
 }
 
 /**
@@ -235,12 +235,10 @@ function parseTokenUsageJsonl(jsonlContent) {
   const hasReportedAIC = summary.entries.some(entry => entry.reportedDeltaAIC !== null || entry.reportedTotalAIC !== null);
   const hasAnyReportedAICFields = summary.entries.some(entry => entry.hasReportedDeltaField || entry.hasReportedTotalField);
   const hasExplicitCacheSemantics = summary.entries.some(entry => typeof entry.inputTokensIncludeCache === "boolean");
-  const invalidCacheSemanticsCount = summary.entries.filter(entry => entry.hasInputTokensIncludeCacheField && typeof entry.inputTokensIncludeCache !== "boolean").length;
-  if (invalidCacheSemanticsCount > 0) {
-    summary.aiCreditsWarnings.push(`${invalidCacheSemanticsCount} token usage record(s) had invalid input_tokens_include_cache values; legacy provider cache semantics were used.`);
-  }
+  let invalidCacheSemanticsCount = 0;
 
   if (!hasAnyReportedAICFields && !hasExplicitCacheSemantics) {
+    invalidCacheSemanticsCount = summary.entries.filter(entry => entry.hasInputTokensIncludeCacheField && typeof entry.inputTokensIncludeCache !== "boolean").length;
     // Preserve the legacy aggregation contract exactly for records emitted before
     // AWF added reported AIC and explicit cache-semantics fields.
     let totalAIC = 0;
@@ -294,9 +292,13 @@ function parseTokenUsageJsonl(jsonlContent) {
       const reportedFieldsMissingOrInvalid = hasAnyReportedAICFields && (!entry.hasReportedDeltaField || entry.reportedDeltaAIC === null || !entry.hasReportedTotalField || entry.reportedTotalAIC === null);
       if (reportedFieldsMissingOrInvalid) fallbackRecordCount++;
 
-      entry.deltaAIC =
-        entry.reportedDeltaAIC ??
-        computeInferenceAIC({
+      if (entry.reportedDeltaAIC !== null) {
+        entry.deltaAIC = entry.reportedDeltaAIC;
+      } else {
+        if (entry.hasInputTokensIncludeCacheField && typeof entry.inputTokensIncludeCache !== "boolean") {
+          invalidCacheSemanticsCount++;
+        }
+        entry.deltaAIC = computeInferenceAIC({
           provider: entry.provider || "",
           model: entry.model,
           inputTokens: entry.inputTokens,
@@ -306,6 +308,7 @@ function parseTokenUsageJsonl(jsonlContent) {
           reasoningTokens: entry.reasoningTokens || 0,
           inputTokensIncludeCache: entry.inputTokensIncludeCache,
         });
+      }
       summary.byModel[entry.model].aic += entry.deltaAIC;
       runningAIC = entry.reportedTotalAIC ?? runningAIC + entry.deltaAIC;
       entry.runningAIC = runningAIC;
@@ -321,6 +324,9 @@ function parseTokenUsageJsonl(jsonlContent) {
       summary.aiCreditsWarnings.push("The AWF-reported cumulative AI Credits total differs from the sum of per-request credits; the cumulative total was preserved for reporting.");
     }
   }
+  if (invalidCacheSemanticsCount > 0) {
+    summary.aiCreditsWarnings.push(`${invalidCacheSemanticsCount} token usage record(s) had invalid input_tokens_include_cache values; legacy provider cache semantics were used.`);
+  }
 
   return summary;
 }
@@ -396,7 +402,7 @@ async function writeStepSummaryWithTokenUsage(coreObj) {
       for (const warning of parsedSummary?.aiCreditsWarnings || []) {
         coreObj.warning?.(`[ai-credits] ${warning}`);
       }
-      if (parsedSummary && parsedSummary.totalAIC > 0) {
+      if (parsedSummary && (parsedSummary.aiCreditsSource === "awf_reported" || parsedSummary.totalAIC > 0)) {
         const aic = formatAICForOutput(parsedSummary.totalAIC, parsedSummary.aiCreditsSource);
         coreObj.exportVariable("GH_AW_AIC", aic);
         coreObj.setOutput("aic", aic);

actions/setup/js/parse_mcp_gateway_log.cjs

@@ -1999,6 +1999,45 @@ Some content here.`;
       expect(summary.entries[0].deltaAIC).toBe(0);
       expect(summary.totalAIC).toBe(0);
       expect(summary.aiCreditsWarnings).toEqual([]);
+      expect(generateTokenUsageSummary(summary)).toContain("| **Total** | | **1,000** | **100** | **0** | **0** | | **0** |");
+    });
+
+    test("exports AWF-reported zero to the main job output", async () => {
+      const tokenUsagePath = "/tmp/gh-aw/sandbox/firewall/logs/api-proxy-logs/token-usage.jsonl";
+      const content = JSON.stringify({
+        provider: "copilot",
+        model: "gpt-4o-mini-2024-07-18",
+        input_tokens: 1000,
+        output_tokens: 100,
+        ai_credits_this_response: 0,
+        ai_credits_total: 0,
+      });
+      const existsSpy = vi.spyOn(fs, "existsSync").mockImplementation(filePath => filePath === tokenUsagePath);
+      const readSpy = vi.spyOn(fs, "readFileSync").mockImplementation(filePath => {
+        if (filePath === tokenUsagePath) return content;
+        return "";
+      });
+      const coreObj = {
+        debug: vi.fn(),
+        info: vi.fn(),
+        exportVariable: vi.fn(),
+        setOutput: vi.fn(),
+        summary: {
+          addRaw: vi.fn(),
+          write: vi.fn().mockResolvedValue(undefined),
+        },
+      };
+
+      try {
+        await writeStepSummaryWithTokenUsage(coreObj);
+      } finally {
+        existsSpy.mockRestore();
+        readSpy.mockRestore();
+      }
+
+      expect(coreObj.exportVariable).toHaveBeenCalledWith("GH_AW_AIC", "0");
+      expect(coreObj.setOutput).toHaveBeenCalledWith("aic", "0");
+      expect(coreObj.info).toHaveBeenCalledWith("AI Credits: 0");
     });
 
     test("aggregates AWF-reported credits by model without changing the run total", () => {
@@ -2083,6 +2122,25 @@ Some content here.`;
       expect(summary.aiCreditsWarnings).toEqual([expect.stringContaining("invalid input_tokens_include_cache")]);
     });
 
+    test("does not warn for invalid input_tokens_include_cache when AWF delta is valid", () => {
+      const summary = parseTokenUsageJsonl(
+        JSON.stringify({
+          provider: "copilot",
+          model: "gpt-4o-mini-2024-07-18",
+          input_tokens: 1000,
+          output_tokens: 100,
+          cache_read_tokens: 400,
+          cache_write_tokens: 100,
+          input_tokens_include_cache: "invalid",
+          ai_credits_this_response: 0.123,
+          ai_credits_total: 0.123,
+        })
+      );
+
+      expect(summary.totalAIC).toBe(0.123);
+      expect(summary.aiCreditsWarnings).toEqual([]);
+    });
+
     test("uses the chronologically last valid AWF-reported total", () => {
       const content = [
         {

actions/setup/js/parse_mcp_gateway_log.test.cjs

@@ -45,17 +45,19 @@ function getReadableTokenUsagePaths(paths) {
 }
 
 /**
- * Extracts request_id with lightweight matching (no full JSON parse).
+ * Extracts a cross-file dedupe key with lightweight matching (no full JSON parse).
  * @param {string} line
  * @returns {string}
  */
-function extractRequestId(line) {
-  const match = line.match(/"request_id"\s*:\s*"((?:\\.|[^"\\])*)"/);
-  return match ? match[1] : "";
+function extractTokenUsageDedupeKey(line) {
+  const requestMatch = line.match(/"request_id"\s*:\s*"((?:\\.|[^"\\])*)"/);
+  if (!requestMatch) return "";
+  const eventMatch = line.match(/"event"\s*:\s*"((?:\\.|[^"\\])*)"/);
+  return `${eventMatch ? eventMatch[1] : "token_usage"}:${requestMatch[1]}`;
 }
 
 /**
- * Reads token usage files and deduplicates overlapping lines by request_id.
+ * Reads token usage files and deduplicates overlapping lines by event and request_id.
  * Falls back to raw line dedupe when request_id is absent.
  * @param {string[]} paths
  * @returns {string}
@@ -76,8 +78,7 @@ function readDedupedTokenUsage(paths) {
     for (const line of fileContent.split("\n")) {
       const trimmed = line.trim();
       if (!trimmed) continue;
-      const requestId = extractRequestId(trimmed);
-      const dedupeKey = requestId ? `request_id:${requestId}` : trimmed;
+      const dedupeKey = extractTokenUsageDedupeKey(trimmed) || trimmed;
       if (uniqueLineKeys.has(dedupeKey)) continue;
       uniqueLineKeys.add(dedupeKey);
       dedupedLines.push(trimmed);
@@ -247,7 +248,7 @@ async function main() {
       core.setOutput("primary_model", primaryModel);
       core.info(`Primary model: ${primaryModel}`);
     }
-    if (summary.totalAIC > 0) {
+    if (summary.aiCreditsSource === "awf_reported" || summary.totalAIC > 0) {
       const aic = formatAICForOutput(summary.totalAIC, summary.aiCreditsSource);
       core.exportVariable("GH_AW_AIC", aic);
       core.setOutput("aic", aic);
@@ -269,7 +270,11 @@ if (typeof module !== "undefined" && module.exports) {
   module.exports = {
     main,
     getReadableTokenUsagePaths,
-    extractRequestId,
+    extractRequestId: line => {
+      const key = extractTokenUsageDedupeKey(line);
+      return key ? key.slice(key.indexOf(":") + 1) : "";
+    },
+    extractTokenUsageDedupeKey,
     readDedupedTokenUsage,
     getSummaryTitle,
     buildStepSummarySection,

actions/setup/js/parse_token_usage.cjs

@@ -9,6 +9,7 @@ const {
   main,
   getReadableTokenUsagePaths,
   extractRequestId,
+  extractTokenUsageDedupeKey,
   readDedupedTokenUsage,
   getSummaryTitle,
   buildStepSummarySection,
@@ -356,6 +357,48 @@ describe("parse_token_usage", () => {
       expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("1.03602"));
     });
 
+    test("exports AWF-reported zero AIC instead of treating it as missing", async () => {
+      const agentUsageFile = path.join(tmpDir, "agent_usage.json");
+      const zeroEntry = JSON.stringify({
+        model: "gpt-4o-mini-2024-07-18",
+        provider: "copilot",
+        input_tokens: 1000,
+        output_tokens: 100,
+        ai_credits_this_response: 0,
+        ai_credits_total: 0,
+      });
+
+      fs.existsSync = vi.fn(p => {
+        if (p === TOKEN_USAGE_PATH) return true;
+        if (p === TOKEN_USAGE_AUDIT_PATH || p === TOKEN_USAGE_AWF_AUDIT_PATH) return false;
+        return originalExistsSync(p);
+      });
+      fs.statSync = vi.fn(p => {
+        if (p === TOKEN_USAGE_PATH) return { size: zeroEntry.length };
+        if (p === TOKEN_USAGE_AUDIT_PATH || p === TOKEN_USAGE_AWF_AUDIT_PATH) return { size: 0 };
+        return originalStatSync(p);
+      });
+      fs.readFileSync = vi.fn((p, enc) => {
+        if (p === TOKEN_USAGE_PATH) return zeroEntry;
+        if (p === TOKEN_USAGE_AUDIT_PATH || p === TOKEN_USAGE_AWF_AUDIT_PATH) return "";
+        return originalReadFileSync(p, enc);
+      });
+      fs.writeFileSync = vi.fn((p, data) => {
+        if (p === AGENT_USAGE_PATH) {
+          originalWriteFileSync(agentUsageFile, data);
+        } else {
+          originalWriteFileSync(p, data);
+        }
+      });
+
+      await main();
+
+      const agentUsage = JSON.parse(originalReadFileSync(agentUsageFile, "utf8"));
+      expect(agentUsage.ai_credits).toBe(0);
+      expect(mockCore.exportVariable).toHaveBeenCalledWith("GH_AW_AIC", "0");
+      expect(mockCore.setOutput).toHaveBeenCalledWith("aic", "0");
+    });
+
     test("surfaces fallback accounting warnings", async () => {
       const malformedEntry = JSON.stringify({
         model: "gpt-4o-mini-2024-07-18",
@@ -589,6 +632,13 @@ describe("parse_token_usage", () => {
       expect(extractRequestId('{"model":"m"}')).toBe("");
     });
 
+    test("extractTokenUsageDedupeKey includes event and request_id", () => {
+      expect(extractTokenUsageDedupeKey('{"event":"token_usage","request_id":"req-123","model":"m"}')).toBe("token_usage:req-123");
+      expect(extractTokenUsageDedupeKey('{"event":"other","request_id":"req-123","model":"m"}')).toBe("other:req-123");
+      expect(extractTokenUsageDedupeKey('{"request_id":"req-123","model":"m"}')).toBe("token_usage:req-123");
+      expect(extractTokenUsageDedupeKey('{"model":"m"}')).toBe("");
+    });
+
     test("getReadableTokenUsagePaths skips failing stat path and keeps valid path", () => {
       fs.existsSync = vi.fn(p => p === TOKEN_USAGE_AUDIT_PATH || p === TOKEN_USAGE_PATH);
       fs.statSync = vi.fn(p => {
@@ -618,6 +668,22 @@ describe("parse_token_usage", () => {
       expect(deduped.match(/"request_id":"req-1"/g)).toHaveLength(1);
     });
 
+    test("readDedupedTokenUsage keeps different events with the same request_id", () => {
+      const fileA = '{"event":"token_usage","request_id":"req-1","model":"m1","input_tokens":1}';
+      const fileB = '{"event":"token_steering","request_id":"req-1","model":"m1","input_tokens":2}';
+
+      fs.readFileSync = vi.fn(p => {
+        if (p === TOKEN_USAGE_AUDIT_PATH) return fileA;
+        if (p === TOKEN_USAGE_PATH) return fileB;
+        return originalReadFileSync(p, "utf8");
+      });
+
+      const deduped = readDedupedTokenUsage([TOKEN_USAGE_AUDIT_PATH, TOKEN_USAGE_PATH]);
+      expect(deduped).toContain('"event":"token_usage"');
+      expect(deduped).toContain('"event":"token_steering"');
+      expect(deduped.match(/"request_id":"req-1"/g)).toHaveLength(2);
+    });
+
     test("deduplicates mirrored AWF records before aggregating reported credits", () => {
       const fixture = originalReadFileSync(path.join(__dirname, "fixtures", "awf-v0.28.7-aic-token-usage.jsonl"), "utf8");
       fs.readFileSync = vi.fn(p => {

actions/setup/js/parse_token_usage.test.cjs

@@ -148,7 +148,7 @@ The unified `agent` artifact contains agent job outputs:
 - Agent execution logs
 - Safe output data (`agent_output.json`)
 - GitHub API rate limit logs (`github_rate_limits.jsonl`)
-- Token usage summary (`agent_usage.json`) — aggregated totals only; per-request data is in `firewall-audit-logs`. When AWF records include `ai_credits_this_response` and `ai_credits_total`, the summary preserves those reported values instead of repricing the tokens.
+- Token usage summary (`agent_usage.json`) — aggregated totals only; per-request data is in `firewall-audit-logs`. When AWF records include valid `ai_credits_this_response` and `ai_credits_total` values, the summary preserves those reported values instead of repricing the tokens.
 - `otel.jsonl` — OTLP span mirror written by gh-aw's JavaScript span exporters when `observability.otlp` is configured
 
 For OTLP configuration, runtime environment variables, and span semantics, see the [OpenTelemetry guide](/gh-aw/reference/open-telemetry/).

docs/src/content/docs/reference/artifacts.md

@@ -80,13 +80,16 @@ func buildAgentModelTokenUsage(entry agentUsageEntry, provider string) *ModelTok
 }
 
 func populateAgentUsageAIC(summary *TokenUsageSummary, entry agentUsageEntry, model, provider string, hasRawTokenData bool) {
-	if entry.AICredits <= 0 {
+	aic, present, valid := parseOptionalNonNegativeFloat(entry.AICredits)
+	if !present || !valid {
 		if hasRawTokenData {
 			populateAIC(summary)
+			summary.AICFound = summary.TotalAIC > 0
 		}
 		return
 	}
-	summary.TotalAIC = entry.AICredits
+	summary.TotalAIC = aic
+	summary.AICFound = true
 	if summary.ByModel[model] == nil {
 		summary.ByModel[model] = &ModelTokenUsage{}
 	}
@@ -97,5 +100,5 @@ func populateAgentUsageAIC(summary *TokenUsageSummary, entry agentUsageEntry, mo
 	usage.CacheReadTokens = entry.CacheReadTokens
 	usage.CacheWriteTokens = entry.CacheWriteTokens
 	usage.ReasoningTokens = entry.ReasoningTokens
-	usage.AIC = entry.AICredits
+	usage.AIC = aic
 }