diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b0ae2871b..748cb31ac 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -35,13 +35,17 @@ 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: @@ -49,18 +53,40 @@ jobs: 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" diff --git a/.github/workflows/runtime-live-e2e.yml b/.github/workflows/runtime-live-e2e.yml index e4f25d62a..f05ac8ede 100644 --- a/.github/workflows/runtime-live-e2e.yml +++ b/.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 }}" diff --git a/cmd/spacedock-release/journey_costs_test.go b/cmd/spacedock-release/journey_costs_test.go index 4328b9ba0..596dfd06a 100644 --- a/cmd/spacedock-release/journey_costs_test.go +++ b/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) diff --git a/cmd/spacedock-release/journey_delta.go b/cmd/spacedock-release/journey_delta.go new file mode 100644 index 000000000..ab09dc630 --- /dev/null +++ b/cmd/spacedock-release/journey_delta.go @@ -0,0 +1,145 @@ +// ABOUTME: `spacedock-release journey-delta` renders and posts the AC-3 per-PR +// ABOUTME: journey-cost delta comment against the previously published release ledger. +package main + +import ( + "encoding/json" + "fmt" + "os" + "os/exec" + "strings" + + "github.com/spacedock-dev/spacedock/internal/journeymetrics" + "github.com/spacedock-dev/spacedock/internal/release" +) + +// postCommentFunc posts (or updates) the rendered PR comment. It is a seam so +// the argv is testable without a live `gh` call. +type postCommentFunc func(args []string) error + +// findCommentFunc looks up an existing journey-delta comment's numeric id on +// the PR by its sticky marker (release.JourneyDeltaCommentMarker), returning "" +// if none exists yet. It is a seam so the lookup is testable without a live +// `gh api` call. +type findCommentFunc func(pr string) (string, error) + +// ghPostComment invokes the real `gh` CLI. It is the production postCommentFunc; +// tests substitute a stub that records args instead. +func ghPostComment(args []string) error { + cmd := exec.Command("gh", args...) + cmd.Stdout, cmd.Stderr = os.Stdout, os.Stderr + return cmd.Run() +} + +// ghFindComment is the production findCommentFunc: it lists the PR's issue +// comments via `gh api` and returns the id of the first one whose body starts +// with release.JourneyDeltaCommentMarker, or "" if none is found. +func ghFindComment(pr string) (string, error) { + jq := fmt.Sprintf(`[.[] | select(.body | startswith(%q))][0].id // empty`, release.JourneyDeltaCommentMarker) + cmd := exec.Command("gh", "api", "repos/{owner}/{repo}/issues/"+pr+"/comments", "--paginate", "--jq", jq) + cmd.Stderr = os.Stderr + out, err := cmd.Output() + if err != nil { + return "", err + } + return strings.TrimSpace(string(out)), nil +} + +// journeyDelta computes and posts the AC-3 per-PR journey-cost delta comment: +// it diffs the current PR run's freshly emitted records (--metrics-dir) against +// the latest-by-captured_at observation per scenario/model in the previously +// published release ledger (the positional argument), +// renders the result as a single Markdown comment, and posts it as a single +// sticky comment — found by its marker (find), not by "the poster's last +// comment on the PR" (which can target the wrong comment if any other +// automated comment interleaves) — updating it in place if found, or creating +// it if this is the first run on this PR. +func journeyDelta(args []string, find findCommentFunc, post postCommentFunc) int { + if len(args) < 5 { + fmt.Fprintln(os.Stderr, "spacedock-release journey-delta: need --metrics-dir --pr ") + return 2 + } + ledgerPath := args[0] + var metricsDir, prNumber string + for i := 1; i < len(args); i++ { + switch args[i] { + case "--metrics-dir": + i++ + if i >= len(args) { + fmt.Fprintln(os.Stderr, "spacedock-release journey-delta: --metrics-dir needs a value") + return 2 + } + metricsDir = args[i] + case "--pr": + i++ + if i >= len(args) { + fmt.Fprintln(os.Stderr, "spacedock-release journey-delta: --pr needs a value") + return 2 + } + prNumber = args[i] + default: + fmt.Fprintf(os.Stderr, "spacedock-release journey-delta: unknown argument %q\n", args[i]) + return 2 + } + } + if ledgerPath == "" || metricsDir == "" || prNumber == "" { + fmt.Fprintln(os.Stderr, "spacedock-release journey-delta: need --metrics-dir --pr ") + return 2 + } + + ledgerData, err := os.ReadFile(ledgerPath) + if err != nil { + fmt.Fprintf(os.Stderr, "read previous ledger: %v\n", err) + return 1 + } + var ledger journeymetrics.Ledger + if err := json.Unmarshal(ledgerData, &ledger); err != nil { + fmt.Fprintf(os.Stderr, "parse previous ledger: %v\n", err) + return 1 + } + + current, err := journeymetrics.ReadRecordsDir(metricsDir) + if err != nil { + fmt.Fprintf(os.Stderr, "read current run metrics: %v\n", err) + return 1 + } + + deltas := release.ComputeJourneyDeltas(ledger, current) + body := release.RenderJourneyDeltaComment(deltas) + + bodyFile, err := os.CreateTemp("", "journey-delta-comment-*.md") + if err != nil { + fmt.Fprintf(os.Stderr, "create comment body file: %v\n", err) + return 1 + } + defer os.Remove(bodyFile.Name()) + if _, err := bodyFile.WriteString(body); err != nil { + bodyFile.Close() + fmt.Fprintf(os.Stderr, "write comment body file: %v\n", err) + return 1 + } + if err := bodyFile.Close(); err != nil { + fmt.Fprintf(os.Stderr, "close comment body file: %v\n", err) + return 1 + } + + commentID, err := find(prNumber) + if err != nil { + fmt.Fprintf(os.Stderr, "look up existing journey-delta comment: %v\n", err) + return 1 + } + if commentID != "" { + if err := post(release.JourneyDeltaUpdateCommentArgs(commentID, bodyFile.Name())); err != nil { + fmt.Fprintf(os.Stderr, "update PR comment %s: %v\n", commentID, err) + return 1 + } + fmt.Printf("updated journey-cost delta comment %s on PR %s (%d scenario/model rows)\n", commentID, prNumber, len(deltas)) + return 0 + } + if err := post(release.JourneyDeltaCreateCommentArgs(prNumber, bodyFile.Name())); err != nil { + fmt.Fprintf(os.Stderr, "post PR comment: %v\n", err) + return 1 + } + fmt.Printf("posted journey-cost delta comment on PR %s (%d scenario/model rows)\n", prNumber, len(deltas)) + return 0 +} diff --git a/cmd/spacedock-release/journey_delta_test.go b/cmd/spacedock-release/journey_delta_test.go new file mode 100644 index 000000000..8fc89d3a4 --- /dev/null +++ b/cmd/spacedock-release/journey_delta_test.go @@ -0,0 +1,111 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/spacedock-dev/spacedock/internal/journeymetrics" +) + +// TestJourneyDeltaCommandRejectsMissingArgs proves the CLI's argument-validation +// exit code, mirroring the sibling journey-costs command's shape. +func TestJourneyDeltaCommandRejectsMissingArgs(t *testing.T) { + stubFind := func(string) (string, error) { return "", nil } + stubPost := func([]string) error { return nil } + if code := journeyDelta([]string{"ledger.json", "--metrics-dir", t.TempDir()}, stubFind, stubPost); code != 2 { + t.Fatalf("journeyDelta with a missing --pr exit = %d, want 2", code) + } +} + +// TestJourneyDeltaCommandCreatesNewCommentWhenNoneExists and +// TestJourneyDeltaCommandUpdatesExistingCommentWhenFound are the find-by-marker +// replacement for --edit-last: journeyDelta's own branching logic (not a +// generic argv pass-through) must call the CREATE args when find reports no +// existing comment, and the UPDATE args (targeting the exact found id) when it +// does — proving the job posts once and then edits the SAME comment in place, +// located by content rather than by "the poster's last comment on the PR." +func TestJourneyDeltaCommandCreatesNewCommentWhenNoneExists(t *testing.T) { + ledgerPath := writeDeltaFixtureLedger(t) + metricsDir := t.TempDir() + writeMetricRecord(t, metricsDir, journeymetrics.Record{ + ScenarioID: "shallow-boot-window", Runtime: "claude", Model: "claude-sonnet-4-6", + MetricsState: journeymetrics.StateMeasured, Outcome: journeymetrics.Outcome{Status: "passed"}, + Turns: 20, Tokens: journeymetrics.TokenTotals{Total: 1600}, + }) + + find := func(string) (string, error) { return "", nil } + var calls [][]string + post := func(args []string) error { + calls = append(calls, args) + return nil + } + + if code := journeyDelta([]string{ledgerPath, "--metrics-dir", metricsDir, "--pr", "42"}, find, post); code != 0 { + t.Fatalf("journeyDelta exit = %d, want 0", code) + } + if len(calls) != 1 { + t.Fatalf("post invocations = %d, want 1", len(calls)) + } + joined := strings.Join(calls[0], " ") + if !strings.Contains(joined, "pr comment 42") || strings.Contains(joined, "PATCH") { + t.Fatalf("with no existing comment, journeyDelta must CREATE (pr comment), got: %v", calls[0]) + } +} + +func TestJourneyDeltaCommandUpdatesExistingCommentWhenFound(t *testing.T) { + ledgerPath := writeDeltaFixtureLedger(t) + metricsDir := t.TempDir() + writeMetricRecord(t, metricsDir, journeymetrics.Record{ + ScenarioID: "shallow-boot-window", Runtime: "claude", Model: "claude-sonnet-4-6", + MetricsState: journeymetrics.StateMeasured, Outcome: journeymetrics.Outcome{Status: "passed"}, + Turns: 25, Tokens: journeymetrics.TokenTotals{Total: 1700}, + }) + + find := func(string) (string, error) { return "987654321", nil } + var calls [][]string + post := func(args []string) error { + calls = append(calls, args) + return nil + } + + if code := journeyDelta([]string{ledgerPath, "--metrics-dir", metricsDir, "--pr", "42"}, find, post); code != 0 { + t.Fatalf("journeyDelta exit = %d, want 0", code) + } + if len(calls) != 1 { + t.Fatalf("post invocations = %d, want 1", len(calls)) + } + joined := strings.Join(calls[0], " ") + if !strings.Contains(joined, "issues/comments/987654321") || !strings.Contains(joined, "PATCH") { + t.Fatalf("with an existing comment id, journeyDelta must UPDATE that exact comment via gh api PATCH, got: %v", calls[0]) + } +} + +func writeDeltaFixtureLedger(t *testing.T) string { + t.Helper() + ledger := journeymetrics.Ledger{ + Scenarios: []journeymetrics.ScenarioLedgerEntry{ + { + ScenarioID: "shallow-boot-window", + Observations: []journeymetrics.Record{ + { + ScenarioID: "shallow-boot-window", Runtime: "claude", Model: "claude-sonnet-4-6", + Turns: 18, Tokens: journeymetrics.TokenTotals{Total: 1562}, CapturedAt: "2026-06-20T00:00:00Z", + RunURL: "https://github.com/spacedock-dev/spacedock/actions/runs/27931963802", + }, + }, + }, + }, + } + data, err := json.Marshal(ledger) + if err != nil { + t.Fatal(err) + } + path := filepath.Join(t.TempDir(), "journey-costs-previous.json") + if err := os.WriteFile(path, data, 0o644); err != nil { + t.Fatal(err) + } + return path +} diff --git a/cmd/spacedock-release/main.go b/cmd/spacedock-release/main.go index b213ceba4..8d65e411e 100644 --- a/cmd/spacedock-release/main.go +++ b/cmd/spacedock-release/main.go @@ -22,6 +22,7 @@ import ( // spacedock-release stamp-version [ ...] // spacedock-release bump-calendar // spacedock-release dev-preversion +// spacedock-release journey-delta --metrics-dir --pr // spacedock-release e2e-gate // // stamp-version rewrites each argument to the release version: a `.json` @@ -31,7 +32,16 @@ import ( // erroring unless the literal appears exactly once. bump-calendar advances the // marketplace plugin entry's calendar key to today's `0.0.YYYYMMDDNN` (AC-2d). // All rewrite in place. dev-preversion prints the post-release dev pre-version -// (X.(Y+1).0-pre1) the stable-tag edge advance stamps onto `next`. e2e-gate is the +// (X.(Y+1).0-pre1) the stable-tag edge advance stamps onto `next`. journey-delta +// renders and posts the per-PR journey-cost delta against the previously +// published release ledger's latest-by-captured_at baseline per scenario/model, +// updating a single sticky PR comment found by its HTML marker. The AC-4 +// release-ledger backfill (a captain-flagged one-time manual procedure, never a +// CI step) is a documented runbook, not a shipped subcommand: extraction reuses +// the exported ensigncycle.BuildShallowBootWindowRecord via a throwaway `go run` +// script against each archived stream, and the pre-upload safeguard is +// `jq -S .scenarios | diff - <(jq -S .scenarios )` — +// any diff output means STOP, do not upload. e2e-gate is the // release-time precondition: it passes (exit 0) only when a conclusion:success // Runtime Live E2E run exists for the commit, or when SPACEDOCK_E2E_GATE_WAIVER // is set, and blocks the cut (exit 1) otherwise. manifest-tag-gate blocks the cut @@ -55,6 +65,8 @@ func main() { os.Exit(devPreversion(os.Args[2:])) case "journey-costs": os.Exit(journeyCosts(os.Args[2:])) + case "journey-delta": + os.Exit(journeyDelta(os.Args[2:], ghFindComment, ghPostComment)) case "e2e-gate": os.Exit(runE2EGate(os.Args[2:], ghRunListForCommit)) case "manifest-tag-gate": @@ -363,6 +375,7 @@ Usage: spacedock-release bump-calendar spacedock-release dev-preversion spacedock-release journey-costs --metrics-dir --out + spacedock-release journey-delta --metrics-dir --pr spacedock-release e2e-gate spacedock-release manifest-tag-gate [ ...] spacedock-release notes diff --git a/internal/ensigncycle/claude_live_runner_test.go b/internal/ensigncycle/claude_live_runner_test.go index fbfbd4820..b3d0b9880 100644 --- a/internal/ensigncycle/claude_live_runner_test.go +++ b/internal/ensigncycle/claude_live_runner_test.go @@ -346,11 +346,16 @@ func runClaudeShallowBootScenario(t *testing.T, runner liveDriver, scenario shar if err := assertGreetInvokesNoDeferredFOSkill(result.stream); err != nil { t.Fatalf("%v\nArtifacts: %s", err, result.artifactDir) } - // AC-6: the greet-turn context is below the ceiling and no pre-greet 89k - // cache_creation spike (measured, over the captured token stream). + // The boot-window oracle: a greet turn was produced (structural only — the + // former ~60k ceiling/spike thresholds no longer gate CI, see + // assertShallowBootMeasuredTurns). if err := assertShallowBootMeasured(result.stream); err != nil { t.Fatalf("%v\nArtifacts: %s", err, result.artifactDir) } + // Record (don't gate on) the greet turn's full token usage as a distinct + // shallow-boot-window observation, riding the same journeymetrics ledger pipe + // emitClaudeScenarioMetrics below already uses. + emitShallowBootWindowMetrics(t, result.stream, runner.model()) emitClaudeScenarioMetrics(t, scenario, result, runner.model()) } diff --git a/internal/ensigncycle/journey_metrics_live_test.go b/internal/ensigncycle/journey_metrics_live_test.go index 933d24e58..471afe6b7 100644 --- a/internal/ensigncycle/journey_metrics_live_test.go +++ b/internal/ensigncycle/journey_metrics_live_test.go @@ -75,6 +75,29 @@ func ensignTranscripts(result liveResult) [][]byte { return transcripts } +// emitShallowBootWindowMetrics emits the shallow-boot-window observation (AC-1) +// alongside the whole-run "shallow-boot" record emitClaudeScenarioMetrics already +// publishes, into the same SPACEDOCK_JOURNEY_METRICS_DIR/shared-scenarios dir, so +// the two sibling records land together without either overwriting the other. +func emitShallowBootWindowMetrics(t *testing.T, stream string, model string) { + t.Helper() + dir := os.Getenv("SPACEDOCK_JOURNEY_METRICS_DIR") + if dir == "" { + return + } + turns, err := journeymetrics.ParseClaudeTurns([]byte(stream)) + if err != nil { + t.Fatalf("parse Claude turns for shallow-boot-window: %v", err) + } + record, err := BuildShallowBootWindowRecord(turns, model, journeymetrics.ParseClaudeCodeVersion([]byte(stream)), journeymetrics.ParseClaudeInitModel([]byte(stream))) + if err != nil { + t.Fatalf("build shallow-boot-window record: %v", err) + } + if err := journeymetrics.EmitRecord(filepath.Join(dir, "shared-scenarios"), record); err != nil { + t.Fatalf("emit shallow-boot-window record: %v", err) + } +} + func emitCodexScenarioMetrics(t *testing.T, scenario sharedRuntimeScenario, result codexScenarioResult) { t.Helper() dir := os.Getenv("SPACEDOCK_JOURNEY_METRICS_DIR") diff --git a/internal/ensigncycle/shallow_boot_measure_test.go b/internal/ensigncycle/shallow_boot_measure_test.go index a9ecb0f1b..6c16dbc4e 100644 --- a/internal/ensigncycle/shallow_boot_measure_test.go +++ b/internal/ensigncycle/shallow_boot_measure_test.go @@ -17,49 +17,6 @@ import ( // skill ARGUMENT, not on any Skill call. var deferredFOSkillNames = []string{"fo-status-viewer", "fo-write-core", "fo-dispatch-recovery"} -// AC-6 boot-window measurement thresholds. The ceiling is the milestone's ~60k -// greet-turn context ceiling; the spike threshold is set below the ~89k team-mode -// prefix re-cache (TeamCreate re-caching the whole conversation prefix to the 1h -// cache) so any cache_creation on that order is caught, while the small -// per-turn cache_creation a healthy boot writes (a few thousand tokens) is not. -const ( - greetContextCeiling = 60000 - teamRecacheSpikeFloor = 60000 -) - -// dispatchToolNames are the tool_use names that mark a worker dispatch / team -// creation — the boundary AC-6 uses to bound the pre-greet window. A turn that -// names one of these is NOT a greet turn. -var dispatchToolNames = map[string]bool{ - "Agent": true, - "Task": true, - "TeamCreate": true, -} - -// greetTurnIndex returns the index of the greet turn in turns: the LAST assistant -// turn that emits no dispatch tool_use (the FO greets via text output and stops, -// it does not dispatch in the same turn). It returns -1 when every turn dispatches -// (no greet was produced). A shallow boot has no dispatch at all, so the greet turn -// is simply the final turn; an eager-team boot fires TeamCreate before the greet, -// so the greet turn is still the final non-dispatch turn and the TeamCreate turn -// stays inside the pre-greet window where the spike check can see it. -func greetTurnIndex(turns []journeymetrics.ClaudeTurn) int { - idx := -1 - for i, t := range turns { - dispatches := false - for _, name := range t.ToolNames { - if dispatchToolNames[name] { - dispatches = true - break - } - } - if !dispatches { - idx = i - } - } - return idx -} - // teamToolNames are the Claude team-mode tool calls whose presence before the // greet would mean an eager team was created. TeamCreate is the one the FO would // fire; the others are listed so any team-lifecycle call pre-greet is caught. @@ -131,19 +88,14 @@ func assertGreetInvokesNoDeferredFOSkill(stream string) error { return nil } -// assertShallowBootMeasured is the AC-6 measured-saving oracle over a captured -// claude-stream.jsonl: it parses the stream per turn, identifies the greet turn and -// the pre-greet window (turns up to and including the greet turn), and asserts -// -// (1) the greet-turn context (input + cache_read + cache_creation) is below the -// ~60k ceiling, and -// (2) no pre-greet turn shows a cache_creation spike on the order of the ~89k -// team-mode prefix re-cache. -// -// It grades the host's emitted usage numbers — an independent source the contract -// cannot fake — never a prose match. A regression that re-introduced an eager team -// create or a heavy boot read pushes the greet context over the ceiling or surfaces -// the spike, failing this oracle. +// assertShallowBootMeasured is the AC-6 structural oracle over a captured +// claude-stream.jsonl: it parses the stream per turn and asserts a greet turn was +// produced. It no longer gates on the ~60k greet-context ceiling or the ~89k +// pre-greet cache_creation spike — those were absolute constants calibrated once +// against a since-superseded baseline (task j9, v0.20.3) that carried no +// information about whether a given boot got better or worse. The greet turn's +// full token usage rides instead as a recorded (not gated) shallow-boot-window +// journeymetrics observation — see BuildShallowBootWindowRecord. func assertShallowBootMeasured(stream string) error { turns, err := journeymetrics.ParseClaudeTurns([]byte(stream)) if err != nil { @@ -152,24 +104,17 @@ func assertShallowBootMeasured(stream string) error { return assertShallowBootMeasuredTurns(turns) } -// assertShallowBootMeasuredTurns is the turn-level half of the AC-6 oracle, split -// out so the offline unit cases can drive the ceiling and spike checks directly -// without a stream fixture. +// assertShallowBootMeasuredTurns is the turn-level half of the boot oracle, split +// out so the offline unit cases can drive it directly without a stream fixture. +// Only the structural checks remain a hard failure — the two former threshold +// branches (greet-context ceiling, pre-greet cache_creation spike) were removed +// here, at their actual location, not merely skipped by the caller. func assertShallowBootMeasuredTurns(turns []journeymetrics.ClaudeTurn) error { if len(turns) == 0 { return fmt.Errorf("stream carried no assistant turns — nothing to measure") } - greet := greetTurnIndex(turns) - if greet < 0 { + if greetTurnIndex(turns) < 0 { return fmt.Errorf("every assistant turn dispatched — no greet turn produced") } - if ctx := turns[greet].Context(); ctx >= greetContextCeiling { - return fmt.Errorf("greet-turn context %d is not below the ~%dk ceiling — a heavy boot read or eager team create regressed the saving", ctx, greetContextCeiling/1000) - } - for i := 0; i <= greet; i++ { - if cc := turns[i].Usage.CacheCreation; cc >= teamRecacheSpikeFloor { - return fmt.Errorf("pre-greet turn %d shows a cache_creation spike of %d (>= %d) — the ~89k team-mode prefix re-cache fired before the greet", i, cc, teamRecacheSpikeFloor) - } - } return nil } diff --git a/internal/ensigncycle/shallow_boot_measure_unit_test.go b/internal/ensigncycle/shallow_boot_measure_unit_test.go index 6a8cb3a73..25c7f4c0b 100644 --- a/internal/ensigncycle/shallow_boot_measure_unit_test.go +++ b/internal/ensigncycle/shallow_boot_measure_unit_test.go @@ -22,19 +22,21 @@ func readMeasureFixture(t *testing.T, name string) string { return string(data) } -// TestAssertShallowBootMeasuredOffline is the AC-6 de-risk: it validates the -// measured-saving oracle against committed real-shape streams BEFORE the live -// shallow-boot run spends a model on it. The positive fixture (a greet-and-stop -// boot, no TeamCreate, greet context under the ceiling) passes; the negative -// fixture (an eager-team boot with the ~89k cache_creation spike before the greet -// and a greet context over the ceiling) fails — proving the measurement -// distinguishes the realized saving from its absence, the AC-6 negative control. +// TestAssertShallowBootMeasuredOffline validates that assertShallowBootMeasured's +// only remaining checks are structural (the stream produced a greet turn) now that +// the former ~60k ceiling/spike thresholds are removed at their actual location +// (assertShallowBootMeasuredTurns), not merely skipped by the caller. Both the +// shallow-boot positive fixture and the eager-team-boot fixture — whose ~89k +// pre-greet spike and over-ceiling greet used to FAIL this oracle — now pass, +// since neither fixture violates the remaining structural checks. The +// ceiling/spike signal itself now rides the shallow-boot-window journeymetrics +// observation instead of a pass/fail gate — see TestBuildShallowBootWindowRecord. func TestAssertShallowBootMeasuredOffline(t *testing.T) { if err := assertShallowBootMeasured(readMeasureFixture(t, "shallow-boot-greet.stream.jsonl")); err != nil { - t.Fatalf("shallow-boot positive fixture must pass the measured-saving oracle: %v", err) + t.Fatalf("shallow-boot positive fixture must pass the structural boot oracle: %v", err) } - if err := assertShallowBootMeasured(readMeasureFixture(t, "eager-team-boot.stream.jsonl")); err == nil { - t.Fatal("eager-team negative fixture must FAIL the measured-saving oracle (89k spike before greet + greet context over ceiling) — else the measurement does not distinguish the realized saving from its absence") + if err := assertShallowBootMeasured(readMeasureFixture(t, "eager-team-boot.stream.jsonl")); err != nil { + t.Fatalf("eager-team fixture must pass now that the ceiling/spike thresholds are removed (it still produces a valid greet turn): %v", err) } } @@ -137,28 +139,150 @@ func TestParserExtractsTeamCallsFromRealHangCapture(t *testing.T) { } } -// TestShallowBootMeasureSignalsAreIndependent isolates the two AC-6 signals so -// neither can be silently dropped: a stream that fails ONLY the ceiling check (a -// heavy greet, no spike) and a stream that fails ONLY the spike check (a pre-greet -// 89k cache_creation, but a light greet) must each go red. +// TestBuildShallowBootWindowRecord is AC-1's primary offline fixture unit test: it +// proves EmitRecord writes a shallow-boot-window--claude--llm--llm-live---- +// measured.json file DISTINCT from the pre-existing whole-run +// shallow-boot--...--measured.json record (both present after one scenario run, +// neither overwriting the other), carrying Turns == greetIndex+1 and Tokens equal +// to the greet turn's full actual TokenTotals (input, output, cache_read, +// cache_creation) from the fixture stream — not CacheCreation alone. It also +// proves the follow-on fields the fixture now carries: BaselineTokens equals +// the FIRST turn's full TokenTotals (so a reader can compute net boot cost +// without re-parsing the stream), ClaudeCodeVersion is threaded through from +// the fixture's system/init event's claude_code_version field, and ResolvedModel +// (the fixture's actual "claude-opus-4-8") differs from the caller-supplied +// Model alias ("claude-test-model") — proving the two fields serve distinct +// purposes rather than duplicating each other. +func TestBuildShallowBootWindowRecord(t *testing.T) { + stream := readMeasureFixture(t, "shallow-boot-greet.stream.jsonl") + turns, err := journeymetrics.ParseClaudeTurns([]byte(stream)) + if err != nil { + t.Fatalf("ParseClaudeTurns: %v", err) + } + greet := greetTurnIndex(turns) + if greet < 0 { + t.Fatal("fixture must produce a greet turn") + } + claudeCodeVersion := journeymetrics.ParseClaudeCodeVersion([]byte(stream)) + if claudeCodeVersion == "" { + t.Fatal("fixture must carry a claude_code_version on its system/init event") + } + resolvedModel := journeymetrics.ParseClaudeInitModel([]byte(stream)) + if resolvedModel == "" { + t.Fatal("fixture must carry a model on its system/init event") + } + + const model = "claude-test-model" + record, err := BuildShallowBootWindowRecord(turns, model, claudeCodeVersion, resolvedModel) + if err != nil { + t.Fatalf("BuildShallowBootWindowRecord: %v", err) + } + if record.ScenarioID != "shallow-boot-window" { + t.Fatalf("ScenarioID = %q, want shallow-boot-window", record.ScenarioID) + } + if record.Turns != greet+1 { + t.Fatalf("Turns = %d, want %d (greetIndex+1)", record.Turns, greet+1) + } + want := turns[greet].Usage + if record.Tokens.Input != want.Input || record.Tokens.Output != want.Output || + record.Tokens.CacheCreation != want.CacheCreation || record.Tokens.CacheRead != want.CacheRead { + t.Fatalf("Tokens = %+v, want the greet turn's full TokenTotals %+v (not CacheCreation alone)", record.Tokens, want) + } + wantBaseline := turns[0].Usage + if record.BaselineTokens.Input != wantBaseline.Input || record.BaselineTokens.Output != wantBaseline.Output || + record.BaselineTokens.CacheCreation != wantBaseline.CacheCreation || record.BaselineTokens.CacheRead != wantBaseline.CacheRead { + t.Fatalf("BaselineTokens = %+v, want the FIRST turn's full TokenTotals %+v", record.BaselineTokens, wantBaseline) + } + if record.ClaudeCodeVersion != claudeCodeVersion { + t.Fatalf("ClaudeCodeVersion = %q, want %q from the fixture's system/init event", record.ClaudeCodeVersion, claudeCodeVersion) + } + if record.Model != model { + t.Fatalf("Model = %q, want the caller-supplied alias %q unchanged", record.Model, model) + } + if record.ResolvedModel != resolvedModel || record.ResolvedModel == record.Model { + t.Fatalf("ResolvedModel = %q, want the fixture's actual model %q, distinct from Model %q", record.ResolvedModel, resolvedModel, record.Model) + } + + // The whole-run "shallow-boot" record the same scenario run already publishes + // (see emitClaudeScenarioMetrics) must survive untouched as a sibling file. + dir := t.TempDir() + wholeRun := journeymetrics.BuildRecord(journeymetrics.JourneySpec{ + ScenarioID: "shallow-boot", + Source: "live-harness", + Mode: journeymetrics.ModeLLMLive, + Runtime: "claude", + Executor: "llm", + Host: "claude", + Model: model, + }, journeymetrics.BehaviorResult{Passed: true}, journeymetrics.Observation{}) + if err := journeymetrics.EmitRecord(dir, wholeRun); err != nil { + t.Fatalf("emit whole-run shallow-boot record: %v", err) + } + if err := journeymetrics.EmitRecord(dir, record); err != nil { + t.Fatalf("emit shallow-boot-window record: %v", err) + } + + windowPath := filepath.Join(dir, "shallow-boot-window--claude--llm--llm-live--"+model+"--measured.json") + if _, err := os.Stat(windowPath); err != nil { + t.Fatalf("expected shallow-boot-window record file at %s: %v", windowPath, err) + } + wholeRunPath := filepath.Join(dir, "shallow-boot--claude--llm--llm-live--"+model+"--measured.json") + if _, err := os.Stat(wholeRunPath); err != nil { + t.Fatalf("the pre-existing shallow-boot record must survive unmodified at %s: %v", wholeRunPath, err) + } + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + if len(entries) != 2 { + t.Fatalf("expected exactly 2 record files (shallow-boot and shallow-boot-window), got %d", len(entries)) + } +} + +// TestShallowBootMeasureSignalsAreIndependent previously proved the ceiling and +// spike checks could each fail independently. Now that both threshold branches are +// removed from assertShallowBootMeasuredTurns (not merely bypassed by the caller), +// it proves the gate is gone at its actual location — none of the three cases that +// used to trip a threshold return an error — while BuildShallowBootWindowRecord +// still captures each case's full greet-turn TokenTotals, so the former +// ceiling/spike signal stays reconstructable from the recorded telemetry even +// though it no longer gates CI. func TestShallowBootMeasureSignalsAreIndependent(t *testing.T) { - // Only the ceiling fails: a single text greet turn whose context exceeds the - // ceiling, with no cache_creation spike anywhere. + // Formerly failed only the ceiling check: a single text greet turn whose + // context exceeds the old ~60k ceiling, with no cache_creation spike anywhere. heavyGreet := []journeymetrics.ClaudeTurn{ - {ID: "greet", Usage: journeymetrics.TokenTotals{Input: 100, CacheRead: greetContextCeiling, CacheCreation: 0}}, + {ID: "greet", Usage: journeymetrics.TokenTotals{Input: 100, CacheRead: 60000, CacheCreation: 0}}, } - if err := assertShallowBootMeasuredTurns(heavyGreet); err == nil { - t.Fatal("a greet turn whose context exceeds the ceiling (no spike) must fail on the ceiling check") + if err := assertShallowBootMeasuredTurns(heavyGreet); err != nil { + t.Fatalf("a greet turn over the former ceiling must no longer fail now that the threshold gate is removed: %v", err) + } + if record, err := BuildShallowBootWindowRecord(heavyGreet, "test-model", "2.1.196", "claude-sonnet-4-6"); err != nil { + t.Fatalf("BuildShallowBootWindowRecord(heavyGreet): %v", err) + } else if record.Tokens.CacheRead != 60000 { + t.Fatalf("recorded Tokens must preserve the full greet-turn usage that used to trip the ceiling check, got %+v", record.Tokens) + } else if record.ClaudeCodeVersion != "2.1.196" { + t.Fatalf("ClaudeCodeVersion = %q, want the passed-through version 2.1.196", record.ClaudeCodeVersion) + } else if record.ResolvedModel != "claude-sonnet-4-6" { + t.Fatalf("ResolvedModel = %q, want the passed-through resolved model claude-sonnet-4-6", record.ResolvedModel) } - // Only the spike fails: a pre-greet dispatch turn carrying the ~89k spike, then - // a light text greet under the ceiling. + // Formerly failed only the spike check: a pre-greet dispatch turn carrying the + // ~89k spike, then a light text greet under the old ceiling. spikeThenLightGreet := []journeymetrics.ClaudeTurn{ {ID: "team", Usage: journeymetrics.TokenTotals{Input: 8, CacheCreation: 89000, CacheRead: 16000}, ToolNames: []string{"TeamCreate"}}, {ID: "greet", Usage: journeymetrics.TokenTotals{Input: 100, CacheRead: 5000, CacheCreation: 0}}, } - if err := assertShallowBootMeasuredTurns(spikeThenLightGreet); err == nil { - t.Fatal("a pre-greet ~89k cache_creation spike (with a light greet) must fail on the spike check") + if err := assertShallowBootMeasuredTurns(spikeThenLightGreet); err != nil { + t.Fatalf("a pre-greet ~89k cache_creation spike (light greet) must no longer fail now that the threshold gate is removed: %v", err) + } + if record, err := BuildShallowBootWindowRecord(spikeThenLightGreet, "test-model", "", ""); err != nil { + t.Fatalf("BuildShallowBootWindowRecord(spikeThenLightGreet): %v", err) + } else if record.Turns != 2 { + t.Fatalf("Turns = %d, want 2 (greetIndex+1, greet is turns[1])", record.Turns) + } else if record.BaselineTokens.CacheCreation != 89000 { + t.Fatalf("BaselineTokens must preserve the pre-greet ~89k spike turn's full usage (turns[0]), got %+v", record.BaselineTokens) + } else if record.PreGreetPeakCacheCreation != 89000 { + t.Fatalf("PreGreetPeakCacheCreation = %d, want 89000 (the former teamRecacheSpikeFloor signal)", record.PreGreetPeakCacheCreation) } // Both clean: a light greet, no pre-greet spike — the realized-saving end-state. @@ -170,3 +294,28 @@ func TestShallowBootMeasureSignalsAreIndependent(t *testing.T) { t.Fatalf("a clean shallow boot (light greet, no spike) must pass: %v", err) } } + +// TestPreGreetPeakCacheCreationFindsSpikeNotOnFirstTurn is the regression proof +// for the captain-review finding that BaselineTokens (turns[0].Usage alone) +// does NOT reconstruct the former teamRecacheSpikeFloor signal for a REALISTIC +// multi-turn pre-greet window where the spike lands on a LATER pre-greet turn, +// not the first one. The original teamRecacheSpikeFloor check looped EVERY +// pre-greet turn (assertShallowBootMeasuredTurns's removed `for i := 0; i <= +// greet; i++` branch) — a max over the whole window, not just turns[0]. +func TestPreGreetPeakCacheCreationFindsSpikeNotOnFirstTurn(t *testing.T) { + turns := []journeymetrics.ClaudeTurn{ + {ID: "boot", Usage: journeymetrics.TokenTotals{Input: 4, CacheCreation: 500, CacheRead: 8000}, ToolNames: []string{"Bash"}}, + {ID: "team", Usage: journeymetrics.TokenTotals{Input: 8, CacheCreation: 89000, CacheRead: 16000}, ToolNames: []string{"TeamCreate"}}, + {ID: "greet", Usage: journeymetrics.TokenTotals{Input: 100, CacheRead: 5000, CacheCreation: 0}}, + } + record, err := BuildShallowBootWindowRecord(turns, "test-model", "", "") + if err != nil { + t.Fatalf("BuildShallowBootWindowRecord: %v", err) + } + if record.BaselineTokens.CacheCreation != 500 { + t.Fatalf("BaselineTokens (turns[0]) = %+v, want CacheCreation 500 — it is NOT the spike turn here, proving it alone cannot reconstruct the spike signal", record.BaselineTokens) + } + if record.PreGreetPeakCacheCreation != 89000 { + t.Fatalf("PreGreetPeakCacheCreation = %d, want 89000 — the spike on turns[1] (NOT turns[0]) must still be found", record.PreGreetPeakCacheCreation) + } +} diff --git a/internal/ensigncycle/shallow_boot_window_record.go b/internal/ensigncycle/shallow_boot_window_record.go new file mode 100644 index 000000000..19218cf4b --- /dev/null +++ b/internal/ensigncycle/shallow_boot_window_record.go @@ -0,0 +1,124 @@ +// ABOUTME: Boot-window telemetry extraction shared by the live shallow-boot +// ABOUTME: scenario test and the release-ledger backfill runbook (AC-1/AC-4). +package ensigncycle + +import ( + "fmt" + + "github.com/spacedock-dev/spacedock/internal/journeymetrics" +) + +// shallowBootWindowScenarioID is the journeymetrics ScenarioID the boot-window +// observation (see BuildShallowBootWindowRecord) publishes under. It MUST differ +// from the whole-run "shallow-boot" scenario ID the same run already publishes +// (see emitClaudeScenarioMetrics) — journeymetrics.recordFilename has no +// run-distinguishing component, so reusing "shallow-boot" here would silently +// overwrite that record instead of adding a sibling observation. +const shallowBootWindowScenarioID = "shallow-boot-window" + +// dispatchToolNames are the tool_use names that mark a worker dispatch / team +// creation — the boundary the boot-window measurement uses to bound the +// pre-greet window. A turn that names one of these is NOT a greet turn. +var dispatchToolNames = map[string]bool{ + "Agent": true, + "Task": true, + "TeamCreate": true, +} + +// greetTurnIndex returns the index of the greet turn in turns: the LAST assistant +// turn that emits no dispatch tool_use (the FO greets via text output and stops, +// it does not dispatch in the same turn). It returns -1 when every turn dispatches +// (no greet was produced). A shallow boot has no dispatch at all, so the greet turn +// is simply the final turn; an eager-team boot fires TeamCreate before the greet, +// so the greet turn is still the final non-dispatch turn and the TeamCreate turn +// stays inside the pre-greet window where the spike check can see it. +func greetTurnIndex(turns []journeymetrics.ClaudeTurn) int { + idx := -1 + for i, t := range turns { + dispatches := false + for _, name := range t.ToolNames { + if dispatchToolNames[name] { + dispatches = true + break + } + } + if !dispatches { + idx = i + } + } + return idx +} + +// preGreetPeakCacheCreation returns the MAXIMUM cache_creation across every +// turn strictly before greet — the exact aggregate the removed +// teamRecacheSpikeFloor check compared against its floor (it looped every +// pre-greet turn and tripped if ANY turn's cache_creation reached the floor, +// which is precisely a max-over-the-window comparison). Greet's own +// cache_creation is excluded: it is already captured in full by Tokens, so +// including it here would double-record the same data point under a +// "pre-greet" name. Returns 0 when greet is the first turn (no pre-greet +// window at all) — correctly signaling "no spike, nothing happened before +// greet" rather than a sentinel. +func preGreetPeakCacheCreation(turns []journeymetrics.ClaudeTurn, greet int) int { + peak := 0 + for i := 0; i < greet; i++ { + if cc := turns[i].Usage.CacheCreation; cc > peak { + peak = cc + } + } + return peak +} + +// BuildShallowBootWindowRecord builds the shallow-boot-window journeymetrics.Record +// from a parsed turn list: Turns is greetIndex+1, and Tokens is the greet turn's +// FULL TokenTotals (input, output, cache_read, and cache_creation — not +// cache_creation alone), so a future reader can reconstruct the former ceiling +// signal (Context() = input+cache_read+cache_creation) from this one recorded +// observation. PreGreetPeakCacheCreation is the MAX cache_creation across the +// turns BEFORE greet — the exact aggregate the removed teamRecacheSpikeFloor +// check compared against its floor — so the former pre-greet-spike signal +// stays reconstructable too; Tokens.CacheCreation alone does NOT do this for a +// multi-turn pre-greet window, since the spike can land on any turn before +// greet, not necessarily the greet turn itself. BaselineTokens carries the +// FIRST turn's full TokenTotals as a reference point for how the conversation +// started; because per-turn usage is NOT cumulative across turns, only +// Tokens.Context() - BaselineTokens.Context() is a defensible "context grown +// since the first turn" quantity (Context sums input+cache_read+cache_creation, +// which track the conversation's accumulating cached prefix) — the Input/Output +// fields are NOT independently subtractable between two unrelated single +// requests, so no such subtraction is implied for them. +// claudeCodeVersion and resolvedModel (the caller's +// journeymetrics.ParseClaudeCodeVersion(data) and +// journeymetrics.ParseClaudeInitModel(data) results) thread the Claude Code +// CLI client version and the actually-resolved model identifier through, so a +// future trend reader can attribute a boot-cost shift to a client update or a +// silent alias-resolution change (e.g. "sonnet" moving to a new snapshot) +// rather than the FO's own contract. +// Exported (unlike the live-scenario test glue around it) so the AC-4 release- +// ledger backfill runbook's throwaway `go run` script can apply the SAME +// extraction logic to archived streams, standalone, rather than duplicating it. +func BuildShallowBootWindowRecord(turns []journeymetrics.ClaudeTurn, model string, claudeCodeVersion string, resolvedModel string) (journeymetrics.Record, error) { + if len(turns) == 0 { + return journeymetrics.Record{}, fmt.Errorf("stream carried no assistant turns — nothing to record") + } + greet := greetTurnIndex(turns) + if greet < 0 { + return journeymetrics.Record{}, fmt.Errorf("every assistant turn dispatched — no greet turn produced") + } + return journeymetrics.BuildRecord(journeymetrics.JourneySpec{ + ScenarioID: shallowBootWindowScenarioID, + Source: "live-harness", + Mode: journeymetrics.ModeLLMLive, + Runtime: "claude", + Executor: "llm", + Host: "claude", + Model: model, + }, journeymetrics.BehaviorResult{Passed: true}, journeymetrics.Observation{ + Turns: greet + 1, + Tokens: turns[greet].Usage, + BaselineTokens: turns[0].Usage, + PreGreetPeakCacheCreation: preGreetPeakCacheCreation(turns, greet), + ClaudeCodeVersion: claudeCodeVersion, + ResolvedModel: resolvedModel, + }), nil +} diff --git a/internal/ensigncycle/testdata/shallow-boot-greet.stream.jsonl b/internal/ensigncycle/testdata/shallow-boot-greet.stream.jsonl index 30df03688..64096b54d 100644 --- a/internal/ensigncycle/testdata/shallow-boot-greet.stream.jsonl +++ b/internal/ensigncycle/testdata/shallow-boot-greet.stream.jsonl @@ -1,4 +1,4 @@ -{"type":"system","subtype":"init","model":"claude-opus-4-8"} +{"type":"system","subtype":"init","model":"claude-opus-4-8","claude_code_version":"2.1.196"} {"type":"assistant","message":{"id":"msg_boot1","model":"claude-opus-4-8","usage":{"input_tokens":4,"output_tokens":12,"cache_creation_input_tokens":1200,"cache_read_input_tokens":8000},"content":[{"type":"thinking","thinking":"check the version"}]}} {"type":"assistant","message":{"id":"msg_boot1","model":"claude-opus-4-8","usage":{"input_tokens":4,"output_tokens":12,"cache_creation_input_tokens":1200,"cache_read_input_tokens":8000},"content":[{"type":"tool_use","id":"toolu_ver","name":"Bash","input":{"command":"spacedock --version"}}]}} {"type":"assistant","message":{"id":"msg_boot2","model":"claude-opus-4-8","usage":{"input_tokens":6,"output_tokens":40,"cache_creation_input_tokens":900,"cache_read_input_tokens":18000},"content":[{"type":"thinking","thinking":"read the boot state"}]}} diff --git a/internal/journeymetrics/claude.go b/internal/journeymetrics/claude.go index 1b32efd2d..b34fea6a1 100644 --- a/internal/journeymetrics/claude.go +++ b/internal/journeymetrics/claude.go @@ -238,6 +238,56 @@ func ParseClaudeTurns(data []byte) ([]ClaudeTurn, error) { return turns, nil } +// claudeInitEvent scans a stream-json transcript for the "system"/"init" event's +// raw fields and returns nil if none is found. Shared by ParseClaudeCodeVersion +// and ParseClaudeInitModel so both read off the SAME line without a duplicate +// scan of the (potentially large) stream. +func claudeInitEvent(data []byte) map[string]json.RawMessage { + scanner := bufio.NewScanner(bytes.NewReader(data)) + scanner.Buffer(make([]byte, 0, 64*1024), 4*1024*1024) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if line == "" || !strings.HasPrefix(line, "{") { + continue + } + var row map[string]json.RawMessage + if err := json.Unmarshal([]byte(line), &row); err != nil { + continue + } + if rawString(row["type"]) == "system" && rawString(row["subtype"]) == "init" { + return row + } + } + return nil +} + +// ParseClaudeCodeVersion scans a stream-json transcript for the "system"/"init" +// event's claude_code_version field — the Claude Code CLI client version that +// produced the stream, distinct from the model identifier. Returns "" if no +// init event carries the field, e.g. a trimmed fixture stream or a stream +// captured before the field existed. +func ParseClaudeCodeVersion(data []byte) string { + row := claudeInitEvent(data) + if row == nil { + return "" + } + return rawString(row["claude_code_version"]) +} + +// ParseClaudeInitModel scans a stream-json transcript for the "system"/"init" +// event's model field — the model identifier the runtime actually resolved at +// boot. This can be MORE PRECISE than a CI-matrix alias recorded as +// Record.Model: e.g. real CI-captured streams launched with the "sonnet" alias +// report "claude-sonnet-4-6" here, while Record.Model stays "sonnet" (the +// stable per-leg grouping key CI passes). Returns "" if no init event is found. +func ParseClaudeInitModel(data []byte) string { + row := claudeInitEvent(data) + if row == nil { + return "" + } + return rawString(row["model"]) +} + type claudeAssistant struct { ID string Model string diff --git a/internal/journeymetrics/claude_test.go b/internal/journeymetrics/claude_test.go index f93f17290..e4081c1ba 100644 --- a/internal/journeymetrics/claude_test.go +++ b/internal/journeymetrics/claude_test.go @@ -182,6 +182,63 @@ func TestParseClaudeTurnsExtractsSkillNamesAcrossDeltas(t *testing.T) { } } +// TestParseClaudeCodeVersionExtractsFromInitEvent proves the Claude Code CLI +// client version is read from the system/init event's claude_code_version +// field, distinct from the per-turn model identifier. +func TestParseClaudeCodeVersionExtractsFromInitEvent(t *testing.T) { + stream := `{"type":"system","subtype":"init","model":"claude-opus-4-8","claude_code_version":"2.1.197"} +{"type":"assistant","message":{"id":"msg_1","model":"claude-opus-4-8","usage":{"input_tokens":1,"output_tokens":1}}}` + + if got := ParseClaudeCodeVersion([]byte(stream)); got != "2.1.197" { + t.Fatalf("ParseClaudeCodeVersion = %q, want 2.1.197", got) + } +} + +// TestParseClaudeCodeVersionEmptyWhenAbsent proves a stream with no system/init +// event at all (a trimmed fixture, or a stream captured before the field +// existed) returns "" rather than erroring. +func TestParseClaudeCodeVersionEmptyWhenAbsent(t *testing.T) { + data := readTestdata(t, "claude_no_terminal.stream.jsonl") + if got := ParseClaudeCodeVersion(data); got != "" { + t.Fatalf("ParseClaudeCodeVersion = %q, want empty for a stream with no system/init event", got) + } +} + +// TestParseClaudeCodeVersionSkipsNonInitSystemEvents proves a non-init system +// event (e.g. a thinking_tokens progress event) is not mistaken for the init +// event that carries claude_code_version. +func TestParseClaudeCodeVersionSkipsNonInitSystemEvents(t *testing.T) { + stream := `{"type":"system","subtype":"thinking_tokens","estimated_tokens":50} +{"type":"system","subtype":"init","model":"claude-opus-4-8","claude_code_version":"2.1.197"}` + + if got := ParseClaudeCodeVersion([]byte(stream)); got != "2.1.197" { + t.Fatalf("ParseClaudeCodeVersion = %q, want 2.1.197 (ignoring the non-init system event)", got) + } +} + +// TestParseClaudeInitModelExtractsResolvedIdentifier proves the model field is +// read from the system/init event — the identifier the runtime actually +// resolved, which real CI-captured streams show can be MORE PRECISE than a +// CI-matrix alias (e.g. "sonnet" resolves to "claude-sonnet-4-6"). +func TestParseClaudeInitModelExtractsResolvedIdentifier(t *testing.T) { + stream := `{"type":"system","subtype":"init","model":"claude-sonnet-4-6","claude_code_version":"2.1.161"} +{"type":"assistant","message":{"id":"msg_1","model":"claude-sonnet-4-6","usage":{"input_tokens":1,"output_tokens":1}}}` + + if got := ParseClaudeInitModel([]byte(stream)); got != "claude-sonnet-4-6" { + t.Fatalf("ParseClaudeInitModel = %q, want claude-sonnet-4-6", got) + } +} + +// TestParseClaudeInitModelEmptyWhenAbsent mirrors +// TestParseClaudeCodeVersionEmptyWhenAbsent: a stream with no system/init event +// returns "" rather than erroring. +func TestParseClaudeInitModelEmptyWhenAbsent(t *testing.T) { + data := readTestdata(t, "claude_no_terminal.stream.jsonl") + if got := ParseClaudeInitModel(data); got != "" { + t.Fatalf("ParseClaudeInitModel = %q, want empty for a stream with no system/init event", got) + } +} + func readTestdata(t *testing.T, name string) []byte { t.Helper() data, err := os.ReadFile(filepath.Join("testdata", name)) diff --git a/internal/journeymetrics/record.go b/internal/journeymetrics/record.go index 0141a219b..b55db083f 100644 --- a/internal/journeymetrics/record.go +++ b/internal/journeymetrics/record.go @@ -39,26 +39,30 @@ func BuildRecord(spec JourneySpec, result BehaviorResult, observation Observatio outcome = Outcome{Status: "passed"} } record := Record{ - SchemaVersion: RecordSchemaVersion, - ScenarioID: firstNonEmpty(spec.ScenarioID, spec.ID), - Source: spec.Source, - Mode: spec.Mode, - Runtime: firstNonEmpty(spec.Runtime, spec.Host), - Executor: spec.Executor, - Host: spec.Host, - Model: spec.Model, - MetricsState: state, - Outcome: outcome, - DurationMS: observation.Duration.Milliseconds(), - Turns: observation.Turns, - ToolCalls: observation.ToolCalls, - ToolCallsByName: observation.ToolCallsByName, - StatusReadCalls: observation.StatusReadCalls, - ScopedReadCalls: observation.ScopedReadCalls, - Tokens: observation.Tokens.withTotal(), - TotalCostUSD: observation.TotalCostUSD, - ModelUsage: normalizeModelUsage(observation.ModelUsage), - Budget: spec.Budget, + SchemaVersion: RecordSchemaVersion, + ScenarioID: firstNonEmpty(spec.ScenarioID, spec.ID), + Source: spec.Source, + Mode: spec.Mode, + Runtime: firstNonEmpty(spec.Runtime, spec.Host), + Executor: spec.Executor, + Host: spec.Host, + Model: spec.Model, + MetricsState: state, + Outcome: outcome, + DurationMS: observation.Duration.Milliseconds(), + Turns: observation.Turns, + ToolCalls: observation.ToolCalls, + ToolCallsByName: observation.ToolCallsByName, + StatusReadCalls: observation.StatusReadCalls, + ScopedReadCalls: observation.ScopedReadCalls, + Tokens: observation.Tokens.withTotal(), + BaselineTokens: observation.BaselineTokens.withTotal(), + PreGreetPeakCacheCreation: observation.PreGreetPeakCacheCreation, + TotalCostUSD: observation.TotalCostUSD, + ModelUsage: normalizeModelUsage(observation.ModelUsage), + Budget: spec.Budget, + ClaudeCodeVersion: observation.ClaudeCodeVersion, + ResolvedModel: observation.ResolvedModel, } if hasBudget(spec.Budget) { result := EvaluateBudget(record, spec.Budget) @@ -69,6 +73,7 @@ func BuildRecord(spec JourneySpec, result BehaviorResult, observation Observatio func EmitRecord(dir string, record Record) error { record = normalizeRecord(record) + record = stampProvenance(record) if strings.TrimSpace(record.ScenarioID) == "" { return fmt.Errorf("scenario id is required") } @@ -83,6 +88,32 @@ func EmitRecord(dir string, record Record) error { return os.WriteFile(filepath.Join(dir, recordFilename(record)), data, 0o644) } +// stampProvenance fills in the run-provenance fields a caller left unset. +// CapturedAt defaults to the current time, so every freshly emitted record can be +// ordered chronologically once it lands in a published ledger. RunID/RunURL +// default from the GitHub Actions run environment ($GITHUB_RUN_ID / +// $GITHUB_SERVER_URL / $GITHUB_REPOSITORY) when present; outside CI they stay +// empty and the omitempty tags drop them from the JSON. It deliberately runs ONLY +// at emission time, never inside normalizeRecord — normalizeRecord also runs when +// re-reading an already-emitted record (ReadRecordsDir, AggregateLedger), and +// re-stamping CapturedAt there would silently overwrite a historical record's real +// capture time with "now" on every rebuild. +func stampProvenance(record Record) Record { + if record.CapturedAt == "" { + record.CapturedAt = time.Now().UTC().Format(time.RFC3339) + } + if record.RunID == "" { + record.RunID = os.Getenv("GITHUB_RUN_ID") + } + if record.RunURL == "" && record.RunID != "" { + serverURL, repo := os.Getenv("GITHUB_SERVER_URL"), os.Getenv("GITHUB_REPOSITORY") + if serverURL != "" && repo != "" { + record.RunURL = strings.TrimSuffix(serverURL, "/") + "/" + repo + "/actions/runs/" + record.RunID + } + } + return record +} + func EvaluateBudget(record Record, budget Budget) BudgetResult { var violations []string if budget.MaxTotalTokens != nil { @@ -110,6 +141,7 @@ func normalizeRecord(record Record) Record { record.JourneyID = "" record.Runtime = firstNonEmpty(record.Runtime, record.Host) record.Tokens = record.Tokens.withTotal() + record.BaselineTokens = record.BaselineTokens.withTotal() record.ModelUsage = normalizeModelUsage(record.ModelUsage) return record } diff --git a/internal/journeymetrics/types.go b/internal/journeymetrics/types.go index 6fe65c3d1..25e665890 100644 --- a/internal/journeymetrics/types.go +++ b/internal/journeymetrics/types.go @@ -46,8 +46,31 @@ type Observation struct { StatusReadCalls int ScopedReadCalls int Tokens TokenTotals - TotalCostUSD float64 - ModelUsage map[string]ModelUsage + // BaselineTokens is the first turn's full TokenTotals, a reference point for + // how the observation started. Per-turn usage is NOT cumulative across + // turns, so only Tokens.Context() - BaselineTokens.Context() is a + // defensible derived quantity (Context tracks the conversation's + // accumulating cached prefix); Input/Output are not independently + // subtractable between two unrelated single requests. Left zero-valued for + // scenarios that don't measure a pre-observation baseline. + BaselineTokens TokenTotals + // PreGreetPeakCacheCreation is the MAXIMUM cache_creation across the turns + // before the measured turn — e.g. the shallow-boot-window scenario's former + // teamRecacheSpikeFloor signal. Left zero when the scenario has no + // pre-observation window (a single-turn observation) or doesn't track it. + PreGreetPeakCacheCreation int + TotalCostUSD float64 + ModelUsage map[string]ModelUsage + // ClaudeCodeVersion is the Claude Code CLI client version (the stream's + // system/init event's claude_code_version field) that produced this + // observation, distinct from Model — left empty for non-Claude runtimes or + // streams that predate the field. + ClaudeCodeVersion string + // ResolvedModel is the model identifier the runtime actually resolved at + // boot (the stream's system/init event's model field), which can be more + // precise than a CI-matrix alias recorded as JourneySpec.Model — e.g. + // "sonnet" resolves to "claude-sonnet-4-6". Left empty when unavailable. + ResolvedModel string } type TokenTotals struct { @@ -99,54 +122,92 @@ type BudgetResult struct { } type Record struct { - SchemaVersion int `json:"schema_version"` - ScenarioID string `json:"scenario_id"` - JourneyID string `json:"journey_id,omitempty"` - Source string `json:"source"` - Mode string `json:"mode,omitempty"` - Runtime string `json:"runtime,omitempty"` - Executor string `json:"executor,omitempty"` - Host string `json:"host"` - Model string `json:"model"` - MetricsState MetricsState `json:"metrics_state"` - Outcome Outcome `json:"outcome"` - DurationMS int64 `json:"duration_ms,omitempty"` - Turns int `json:"turns,omitempty"` - ToolCalls int `json:"tool_calls,omitempty"` - ToolCallsByName map[string]int `json:"tool_calls_by_name,omitempty"` - StatusReadCalls int `json:"status_read_calls,omitempty"` - ScopedReadCalls int `json:"scoped_read_calls,omitempty"` - Tokens TokenTotals `json:"tokens,omitempty"` - TotalCostUSD float64 `json:"total_cost_usd,omitempty"` - ModelUsage map[string]ModelUsage `json:"model_usage,omitempty"` - Budget Budget `json:"budget,omitempty"` - BudgetResult *BudgetResult `json:"budget_result,omitempty"` - CodexCharacter *CodexCharacterization `json:"codex_characterization,omitempty"` + SchemaVersion int `json:"schema_version"` + ScenarioID string `json:"scenario_id"` + JourneyID string `json:"journey_id,omitempty"` + Source string `json:"source"` + Mode string `json:"mode,omitempty"` + Runtime string `json:"runtime,omitempty"` + Executor string `json:"executor,omitempty"` + Host string `json:"host"` + Model string `json:"model"` + MetricsState MetricsState `json:"metrics_state"` + Outcome Outcome `json:"outcome"` + DurationMS int64 `json:"duration_ms,omitempty"` + Turns int `json:"turns,omitempty"` + ToolCalls int `json:"tool_calls,omitempty"` + ToolCallsByName map[string]int `json:"tool_calls_by_name,omitempty"` + StatusReadCalls int `json:"status_read_calls,omitempty"` + ScopedReadCalls int `json:"scoped_read_calls,omitempty"` + Tokens TokenTotals `json:"tokens,omitempty"` + // BaselineTokens is the first turn's full TokenTotals — a reference point + // for how the observation started. Per-turn usage is NOT cumulative across + // turns, so only Tokens.Context() - BaselineTokens.Context() is a + // defensible derived quantity; Input/Output are not independently + // subtractable between two unrelated single requests. Empty for scenarios + // that don't record a pre-observation baseline. + BaselineTokens TokenTotals `json:"baseline_tokens,omitempty"` + // PreGreetPeakCacheCreation is the MAXIMUM cache_creation across the turns + // before the measured turn (e.g. shallow-boot-window's former + // teamRecacheSpikeFloor signal). Zero when there is no pre-observation + // window or the scenario doesn't track it. + PreGreetPeakCacheCreation int `json:"pre_greet_peak_cache_creation,omitempty"` + TotalCostUSD float64 `json:"total_cost_usd,omitempty"` + ModelUsage map[string]ModelUsage `json:"model_usage,omitempty"` + Budget Budget `json:"budget,omitempty"` + BudgetResult *BudgetResult `json:"budget_result,omitempty"` + CodexCharacter *CodexCharacterization `json:"codex_characterization,omitempty"` + // RunID, RunURL, and CapturedAt are run-provenance fields stamped at emission + // time (see EmitRecord's stampProvenance) so a scenario/model that accumulates + // multiple observations in a published ledger can be traced back to the run + // that produced each one, and ordered chronologically. A record emitted outside + // CI (no GITHUB_RUN_ID) simply omits RunID/RunURL. + RunID string `json:"run_id,omitempty"` + RunURL string `json:"run_url,omitempty"` + CapturedAt string `json:"captured_at,omitempty"` + // ClaudeCodeVersion is the Claude Code CLI client version that produced this + // record (the stream's system/init event's claude_code_version field), so a + // future trend reader can attribute a boot-cost shift to a client update + // rather than the FO's own contract. Empty for non-Claude runtimes or + // streams captured before the field existed. + ClaudeCodeVersion string `json:"claude_code_version,omitempty"` + // ResolvedModel is the model identifier the runtime actually resolved at + // boot, distinct from Model when Model is a CI-matrix alias (e.g. "sonnet" + // resolves to "claude-sonnet-4-6") — so a trend reader can tell a silent + // alias-resolution change from a real regression. Empty when unavailable. + ResolvedModel string `json:"resolved_model,omitempty"` } type recordJSON struct { - SchemaVersion int `json:"schema_version"` - ScenarioID string `json:"scenario_id"` - Source string `json:"source"` - Mode string `json:"mode,omitempty"` - Runtime string `json:"runtime,omitempty"` - Executor string `json:"executor,omitempty"` - Host string `json:"host"` - Model string `json:"model"` - MetricsState MetricsState `json:"metrics_state"` - Outcome Outcome `json:"outcome"` - DurationMS int64 `json:"duration_ms,omitempty"` - Turns int `json:"turns,omitempty"` - ToolCalls int `json:"tool_calls,omitempty"` - ToolCallsByName map[string]int `json:"tool_calls_by_name,omitempty"` - StatusReadCalls int `json:"status_read_calls,omitempty"` - ScopedReadCalls int `json:"scoped_read_calls,omitempty"` - Tokens *TokenTotals `json:"tokens,omitempty"` - TotalCostUSD float64 `json:"total_cost_usd,omitempty"` - ModelUsage map[string]ModelUsage `json:"model_usage,omitempty"` - Budget *Budget `json:"budget,omitempty"` - BudgetResult *BudgetResult `json:"budget_result,omitempty"` - CodexCharacter *CodexCharacterization `json:"codex_characterization,omitempty"` + SchemaVersion int `json:"schema_version"` + ScenarioID string `json:"scenario_id"` + Source string `json:"source"` + Mode string `json:"mode,omitempty"` + Runtime string `json:"runtime,omitempty"` + Executor string `json:"executor,omitempty"` + Host string `json:"host"` + Model string `json:"model"` + MetricsState MetricsState `json:"metrics_state"` + Outcome Outcome `json:"outcome"` + DurationMS int64 `json:"duration_ms,omitempty"` + Turns int `json:"turns,omitempty"` + ToolCalls int `json:"tool_calls,omitempty"` + ToolCallsByName map[string]int `json:"tool_calls_by_name,omitempty"` + StatusReadCalls int `json:"status_read_calls,omitempty"` + ScopedReadCalls int `json:"scoped_read_calls,omitempty"` + Tokens *TokenTotals `json:"tokens,omitempty"` + BaselineTokens *TokenTotals `json:"baseline_tokens,omitempty"` + PreGreetPeakCacheCreation int `json:"pre_greet_peak_cache_creation,omitempty"` + TotalCostUSD float64 `json:"total_cost_usd,omitempty"` + ModelUsage map[string]ModelUsage `json:"model_usage,omitempty"` + Budget *Budget `json:"budget,omitempty"` + BudgetResult *BudgetResult `json:"budget_result,omitempty"` + CodexCharacter *CodexCharacterization `json:"codex_characterization,omitempty"` + RunID string `json:"run_id,omitempty"` + RunURL string `json:"run_url,omitempty"` + CapturedAt string `json:"captured_at,omitempty"` + ClaudeCodeVersion string `json:"claude_code_version,omitempty"` + ResolvedModel string `json:"resolved_model,omitempty"` } func (r Record) MarshalJSON() ([]byte, error) { @@ -155,33 +216,45 @@ func (r Record) MarshalJSON() ([]byte, error) { t := r.Tokens.withTotal() tokens = &t } + var baselineTokens *TokenTotals + if !r.BaselineTokens.isZero() { + t := r.BaselineTokens.withTotal() + baselineTokens = &t + } var budget *Budget if r.Budget.MaxTotalTokens != nil || r.Budget.MaxToolCalls != nil { b := r.Budget budget = &b } return marshalRecordJSON(recordJSON{ - SchemaVersion: RecordSchemaVersion, - ScenarioID: firstNonEmpty(r.ScenarioID, r.JourneyID), - Source: r.Source, - Mode: r.Mode, - Runtime: firstNonEmpty(r.Runtime, r.Host), - Executor: r.Executor, - Host: r.Host, - Model: r.Model, - MetricsState: r.MetricsState, - Outcome: r.Outcome, - DurationMS: r.DurationMS, - Turns: r.Turns, - ToolCalls: r.ToolCalls, - ToolCallsByName: r.ToolCallsByName, - StatusReadCalls: r.StatusReadCalls, - ScopedReadCalls: r.ScopedReadCalls, - Tokens: tokens, - TotalCostUSD: r.TotalCostUSD, - ModelUsage: r.ModelUsage, - Budget: budget, - BudgetResult: r.BudgetResult, - CodexCharacter: r.CodexCharacter, + SchemaVersion: RecordSchemaVersion, + ScenarioID: firstNonEmpty(r.ScenarioID, r.JourneyID), + Source: r.Source, + Mode: r.Mode, + Runtime: firstNonEmpty(r.Runtime, r.Host), + Executor: r.Executor, + Host: r.Host, + Model: r.Model, + MetricsState: r.MetricsState, + Outcome: r.Outcome, + DurationMS: r.DurationMS, + Turns: r.Turns, + ToolCalls: r.ToolCalls, + ToolCallsByName: r.ToolCallsByName, + StatusReadCalls: r.StatusReadCalls, + ScopedReadCalls: r.ScopedReadCalls, + Tokens: tokens, + BaselineTokens: baselineTokens, + PreGreetPeakCacheCreation: r.PreGreetPeakCacheCreation, + TotalCostUSD: r.TotalCostUSD, + ModelUsage: r.ModelUsage, + Budget: budget, + BudgetResult: r.BudgetResult, + CodexCharacter: r.CodexCharacter, + RunID: r.RunID, + RunURL: r.RunURL, + CapturedAt: r.CapturedAt, + ClaudeCodeVersion: r.ClaudeCodeVersion, + ResolvedModel: r.ResolvedModel, }) } diff --git a/internal/release/journey_delta_workflow_test.go b/internal/release/journey_delta_workflow_test.go new file mode 100644 index 000000000..80b4e1c36 --- /dev/null +++ b/internal/release/journey_delta_workflow_test.go @@ -0,0 +1,41 @@ +package release + +import ( + "testing" + + "gopkg.in/yaml.v3" +) + +// TestRuntimeLiveWorkflowJourneyDeltaJobHasPRCommentPermission is AC-3's +// workflow-shape guard: the job that posts the per-PR journey-cost delta +// comment must declare its own `permissions: pull-requests: write` (the +// workflow-level default is `contents: read` only, which cannot post a PR +// comment), must run only on `pull_request` (a workflow_dispatch run has no PR +// to comment on), and must actually invoke the journey-delta CLI. +func TestRuntimeLiveWorkflowJourneyDeltaJobHasPRCommentPermission(t *testing.T) { + live := readWorkflow(t, "runtime-live-e2e.yml") + + var doc struct { + Jobs map[string]struct { + If string `yaml:"if"` + Permissions map[string]string `yaml:"permissions"` + } `yaml:"jobs"` + } + if err := yaml.Unmarshal([]byte(live), &doc); err != nil { + t.Fatalf("parse runtime-live-e2e.yml: %v", err) + } + + job, ok := doc.Jobs["journey-delta-comment"] + if !ok { + t.Fatal("runtime-live-e2e.yml has no journey-delta-comment job") + } + if job.Permissions["pull-requests"] != "write" { + t.Fatalf("journey-delta-comment job permissions = %+v, want pull-requests: write", job.Permissions) + } + if job.If != "github.event_name == 'pull_request'" { + t.Fatalf("journey-delta-comment job if = %q, want it scoped to pull_request events", job.If) + } + if !workflowHasExecutableCommandContaining(live, "go run ./cmd/spacedock-release journey-delta") { + t.Fatal("runtime-live-e2e.yml journey-delta-comment job does not invoke the journey-delta CLI") + } +} diff --git a/internal/release/journey_workflow_test.go b/internal/release/journey_workflow_test.go index b9f9d46e4..213711519 100644 --- a/internal/release/journey_workflow_test.go +++ b/internal/release/journey_workflow_test.go @@ -583,6 +583,244 @@ func TestReleaseDownloadSkipBranchToleratesGhError(t *testing.T) { } } +// ghDownloadStepStub is the stubbed `gh` binary for the download step's +// positive multi-run path: `gh release list` returns a since-tag anchor (not +// the epoch fallback), `gh run list` returns two run ids, and `gh run +// download` populates each run's own artifacts directory with a SAME-filename, +// DIFFERENT-content journey-metrics record — the exact collision shape +// journeymetrics.recordFilename produces for two runs of the same +// scenario/model (no run-distinguishing component), so the download step's own +// per-run-subdirectory copy is what has to keep them apart, not an accidental +// filename difference. +const ghDownloadStepStub = `#!/bin/sh +case "$1" in + release) + echo "2026-06-01T00:00:00Z" + ;; + run) + case "$2" in + list) + echo "1001" + echo "1002" + ;; + download) + run_id="$3" + shift 3 + dir="" + while [ $# -gt 0 ]; do + case "$1" in + --dir) dir="$2"; shift 2 ;; + *) shift ;; + esac + done + mkdir -p "$dir/journey-metrics" + printf '{"scenario_id":"shallow-boot-window","run_id":"%s"}' "$run_id" > "$dir/journey-metrics/shallow-boot-window--claude--llm--llm-live--claude-sonnet-4-6--measured.json" + ;; + esac + ;; +esac +exit 0 +` + +// runDownloadStep EXECUTES script (the extracted, possibly mutated, "Download +// latest journey metrics artifacts" run block) against ghDownloadStepStub's two +// stubbed runs and returns the RUNNER_TEMP directory the script populated, so +// callers can inspect the resulting $RUNNER_TEMP/journey-metrics/ layout. +func runDownloadStep(t *testing.T, script string) string { + t.Helper() + dir := t.TempDir() + binDir := filepath.Join(dir, "bin") + if err := os.MkdirAll(binDir, 0o755); err != nil { + t.Fatalf("mkdir bin: %v", err) + } + if err := os.WriteFile(filepath.Join(binDir, "gh"), []byte(ghDownloadStepStub), 0o755); err != nil { + t.Fatalf("write gh stub: %v", err) + } + outPath := filepath.Join(dir, "github_output") + if err := os.WriteFile(outPath, nil, 0o644); err != nil { + t.Fatalf("seed github_output: %v", err) + } + + cmd := exec.Command("bash", "-c", script) + cmd.Dir = dir + cmd.Env = append(os.Environ(), + "PATH="+binDir+string(os.PathListSeparator)+os.Getenv("PATH"), + "RUNNER_TEMP="+dir, + "GITHUB_OUTPUT="+outPath, + "GH_TOKEN=stub", + "GITHUB_REF_NAME=v0.99.0", + ) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("download step exited non-zero with two stubbed runs: %v\n%s", err, out) + } + + gotOutput, err := os.ReadFile(outPath) + if err != nil { + t.Fatalf("read github_output: %v", err) + } + if !strings.Contains(string(gotOutput), "found=true") { + t.Fatalf("download step did not emit found=true with two stubbed runs; got:\n%s", gotOutput) + } + return dir +} + +// TestReleaseDownloadStepProducesPerRunSubdirectories EXERCISES the real +// download script's positive multi-run path (the AC-2 headline mechanism this +// task exists to ship): two stubbed runs, each carrying a journey-metrics +// record with the SAME on-disk filename but different content. It asserts the +// script's OWN per-run-subdirectory copy — not merely its Go-side consumer +// (journeymetrics.AggregateLedger) — keeps both runs' records intact and +// separate, so a positive multi-run cut actually aggregates N observations +// instead of collapsing to one via silent overwrite. +func TestReleaseDownloadStepProducesPerRunSubdirectories(t *testing.T) { + release := readWorkflow(t, "release.yml") + script := extractStepRun(t, release, "Download latest journey metrics artifacts") + + runnerTemp := runDownloadStep(t, script) + + for _, tc := range []struct{ runID, wantContains string }{ + {"1001", `"run_id":"1001"`}, + {"1002", `"run_id":"1002"`}, + } { + path := filepath.Join(runnerTemp, "journey-metrics", tc.runID, "shallow-boot-window--claude--llm--llm-live--claude-sonnet-4-6--measured.json") + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("run %s's record missing from its own subdirectory: %v", tc.runID, err) + } + if !strings.Contains(string(data), tc.wantContains) { + t.Fatalf("run %s's record does not carry its own run id (overwritten by another run's content) — %s:\n%s", tc.runID, path, data) + } + } +} + +// TestReleaseDownloadStepGuardRejectsFlatCopyRegression is the adversarial +// twin: reverting the per-run cp target back to the flat "$RUNNER_TEMP/journey- +// metrics/" directory (the exact bug AC-2 fixes, and the mutation the +// validation-stage adversarial audit applied by hand) must collapse the two +// stubbed runs' same-named records into ONE file. This proves +// TestReleaseDownloadStepProducesPerRunSubdirectories actually discriminates a +// fixed script from a broken one, rather than passing vacuously. +func TestReleaseDownloadStepGuardRejectsFlatCopyRegression(t *testing.T) { + release := readWorkflow(t, "release.yml") + const fixedCopy = `find "$RUNNER_TEMP/runtime-live-artifacts/$run_id" -path '*/journey-metrics/*.json' -type f -exec cp {} "$run_dir/" \;` + const flatCopy = `find "$RUNNER_TEMP/runtime-live-artifacts/$run_id" -path '*/journey-metrics/*.json' -type f -exec cp {} "$RUNNER_TEMP/journey-metrics/" \;` + adversarial := strings.Replace(release, fixedCopy, flatCopy, 1) + if adversarial == release { + t.Fatal("fixture workflow missing the per-run cp target to mutate") + } + script := extractStepRun(t, adversarial, "Download latest journey metrics artifacts") + + runnerTemp := runDownloadStep(t, script) + + matches, err := filepath.Glob(filepath.Join(runnerTemp, "journey-metrics", "*.json")) + if err != nil { + t.Fatal(err) + } + if len(matches) != 1 { + t.Fatalf("flat-copy regression should collapse both runs' same-named records into exactly one flat file, got %d: %v", len(matches), matches) + } + for _, runID := range []string{"1001", "1002"} { + subdirMatches, err := filepath.Glob(filepath.Join(runnerTemp, "journey-metrics", runID, "*.json")) + if err != nil { + t.Fatal(err) + } + if len(subdirMatches) != 0 { + t.Fatalf("run %s's subdirectory unexpectedly has records under the flat-copy regression: %v", runID, subdirMatches) + } + } +} + +// TestJourneyDeltaLocateMetricsStepFindsNestedJourneyMetricsFiles is the AC-3 +// production-bug proof: runtime-live-e2e.yml's journey-delta-comment job used to +// hardcode --metrics-dir to an exact subpath under the download-artifact +// destination, but a real run's downloaded artifact zip nests the journey-metrics +// JSON several directories deeper (verified against run 28432388663) — the exact +// hardcoded path is EMPTY, so journeymetrics.ReadRecordsDir errored and REDed +// every PR's delta-comment job under set -euo pipefail. This exercises the REAL +// "Locate this run's journey metrics" step (extracted from the live workflow, +// not a reimplementation) against that realistic nested layout and proves it +// finds the file regardless of the exact nesting depth. +func TestJourneyDeltaLocateMetricsStepFindsNestedJourneyMetricsFiles(t *testing.T) { + live := readWorkflow(t, "runtime-live-e2e.yml") + script := extractStepRun(t, live, "Locate this run's journey metrics") + + dir := t.TempDir() + // The verified real nesting: several directories deeper than the + // "live-artifacts/journey-metrics/" subpath the removed hardcoded + // --metrics-dir assumed. + nested := filepath.Join(dir, "current-run-artifacts", "spacedock", "spacedock", "live-artifacts", "journey-metrics", "claude", "claude-opus-4-8") + if err := os.MkdirAll(nested, 0o755); err != nil { + t.Fatal(err) + } + metricsFile := filepath.Join(nested, "shallow-boot--claude--llm--llm-live--claude-opus-4-8--measured.json") + if err := os.WriteFile(metricsFile, []byte(`{"scenario_id":"shallow-boot"}`), 0o644); err != nil { + t.Fatal(err) + } + + // Prove the bug was real: the OLD hardcoded path finds nothing in this + // realistic layout. + oldHardcodedPath := filepath.Join(dir, "current-run-artifacts", "live-artifacts", "journey-metrics") + if _, err := os.Stat(oldHardcodedPath); err == nil { + t.Fatalf("test fixture is unrealistic: the OLD hardcoded path %s must NOT exist", oldHardcodedPath) + } + + cmd := exec.Command("bash", "-c", script) + cmd.Dir = dir + cmd.Env = append(os.Environ(), "RUNNER_TEMP="+dir) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("locate-metrics step exited non-zero: %v\n%s", err, out) + } + + matches, err := filepath.Glob(filepath.Join(dir, "current-run-metrics", "*.json")) + if err != nil { + t.Fatal(err) + } + if len(matches) != 1 { + t.Fatalf("expected the nested journey-metrics JSON to be located and copied into current-run-metrics, got %d matches: %v", len(matches), matches) + } +} + +// TestJourneyDeltaLocateAndPostStepsShareMetricsDir closes the cross-step +// wiring gap validation's cycle-3 adversarial audit found: the "Locate this +// run's journey metrics" step's own mechanism is tested in isolation above, +// but nothing previously verified that the "Post the journey-cost delta PR +// comment" step's --metrics-dir argument stays wired to the Locate step's +// actual output directory. A one-line regression reverting ONLY the Post +// step's --metrics-dir back to the old hardcoded broken path left the full +// suite green, because each step's script was only ever checked in isolation. +// This derives BOTH directories from the REAL extracted step scripts (not a +// hardcoded assumption on either side) and asserts they're the same value. +func TestJourneyDeltaLocateAndPostStepsShareMetricsDir(t *testing.T) { + live := readWorkflow(t, "runtime-live-e2e.yml") + locateScript := extractStepRun(t, live, "Locate this run's journey metrics") + postScript := extractStepRun(t, live, "Post the journey-cost delta PR comment") + + locateDir := firstQuotedArg(t, locateScript, `mkdir -p "`) + postDir := firstQuotedArg(t, postScript, `--metrics-dir "`) + if locateDir != postDir { + t.Fatalf("Locate step writes to %q but Post step's --metrics-dir reads from %q — the two steps are no longer wired together", locateDir, postDir) + } +} + +// firstQuotedArg extracts the double-quoted value immediately following the +// given prefix (e.g. `mkdir -p "`) in script, failing the test if the prefix +// or its closing quote is not found. +func firstQuotedArg(t *testing.T, script, prefix string) string { + t.Helper() + start := strings.Index(script, prefix) + if start < 0 { + t.Fatalf("script does not contain %q:\n%s", prefix, script) + } + start += len(prefix) + end := strings.Index(script[start:], `"`) + if end < 0 { + t.Fatalf("unterminated quoted argument after %q:\n%s", prefix, script) + } + return script[start : start+end] +} + // extractStepRun pulls a named step's `run: |` block out of a workflow document, // dedented to a runnable shell script, so tests exercise the EXACT script CI // runs rather than a hand-copied duplicate. diff --git a/internal/release/journeydelta.go b/internal/release/journeydelta.go new file mode 100644 index 000000000..850e2f9d2 --- /dev/null +++ b/internal/release/journeydelta.go @@ -0,0 +1,173 @@ +// ABOUTME: Per-PR journey-cost delta — selects the latest-by-captured_at baseline +// ABOUTME: observation per scenario/model from a published ledger and diffs it against a PR run. +package release + +import ( + "fmt" + "sort" + "strings" + "time" + + "github.com/spacedock-dev/spacedock/internal/journeymetrics" +) + +// journeyDeltaKey identifies one scenario/runtime/model line the PR delta +// comment reports. +type journeyDeltaKey struct { + ScenarioID string + Runtime string + Model string +} + +// LatestObservations selects, for every distinct scenario/runtime/model in a +// published ledger, the single observation with the LATEST captured_at +// timestamp — the AC-3 delta baseline. A published ledger holds N observations +// per scenario/model once AC-2's widened aggregation ships, so "the baseline" +// is ambiguous without this reduction; latest-by-captured_at (not mean, not +// max, not array position) is the one this entity pins. An observation with an +// empty or unparseable captured_at sorts as the oldest possible time, so any +// genuinely-timestamped peer always wins over it. +func LatestObservations(ledger journeymetrics.Ledger) map[string]journeymetrics.Record { + latest := map[string]journeymetrics.Record{} + latestTime := map[string]time.Time{} + for _, scenario := range ledger.Scenarios { + for _, obs := range scenario.Observations { + key := deltaKeyString(journeyDeltaKey{ScenarioID: obs.ScenarioID, Runtime: obs.Runtime, Model: obs.Model}) + t := parseCapturedAt(obs.CapturedAt) + if cur, ok := latestTime[key]; !ok || t.After(cur) { + latest[key] = obs + latestTime[key] = t + } + } + } + return latest +} + +func parseCapturedAt(s string) time.Time { + t, err := time.Parse(time.RFC3339, s) + if err != nil { + return time.Time{} + } + return t +} + +func deltaKeyString(k journeyDeltaKey) string { + return strings.Join([]string{k.ScenarioID, k.Runtime, k.Model}, "\x00") +} + +// JourneyDelta is one scenario/runtime/model line of the AC-3 PR comment: a +// PR run's fresh observation against the previously published release's +// latest-by-captured_at baseline for the same scenario/runtime/model. +type JourneyDelta struct { + ScenarioID string + Runtime string + Model string + HasBaseline bool + BaselineRunURL string + TurnsDelta int + DurationMSDelta int64 + TokensDelta journeymetrics.TokenTotals + CostDeltaUSD float64 +} + +// ComputeJourneyDeltas computes, for every scenario/runtime/model the current +// PR run produced, the delta against the baseline ledger's latest-by- +// captured_at observation for that same scenario/runtime/model (see +// LatestObservations): delta = PR value - baseline value, exactly, across +// turns, the full token breakdown, and cost — whatever the ledger already +// tracks generically, not a boot-window special case. A scenario/model with no +// matching baseline observation is still reported (HasBaseline=false, delta +// equal to the PR's own value) rather than silently dropped, so a brand-new +// scenario doesn't vanish from the comment. +func ComputeJourneyDeltas(baseline journeymetrics.Ledger, current []journeymetrics.Record) []JourneyDelta { + latest := LatestObservations(baseline) + deltas := make([]JourneyDelta, 0, len(current)) + for _, cur := range current { + key := deltaKeyString(journeyDeltaKey{ScenarioID: cur.ScenarioID, Runtime: cur.Runtime, Model: cur.Model}) + base, ok := latest[key] + d := JourneyDelta{ + ScenarioID: cur.ScenarioID, + Runtime: cur.Runtime, + Model: cur.Model, + HasBaseline: ok, + } + if ok { + d.BaselineRunURL = base.RunURL + } + d.TurnsDelta = cur.Turns - base.Turns + d.DurationMSDelta = cur.DurationMS - base.DurationMS + d.TokensDelta = journeymetrics.TokenTotals{ + Input: cur.Tokens.Input - base.Tokens.Input, + Output: cur.Tokens.Output - base.Tokens.Output, + CacheCreation: cur.Tokens.CacheCreation - base.Tokens.CacheCreation, + CacheRead: cur.Tokens.CacheRead - base.Tokens.CacheRead, + Total: cur.Tokens.Total - base.Tokens.Total, + } + d.CostDeltaUSD = cur.TotalCostUSD - base.TotalCostUSD + deltas = append(deltas, d) + } + sort.Slice(deltas, func(i, j int) bool { + if deltas[i].ScenarioID != deltas[j].ScenarioID { + return deltas[i].ScenarioID < deltas[j].ScenarioID + } + if deltas[i].Runtime != deltas[j].Runtime { + return deltas[i].Runtime < deltas[j].Runtime + } + return deltas[i].Model < deltas[j].Model + }) + return deltas +} + +// JourneyDeltaCommentMarker is stamped into every rendered comment. A caller +// finds its own prior comment by searching the PR's comments for one whose +// body starts with this marker (JourneyDeltaUpdateCommentArgs), rather than by +// editing "the poster's last comment" (`--edit-last`) — --edit-last targets the +// wrong comment if any OTHER automated comment from the same bot account lands +// on the PR in between. +const JourneyDeltaCommentMarker = "" + +// RenderJourneyDeltaComment renders the AC-3 PR comment body: one table row per +// scenario/runtime/model. A row with a baseline shows the exact (PR value - +// baseline value) delta for turns, each token class, and cost; a row with NO +// baseline (a brand-new scenario/model this ledger has never seen) renders +// "n/a (new)" in every delta cell instead of a self-delta against an implicit +// zero baseline, which would otherwise read as a huge, meaningless "increase." +func RenderJourneyDeltaComment(deltas []JourneyDelta) string { + var b strings.Builder + b.WriteString(JourneyDeltaCommentMarker) + b.WriteString("\n### Journey cost delta\n\n") + if len(deltas) == 0 { + b.WriteString("No journey metrics observations were produced by this run.\n") + return b.String() + } + b.WriteString("| Scenario | Runtime | Model | Turns Δ | Cache Read Δ | Cache Creation Δ | Tokens Δ (total) | Cost Δ (USD) | Duration Δ (ms) | Baseline |\n") + b.WriteString("| --- | --- | --- | --- | --- | --- | --- | --- | --- | --- |\n") + for _, d := range deltas { + if !d.HasBaseline { + fmt.Fprintf(&b, "| %s | %s | %s | n/a (new) | n/a (new) | n/a (new) | n/a (new) | n/a (new) | n/a (new) | none (new observation) |\n", + d.ScenarioID, d.Runtime, d.Model) + continue + } + baseline := "latest published" + if d.BaselineRunURL != "" { + baseline = fmt.Sprintf("[latest published](%s)", d.BaselineRunURL) + } + fmt.Fprintf(&b, "| %s | %s | %s | %+d | %+d | %+d | %+d | %+.4f | %+d | %s |\n", + d.ScenarioID, d.Runtime, d.Model, d.TurnsDelta, d.TokensDelta.CacheRead, d.TokensDelta.CacheCreation, d.TokensDelta.Total, d.CostDeltaUSD, d.DurationMSDelta, baseline) + } + return b.String() +} + +// JourneyDeltaCreateCommentArgs returns the `gh pr comment` argv that posts a +// NEW comment — used when no prior journey-delta comment exists on the PR yet. +func JourneyDeltaCreateCommentArgs(prNumber, bodyFile string) []string { + return []string{"pr", "comment", prNumber, "--body-file", bodyFile} +} + +// JourneyDeltaUpdateCommentArgs returns the `gh api` argv that PATCHes the +// EXACT existing comment by id — the find-by-marker replacement for +// `--edit-last` (which targets "the poster's last comment on the PR," not +// necessarily this job's own prior comment). +func JourneyDeltaUpdateCommentArgs(commentID, bodyFile string) []string { + return []string{"api", "repos/{owner}/{repo}/issues/comments/" + commentID, "-X", "PATCH", "-f", "body=@" + bodyFile} +} diff --git a/internal/release/journeydelta_test.go b/internal/release/journeydelta_test.go new file mode 100644 index 000000000..c7b0d942b --- /dev/null +++ b/internal/release/journeydelta_test.go @@ -0,0 +1,321 @@ +package release + +import ( + "strings" + "testing" + + "github.com/spacedock-dev/spacedock/internal/journeymetrics" +) + +// TestLatestObservationsSelectsByCapturedAtNotArrayPosition is AC-3's proof that +// baseline selection is driven by the captured_at timestamp, not by "last in the +// array": the observations are deliberately given OUT-OF-ORDER captured_at +// values (the chronologically latest one is authored FIRST in the slice), so a +// naive "last element wins" implementation would pick the wrong observation. +func TestLatestObservationsSelectsByCapturedAtNotArrayPosition(t *testing.T) { + older := journeymetrics.Record{ + ScenarioID: "shallow-boot-window", Runtime: "claude", Model: "claude-sonnet-4-6", + Turns: 18, Tokens: journeymetrics.TokenTotals{CacheCreation: 1562, Total: 1562}, + CapturedAt: "2026-06-20T00:00:00Z", RunID: "27931963802", + RunURL: "https://github.com/spacedock-dev/spacedock/actions/runs/27931963802", + } + newer := journeymetrics.Record{ + ScenarioID: "shallow-boot-window", Runtime: "claude", Model: "claude-sonnet-4-6", + Turns: 20, Tokens: journeymetrics.TokenTotals{CacheCreation: 1576, Total: 1576}, + CapturedAt: "2026-06-27T00:00:00Z", RunID: "28432388663", + RunURL: "https://github.com/spacedock-dev/spacedock/actions/runs/28432388663", + } + ledger := journeymetrics.Ledger{ + Scenarios: []journeymetrics.ScenarioLedgerEntry{ + { + ScenarioID: "shallow-boot-window", + // newer is authored FIRST, older SECOND — the reverse of + // chronological order — so an array-position-based ("last wins") + // selection would wrongly pick `older`. + Observations: []journeymetrics.Record{newer, older}, + }, + }, + } + + latest := LatestObservations(ledger) + got, ok := latest[deltaKeyString(journeyDeltaKey{ScenarioID: "shallow-boot-window", Runtime: "claude", Model: "claude-sonnet-4-6"})] + if !ok { + t.Fatal("no baseline observation selected for shallow-boot-window/claude/claude-sonnet-4-6") + } + if got.RunID != "28432388663" { + t.Fatalf("selected baseline RunID = %q, want the chronologically-latest 28432388663 (got Turns=%d, CapturedAt=%s) — selection is not timestamp-based", got.RunID, got.Turns, got.CapturedAt) + } +} + +// TestLatestObservationsTreatsUnparseableCapturedAtAsOldest proves a record with +// no (or malformed) captured_at never wins over a genuinely-timestamped peer — +// it sorts as the oldest possible time rather than erroring or comparing equal. +func TestLatestObservationsTreatsUnparseableCapturedAtAsOldest(t *testing.T) { + noTimestamp := journeymetrics.Record{ + ScenarioID: "gate-guardrail", Runtime: "claude", Model: "claude-sonnet-4-6", Turns: 1, + } + timestamped := journeymetrics.Record{ + ScenarioID: "gate-guardrail", Runtime: "claude", Model: "claude-sonnet-4-6", Turns: 2, + CapturedAt: "2026-01-01T00:00:00Z", + } + ledger := journeymetrics.Ledger{ + Scenarios: []journeymetrics.ScenarioLedgerEntry{ + {ScenarioID: "gate-guardrail", Observations: []journeymetrics.Record{noTimestamp, timestamped}}, + }, + } + latest := LatestObservations(ledger) + got := latest[deltaKeyString(journeyDeltaKey{ScenarioID: "gate-guardrail", Runtime: "claude", Model: "claude-sonnet-4-6"})] + if got.Turns != 2 { + t.Fatalf("Turns = %d, want 2 (the timestamped observation must win over the unparseable one)", got.Turns) + } +} + +// TestComputeJourneyDeltasExactArithmetic is AC-3's primary proof: the delta for +// each scenario/model equals (PR value - the ledger observation with the latest +// captured_at for that scenario/model) EXACTLY, across turns, the full token +// breakdown, and cost. +func TestComputeJourneyDeltasExactArithmetic(t *testing.T) { + baseline := journeymetrics.Ledger{ + Scenarios: []journeymetrics.ScenarioLedgerEntry{ + { + ScenarioID: "shallow-boot-window", + Observations: []journeymetrics.Record{ + { + ScenarioID: "shallow-boot-window", Runtime: "claude", Model: "claude-sonnet-4-6", + Turns: 18, Tokens: journeymetrics.TokenTotals{Input: 100, Output: 50, CacheCreation: 1562, CacheRead: 20000, Total: 21712}, + TotalCostUSD: 1.2500, CapturedAt: "2026-06-20T00:00:00Z", + RunURL: "https://github.com/spacedock-dev/spacedock/actions/runs/27931963802", + }, + // An OLDER observation for the same scenario/model — must be + // ignored in favor of the one above. + { + ScenarioID: "shallow-boot-window", Runtime: "claude", Model: "claude-sonnet-4-6", + Turns: 5, Tokens: journeymetrics.TokenTotals{Total: 500}, + TotalCostUSD: 0.05, CapturedAt: "2026-05-01T00:00:00Z", + }, + }, + }, + }, + } + current := []journeymetrics.Record{ + { + ScenarioID: "shallow-boot-window", Runtime: "claude", Model: "claude-sonnet-4-6", + Turns: 20, Tokens: journeymetrics.TokenTotals{Input: 110, Output: 60, CacheCreation: 1576, CacheRead: 21000, Total: 22746}, + TotalCostUSD: 1.4000, + }, + } + + deltas := ComputeJourneyDeltas(baseline, current) + if len(deltas) != 1 { + t.Fatalf("deltas = %d, want 1", len(deltas)) + } + d := deltas[0] + if !d.HasBaseline { + t.Fatal("expected a matching baseline observation") + } + if d.TurnsDelta != 2 { + t.Fatalf("TurnsDelta = %d, want 2 (20-18)", d.TurnsDelta) + } + wantTokens := journeymetrics.TokenTotals{Input: 10, Output: 10, CacheCreation: 14, CacheRead: 1000, Total: 1034} + if d.TokensDelta != wantTokens { + t.Fatalf("TokensDelta = %+v, want %+v", d.TokensDelta, wantTokens) + } + if diff := d.CostDeltaUSD - 0.15; diff > 1e-9 || diff < -1e-9 { + t.Fatalf("CostDeltaUSD = %v, want 0.15 (1.40-1.25)", d.CostDeltaUSD) + } + if d.BaselineRunURL != "https://github.com/spacedock-dev/spacedock/actions/runs/27931963802" { + t.Fatalf("BaselineRunURL = %q, want the latest-by-captured_at observation's run_url", d.BaselineRunURL) + } +} + +// TestComputeJourneyDeltasReportsScenarioWithNoBaseline proves a scenario/model +// the PR run produced but the baseline ledger never saw is still reported +// (against a zero-value baseline) rather than silently dropped. +func TestComputeJourneyDeltasReportsScenarioWithNoBaseline(t *testing.T) { + baseline := journeymetrics.Ledger{} + current := []journeymetrics.Record{ + {ScenarioID: "shallow-boot-window", Runtime: "claude", Model: "claude-sonnet-4-6", Turns: 20, Tokens: journeymetrics.TokenTotals{Total: 100}}, + } + deltas := ComputeJourneyDeltas(baseline, current) + if len(deltas) != 1 { + t.Fatalf("deltas = %d, want 1", len(deltas)) + } + if deltas[0].HasBaseline { + t.Fatal("expected HasBaseline=false for a scenario/model absent from the baseline ledger") + } + if deltas[0].TurnsDelta != 20 { + t.Fatalf("TurnsDelta = %d, want 20 (delta against a zero baseline equals the PR's own value)", deltas[0].TurnsDelta) + } +} + +// TestRenderJourneyDeltaCommentIncludesExactDeltasAndMarker proves the rendered +// comment BODY (not just the underlying struct) carries the marker used to +// identify the comment's origin, and the exact computed delta values. +func TestRenderJourneyDeltaCommentIncludesExactDeltasAndMarker(t *testing.T) { + deltas := []JourneyDelta{ + { + ScenarioID: "shallow-boot-window", Runtime: "claude", Model: "claude-sonnet-4-6", + HasBaseline: true, BaselineRunURL: "https://github.com/spacedock-dev/spacedock/actions/runs/27931963802", + TurnsDelta: 2, TokensDelta: journeymetrics.TokenTotals{Total: 1034}, CostDeltaUSD: 0.15, + }, + } + body := RenderJourneyDeltaComment(deltas) + if !strings.HasPrefix(body, JourneyDeltaCommentMarker) { + t.Fatalf("comment body does not start with the sticky marker:\n%s", body) + } + for _, want := range []string{"shallow-boot-window", "+2", "+1034", "+0.1500", "27931963802"} { + if !strings.Contains(body, want) { + t.Fatalf("comment body missing %q:\n%s", want, body) + } + } +} + +// TestRenderJourneyDeltaCommentShowsTokenClassBreakdown proves the comment +// renders Cache Read Δ and Cache Creation Δ as their OWN columns, in the +// correct (non-swapped) position — not collapsed into the Tokens Δ (total) +// figure. cache_read and cache_creation differ ~12x in cost and meaning for a +// boot metric, so folding them into one number hides the signal this Minor +// exists to surface. Uses distinguishable, easily-confused-if-swapped values +// (CacheRead=111, CacheCreation=222) and checks the actual table row's cell +// positions, not just substring presence anywhere in the body (which would +// not catch a column swap). +func TestRenderJourneyDeltaCommentShowsTokenClassBreakdown(t *testing.T) { + deltas := []JourneyDelta{ + { + ScenarioID: "shallow-boot-window", Runtime: "claude", Model: "claude-sonnet-4-6", + HasBaseline: true, BaselineRunURL: "https://github.com/spacedock-dev/spacedock/actions/runs/27931963802", + TurnsDelta: 2, TokensDelta: journeymetrics.TokenTotals{CacheRead: 111, CacheCreation: 222, Total: 1034}, CostDeltaUSD: 0.15, + }, + } + body := RenderJourneyDeltaComment(deltas) + + var row string + for _, line := range strings.Split(body, "\n") { + if strings.HasPrefix(line, "| shallow-boot-window") { + row = line + break + } + } + if row == "" { + t.Fatalf("comment body missing the data row:\n%s", body) + } + cells := strings.Split(row, "|") + // | (empty) | Scenario | Runtime | Model | Turns Δ | Cache Read Δ | Cache Creation Δ | Tokens Δ (total) | Cost Δ (USD) | Duration Δ (ms) | Baseline | (empty) | + if len(cells) != 12 { + t.Fatalf("data row has %d cells, want 12 (header shape changed?): %q", len(cells), row) + } + cacheReadCell := strings.TrimSpace(cells[5]) + cacheCreationCell := strings.TrimSpace(cells[6]) + tokensTotalCell := strings.TrimSpace(cells[7]) + if cacheReadCell != "+111" { + t.Fatalf("Cache Read Δ cell = %q, want +111 (got %q for Cache Creation Δ) — columns may be swapped", cacheReadCell, cacheCreationCell) + } + if cacheCreationCell != "+222" { + t.Fatalf("Cache Creation Δ cell = %q, want +222 (got %q for Cache Read Δ) — columns may be swapped", cacheCreationCell, cacheReadCell) + } + if tokensTotalCell != "+1034" { + t.Fatalf("Tokens Δ (total) cell = %q, want +1034", tokensTotalCell) + } +} + +// TestJourneyDeltaCommentShowsDurationDelta is the TDD proof for the +// wallclock/duration delta: computes deltas from a baseline ledger and a +// current PR run with distinct DurationMS values, then asserts both the +// computed struct field and the rendered comment's Duration Δ column show the +// exact millisecond delta (current - baseline), not a total or a placeholder. +func TestJourneyDeltaCommentShowsDurationDelta(t *testing.T) { + baseline := journeymetrics.Ledger{ + Scenarios: []journeymetrics.ScenarioLedgerEntry{ + { + ScenarioID: "shallow-boot-window", + Observations: []journeymetrics.Record{ + { + ScenarioID: "shallow-boot-window", Runtime: "claude", Model: "claude-sonnet-4-6", + DurationMS: 12000, CapturedAt: "2026-06-20T00:00:00Z", + RunURL: "https://github.com/spacedock-dev/spacedock/actions/runs/27931963802", + }, + }, + }, + }, + } + current := []journeymetrics.Record{ + { + ScenarioID: "shallow-boot-window", Runtime: "claude", Model: "claude-sonnet-4-6", + DurationMS: 15500, + }, + } + + deltas := ComputeJourneyDeltas(baseline, current) + if len(deltas) != 1 { + t.Fatalf("deltas = %d, want 1", len(deltas)) + } + if deltas[0].DurationMSDelta != 3500 { + t.Fatalf("DurationMSDelta = %d, want 3500 (15500-12000)", deltas[0].DurationMSDelta) + } + + body := RenderJourneyDeltaComment(deltas) + var row string + for _, line := range strings.Split(body, "\n") { + if strings.HasPrefix(line, "| shallow-boot-window") { + row = line + break + } + } + if row == "" { + t.Fatalf("comment body missing the data row:\n%s", body) + } + if !strings.Contains(row, "+3500") { + t.Fatalf("data row missing Duration Δ +3500(ms): %q", row) + } +} + +// TestRenderJourneyDeltaCommentRendersNoBaselineAsNewNotSelfDelta proves a +// scenario/model with no matching baseline observation renders "n/a (new)" in +// its delta cells rather than a self-delta against an implicit zero baseline +// (which would otherwise print the observation's own full value as if it were +// a huge, meaningless increase). +func TestRenderJourneyDeltaCommentRendersNoBaselineAsNewNotSelfDelta(t *testing.T) { + deltas := []JourneyDelta{ + { + ScenarioID: "brand-new-scenario", Runtime: "claude", Model: "claude-sonnet-4-6", + HasBaseline: false, TurnsDelta: 20, TokensDelta: journeymetrics.TokenTotals{Total: 5000}, CostDeltaUSD: 2.5, + }, + } + body := RenderJourneyDeltaComment(deltas) + if !strings.Contains(body, "brand-new-scenario") { + t.Fatalf("comment body missing the scenario row:\n%s", body) + } + if strings.Contains(body, "+20") || strings.Contains(body, "+5000") || strings.Contains(body, "+2.5000") { + t.Fatalf("comment body rendered a self-delta for a no-baseline row instead of n/a (new):\n%s", body) + } + if !strings.Contains(body, "n/a (new)") { + t.Fatalf("comment body missing n/a (new) for the no-baseline row:\n%s", body) + } +} + +// TestJourneyDeltaCreateCommentArgs proves the gh argv used to post a brand new +// comment (no prior journey-delta comment found on the PR). +func TestJourneyDeltaCreateCommentArgs(t *testing.T) { + args := JourneyDeltaCreateCommentArgs("42", "/tmp/comment.md") + joined := strings.Join(args, " ") + for _, want := range []string{"pr comment", "42", "--body-file /tmp/comment.md"} { + if !strings.Contains(joined, want) { + t.Fatalf("gh argv %v missing %q", args, want) + } + } + if strings.Contains(joined, "--edit-last") { + t.Fatalf("gh argv %v must not carry --edit-last (marker-based lookup already decided this is a fresh comment)", args) + } +} + +// TestJourneyDeltaUpdateCommentArgs proves the gh argv used to PATCH the exact +// existing comment found by marker — the --edit-last replacement. +func TestJourneyDeltaUpdateCommentArgs(t *testing.T) { + args := JourneyDeltaUpdateCommentArgs("987654321", "/tmp/comment.md") + joined := strings.Join(args, " ") + for _, want := range []string{"api", "issues/comments/987654321", "-X PATCH", "body=@/tmp/comment.md"} { + if !strings.Contains(joined, want) { + t.Fatalf("gh argv %v missing %q", args, want) + } + } +}