Merge pull request #470 from spacedock-dev/spacedock-ensign/boot-metrics-record-and-pr-delta

Boot-window metrics — record turns/cache-creation instead of gating on magic thresholds, surface per-PR delta

Author: clkao

.github/workflows/release.yml

@@ -35,32 +35,58 @@ jobs:
         with:
           go-version: "1.22"
 
-      # Find the latest successful Runtime Live E2E run on next and pull its
-      # journey-metrics artifacts. When no such run exists this is a NON-FATAL
-      # skip (`exit 0`, found=false) — runtime-live-e2e.yml has no push:next
-      # trigger, so a fresh cut routinely has no producer run, and blocking the
-      # ledger here must not fail anything. The emitted `found` output gates the
-      # downstream Build/Publish steps so they SKIP (job green) rather than run
-      # `journey-costs` over an empty dir and RED the job.
+      # Pull journey-metrics from EVERY successful Runtime Live E2E run (PR or
+      # main) since the immediately-prior release tag of any kind (stable or
+      # -pre) — an incremental window, so a -pre release does not re-fold runs an
+      # earlier -pre in the same cycle already covered. A run's inclusion does
+      # not depend on its triggering PR's eventual merge state: the observation
+      # is about boot behavior on that commit at that time. When no run exists in
+      # the window this is a NON-FATAL skip (`exit 0`, found=false) — a fresh cut
+      # routinely has no producer run, and blocking the ledger here must not fail
+      # anything. The emitted `found` output gates the downstream Build/Publish
+      # steps so they SKIP (job green) rather than run `journey-costs` over an
+      # empty dir and RED the job.
       - name: Download latest journey metrics artifacts
         id: download_metrics
         env:
           GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         run: |
           set -euo pipefail
           mkdir -p "$RUNNER_TEMP/runtime-live-artifacts" "$RUNNER_TEMP/journey-metrics"
-          # `|| true` degrades a gh error (or a missing gh) to an empty run_id so
+          # `|| true` degrades a gh error (or a missing gh) to the epoch fallback
+          # so a query failure never aborts the best-effort ledger under set -e.
+          # jq reads the tag via $ENV (not bash interpolation) so this survives
+          # set -u even when GITHUB_REF_NAME is unset.
+          since="$(gh release list --limit 100 --json tagName,createdAt --jq \
+            '[sort_by(.createdAt)[]] as $r | ($r | map(.tagName) | index($ENV.GITHUB_REF_NAME)) as $i | if $i == null or $i == 0 then "" else $r[$i-1].createdAt end' \
+            || true)"
+          if [ -z "$since" ]; then
+            since="1970-01-01T00:00:00Z"
+          fi
+          echo "journey ledger aggregation window: runs created after $since" >&2
+          # `|| true` degrades a gh error (or a missing gh) to an empty run list so
           # the no-run skip branch below fires (found=false, exit 0) instead of
           # aborting under set -e. The journey ledger is best-effort; a query
-          # failure must never RED the cut.
-          run_id="$(gh run list --workflow "Runtime Live E2E" --branch main --status success --limit 1 --json databaseId --jq '.[0].databaseId' || true)"
-          if [ -z "$run_id" ] || [ "$run_id" = "null" ]; then
-            echo "::warning::no successful Runtime Live E2E run found on main; skipping journey ledger" >&2
+          # failure must never RED the cut. No --branch filter: PR-triggered runs
+          # count in the window exactly like main runs.
+          run_ids="$(gh run list --workflow "Runtime Live E2E" --status success --created ">$since" --limit 100 --json databaseId --jq '.[].databaseId' || true)"
+          if [ -z "$run_ids" ]; then
+            echo "::warning::no successful Runtime Live E2E run found since $since; skipping journey ledger" >&2
             echo "found=false" >> "$GITHUB_OUTPUT"
             exit 0
           fi
-          gh run download "$run_id" --dir "$RUNNER_TEMP/runtime-live-artifacts"
-          find "$RUNNER_TEMP/runtime-live-artifacts" -path '*/journey-metrics/*.json' -type f -exec cp {} "$RUNNER_TEMP/journey-metrics/" \;
+          # Each run's metrics land in their OWN subdirectory, never flat-copied
+          # into one: journeymetrics.recordFilename has no run-distinguishing
+          # component, so flattening N runs' same-named scenario/model files into
+          # one directory silently collapses them back to one observation via
+          # overwrite. journeymetrics.ReadRecordsDir already walks metrics-dir
+          # recursively, so no Go-side change is needed to consume this layout.
+          for run_id in $run_ids; do
+            run_dir="$RUNNER_TEMP/journey-metrics/$run_id"
+            mkdir -p "$run_dir"
+            gh run download "$run_id" --dir "$RUNNER_TEMP/runtime-live-artifacts/$run_id"
+            find "$RUNNER_TEMP/runtime-live-artifacts/$run_id" -path '*/journey-metrics/*.json' -type f -exec cp {} "$run_dir/" \;
+          done
           if [ -z "$(find "$RUNNER_TEMP/journey-metrics" -name '*.json' -type f -print -quit)" ]; then
             echo "::warning::downloaded Runtime Live E2E artifacts contained no journey metrics JSON; skipping journey ledger" >&2
             echo "found=false" >> "$GITHUB_OUTPUT"

.github/workflows/runtime-live-e2e.yml

@@ -646,3 +646,84 @@ jobs:
             live-artifacts/pi/**
             live-artifacts/journey-metrics/**
           if-no-files-found: warn
+
+  # AC-3: posts a per-PR delta of this run's journey-cost observations against
+  # the latest published release ledger, as a single sticky-updating PR comment
+  # (`gh pr comment --edit-last`, so repeated pushes update ONE comment instead
+  # of appending a new one). PR-only — a workflow_dispatch run has no PR to
+  # comment on. needs: claude-live with no explicit `if:` on that edge means
+  # this job runs only once claude-live's matrix completed successfully;
+  # shallow-boot-window (like every scenario this comments on) is Claude-only.
+  # A job-level permissions block grants pull-requests: write beyond the
+  # workflow-level contents: read default; specifying any job-level permissions
+  # replaces (not merges with) the workflow default, so contents/actions read
+  # are restated here for checkout and cross-job artifact download.
+  journey-delta-comment:
+    needs: claude-live
+    if: github.event_name == 'pull_request'
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      actions: read
+      pull-requests: write
+    steps:
+      - uses: actions/checkout@v5
+
+      - uses: actions/setup-go@v6
+        with:
+          go-version: "1.22"
+
+      - name: Download this run's Claude journey metrics
+        uses: actions/download-artifact@v5
+        with:
+          pattern: runtime-live-e2e-claude-live-*
+          path: ${{ runner.temp }}/current-run-artifacts
+          merge-multiple: true
+
+      # Best-effort: no previously published release (or one with no
+      # journey-costs asset) means no baseline to diff against. `|| true` /
+      # explicit exit-0 branches degrade either gap to a non-fatal skip so a
+      # missing baseline never REDs the PR.
+      - name: Download the latest published release's journey-cost ledger
+        id: download_previous_ledger
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: |
+          set -euo pipefail
+          previous_tag="$(gh release list --limit 1 --json tagName --jq '.[0].tagName' || true)"
+          if [ -z "$previous_tag" ] || [ "$previous_tag" = "null" ]; then
+            echo "::warning::no published release found; skipping journey-cost delta comment" >&2
+            echo "found=false" >> "$GITHUB_OUTPUT"
+            exit 0
+          fi
+          if ! gh release download "$previous_tag" -p 'journey-costs-*.json' -O "$RUNNER_TEMP/previous-journey-costs.json"; then
+            echo "::warning::published release $previous_tag has no journey-costs asset; skipping journey-cost delta comment" >&2
+            echo "found=false" >> "$GITHUB_OUTPUT"
+            exit 0
+          fi
+          echo "found=true" >> "$GITHUB_OUTPUT"
+
+      # download-artifact's merge-multiple flat-copies every matched artifact's
+      # files into current-run-artifacts, but PRESERVES each artifact's own
+      # internal path prefix (verified against a real run's downloaded zip:
+      # the journey-metrics JSON lands several directories deep, not directly
+      # under "live-artifacts/journey-metrics/"). A hardcoded exact subpath is
+      # brittle to that prefix; find + cp (release.yml's own pattern for the
+      # same class of problem) is layout-agnostic — it locates the metrics
+      # files wherever they actually landed.
+      - name: Locate this run's journey metrics
+        if: steps.download_previous_ledger.outputs.found == 'true'
+        run: |
+          set -euo pipefail
+          mkdir -p "$RUNNER_TEMP/current-run-metrics"
+          find "$RUNNER_TEMP/current-run-artifacts" -path '*/journey-metrics/*.json' -type f -exec cp {} "$RUNNER_TEMP/current-run-metrics/" \;
+
+      - name: Post the journey-cost delta PR comment
+        if: steps.download_previous_ledger.outputs.found == 'true'
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: |
+          set -euo pipefail
+          go run ./cmd/spacedock-release journey-delta "$RUNNER_TEMP/previous-journey-costs.json" \
+            --metrics-dir "$RUNNER_TEMP/current-run-metrics" \
+            --pr "${{ github.event.pull_request.number }}"

cmd/spacedock-release/journey_costs_test.go

@@ -101,12 +101,98 @@ func TestJourneyCostsCommandRejectsMismatchedOutputFilename(t *testing.T) {
 	}
 }
 
+// TestJourneyCostsCommandAggregatesPerRunSubdirectories proves the AC-2 fix: once
+// release.yml downloads each discovered run into its OWN subdirectory instead of
+// flat-copying, journeymetrics.ReadRecordsDir's existing recursive walk aggregates
+// both runs' observations for the SAME scenario/model without collision, and each
+// observation carries the distinct, non-empty run_id/run_url of the source run that
+// produced it — proving "traceable to more than one run" against actual
+// provenance data, not just a count.
+func TestJourneyCostsCommandAggregatesPerRunSubdirectories(t *testing.T) {
+	metricsDir := t.TempDir()
+	runADir := filepath.Join(metricsDir, "27931963802")
+	runBDir := filepath.Join(metricsDir, "28432388663")
+
+	base := journeymetrics.Record{
+		SchemaVersion: journeymetrics.RecordSchemaVersion,
+		ScenarioID:    "shallow-boot-window",
+		Source:        "live-harness",
+		Mode:          journeymetrics.ModeLLMLive,
+		Runtime:       "claude",
+		Executor:      "llm",
+		Host:          "claude",
+		Model:         "claude-sonnet-4-6",
+		MetricsState:  journeymetrics.StateMeasured,
+		Outcome:       journeymetrics.Outcome{Status: "passed"},
+	}
+	runA := base
+	runA.Turns, runA.Tokens = 18, journeymetrics.TokenTotals{CacheCreation: 1562}
+	runA.RunID = "27931963802"
+	runA.RunURL = "https://github.com/spacedock-dev/spacedock/actions/runs/27931963802"
+	runA.CapturedAt = "2026-06-20T00:00:00Z"
+	runB := base
+	runB.Turns, runB.Tokens = 20, journeymetrics.TokenTotals{CacheCreation: 1576}
+	runB.RunID = "28432388663"
+	runB.RunURL = "https://github.com/spacedock-dev/spacedock/actions/runs/28432388663"
+	runB.CapturedAt = "2026-06-27T00:00:00Z"
+
+	writeMetricRecord(t, runADir, runA)
+	writeMetricRecord(t, runBDir, runB)
+
+	out := filepath.Join(t.TempDir(), "journey-costs-v1.2.3.json")
+	if code := journeyCosts([]string{"1.2.3", "--metrics-dir", metricsDir, "--out", out}); code != 0 {
+		t.Fatalf("journeyCosts exit = %d, want 0", code)
+	}
+	ledger := readLedger(t, out)
+	entry := findScenario(t, ledger, "shallow-boot-window")
+	if len(entry.Observations) != 2 {
+		t.Fatalf("per-run-subdirectory shape must yield 2 observations for shallow-boot-window, got %d: %+v", len(entry.Observations), entry.Observations)
+	}
+	seenRunIDs := map[string]bool{}
+	for _, obs := range entry.Observations {
+		if obs.RunID == "" || obs.RunURL == "" {
+			t.Fatalf("observation missing run provenance: %+v", obs)
+		}
+		seenRunIDs[obs.RunID] = true
+	}
+	if !seenRunIDs["27931963802"] || !seenRunIDs["28432388663"] {
+		t.Fatalf("observations did not carry the two distinct source run ids, got %+v", seenRunIDs)
+	}
+}
+
+func readLedger(t *testing.T, path string) journeymetrics.Ledger {
+	t.Helper()
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var ledger journeymetrics.Ledger
+	if err := json.Unmarshal(data, &ledger); err != nil {
+		t.Fatalf("parse ledger %s: %v\n%s", path, err, data)
+	}
+	return ledger
+}
+
+func findScenario(t *testing.T, ledger journeymetrics.Ledger, scenarioID string) journeymetrics.ScenarioLedgerEntry {
+	t.Helper()
+	for _, entry := range ledger.Scenarios {
+		if entry.ScenarioID == scenarioID {
+			return entry
+		}
+	}
+	t.Fatalf("ledger has no scenario %q; scenarios: %+v", scenarioID, ledger.Scenarios)
+	return journeymetrics.ScenarioLedgerEntry{}
+}
+
 func writeMetricRecord(t *testing.T, dir string, record journeymetrics.Record) {
 	t.Helper()
 	data, err := json.Marshal(record)
 	if err != nil {
 		t.Fatal(err)
 	}
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		t.Fatal(err)
+	}
 	name := strings.Join([]string{record.ScenarioID, record.Runtime, record.Model}, "--") + ".json"
 	if err := os.WriteFile(filepath.Join(dir, name), data, 0o644); err != nil {
 		t.Fatal(err)