Address grader review follow-up

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot · Sep 13, 2026 · Sep 13, 2026 · Sep 13, 2026 · Sep 13, 2026 · Sep 13, 2026
commit ad5eac0867f8ccb4abced2dbe1450a195edb5e59
diff --git a/actions/setup/js/file_helpers.cjs b/actions/setup/js/file_helpers.cjs
@@ -40,7 +40,21 @@ function findExistingFiles(candidatePaths) {
  * @returns {string | undefined} The first existing candidate
  */
 function findFirstExistingFile(candidatePaths) {
-  return findExistingFiles(candidatePaths)[0];
+  for (const candidate of candidatePaths) {
+    if (!fs.existsSync(candidate)) {
+      continue;
+    }
+    try {
+      if (fs.statSync(candidate).isFile()) {
+        return candidate;
+      }
+    } catch (error) {
+      if (typeof global.core?.debug === "function") {
+        global.core.debug("Candidate file is not usable: " + candidate + ": " + getErrorMessage(error));
+      }
+    }
+  }
+  return undefined;
 }
 
 /**

diff --git a/actions/setup/js/generate_usage_activity_summary.cjs b/actions/setup/js/generate_usage_activity_summary.cjs
@@ -16,7 +16,7 @@ const fs = require("fs");
 const path = require("path");
 const { GRADERS_DIR_NAME, GRADER_RESULTS_FILENAME, TMP_GH_AW_PATH } = require("./constants.cjs");
 const { readExperimentAssignments } = require("./experiment_helpers.cjs");
-const { findExistingFiles, findFirstExistingFile } = require("./file_helpers.cjs");
+const { findFirstExistingFile } = require("./file_helpers.cjs");
 const { calculateWorkingSetFromJSONL } = require("./working_set_metrics.cjs");
 
 require("./shim.cjs");
@@ -795,27 +795,20 @@ function parseExperimentsData() {
  * @returns {{ version?: number, results: any[] } | null} grader document, or null when unavailable
  */
 function parseGraderResults(candidatePaths = GRADER_RESULTS_PATHS) {
-  /** @type {Error | null} */
-  let lastError = null;
-  for (const candidate of findExistingFiles(candidatePaths)) {
-    try {
-      const parsed = JSON.parse(fs.readFileSync(candidate, "utf-8"));
-      if (!parsed || !Array.isArray(parsed.results)) {
-        throw new Error(`Grader results ${candidate} do not contain a results array`);
-      }
-      return parsed;
-    } catch (err) {
-      // Keep trying the remaining candidates: a truncated copy in one artifact
-      // must not hide a usable copy in another. The failure is still reported so a
-      // malformed file does not go unnoticed when a fallback path succeeds.
-      core.warning(`grader results ${candidate} could not be read: ${String(err)}`);
-      lastError = new Error(`Failed to read grader results ${candidate}`, { cause: err });
-    }
+  const candidate = findFirstExistingFile(candidatePaths);
+  if (!candidate) {
+    return null;
   }
-  if (lastError) {
-    throw lastError;
+  try {
+    const parsed = JSON.parse(fs.readFileSync(candidate, "utf-8"));
+    if (!parsed || !Array.isArray(parsed.results)) {
+      throw new Error(`Grader results ${candidate} do not contain a results array`);
+    }
+    return parsed;
+  } catch (err) {
+    core.warning(`grader results ${candidate} could not be read: ${String(err)}`);
+    throw new Error(`Failed to read grader results ${candidate}`, { cause: err });
   }
-  return null;
 }
 
 /**

diff --git a/actions/setup/js/generate_usage_activity_summary.test.cjs b/actions/setup/js/generate_usage_activity_summary.test.cjs
@@ -422,13 +422,13 @@ describe("generate_usage_activity_summary.cjs", () => {
       expect(parseGraderResults([resultsPath])).toEqual(document);
     });
 
-    it("falls back to the next candidate when the first file is malformed", () => {
+    it("throws when the first existing candidate is malformed", () => {
       const brokenPath = path.join(gradersDir, "broken.json");
       const goodPath = path.join(gradersDir, "good.json");
       fs.writeFileSync(brokenPath, "not json");
       fs.writeFileSync(goodPath, JSON.stringify({ version: 1, results: [{ id: "agent" }] }));
 
-      expect(parseGraderResults([brokenPath, goodPath]).results[0].id).toBe("agent");
+      expect(() => parseGraderResults([brokenPath, goodPath])).toThrow(/Failed to read grader results/);
     });
 
     it("prefers the first existing candidate path", () => {

diff --git a/actions/setup/js/run_created_at.cjs b/actions/setup/js/run_created_at.cjs
@@ -28,14 +28,41 @@ const RUN_CREATED_AT_RETRY_CONFIG = {
   jitterMs: 0,
   shouldRetry: error => {
     const status = Number(error?.status ?? error?.response?.status);
-    return (Number.isInteger(status) && status >= 500) || isTransientError(error);
+    return (Number.isInteger(status) && (status === 408 || status >= 500)) || isTransientError(error);
   },
 };
 
 // ISO-8601 date-time with a UTC designator or a numeric offset. Date.parse accepts
 // looser formats (for example "2026" or "2026-09-12 20:58:00"), which must not be
 // promoted to a valid run creation time.
-const ISO_8601_DATE_TIME = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)?(?:Z|[+-]\d{2}:\d{2})$/;
+const ISO_8601_DATE_TIME = /^(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2}):(\d{2})(?:\.\d+)?(?:Z|([+-])(\d{2}):(\d{2}))$/;
+
+function isLeapYear(year) {
+  return year % 4 === 0 && (year % 100 !== 0 || year % 400 === 0);
+}
+
+function daysInMonth(year, month) {
+  return [31, isLeapYear(year) ? 29 : 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31][month - 1] || 0;
+}
+
+function isValidRunCreatedAtMatch(match) {
+  const [, yearText, monthText, dayText, hourText, minuteText, secondText, offsetSign, offsetHourText, offsetMinuteText] = match;
+  const year = Number(yearText);
+  const month = Number(monthText);
+  const day = Number(dayText);
+  const hour = Number(hourText);
+  const minute = Number(minuteText);
+  const second = Number(secondText);
+  if (month < 1 || month > 12 || day < 1 || day > daysInMonth(year, month) || hour > 23 || minute > 59 || second > 59) {
+    return false;
+  }
+  if (offsetSign) {
+    const offsetHour = Number(offsetHourText);
+    const offsetMinute = Number(offsetMinuteText);
+    if (offsetHour > 23 || offsetMinute > 59) return false;
+  }
+  return true;
+}
 
 /**
  * Normalize a workflow-run creation time into the strict UTC ISO-8601 form
@@ -54,7 +81,8 @@ const ISO_8601_DATE_TIME = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)?(?:Z|[
 function normalizeRunCreatedAt(value) {
   if (typeof value !== "string") return "";
   const trimmed = value.trim();
-  if (!ISO_8601_DATE_TIME.test(trimmed)) return "";
+  const match = ISO_8601_DATE_TIME.exec(trimmed);
+  if (!match || !isValidRunCreatedAtMatch(match)) return "";
   const parsed = Date.parse(trimmed);
   if (!Number.isFinite(parsed)) return "";
   return new Date(Math.trunc(parsed / 1000) * 1000).toISOString().replace(/\.\d{3}Z$/, "Z");

diff --git a/actions/setup/js/run_created_at.test.cjs b/actions/setup/js/run_created_at.test.cjs
@@ -2,7 +2,7 @@
 
 import { describe, it, expect, beforeEach, vi } from "vitest";
 
-const { normalizeRunCreatedAt, resolveRunCreatedAtForInfo, resolveRunCreatedAtForGrading } = require("./run_created_at.cjs");
+const { normalizeRunCreatedAt, resolveRunCreatedAtForInfo, resolveRunCreatedAtForGrading, RUN_CREATED_AT_RETRY_CONFIG } = require("./run_created_at.cjs");
 
 // error_recovery.cjs (withRetry) logs through the github-script `core` global.
 const mockCore = { info: vi.fn(), debug: vi.fn(), warning: vi.fn(), error: vi.fn() };
@@ -34,6 +34,27 @@ describe("normalizeRunCreatedAt", () => {
     expect(normalizeRunCreatedAt(undefined)).toBe("");
     expect(normalizeRunCreatedAt(1757710680000)).toBe("");
   });
+
+  it("rejects calendar dates and offsets that Date.parse would normalize", () => {
+    expect(normalizeRunCreatedAt("2026-02-30T20:58:00Z")).toBe("");
+    expect(normalizeRunCreatedAt("2025-02-29T20:58:00Z")).toBe("");
+    expect(normalizeRunCreatedAt("2026-13-01T20:58:00Z")).toBe("");
+    expect(normalizeRunCreatedAt("2026-09-12T24:00:00Z")).toBe("");
+    expect(normalizeRunCreatedAt("2026-09-12T20:60:00Z")).toBe("");
+    expect(normalizeRunCreatedAt("2026-09-12T20:58:60Z")).toBe("");
+    expect(normalizeRunCreatedAt("2026-09-12T20:58:00+24:00")).toBe("");
+    expect(normalizeRunCreatedAt("2026-09-12T20:58:00+02:60")).toBe("");
+  });
+
+  it("accepts leap-day calendar dates", () => {
+    expect(normalizeRunCreatedAt("2024-02-29T20:58:00Z")).toBe("2024-02-29T20:58:00Z");
+  });
+});
+
+describe("RUN_CREATED_AT_RETRY_CONFIG", () => {
+  it("retries status-only request timeouts", () => {
+    expect(RUN_CREATED_AT_RETRY_CONFIG.shouldRetry({ status: 408, message: "Request failed" })).toBe(true);
+  });
 });
 
 describe("resolveRunCreatedAtForGrading", () => {

diff --git a/pkg/cli/audit_report_graders.go b/pkg/cli/audit_report_graders.go
@@ -44,6 +44,12 @@ type GraderResult struct {
 	Diagnostics       map[string]any        `json:"diagnostics,omitempty"`
 	BaselineValue     *float64              `json:"baselineValue,omitempty"`
 	DeltaFromBaseline *float64              `json:"deltaFromBaseline,omitempty"`
+
+	valuePresent             bool
+	observationPresent       bool
+	diagnosticsPresent       bool
+	baselineValuePresent     bool
+	deltaFromBaselinePresent bool
 }
 
 // GraderImplementation identifies the grader runtime that produced a result.
@@ -101,6 +107,63 @@ type graderManifestDocument struct {
 	Graders []graderManifestEntry `json:"graders"`
 }
 
+// MarshalJSON preserves explicitly transported null scalar values and empty
+// payload objects from grader_results.json. The public fields remain pointer/map
+// typed so existing CLI rendering can continue to treat missing and null values
+// identically, but the JSON report keeps the original transport contract.
+func (result GraderResult) MarshalJSON() ([]byte, error) {
+	type alias GraderResult
+	data, err := json.Marshal(alias(result))
+	if err != nil {
+		return nil, err
+	}
+	var obj map[string]any
+	if err := json.Unmarshal(data, &obj); err != nil {
+		return nil, err
+	}
+	if result.valuePresent && result.Value == nil {
+		obj["value"] = nil
+	}
+	if result.observationPresent && result.Observation == nil {
+		obj["observation"] = nil
+	} else if result.observationPresent && len(result.Observation) == 0 {
+		obj["observation"] = map[string]any{}
+	}
+	if result.diagnosticsPresent && result.Diagnostics == nil {
+		obj["diagnostics"] = nil
+	} else if result.diagnosticsPresent && len(result.Diagnostics) == 0 {
+		obj["diagnostics"] = map[string]any{}
+	}
+	if result.baselineValuePresent && result.BaselineValue == nil {
+		obj["baselineValue"] = nil
+	}
+	if result.deltaFromBaselinePresent && result.DeltaFromBaseline == nil {
+		obj["deltaFromBaseline"] = nil
+	}
+	return json.Marshal(obj)
+}
+
+// UnmarshalJSON records which optional keys were present so a JSON round trip
+// through cached log records does not drop explicit nulls or empty objects.
+func (result *GraderResult) UnmarshalJSON(data []byte) error {
+	type alias GraderResult
+	var decoded alias
+	if err := json.Unmarshal(data, &decoded); err != nil {
+		return err
+	}
+	*result = GraderResult(decoded)
+	var raw map[string]json.RawMessage
+	if err := json.Unmarshal(data, &raw); err != nil {
+		return err
+	}
+	_, result.valuePresent = raw["value"]
+	_, result.observationPresent = raw["observation"]
+	_, result.diagnosticsPresent = raw["diagnostics"]
+	_, result.baselineValuePresent = raw["baselineValue"]
+	_, result.deltaFromBaselinePresent = raw["deltaFromBaseline"]
+	return nil
+}
+
 // graderArtifactDirCandidates returns the directories, relative to a run's log directory,
 // that may contain grader output files. The order reflects preference: the compact usage
 // artifact first (it is downloaded for every run), then the dedicated graders artifact,
@@ -152,7 +215,11 @@ func findGraderFile(runDir, filename string) string {
 
 // runHasGraders reports whether a run's output directory contains grader results.
 func runHasGraders(runDir string) bool {
-	return findGraderFile(runDir, constants.GraderResultsFilename.String()) != ""
+	if findGraderFile(runDir, constants.GraderResultsFilename.String()) != "" {
+		return true
+	}
+	doc := graderResultsFromUsageSummary(runDir)
+	return doc != nil && len(doc.Results) > 0
 }
 
 // extractGradersData reads grader results (and the grader manifest, when available) from
@@ -193,22 +260,22 @@ func loadGraderResultsDocument(logsPath string) *graderArtifactFullDocument {
 
 	info, err := os.Stat(resultsPath)
 	if err != nil {
-		return nil
+		return graderResultsFromUsageSummary(logsPath)
 	}
 	if info.Size() > maxGraderResultsBytes {
 		gradersDataLog.Printf("Grader results too large (%d bytes), skipping: %s", info.Size(), resultsPath)
-		return nil
+		return graderResultsFromUsageSummary(logsPath)
 	}
 	data, err := os.ReadFile(resultsPath) // #nosec G304 -- path resolved beneath the run's logs directory
 	if err != nil {
 		gradersDataLog.Printf("Failed to read grader results: %v", err)
-		return nil
+		return graderResultsFromUsageSummary(logsPath)
 	}
 
 	var doc graderArtifactFullDocument
 	if err := json.Unmarshal(data, &doc); err != nil {
 		gradersDataLog.Printf("Failed to parse grader results: %v", err)
-		return nil
+		return graderResultsFromUsageSummary(logsPath)
 	}
 	return &doc
 }
@@ -278,6 +345,12 @@ func buildGraderResult(result graderArtifactFullResult, manifest graderManifestE
 		Implementation: result.Implementation,
 		Observation:    result.Observation,
 		Diagnostics:    result.Diagnostics,
+
+		valuePresent:             len(result.Value) > 0,
+		observationPresent:       result.Observation != nil,
+		diagnosticsPresent:       result.Diagnostics != nil,
+		baselineValuePresent:     len(result.BaselineValue) > 0,
+		deltaFromBaselinePresent: len(result.DeltaFromBaseline) > 0,
 	}
 	if summary.Name == "" {
 		summary.Name = manifest.Name

diff --git a/pkg/cli/audit_report_graders_test.go b/pkg/cli/audit_report_graders_test.go
@@ -252,6 +252,12 @@ func TestExtractGradersDataPreservesOperationalValueContract(t *testing.T) {
 	if err != nil {
 		t.Fatalf("failed to marshal graders data: %v", err)
 	}
+	encodedText := string(encoded)
+	for _, want := range []string{`"id":"operational-value-unavailable"`, `"value":null`, `"baselineValue":null`, `"deltaFromBaseline":null`} {
+		if !strings.Contains(encodedText, want) {
+			t.Fatalf("expected graders JSON to preserve %s, got %s", want, encodedText)
+		}
+	}
 	var decoded GradersData
 	if err := json.Unmarshal(encoded, &decoded); err != nil {
 		t.Fatalf("failed to unmarshal graders data: %v", err)
@@ -312,6 +318,28 @@ func TestExtractGradersDataPreservesOperationalValueContract(t *testing.T) {
 	}
 }
 
+func TestGraderResultJSONPreservesExplicitEmptyPayloads(t *testing.T) {
+	t.Parallel()
+	result := GraderResult{
+		ID:                 "empty-payload",
+		Status:             "pass",
+		Observation:        map[string]any{},
+		Diagnostics:        map[string]any{},
+		observationPresent: true,
+		diagnosticsPresent: true,
+	}
+	encoded, err := json.Marshal(result)
+	if err != nil {
+		t.Fatalf("failed to marshal grader result: %v", err)
+	}
+	encodedText := string(encoded)
+	for _, want := range []string{`"observation":{}`, `"diagnostics":{}`} {
+		if !strings.Contains(encodedText, want) {
+			t.Fatalf("expected grader result JSON to preserve %s, got %s", want, encodedText)
+		}
+	}
+}
+
 // TestExtractGradersDataFromUsageActivitySummary verifies that grader results embedded in
 // the conclusion job's usage activity summary are used when the standalone
 // grader_results.json file is unavailable.
@@ -334,6 +362,9 @@ func TestExtractGradersDataFromUsageActivitySummary(t *testing.T) {
 		t.Fatalf("failed to write summary: %v", err)
 	}
 
+	if !runHasGraders(runDir) {
+		t.Fatal("expected runHasGraders to recognize embedded grader results")
+	}
 	graders := extractGradersData(runDir)
 	if graders == nil {
 		t.Fatal("expected graders data to be extracted from the usage activity summary")
@@ -349,3 +380,37 @@ func TestExtractGradersDataFromUsageActivitySummary(t *testing.T) {
 		t.Fatalf("expected implementation digest to be preserved, got %+v", value.Implementation)
 	}
 }
+
+func TestExtractGradersDataFallsBackToUsageActivitySummaryAfterMalformedStandalone(t *testing.T) {
+	t.Parallel()
+	runDir := t.TempDir()
+	gradersDir := filepath.Join(runDir, constants.UsageArtifactName.String(), constants.GradersDirName.String())
+	if err := os.MkdirAll(gradersDir, 0o755); err != nil {
+		t.Fatalf("failed to create graders dir: %v", err)
+	}
+	if err := os.WriteFile(filepath.Join(gradersDir, constants.GraderResultsFilename.String()), []byte("{not json"), 0o600); err != nil {
+		t.Fatalf("failed to write malformed results: %v", err)
+	}
+	summaryDir := filepath.Join(runDir, constants.UsageArtifactName.String(), "activity")
+	if err := os.MkdirAll(summaryDir, 0o755); err != nil {
+		t.Fatalf("failed to create summary dir: %v", err)
+	}
+	data, err := json.Marshal(map[string]any{
+		"schema":  "usage-activity-summary/v1",
+		"graders": operationalValueGraderResults(),
+	})
+	if err != nil {
+		t.Fatalf("failed to marshal summary: %v", err)
+	}
+	if err := os.WriteFile(filepath.Join(summaryDir, "summary.json"), data, 0o600); err != nil {
+		t.Fatalf("failed to write summary: %v", err)
+	}
+
+	graders := extractGradersData(runDir)
+	if graders == nil {
+		t.Fatal("expected graders data to fall back to the usage activity summary")
+	}
+	if graders.Results[0].ID != "operational-value" {
+		t.Fatalf("unexpected fallback grader data: %+v", graders.Results)
+	}
+}