fix: address review feedback from Copilot

- Guard against division by zero when baseline meanP95 or current p95
  is zero (skips comparison instead of producing Infinity/NaN)
- Validate --baseline argument: error if flag provided without a path
- Fix stderr redirect (2>&1) that would corrupt benchmark-results.json
  with human-readable progress text mixed into JSON output
- Fix cache restore key to use branch-scoped prefix matching instead
  of overly broad fallback that could pull history from other branches
- Add 2 tests for zero p95 edge cases

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: Mossaka

.github/workflows/performance-monitor.yml

@@ -48,9 +48,10 @@ jobs:
         uses: actions/cache/restore@v4
         with:
           path: benchmark-history.json
-          key: benchmark-history-${{ github.ref_name }}
+          key: benchmark-history-${{ github.ref_name }}-${{ github.run_id }}
           restore-keys: |
-            benchmark-history-
+            benchmark-history-${{ github.ref_name }}-
+            benchmark-history-main-
 
       - name: Run benchmarks
         id: benchmark

scripts/ci/benchmark-performance.ts

@@ -286,10 +286,17 @@ function benchmarkNetworkCreation(): BenchmarkResult {
 
 function parseBaselineArg(): string | null {
   const idx = process.argv.indexOf("--baseline");
-  if (idx !== -1 && idx + 1 < process.argv.length) {
-    return process.argv[idx + 1];
+  if (idx === -1) {
+    return null;
   }
-  return null;
+
+  const value = process.argv[idx + 1];
+  if (!value || value.startsWith("--")) {
+    console.error("Error: --baseline requires a file path argument");
+    process.exit(1);
+  }
+
+  return value;
 }
 
 async function main(): Promise<void> {

src/benchmark/history.test.ts

@@ -153,6 +153,33 @@ describe("compareAgainstBaseline", () => {
     expect(comparisons).toHaveLength(0);
   });
 
+  it("skips metrics with zero rolling mean p95 to avoid division by zero", () => {
+    const history: BenchmarkHistory = {
+      version: 1,
+      entries: [makeEntry(0, 0)],
+    };
+    const report = makeReport();
+    const comparisons = compareAgainstBaseline(report, history);
+
+    // Both metrics have 0 baseline p95, so they should be skipped
+    expect(comparisons).toHaveLength(0);
+  });
+
+  it("skips metrics with zero current p95", () => {
+    const history: BenchmarkHistory = {
+      version: 1,
+      entries: [makeEntry(5000, 80)],
+    };
+    const report = makeReport({
+      results: [
+        { metric: "container_startup_warm", unit: "ms", values: [0], mean: 0, median: 0, p95: 0, p99: 0 },
+      ],
+    });
+    const comparisons = compareAgainstBaseline(report, history);
+
+    expect(comparisons).toHaveLength(0);
+  });
+
   it("skips metrics not in history", () => {
     const history: BenchmarkHistory = {
       version: 1,

src/benchmark/history.ts

@@ -125,7 +125,11 @@ export function compareAgainstBaseline(
 
   for (const result of report.results) {
     const baseline = rolling[result.metric];
-    if (!baseline || baseline.count === 0) {
+    if (!baseline || baseline.count === 0 || baseline.meanP95 <= 0) {
+      continue;
+    }
+
+    if (result.p95 <= 0) {
       continue;
     }