From c903ba1b515104cf0f89abb10162c0274bc3b579 Mon Sep 17 00:00:00 2001 From: Spike Test Date: Thu, 2 Jul 2026 20:33:19 +0800 Subject: [PATCH 1/7] AC-1/AC-2: record shallow-boot-window telemetry, add ledger run provenance Removes the two absolute 60k threshold branches from assertShallowBootMeasuredTurns (calibrated once against a since-superseded baseline) and records the greet turn's full token usage as a distinct shallow-boot-window journeymetrics observation instead, so boot cost rides a trend instead of a magic-number gate. Adds RunID/RunURL/CapturedAt provenance fields to journeymetrics.Record, stamped at EmitRecord time, and fixes release.yml's journey-ledger job to download each discovered Runtime Live E2E run into its own subdirectory (instead of flat-copying, which silently collapsed multiple runs' same-named records into one observation) and to widen discovery to every successful run since the previous release tag of any kind. --- .github/workflows/release.yml | 54 +++++-- cmd/spacedock-release/journey_costs_test.go | 134 ++++++++++++++++++ .../ensigncycle/claude_live_runner_test.go | 9 +- .../ensigncycle/journey_metrics_live_test.go | 23 +++ .../ensigncycle/shallow_boot_measure_test.go | 84 ++++++----- .../shallow_boot_measure_unit_test.go | 133 ++++++++++++++--- internal/journeymetrics/record.go | 27 ++++ internal/journeymetrics/types.go | 14 ++ 8 files changed, 404 insertions(+), 74 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 289d1da58..14018ab11 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/cmd/spacedock-release/journey_costs_test.go b/cmd/spacedock-release/journey_costs_test.go index 4328b9ba0..2f5f0d97e 100644 --- a/cmd/spacedock-release/journey_costs_test.go +++ b/cmd/spacedock-release/journey_costs_test.go @@ -101,12 +101,146 @@ func TestJourneyCostsCommandRejectsMismatchedOutputFilename(t *testing.T) { } } +// TestJourneyCostsCommandFlatCopyCollapsesToOneObservation is the AC-2 regression +// test pinning the OLD release.yml behavior: downloading N runs' journey-metrics +// and flat-copying every file into ONE directory. journeymetrics.recordFilename has +// no run-distinguishing component, so two runs of the SAME scenario/model produce +// the SAME on-disk filename in that one directory, and the second cp silently +// overwrites the first — the aggregation collapses back to exactly one observation +// regardless of how many runs were actually discovered. This test proves the old +// shape's collapse is real (not a hypothetical), so the per-run-subdirectory test +// below is provably what closes the gap, not an incidental side effect. +func TestJourneyCostsCommandFlatCopyCollapsesToOneObservation(t *testing.T) { + metricsDir := t.TempDir() // one flat directory — the OLD release.yml `cp` target + 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, runA.CapturedAt = "27931963802", "2026-06-20T00:00:00Z" + runB := base + runB.Turns, runB.Tokens = 20, journeymetrics.TokenTotals{CacheCreation: 1576} + runB.RunID, runB.CapturedAt = "28432388663", "2026-06-27T00:00:00Z" + + // Both records share ScenarioID/Runtime/Model, so writeMetricRecord's filename + // (mirroring journeymetrics.recordFilename's lack of a run-distinguishing + // component) collides — runB's write overwrites runA's file in place, exactly + // as the flat `cp` step would. + writeMetricRecord(t, metricsDir, runA) + writeMetricRecord(t, metricsDir, 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) != 1 { + t.Fatalf("flat-copy shape must still collapse to 1 observation for shallow-boot-window, got %d: %+v", len(entry.Observations), entry.Observations) + } +} + +// 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/internal/ensigncycle/claude_live_runner_test.go b/internal/ensigncycle/claude_live_runner_test.go index 5c4ba79e2..427385667 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..ff069c8df 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) + 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..a9948cfd3 100644 --- a/internal/ensigncycle/shallow_boot_measure_test.go +++ b/internal/ensigncycle/shallow_boot_measure_test.go @@ -7,6 +7,14 @@ import ( "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" + // deferredFOSkillNames are the first-officer-internal skills a greet-and-stop boot // must NOT invoke before the greet: the status-viewer surface, the write/id-style // surface, and the dispatch-failure-recovery surface. Each loads only at its trigger @@ -17,16 +25,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. @@ -131,19 +129,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 +145,45 @@ 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) + return nil +} + +// 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 both the former +// ceiling signal (Context() = input+cache_read+cache_creation) and the former +// pre-greet-spike signal (cache_creation) from this one recorded observation. +func buildShallowBootWindowRecord(turns []journeymetrics.ClaudeTurn, model string) (journeymetrics.Record, error) { + if len(turns) == 0 { + return journeymetrics.Record{}, fmt.Errorf("stream carried no assistant turns — nothing to record") } - 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) - } + greet := greetTurnIndex(turns) + if greet < 0 { + return journeymetrics.Record{}, fmt.Errorf("every assistant turn dispatched — no greet turn produced") } - return nil + 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, + }), nil } diff --git a/internal/ensigncycle/shallow_boot_measure_unit_test.go b/internal/ensigncycle/shallow_boot_measure_unit_test.go index 6a8cb3a73..e8f728b9f 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,113 @@ 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. +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") + } + + const model = "claude-test-model" + record, err := buildShallowBootWindowRecord(turns, model) + 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) + } + + // 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.Fatalf("a greet turn over the former ceiling must no longer fail now that the threshold gate is removed: %v", err) } - 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 record, err := buildShallowBootWindowRecord(heavyGreet, "test-model"); 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) } - // 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) } // Both clean: a light greet, no pre-greet spike — the realized-saving end-state. diff --git a/internal/journeymetrics/record.go b/internal/journeymetrics/record.go index 0141a219b..a6f8847f4 100644 --- a/internal/journeymetrics/record.go +++ b/internal/journeymetrics/record.go @@ -69,6 +69,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 +84,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 { diff --git a/internal/journeymetrics/types.go b/internal/journeymetrics/types.go index 6fe65c3d1..04563a616 100644 --- a/internal/journeymetrics/types.go +++ b/internal/journeymetrics/types.go @@ -122,6 +122,14 @@ type Record struct { 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"` } type recordJSON struct { @@ -147,6 +155,9 @@ type recordJSON struct { 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"` } func (r Record) MarshalJSON() ([]byte, error) { @@ -183,5 +194,8 @@ func (r Record) MarshalJSON() ([]byte, error) { Budget: budget, BudgetResult: r.BudgetResult, CodexCharacter: r.CodexCharacter, + RunID: r.RunID, + RunURL: r.RunURL, + CapturedAt: r.CapturedAt, }) } From 9dfd006368351270fdc672b33d43cb70e198108b Mon Sep 17 00:00:00 2001 From: Spike Test Date: Thu, 2 Jul 2026 20:39:36 +0800 Subject: [PATCH 2/7] AC-3: per-PR journey-cost delta comment against the published ledger Adds `spacedock-release journey-delta`, which selects the latest-by-captured_at observation per scenario/model from the previously published release ledger, diffs it against the current PR run's freshly emitted records, and posts the result as a single sticky-updating PR comment (gh pr comment --edit-last). Wires a new journey-delta-comment job into runtime-live-e2e.yml, gated to pull_request events with its own pull-requests: write permission. --- .github/workflows/runtime-live-e2e.yml | 66 +++++++ cmd/spacedock-release/journey_delta.go | 109 +++++++++++ cmd/spacedock-release/journey_delta_test.go | 100 ++++++++++ cmd/spacedock-release/main.go | 9 +- .../release/journey_delta_workflow_test.go | 66 +++++++ internal/release/journeydelta.go | 159 +++++++++++++++ internal/release/journeydelta_test.go | 185 ++++++++++++++++++ 7 files changed, 693 insertions(+), 1 deletion(-) create mode 100644 cmd/spacedock-release/journey_delta.go create mode 100644 cmd/spacedock-release/journey_delta_test.go create mode 100644 internal/release/journey_delta_workflow_test.go create mode 100644 internal/release/journeydelta.go create mode 100644 internal/release/journeydelta_test.go diff --git a/.github/workflows/runtime-live-e2e.yml b/.github/workflows/runtime-live-e2e.yml index e4f25d62a..5e61fba5b 100644 --- a/.github/workflows/runtime-live-e2e.yml +++ b/.github/workflows/runtime-live-e2e.yml @@ -646,3 +646,69 @@ 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" + + - 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-artifacts/live-artifacts/journey-metrics" \ + --pr "${{ github.event.pull_request.number }}" diff --git a/cmd/spacedock-release/journey_delta.go b/cmd/spacedock-release/journey_delta.go new file mode 100644 index 000000000..9d903d4a3 --- /dev/null +++ b/cmd/spacedock-release/journey_delta.go @@ -0,0 +1,109 @@ +// 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" + + "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 sticky-update argv is testable without a live `gh` call. +type postCommentFunc func(args []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() +} + +// 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 via the sticky +// --edit-last/--create-if-none gh invocation so repeated pushes update ONE +// comment instead of appending a new one each time. +func journeyDelta(args []string, 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 + } + + if err := post(release.JourneyDeltaCommentArgs(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..a5653d634 --- /dev/null +++ b/cmd/spacedock-release/journey_delta_test.go @@ -0,0 +1,100 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/spacedock-dev/spacedock/internal/journeymetrics" +) + +// TestJourneyDeltaCommandReusesStickyCommentOnRepeatedRuns is AC-3's proof that +// the SAME comment id/marker is reused across repeated runs (an update, not a +// second comment): it drives journeyDelta twice — once with an initial PR-run +// fixture, once with an updated one carrying different turns/tokens — and +// asserts BOTH stubbed `gh pr comment` invocations carry the identical sticky +// argv shape (--edit-last --create-if-none against the same PR number), which is +// the mechanism that makes gh edit its own last comment rather than posting a +// new one. +func TestJourneyDeltaCommandReusesStickyCommentOnRepeatedRuns(t *testing.T) { + ledgerPath := writePreviousLedger(t, 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", + }, + }, + }, + }, + }) + + var calls [][]string + stub := func(args []string) error { + calls = append(calls, args) + return nil + } + + metricsDirA := t.TempDir() + writeCurrentRecord(t, metricsDirA, 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}, + }) + if code := journeyDelta([]string{ledgerPath, "--metrics-dir", metricsDirA, "--pr", "42"}, stub); code != 0 { + t.Fatalf("journeyDelta (first run) exit = %d, want 0", code) + } + + metricsDirB := t.TempDir() + writeCurrentRecord(t, metricsDirB, 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}, + }) + if code := journeyDelta([]string{ledgerPath, "--metrics-dir", metricsDirB, "--pr", "42"}, stub); code != 0 { + t.Fatalf("journeyDelta (second run) exit = %d, want 0", code) + } + + if len(calls) != 2 { + t.Fatalf("gh pr comment invocations = %d, want 2", len(calls)) + } + for i, args := range calls { + joined := strings.Join(args, " ") + for _, want := range []string{"pr comment 42", "--edit-last", "--create-if-none"} { + if !strings.Contains(joined, want) { + t.Fatalf("call %d argv %v missing %q — repeated runs must use the identical sticky-update shape", i, args, want) + } + } + } +} + +// TestJourneyDeltaCommandRejectsMissingArgs proves the CLI's argument-validation +// exit code, mirroring the sibling journey-costs command's shape. +func TestJourneyDeltaCommandRejectsMissingArgs(t *testing.T) { + if code := journeyDelta([]string{"ledger.json", "--metrics-dir", t.TempDir()}, func([]string) error { return nil }); code != 2 { + t.Fatalf("journeyDelta with a missing --pr exit = %d, want 2", code) + } +} + +func writePreviousLedger(t *testing.T, ledger journeymetrics.Ledger) string { + t.Helper() + 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 +} + +func writeCurrentRecord(t *testing.T, dir string, record journeymetrics.Record) { + t.Helper() + writeMetricRecord(t, dir, record) +} diff --git a/cmd/spacedock-release/main.go b/cmd/spacedock-release/main.go index 3ae2bd175..459be1a44 100644 --- a/cmd/spacedock-release/main.go +++ b/cmd/spacedock-release/main.go @@ -22,13 +22,17 @@ 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 manifest's top-level `version` to the release // version (AC-4). bump-calendar advances the marketplace plugin entry's calendar // key to today's `0.0.YYYYMMDDNN` (AC-2d). Both 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 +// advance stamps onto `next`. journey-delta renders and posts (via a sticky +// `gh pr comment --edit-last`) the per-PR journey-cost delta against the +// previously published release ledger's latest-by-captured_at baseline per +// scenario/model. 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 @@ -51,6 +55,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:], ghPostComment)) case "e2e-gate": os.Exit(runE2EGate(os.Args[2:], ghRunListForCommit)) case "manifest-tag-gate": @@ -350,6 +356,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/release/journey_delta_workflow_test.go b/internal/release/journey_delta_workflow_test.go new file mode 100644 index 000000000..f03a55573 --- /dev/null +++ b/internal/release/journey_delta_workflow_test.go @@ -0,0 +1,66 @@ +package release + +import ( + "strings" + "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") + } +} + +// TestRuntimeLiveWorkflowJourneyDeltaJobRejectsMissingPermission is the +// adversarial twin: stripping the job's permissions block must trip the guard, +// proving the check is load-bearing rather than vacuously true. +func TestRuntimeLiveWorkflowJourneyDeltaJobRejectsMissingPermission(t *testing.T) { + live := readWorkflow(t, "runtime-live-e2e.yml") + const permBlock = " permissions:\n contents: read\n actions: read\n pull-requests: write\n" + if strings.Count(live, permBlock) != 1 { + t.Fatalf("expected exactly 1 occurrence of the journey-delta-comment permissions block, found %d", strings.Count(live, permBlock)) + } + adversarial := strings.Replace(live, permBlock, "", 1) + + var doc struct { + Jobs map[string]struct { + Permissions map[string]string `yaml:"permissions"` + } `yaml:"jobs"` + } + if err := yaml.Unmarshal([]byte(adversarial), &doc); err != nil { + t.Fatalf("parse adversarial workflow: %v", err) + } + if doc.Jobs["journey-delta-comment"].Permissions["pull-requests"] == "write" { + t.Fatal("mutation did not actually remove the permissions block") + } +} diff --git a/internal/release/journeydelta.go b/internal/release/journeydelta.go new file mode 100644 index 000000000..04677c71f --- /dev/null +++ b/internal/release/journeydelta.go @@ -0,0 +1,159 @@ +// 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 + 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.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. It has no +// behavioral role in THIS process — the sticky-update behavior comes from `gh +// pr comment --edit-last` editing the poster's own last comment — but it lets a +// human (or a future automated check) identify the comment's origin at a +// glance. +const journeyDeltaCommentMarker = "" + +// RenderJourneyDeltaComment renders the AC-3 PR comment body: one table row per +// scenario/runtime/model, each cell an exact (PR value - baseline value) delta. +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 Δ | Tokens Δ | Cost Δ (USD) | Baseline |\n") + b.WriteString("| --- | --- | --- | --- | --- | --- | --- |\n") + for _, d := range deltas { + baseline := "none (new observation)" + switch { + case d.HasBaseline && d.BaselineRunURL != "": + baseline = fmt.Sprintf("[latest published](%s)", d.BaselineRunURL) + case d.HasBaseline: + baseline = "latest published" + } + fmt.Fprintf(&b, "| %s | %s | %s | %+d | %+d | %+.4f | %s |\n", + d.ScenarioID, d.Runtime, d.Model, d.TurnsDelta, d.TokensDelta.Total, d.CostDeltaUSD, baseline) + } + return b.String() +} + +// JourneyDeltaCommentArgs returns the `gh pr comment` argv the AC-3 step +// invokes to post (or update) the PR delta comment. --edit-last edits the +// poster's own last comment on the PR instead of appending a new one on every +// push; --create-if-none falls back to creating the first comment when none +// exists yet, so the FIRST post on a PR still lands. +func JourneyDeltaCommentArgs(prNumber, bodyFile string) []string { + return []string{"pr", "comment", prNumber, "--body-file", bodyFile, "--edit-last", "--create-if-none"} +} diff --git a/internal/release/journeydelta_test.go b/internal/release/journeydelta_test.go new file mode 100644 index 000000000..b9df310cc --- /dev/null +++ b/internal/release/journeydelta_test.go @@ -0,0 +1,185 @@ +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) + } + } +} + +// TestJourneyDeltaCommentArgsUseStickyEditLast proves the gh argv this AC-3 step +// invokes always carries --edit-last --create-if-none, on every call — the +// mechanism that makes repeated pushes update ONE comment instead of appending +// a new one each time. +func TestJourneyDeltaCommentArgsUseStickyEditLast(t *testing.T) { + args := JourneyDeltaCommentArgs("42", "/tmp/comment.md") + joined := strings.Join(args, " ") + for _, want := range []string{"pr comment", "42", "--body-file /tmp/comment.md", "--edit-last", "--create-if-none"} { + if !strings.Contains(joined, want) { + t.Fatalf("gh argv %v missing %q", args, want) + } + } +} From b2c615193e364ceba485bde312c5f559f48a4123 Mon Sep 17 00:00:00 2001 From: Spike Test Date: Thu, 2 Jul 2026 20:56:36 +0800 Subject: [PATCH 3/7] AC-4: backup-then-structural-diff safeguard for release-ledger backfill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two spacedock-release CLI primitives the AC-4 backfill runbook composes: shallow-boot-window-record extracts a shallow-boot-window observation from an archived claude-stream.jsonl (reusing AC-1's extraction logic, hoisted out of its test file into internal/ensigncycle so a non-test binary can import it), and ledger-diff proves a rebuilt journey-costs ledger equals the backed-up original plus exactly the named added scenarios before any `gh release upload --clobber` runs. Both subcommands get CLI-level tests (arg parsing, file I/O, error paths) matching this file's existing per-subcommand test convention, on top of the already-covered package-level logic (BuildShallowBootWindowRecord, DiffAddedScenarios). Fixes a dangling main.go doc-comment reference to a scripts/backfill-shallow-boot-window.sh that was never created. Does not run the live gh release upload or the actual 3-release backfill — that stays an explicit captain-flagged manual step. --- cmd/spacedock-release/ledger_diff.go | 75 ++++++++++++ cmd/spacedock-release/ledger_diff_test.go | 99 ++++++++++++++++ cmd/spacedock-release/main.go | 13 ++- .../shallow_boot_window_record.go | 73 ++++++++++++ .../shallow_boot_window_record_test.go | 91 +++++++++++++++ .../ensigncycle/journey_metrics_live_test.go | 2 +- .../ensigncycle/shallow_boot_measure_test.go | 71 +---------- .../shallow_boot_measure_unit_test.go | 14 +-- .../ensigncycle/shallow_boot_window_record.go | 81 +++++++++++++ internal/release/ledgerdiff.go | 67 +++++++++++ internal/release/ledgerdiff_test.go | 110 ++++++++++++++++++ 11 files changed, 617 insertions(+), 79 deletions(-) create mode 100644 cmd/spacedock-release/ledger_diff.go create mode 100644 cmd/spacedock-release/ledger_diff_test.go create mode 100644 cmd/spacedock-release/shallow_boot_window_record.go create mode 100644 cmd/spacedock-release/shallow_boot_window_record_test.go create mode 100644 internal/ensigncycle/shallow_boot_window_record.go create mode 100644 internal/release/ledgerdiff.go create mode 100644 internal/release/ledgerdiff_test.go diff --git a/cmd/spacedock-release/ledger_diff.go b/cmd/spacedock-release/ledger_diff.go new file mode 100644 index 000000000..d7cbd8486 --- /dev/null +++ b/cmd/spacedock-release/ledger_diff.go @@ -0,0 +1,75 @@ +// ABOUTME: `spacedock-release ledger-diff` is the AC-4 pre-upload safeguard CLI — +// ABOUTME: it exits non-zero if a rebuilt ledger changes anything beyond the named additions. +package main + +import ( + "encoding/json" + "fmt" + "os" + "strings" + + "github.com/spacedock-dev/spacedock/internal/journeymetrics" + "github.com/spacedock-dev/spacedock/internal/release" +) + +// ledgerDiff is the AC-4 safeguard gate: it must run and PASS before any `gh +// release upload --clobber` of a rebuilt ledger. Exit 0 means the rebuilt +// ledger equals the original plus exactly the --added scenario ids, with every +// pre-existing scenario byte-for-byte unchanged; any other outcome exits 1 and +// the caller must NOT upload. +func ledgerDiff(args []string) int { + if len(args) < 4 { + fmt.Fprintln(os.Stderr, "spacedock-release ledger-diff: need --added [,...]") + return 2 + } + originalPath, rebuiltPath := args[0], args[1] + var added string + for i := 2; i < len(args); i++ { + switch args[i] { + case "--added": + i++ + if i >= len(args) { + fmt.Fprintln(os.Stderr, "spacedock-release ledger-diff: --added needs a value") + return 2 + } + added = args[i] + default: + fmt.Fprintf(os.Stderr, "spacedock-release ledger-diff: unknown argument %q\n", args[i]) + return 2 + } + } + if originalPath == "" || rebuiltPath == "" || added == "" { + fmt.Fprintln(os.Stderr, "spacedock-release ledger-diff: need --added [,...]") + return 2 + } + + original, err := readLedgerFile(originalPath) + if err != nil { + fmt.Fprintf(os.Stderr, "read original ledger %s: %v\n", originalPath, err) + return 1 + } + rebuilt, err := readLedgerFile(rebuiltPath) + if err != nil { + fmt.Fprintf(os.Stderr, "read rebuilt ledger %s: %v\n", rebuiltPath, err) + return 1 + } + + if err := release.DiffAddedScenarios(original, rebuilt, strings.Split(added, ",")...); err != nil { + fmt.Fprintf(os.Stderr, "ledger-diff: UNSAFE TO UPLOAD — %v\n", err) + return 1 + } + fmt.Printf("ledger-diff: safe to upload — %s equals %s plus exactly %s\n", rebuiltPath, originalPath, added) + return 0 +} + +func readLedgerFile(path string) (journeymetrics.Ledger, error) { + data, err := os.ReadFile(path) + if err != nil { + return journeymetrics.Ledger{}, err + } + var ledger journeymetrics.Ledger + if err := json.Unmarshal(data, &ledger); err != nil { + return journeymetrics.Ledger{}, err + } + return ledger, nil +} diff --git a/cmd/spacedock-release/ledger_diff_test.go b/cmd/spacedock-release/ledger_diff_test.go new file mode 100644 index 000000000..8d2f25f9f --- /dev/null +++ b/cmd/spacedock-release/ledger_diff_test.go @@ -0,0 +1,99 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/spacedock-dev/spacedock/internal/journeymetrics" +) + +func writeLedgerFile(t *testing.T, path string, ledger journeymetrics.Ledger) { + t.Helper() + data, err := json.Marshal(ledger) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, data, 0o644); err != nil { + t.Fatal(err) + } +} + +func ledgerDiffFixtureScenario(id string, turns int) journeymetrics.ScenarioLedgerEntry { + return journeymetrics.ScenarioLedgerEntry{ + ScenarioID: id, + Source: "live-harness", + Observations: []journeymetrics.Record{ + {ScenarioID: id, Runtime: "claude", Model: "claude-sonnet-4-6", Turns: turns, Outcome: journeymetrics.Outcome{Status: "passed"}}, + }, + } +} + +// TestLedgerDiffCommandAcceptsExactlyOneAddedScenario is AC-4's CLI-level proof +// that the pre-upload safeguard (release.DiffAddedScenarios) is actually wired +// into the `ledger-diff` subcommand the backfill invokes before any `gh release +// upload --clobber`, not just exercised as an internal package function. +func TestLedgerDiffCommandAcceptsExactlyOneAddedScenario(t *testing.T) { + dir := t.TempDir() + originalPath := filepath.Join(dir, "original.json") + rebuiltPath := filepath.Join(dir, "rebuilt.json") + writeLedgerFile(t, originalPath, journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ + ledgerDiffFixtureScenario("gate-guardrail", 5), + }}) + writeLedgerFile(t, rebuiltPath, journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ + ledgerDiffFixtureScenario("gate-guardrail", 5), + ledgerDiffFixtureScenario("shallow-boot-window", 3), + }}) + + if code := ledgerDiff([]string{originalPath, rebuiltPath, "--added", "shallow-boot-window"}); code != 0 { + t.Fatalf("ledgerDiff exit = %d, want 0 for a rebuild that only adds the named scenario", code) + } +} + +// TestLedgerDiffCommandRejectsMutatedExistingScenario proves the CLI surfaces a +// non-zero exit — the signal the backfill's upload step must gate on — when the +// rebuild alters a pre-existing historical scenario. +func TestLedgerDiffCommandRejectsMutatedExistingScenario(t *testing.T) { + dir := t.TempDir() + originalPath := filepath.Join(dir, "original.json") + rebuiltPath := filepath.Join(dir, "rebuilt.json") + writeLedgerFile(t, originalPath, journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ + ledgerDiffFixtureScenario("gate-guardrail", 5), + }}) + writeLedgerFile(t, rebuiltPath, journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ + ledgerDiffFixtureScenario("gate-guardrail", 6), // mutated + ledgerDiffFixtureScenario("shallow-boot-window", 3), + }}) + + if code := ledgerDiff([]string{originalPath, rebuiltPath, "--added", "shallow-boot-window"}); code == 0 { + t.Fatal("ledgerDiff exit = 0 for a rebuild that mutated a pre-existing scenario; want non-zero") + } +} + +// TestLedgerDiffCommandRejectsMissingArgs proves a miswired invocation is a +// usage error rather than a silent pass that would let an upload proceed +// unguarded. +func TestLedgerDiffCommandRejectsMissingArgs(t *testing.T) { + if code := ledgerDiff(nil); code == 0 { + t.Fatalf("ledgerDiff exit = 0 with no arguments; want non-zero") + } + if code := ledgerDiff([]string{"only-one-arg.json"}); code == 0 { + t.Fatalf("ledgerDiff exit = 0 with a single positional argument; want non-zero") + } +} + +// TestLedgerDiffCommandRejectsMissingFile proves an unreadable ledger path fails +// loud instead of comparing against an empty ledger. +func TestLedgerDiffCommandRejectsMissingFile(t *testing.T) { + dir := t.TempDir() + rebuiltPath := filepath.Join(dir, "rebuilt.json") + writeLedgerFile(t, rebuiltPath, journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ + ledgerDiffFixtureScenario("gate-guardrail", 5), + }}) + + code := ledgerDiff([]string{filepath.Join(dir, "does-not-exist.json"), rebuiltPath, "--added", "shallow-boot-window"}) + if code == 0 { + t.Fatalf("ledgerDiff exit = 0 with a missing original ledger file; want non-zero") + } +} diff --git a/cmd/spacedock-release/main.go b/cmd/spacedock-release/main.go index 459be1a44..1fccc2c1a 100644 --- a/cmd/spacedock-release/main.go +++ b/cmd/spacedock-release/main.go @@ -23,6 +23,8 @@ import ( // spacedock-release bump-calendar // spacedock-release dev-preversion // spacedock-release journey-delta --metrics-dir --pr +// spacedock-release shallow-boot-window-record --model --out +// spacedock-release ledger-diff --added [,...] // spacedock-release e2e-gate // // stamp-version rewrites each manifest's top-level `version` to the release @@ -32,7 +34,10 @@ import ( // advance stamps onto `next`. journey-delta renders and posts (via a sticky // `gh pr comment --edit-last`) the per-PR journey-cost delta against the // previously published release ledger's latest-by-captured_at baseline per -// scenario/model. e2e-gate is the +// scenario/model. shallow-boot-window-record and ledger-diff are the AC-4 +// release-ledger backfill's extraction and pre-upload safeguard steps, invoked +// standalone against archived data by whoever performs the backfill (a +// captain-flagged manual procedure, not a CI step). 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 @@ -57,6 +62,10 @@ func main() { os.Exit(journeyCosts(os.Args[2:])) case "journey-delta": os.Exit(journeyDelta(os.Args[2:], ghPostComment)) + case "shallow-boot-window-record": + os.Exit(shallowBootWindowRecord(os.Args[2:])) + case "ledger-diff": + os.Exit(ledgerDiff(os.Args[2:])) case "e2e-gate": os.Exit(runE2EGate(os.Args[2:], ghRunListForCommit)) case "manifest-tag-gate": @@ -357,6 +366,8 @@ Usage: spacedock-release dev-preversion spacedock-release journey-costs --metrics-dir --out spacedock-release journey-delta --metrics-dir --pr + spacedock-release shallow-boot-window-record --model --out + spacedock-release ledger-diff --added [,...] spacedock-release e2e-gate spacedock-release manifest-tag-gate [ ...] spacedock-release notes diff --git a/cmd/spacedock-release/shallow_boot_window_record.go b/cmd/spacedock-release/shallow_boot_window_record.go new file mode 100644 index 000000000..9983070b9 --- /dev/null +++ b/cmd/spacedock-release/shallow_boot_window_record.go @@ -0,0 +1,73 @@ +// ABOUTME: `spacedock-release shallow-boot-window-record` extracts the AC-1 +// ABOUTME: boot-window observation from an archived stream, for the AC-4 backfill. +package main + +import ( + "fmt" + "os" + + "github.com/spacedock-dev/spacedock/internal/ensigncycle" + "github.com/spacedock-dev/spacedock/internal/journeymetrics" +) + +// shallowBootWindowRecord applies AC-1's extraction logic +// (ensigncycle.BuildShallowBootWindowRecord) to an ARCHIVED claude-stream.jsonl, +// standalone rather than at live-test time, and emits the resulting +// shallow-boot-window record into --out — the same mechanism the AC-4 backfill +// uses to add the observation to a historical run's already-recovered metrics +// dir before that run's ledger is rebuilt. +func shallowBootWindowRecord(args []string) int { + if len(args) < 5 { + fmt.Fprintln(os.Stderr, "spacedock-release shallow-boot-window-record: need --model --out ") + return 2 + } + streamPath := args[0] + var model, outDir string + for i := 1; i < len(args); i++ { + switch args[i] { + case "--model": + i++ + if i >= len(args) { + fmt.Fprintln(os.Stderr, "spacedock-release shallow-boot-window-record: --model needs a value") + return 2 + } + model = args[i] + case "--out": + i++ + if i >= len(args) { + fmt.Fprintln(os.Stderr, "spacedock-release shallow-boot-window-record: --out needs a value") + return 2 + } + outDir = args[i] + default: + fmt.Fprintf(os.Stderr, "spacedock-release shallow-boot-window-record: unknown argument %q\n", args[i]) + return 2 + } + } + if streamPath == "" || model == "" || outDir == "" { + fmt.Fprintln(os.Stderr, "spacedock-release shallow-boot-window-record: need --model --out ") + return 2 + } + + data, err := os.ReadFile(streamPath) + if err != nil { + fmt.Fprintf(os.Stderr, "read %s: %v\n", streamPath, err) + return 1 + } + turns, err := journeymetrics.ParseClaudeTurns(data) + if err != nil { + fmt.Fprintf(os.Stderr, "parse claude turns: %v\n", err) + return 1 + } + record, err := ensigncycle.BuildShallowBootWindowRecord(turns, model) + if err != nil { + fmt.Fprintf(os.Stderr, "build shallow-boot-window record: %v\n", err) + return 1 + } + if err := journeymetrics.EmitRecord(outDir, record); err != nil { + fmt.Fprintf(os.Stderr, "emit record: %v\n", err) + return 1 + } + fmt.Printf("wrote shallow-boot-window record to %s (turns=%d, tokens=%+v)\n", outDir, record.Turns, record.Tokens) + return 0 +} diff --git a/cmd/spacedock-release/shallow_boot_window_record_test.go b/cmd/spacedock-release/shallow_boot_window_record_test.go new file mode 100644 index 000000000..4148cc8dd --- /dev/null +++ b/cmd/spacedock-release/shallow_boot_window_record_test.go @@ -0,0 +1,91 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/spacedock-dev/spacedock/internal/journeymetrics" +) + +const shallowBootWindowFixtureStream = `{"type":"system","subtype":"init","model":"claude-test-model"} +{"type":"assistant","message":{"id":"msg_1","model":"claude-test-model","usage":{"input_tokens":10,"output_tokens":5,"cache_creation_input_tokens":100,"cache_read_input_tokens":50},"content":[{"type":"tool_use","id":"toolu_1","name":"Bash","input":{"command":"echo hi"}}]}} +{"type":"assistant","message":{"id":"msg_2","model":"claude-test-model","usage":{"input_tokens":20,"output_tokens":8,"cache_creation_input_tokens":200,"cache_read_input_tokens":90},"content":[{"type":"text","text":"Hello!"}]}} +` + +// TestShallowBootWindowRecordCommandExtractsFromArchivedStream is AC-4's proof +// that the CLI applies AC-1's extraction logic (BuildShallowBootWindowRecord) to +// a standalone archived claude-stream.jsonl — the mechanism the release-ledger +// backfill uses instead of duplicating extraction logic. The fixture's greet +// turn is turns[1] (the only turn with no dispatch tool_use), so Turns == 2 and +// Tokens carries turns[1]'s full usage. +func TestShallowBootWindowRecordCommandExtractsFromArchivedStream(t *testing.T) { + streamPath := filepath.Join(t.TempDir(), "claude-stream.jsonl") + if err := os.WriteFile(streamPath, []byte(shallowBootWindowFixtureStream), 0o644); err != nil { + t.Fatal(err) + } + outDir := t.TempDir() + + if code := shallowBootWindowRecord([]string{streamPath, "--model", "claude-test-model", "--out", outDir}); code != 0 { + t.Fatalf("shallowBootWindowRecord exit = %d, want 0", code) + } + + entries, err := os.ReadDir(outDir) + if err != nil { + t.Fatal(err) + } + if len(entries) != 1 { + t.Fatalf("expected exactly one emitted record file, got %d: %+v", len(entries), entries) + } + data, err := os.ReadFile(filepath.Join(outDir, entries[0].Name())) + if err != nil { + t.Fatal(err) + } + var record journeymetrics.Record + if err := json.Unmarshal(data, &record); err != nil { + t.Fatal(err) + } + if record.ScenarioID != "shallow-boot-window" { + t.Fatalf("ScenarioID = %q, want shallow-boot-window", record.ScenarioID) + } + if record.Turns != 2 { + t.Fatalf("Turns = %d, want 2", record.Turns) + } + want := journeymetrics.TokenTotals{Input: 20, Output: 8, CacheCreation: 200, CacheRead: 90, Total: 318} + if record.Tokens != want { + t.Fatalf("Tokens = %+v, want %+v", record.Tokens, want) + } +} + +// TestShallowBootWindowRecordCommandRejectsMissingArgs proves a miswired +// invocation (missing the required stream/--model/--out) is a usage error, not a +// silent no-op that would leave the AC-4 backfill believing extraction ran. +func TestShallowBootWindowRecordCommandRejectsMissingArgs(t *testing.T) { + if code := shallowBootWindowRecord(nil); code == 0 { + t.Fatalf("shallowBootWindowRecord exit = 0 with no arguments; want non-zero") + } +} + +// TestShallowBootWindowRecordCommandRejectsMissingStreamFile proves an +// unreadable stream path fails loud instead of emitting an empty/zero record. +func TestShallowBootWindowRecordCommandRejectsMissingStreamFile(t *testing.T) { + code := shallowBootWindowRecord([]string{filepath.Join(t.TempDir(), "does-not-exist.jsonl"), "--model", "claude-test-model", "--out", t.TempDir()}) + if code == 0 { + t.Fatalf("shallowBootWindowRecord exit = 0 with a missing stream file; want non-zero") + } +} + +// TestShallowBootWindowRecordCommandRejectsStreamWithNoAssistantTurns proves a +// stream that produced no assistant turns at all (nothing to extract a +// boot-window observation from) is rejected rather than silently skipped. +func TestShallowBootWindowRecordCommandRejectsStreamWithNoAssistantTurns(t *testing.T) { + streamPath := filepath.Join(t.TempDir(), "claude-stream.jsonl") + if err := os.WriteFile(streamPath, []byte(`{"type":"system","subtype":"init","model":"claude-test-model"}`+"\n"), 0o644); err != nil { + t.Fatal(err) + } + code := shallowBootWindowRecord([]string{streamPath, "--model", "claude-test-model", "--out", t.TempDir()}) + if code == 0 { + t.Fatalf("shallowBootWindowRecord exit = 0 on a stream with no assistant turns; want non-zero") + } +} diff --git a/internal/ensigncycle/journey_metrics_live_test.go b/internal/ensigncycle/journey_metrics_live_test.go index ff069c8df..c941b4192 100644 --- a/internal/ensigncycle/journey_metrics_live_test.go +++ b/internal/ensigncycle/journey_metrics_live_test.go @@ -89,7 +89,7 @@ func emitShallowBootWindowMetrics(t *testing.T, stream string, model string) { if err != nil { t.Fatalf("parse Claude turns for shallow-boot-window: %v", err) } - record, err := buildShallowBootWindowRecord(turns, model) + record, err := BuildShallowBootWindowRecord(turns, model) if err != nil { t.Fatalf("build shallow-boot-window record: %v", err) } diff --git a/internal/ensigncycle/shallow_boot_measure_test.go b/internal/ensigncycle/shallow_boot_measure_test.go index a9948cfd3..6c16dbc4e 100644 --- a/internal/ensigncycle/shallow_boot_measure_test.go +++ b/internal/ensigncycle/shallow_boot_measure_test.go @@ -7,14 +7,6 @@ import ( "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" - // deferredFOSkillNames are the first-officer-internal skills a greet-and-stop boot // must NOT invoke before the greet: the status-viewer surface, the write/id-style // surface, and the dispatch-failure-recovery surface. Each loads only at its trigger @@ -25,39 +17,6 @@ const shallowBootWindowScenarioID = "shallow-boot-window" // skill ARGUMENT, not on any Skill call. var deferredFOSkillNames = []string{"fo-status-viewer", "fo-write-core", "fo-dispatch-recovery"} -// 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. @@ -136,7 +95,7 @@ func assertGreetInvokesNoDeferredFOSkill(stream string) error { // 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. +// journeymetrics observation — see BuildShallowBootWindowRecord. func assertShallowBootMeasured(stream string) error { turns, err := journeymetrics.ParseClaudeTurns([]byte(stream)) if err != nil { @@ -159,31 +118,3 @@ func assertShallowBootMeasuredTurns(turns []journeymetrics.ClaudeTurn) error { } return nil } - -// 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 both the former -// ceiling signal (Context() = input+cache_read+cache_creation) and the former -// pre-greet-spike signal (cache_creation) from this one recorded observation. -func buildShallowBootWindowRecord(turns []journeymetrics.ClaudeTurn, model 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, - }), nil -} diff --git a/internal/ensigncycle/shallow_boot_measure_unit_test.go b/internal/ensigncycle/shallow_boot_measure_unit_test.go index e8f728b9f..cc655adc7 100644 --- a/internal/ensigncycle/shallow_boot_measure_unit_test.go +++ b/internal/ensigncycle/shallow_boot_measure_unit_test.go @@ -158,9 +158,9 @@ func TestBuildShallowBootWindowRecord(t *testing.T) { } const model = "claude-test-model" - record, err := buildShallowBootWindowRecord(turns, model) + record, err := BuildShallowBootWindowRecord(turns, model) if err != nil { - t.Fatalf("buildShallowBootWindowRecord: %v", err) + t.Fatalf("BuildShallowBootWindowRecord: %v", err) } if record.ScenarioID != "shallow-boot-window" { t.Fatalf("ScenarioID = %q, want shallow-boot-window", record.ScenarioID) @@ -214,7 +214,7 @@ func TestBuildShallowBootWindowRecord(t *testing.T) { // 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 +// 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. @@ -227,8 +227,8 @@ func TestShallowBootMeasureSignalsAreIndependent(t *testing.T) { 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"); err != nil { - t.Fatalf("buildShallowBootWindowRecord(heavyGreet): %v", err) + if record, err := BuildShallowBootWindowRecord(heavyGreet, "test-model"); 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) } @@ -242,8 +242,8 @@ func TestShallowBootMeasureSignalsAreIndependent(t *testing.T) { 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) + 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) } diff --git a/internal/ensigncycle/shallow_boot_window_record.go b/internal/ensigncycle/shallow_boot_window_record.go new file mode 100644 index 000000000..a20229ce1 --- /dev/null +++ b/internal/ensigncycle/shallow_boot_window_record.go @@ -0,0 +1,81 @@ +// ABOUTME: Boot-window telemetry extraction shared by the live shallow-boot +// ABOUTME: scenario test and the release-ledger backfill CLI (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 +} + +// 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 both the former +// ceiling signal (Context() = input+cache_read+cache_creation) and the former +// pre-greet-spike signal (cache_creation) from this one recorded observation. +// Exported (unlike the live-scenario test glue around it) so the AC-4 release- +// ledger backfill CLI can apply the SAME extraction logic to archived streams, +// standalone, rather than duplicating it. +func BuildShallowBootWindowRecord(turns []journeymetrics.ClaudeTurn, model 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, + }), nil +} diff --git a/internal/release/ledgerdiff.go b/internal/release/ledgerdiff.go new file mode 100644 index 000000000..e5459cecd --- /dev/null +++ b/internal/release/ledgerdiff.go @@ -0,0 +1,67 @@ +// ABOUTME: Structural pre-upload safeguard for the AC-4 release-ledger backfill — +// ABOUTME: proves a rebuilt ledger equals the original plus exactly the added scenarios. +package release + +import ( + "encoding/json" + "fmt" + "slices" + "sort" + + "github.com/spacedock-dev/spacedock/internal/journeymetrics" +) + +// DiffAddedScenarios is the AC-4 pre-upload safeguard: it proves a rebuilt +// journey-cost ledger equals the backed-up original ledger's scenarios array +// PLUS EXACTLY the named added scenario ids, with every pre-existing scenario +// byte-for-byte unchanged. Rebuilding via `journey-costs` re-normalizes every +// historical record (normalizeRecord restamps schema_version and recomputes +// token totals on every record it reads), so a rebuild quirk could otherwise +// silently mutate already-published historical data that has nothing to do +// with the backfill. It returns a descriptive error on ANY deviation — a +// changed, missing, or unexpectedly-extra scenario — so the caller can abort +// BEFORE an upload instead of publishing a corrupted ledger. +func DiffAddedScenarios(original, rebuilt journeymetrics.Ledger, wantAdded ...string) error { + originalByID := scenarioByID(original) + rebuiltByID := scenarioByID(rebuilt) + + for id, origEntry := range originalByID { + rebuiltEntry, ok := rebuiltByID[id] + if !ok { + return fmt.Errorf("scenario %q present in the original ledger is MISSING from the rebuilt ledger", id) + } + origJSON, err := json.Marshal(origEntry) + if err != nil { + return fmt.Errorf("marshal original scenario %q: %w", id, err) + } + rebuiltJSON, err := json.Marshal(rebuiltEntry) + if err != nil { + return fmt.Errorf("marshal rebuilt scenario %q: %w", id, err) + } + if string(origJSON) != string(rebuiltJSON) { + return fmt.Errorf("scenario %q changed by the rebuild (a rebuild must only ADD the backfilled scenario, never alter an existing one) — original:\n%s\nrebuilt:\n%s", id, origJSON, rebuiltJSON) + } + } + + var extra []string + for id := range rebuiltByID { + if _, ok := originalByID[id]; !ok { + extra = append(extra, id) + } + } + sort.Strings(extra) + wantSorted := append([]string(nil), wantAdded...) + sort.Strings(wantSorted) + if !slices.Equal(extra, wantSorted) { + return fmt.Errorf("rebuilt ledger added scenarios %v, want exactly %v", extra, wantSorted) + } + return nil +} + +func scenarioByID(ledger journeymetrics.Ledger) map[string]journeymetrics.ScenarioLedgerEntry { + out := make(map[string]journeymetrics.ScenarioLedgerEntry, len(ledger.Scenarios)) + for _, entry := range ledger.Scenarios { + out[entry.ScenarioID] = entry + } + return out +} diff --git a/internal/release/ledgerdiff_test.go b/internal/release/ledgerdiff_test.go new file mode 100644 index 000000000..01637a0f2 --- /dev/null +++ b/internal/release/ledgerdiff_test.go @@ -0,0 +1,110 @@ +package release + +import ( + "strings" + "testing" + + "github.com/spacedock-dev/spacedock/internal/journeymetrics" +) + +func fixtureScenario(id string, turns int) journeymetrics.ScenarioLedgerEntry { + return journeymetrics.ScenarioLedgerEntry{ + ScenarioID: id, + Source: "live-harness", + Observations: []journeymetrics.Record{ + {ScenarioID: id, Runtime: "claude", Model: "claude-sonnet-4-6", Turns: turns, Outcome: journeymetrics.Outcome{Status: "passed"}}, + }, + } +} + +// TestDiffAddedScenariosAcceptsExactlyOneAddedScenario is AC-4's primary safety +// proof: a rebuilt ledger that equals the original PLUS exactly one new +// shallow-boot-window entry, with every pre-existing scenario byte-identical, +// must pass. +func TestDiffAddedScenariosAcceptsExactlyOneAddedScenario(t *testing.T) { + original := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ + fixtureScenario("gate-guardrail", 5), + fixtureScenario("shallow-boot", 3), + }} + rebuilt := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ + fixtureScenario("gate-guardrail", 5), + fixtureScenario("shallow-boot", 3), + fixtureScenario("shallow-boot-window", 3), + }} + if err := DiffAddedScenarios(original, rebuilt, "shallow-boot-window"); err != nil { + t.Fatalf("expected the exactly-one-addition case to pass, got: %v", err) + } +} + +// TestDiffAddedScenariosRejectsMutatedExistingScenario is AC-4's proof that a +// rebuild quirk mutating a pre-existing historical scenario (e.g. normalizeRecord +// restamping schema_version or recomputing token totals differently) is caught, +// not silently republished. +func TestDiffAddedScenariosRejectsMutatedExistingScenario(t *testing.T) { + original := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ + fixtureScenario("gate-guardrail", 5), + }} + rebuilt := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ + fixtureScenario("gate-guardrail", 6), // mutated: Turns 5 -> 6 + fixtureScenario("shallow-boot-window", 3), + }} + err := DiffAddedScenarios(original, rebuilt, "shallow-boot-window") + if err == nil { + t.Fatal("expected the mutated pre-existing scenario to be rejected") + } + if !strings.Contains(err.Error(), "gate-guardrail") { + t.Fatalf("error does not name the mutated scenario: %v", err) + } +} + +// TestDiffAddedScenariosRejectsMissingExistingScenario proves a rebuild that +// silently DROPPED a pre-existing scenario is caught. +func TestDiffAddedScenariosRejectsMissingExistingScenario(t *testing.T) { + original := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ + fixtureScenario("gate-guardrail", 5), + fixtureScenario("shallow-boot", 3), + }} + rebuilt := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ + fixtureScenario("shallow-boot", 3), + fixtureScenario("shallow-boot-window", 3), + }} + err := DiffAddedScenarios(original, rebuilt, "shallow-boot-window") + if err == nil { + t.Fatal("expected the dropped pre-existing scenario to be rejected") + } + if !strings.Contains(err.Error(), "gate-guardrail") { + t.Fatalf("error does not name the missing scenario: %v", err) + } +} + +// TestDiffAddedScenariosRejectsUnexpectedExtraScenario proves a rebuild that +// added something OTHER than (or in addition to) the expected scenario is +// caught, rather than treated as a benign superset. +func TestDiffAddedScenariosRejectsUnexpectedExtraScenario(t *testing.T) { + original := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ + fixtureScenario("gate-guardrail", 5), + }} + rebuilt := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ + fixtureScenario("gate-guardrail", 5), + fixtureScenario("shallow-boot-window", 3), + fixtureScenario("mystery-scenario", 1), + }} + if err := DiffAddedScenarios(original, rebuilt, "shallow-boot-window"); err == nil { + t.Fatal("expected an unexpected extra scenario beyond the named addition to be rejected") + } +} + +// TestDiffAddedScenariosRejectsMissingExpectedAddition proves a rebuild that did +// NOT actually add the expected scenario (e.g. the extraction silently no-opped) +// is caught rather than passing vacuously. +func TestDiffAddedScenariosRejectsMissingExpectedAddition(t *testing.T) { + original := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ + fixtureScenario("gate-guardrail", 5), + }} + rebuilt := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ + fixtureScenario("gate-guardrail", 5), + }} + if err := DiffAddedScenarios(original, rebuilt, "shallow-boot-window"); err == nil { + t.Fatal("expected a rebuild that did not add the expected scenario to be rejected") + } +} From d019ca60afc76afe4156670541a2b05adf0fd158 Mon Sep 17 00:00:00 2001 From: Spike Test Date: Thu, 2 Jul 2026 21:17:20 +0800 Subject: [PATCH 4/7] AC-2: cover release.yml's per-run-subdirectory download fix directly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validation's detached adversarial audit found that reverting the "Download latest journey metrics artifacts" step's per-run cp target back to the flat- copy overwrite bug (the exact collision AC-2 fixes) left the full test suite green — the Go-side consumer (AggregateLedger/ReadRecordsDir) was fixture- tested, but the shell script producing the layout never was. Extends internal/release/journey_workflow_test.go's existing extractStepRun + stubbed-gh harness with a positive-path test (two stubbed runs, same record filename, different content, asserts both land in their own subdirectory intact) and an adversarial twin that reverts the cp target and asserts the two runs collapse into one file — proving the positive test actually discriminates a fixed script from a broken one. --- internal/release/journey_workflow_test.go | 148 ++++++++++++++++++++++ 1 file changed, 148 insertions(+) diff --git a/internal/release/journey_workflow_test.go b/internal/release/journey_workflow_test.go index b9f9d46e4..984e000d9 100644 --- a/internal/release/journey_workflow_test.go +++ b/internal/release/journey_workflow_test.go @@ -583,6 +583,154 @@ 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) + } + } +} + // 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. From 06094d50dd88f34bdddd49868bae7251e93f522c Mon Sep 17 00:00:00 2001 From: Spike Test Date: Fri, 3 Jul 2026 10:50:24 +0800 Subject: [PATCH 5/7] Address captain-ruled fable review: C1/C2/C4 fixes, AC-4 TRIM, comment Minors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C1 (production bug): runtime-live-e2e.yml's journey-delta-comment job hardcoded --metrics-dir to a path that doesn't match download-artifact's real nested layout (verified against run 28432388663's zip), REDing the job on every PR. Adds a "Locate this run's journey metrics" step using release.yml's own layout-agnostic find+cp pattern, tested against a realistic nested fixture. C2: BuildShallowBootWindowRecord's BaselineTokens (turns[0].Usage) does not reconstruct the removed teamRecacheSpikeFloor signal for a multi-turn pre-greet window, since the spike can land on any pre-greet turn, not just the first. Adds PreGreetPeakCacheCreation (max cache_creation across pre-greet turns) — the exact aggregate the original threshold check compared — with a regression test proving BaselineTokens alone misses a spike not on turn 0. C4: corrects BaselineTokens' documented semantics — per-turn usage is not cumulative, so only Tokens.Context() - BaselineTokens.Context() is a defensible derived quantity, not a full-struct subtraction. TRIM (captain-ordered de-scope): removes the AC-4 backfill's shipped CLI subcommands (shallow-boot-window-record, ledger-diff) and internal/release's ledgerdiff package + tests (~530 lines) in favor of a documented runbook using jq -S .scenarios | diff plus a throwaway go run script over the exported BuildShallowBootWindowRecord — the extraction/safeguard logic stays reachable, just not shipped as permanent CLI surface for a one-time procedure. Also removes three named meta-tests that verified test-fixture setup rather than product behavior. Minors: the PR delta comment now shows cache_read/cache_creation deltas separately (not just the total — they differ ~12x in cost and meaning for a boot metric); a no-baseline row renders "n/a (new)" instead of a self-delta against an implicit zero baseline; the sticky comment is now found by its marker and updated by id (gh api PATCH) instead of --edit-last, which could target the wrong comment if another automated comment interleaves. --- .github/workflows/runtime-live-e2e.yml | 17 +- cmd/spacedock-release/journey_costs_test.go | 48 ---- cmd/spacedock-release/journey_delta.go | 48 +++- cmd/spacedock-release/journey_delta_test.go | 125 ++++++----- cmd/spacedock-release/ledger_diff.go | 75 ------- cmd/spacedock-release/ledger_diff_test.go | 99 --------- cmd/spacedock-release/main.go | 27 +-- .../shallow_boot_window_record.go | 73 ------ .../shallow_boot_window_record_test.go | 91 -------- .../ensigncycle/journey_metrics_live_test.go | 2 +- .../shallow_boot_measure_unit_test.go | 70 +++++- .../ensigncycle/shallow_boot_window_record.go | 55 ++++- .../testdata/shallow-boot-greet.stream.jsonl | 2 +- internal/journeymetrics/claude.go | 50 +++++ internal/journeymetrics/claude_test.go | 57 +++++ internal/journeymetrics/record.go | 45 ++-- internal/journeymetrics/types.go | 209 +++++++++++------- .../release/journey_delta_workflow_test.go | 25 --- internal/release/journey_workflow_test.go | 51 +++++ internal/release/journeydelta.go | 60 +++-- internal/release/journeydelta_test.go | 53 ++++- internal/release/ledgerdiff.go | 67 ------ internal/release/ledgerdiff_test.go | 110 --------- 23 files changed, 652 insertions(+), 807 deletions(-) delete mode 100644 cmd/spacedock-release/ledger_diff.go delete mode 100644 cmd/spacedock-release/ledger_diff_test.go delete mode 100644 cmd/spacedock-release/shallow_boot_window_record.go delete mode 100644 cmd/spacedock-release/shallow_boot_window_record_test.go delete mode 100644 internal/release/ledgerdiff.go delete mode 100644 internal/release/ledgerdiff_test.go diff --git a/.github/workflows/runtime-live-e2e.yml b/.github/workflows/runtime-live-e2e.yml index 5e61fba5b..f05ac8ede 100644 --- a/.github/workflows/runtime-live-e2e.yml +++ b/.github/workflows/runtime-live-e2e.yml @@ -703,6 +703,21 @@ jobs: 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: @@ -710,5 +725,5 @@ jobs: run: | set -euo pipefail go run ./cmd/spacedock-release journey-delta "$RUNNER_TEMP/previous-journey-costs.json" \ - --metrics-dir "$RUNNER_TEMP/current-run-artifacts/live-artifacts/journey-metrics" \ + --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 2f5f0d97e..596dfd06a 100644 --- a/cmd/spacedock-release/journey_costs_test.go +++ b/cmd/spacedock-release/journey_costs_test.go @@ -101,54 +101,6 @@ func TestJourneyCostsCommandRejectsMismatchedOutputFilename(t *testing.T) { } } -// TestJourneyCostsCommandFlatCopyCollapsesToOneObservation is the AC-2 regression -// test pinning the OLD release.yml behavior: downloading N runs' journey-metrics -// and flat-copying every file into ONE directory. journeymetrics.recordFilename has -// no run-distinguishing component, so two runs of the SAME scenario/model produce -// the SAME on-disk filename in that one directory, and the second cp silently -// overwrites the first — the aggregation collapses back to exactly one observation -// regardless of how many runs were actually discovered. This test proves the old -// shape's collapse is real (not a hypothetical), so the per-run-subdirectory test -// below is provably what closes the gap, not an incidental side effect. -func TestJourneyCostsCommandFlatCopyCollapsesToOneObservation(t *testing.T) { - metricsDir := t.TempDir() // one flat directory — the OLD release.yml `cp` target - 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, runA.CapturedAt = "27931963802", "2026-06-20T00:00:00Z" - runB := base - runB.Turns, runB.Tokens = 20, journeymetrics.TokenTotals{CacheCreation: 1576} - runB.RunID, runB.CapturedAt = "28432388663", "2026-06-27T00:00:00Z" - - // Both records share ScenarioID/Runtime/Model, so writeMetricRecord's filename - // (mirroring journeymetrics.recordFilename's lack of a run-distinguishing - // component) collides — runB's write overwrites runA's file in place, exactly - // as the flat `cp` step would. - writeMetricRecord(t, metricsDir, runA) - writeMetricRecord(t, metricsDir, 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) != 1 { - t.Fatalf("flat-copy shape must still collapse to 1 observation for shallow-boot-window, got %d: %+v", len(entry.Observations), entry.Observations) - } -} - // 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 diff --git a/cmd/spacedock-release/journey_delta.go b/cmd/spacedock-release/journey_delta.go index 9d903d4a3..ab09dc630 100644 --- a/cmd/spacedock-release/journey_delta.go +++ b/cmd/spacedock-release/journey_delta.go @@ -7,15 +7,22 @@ import ( "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 sticky-update argv is testable without a live `gh` call. +// 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 { @@ -24,14 +31,30 @@ func ghPostComment(args []string) error { 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 via the sticky -// --edit-last/--create-if-none gh invocation so repeated pushes update ONE -// comment instead of appending a new one each time. -func journeyDelta(args []string, post postCommentFunc) int { +// 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 @@ -100,7 +123,20 @@ func journeyDelta(args []string, post postCommentFunc) int { return 1 } - if err := post(release.JourneyDeltaCommentArgs(prNumber, bodyFile.Name())); err != nil { + 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 } diff --git a/cmd/spacedock-release/journey_delta_test.go b/cmd/spacedock-release/journey_delta_test.go index a5653d634..8fc89d3a4 100644 --- a/cmd/spacedock-release/journey_delta_test.go +++ b/cmd/spacedock-release/journey_delta_test.go @@ -10,79 +10,95 @@ import ( "github.com/spacedock-dev/spacedock/internal/journeymetrics" ) -// TestJourneyDeltaCommandReusesStickyCommentOnRepeatedRuns is AC-3's proof that -// the SAME comment id/marker is reused across repeated runs (an update, not a -// second comment): it drives journeyDelta twice — once with an initial PR-run -// fixture, once with an updated one carrying different turns/tokens — and -// asserts BOTH stubbed `gh pr comment` invocations carry the identical sticky -// argv shape (--edit-last --create-if-none against the same PR number), which is -// the mechanism that makes gh edit its own last comment rather than posting a -// new one. -func TestJourneyDeltaCommandReusesStickyCommentOnRepeatedRuns(t *testing.T) { - ledgerPath := writePreviousLedger(t, 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", - }, - }, - }, - }, +// 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 - stub := func(args []string) error { + post := func(args []string) error { calls = append(calls, args) return nil } - metricsDirA := t.TempDir() - writeCurrentRecord(t, metricsDirA, 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}, - }) - if code := journeyDelta([]string{ledgerPath, "--metrics-dir", metricsDirA, "--pr", "42"}, stub); code != 0 { - t.Fatalf("journeyDelta (first run) exit = %d, want 0", code) + 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]) } +} - metricsDirB := t.TempDir() - writeCurrentRecord(t, metricsDirB, journeymetrics.Record{ +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}, }) - if code := journeyDelta([]string{ledgerPath, "--metrics-dir", metricsDirB, "--pr", "42"}, stub); code != 0 { - t.Fatalf("journeyDelta (second run) exit = %d, want 0", code) + + find := func(string) (string, error) { return "987654321", nil } + var calls [][]string + post := func(args []string) error { + calls = append(calls, args) + return nil } - if len(calls) != 2 { - t.Fatalf("gh pr comment invocations = %d, want 2", len(calls)) + if code := journeyDelta([]string{ledgerPath, "--metrics-dir", metricsDir, "--pr", "42"}, find, post); code != 0 { + t.Fatalf("journeyDelta exit = %d, want 0", code) } - for i, args := range calls { - joined := strings.Join(args, " ") - for _, want := range []string{"pr comment 42", "--edit-last", "--create-if-none"} { - if !strings.Contains(joined, want) { - t.Fatalf("call %d argv %v missing %q — repeated runs must use the identical sticky-update shape", i, args, want) - } - } + if len(calls) != 1 { + t.Fatalf("post invocations = %d, want 1", len(calls)) } -} - -// TestJourneyDeltaCommandRejectsMissingArgs proves the CLI's argument-validation -// exit code, mirroring the sibling journey-costs command's shape. -func TestJourneyDeltaCommandRejectsMissingArgs(t *testing.T) { - if code := journeyDelta([]string{"ledger.json", "--metrics-dir", t.TempDir()}, func([]string) error { return nil }); code != 2 { - t.Fatalf("journeyDelta with a missing --pr exit = %d, want 2", code) + 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 writePreviousLedger(t *testing.T, ledger journeymetrics.Ledger) string { +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) @@ -93,8 +109,3 @@ func writePreviousLedger(t *testing.T, ledger journeymetrics.Ledger) string { } return path } - -func writeCurrentRecord(t *testing.T, dir string, record journeymetrics.Record) { - t.Helper() - writeMetricRecord(t, dir, record) -} diff --git a/cmd/spacedock-release/ledger_diff.go b/cmd/spacedock-release/ledger_diff.go deleted file mode 100644 index d7cbd8486..000000000 --- a/cmd/spacedock-release/ledger_diff.go +++ /dev/null @@ -1,75 +0,0 @@ -// ABOUTME: `spacedock-release ledger-diff` is the AC-4 pre-upload safeguard CLI — -// ABOUTME: it exits non-zero if a rebuilt ledger changes anything beyond the named additions. -package main - -import ( - "encoding/json" - "fmt" - "os" - "strings" - - "github.com/spacedock-dev/spacedock/internal/journeymetrics" - "github.com/spacedock-dev/spacedock/internal/release" -) - -// ledgerDiff is the AC-4 safeguard gate: it must run and PASS before any `gh -// release upload --clobber` of a rebuilt ledger. Exit 0 means the rebuilt -// ledger equals the original plus exactly the --added scenario ids, with every -// pre-existing scenario byte-for-byte unchanged; any other outcome exits 1 and -// the caller must NOT upload. -func ledgerDiff(args []string) int { - if len(args) < 4 { - fmt.Fprintln(os.Stderr, "spacedock-release ledger-diff: need --added [,...]") - return 2 - } - originalPath, rebuiltPath := args[0], args[1] - var added string - for i := 2; i < len(args); i++ { - switch args[i] { - case "--added": - i++ - if i >= len(args) { - fmt.Fprintln(os.Stderr, "spacedock-release ledger-diff: --added needs a value") - return 2 - } - added = args[i] - default: - fmt.Fprintf(os.Stderr, "spacedock-release ledger-diff: unknown argument %q\n", args[i]) - return 2 - } - } - if originalPath == "" || rebuiltPath == "" || added == "" { - fmt.Fprintln(os.Stderr, "spacedock-release ledger-diff: need --added [,...]") - return 2 - } - - original, err := readLedgerFile(originalPath) - if err != nil { - fmt.Fprintf(os.Stderr, "read original ledger %s: %v\n", originalPath, err) - return 1 - } - rebuilt, err := readLedgerFile(rebuiltPath) - if err != nil { - fmt.Fprintf(os.Stderr, "read rebuilt ledger %s: %v\n", rebuiltPath, err) - return 1 - } - - if err := release.DiffAddedScenarios(original, rebuilt, strings.Split(added, ",")...); err != nil { - fmt.Fprintf(os.Stderr, "ledger-diff: UNSAFE TO UPLOAD — %v\n", err) - return 1 - } - fmt.Printf("ledger-diff: safe to upload — %s equals %s plus exactly %s\n", rebuiltPath, originalPath, added) - return 0 -} - -func readLedgerFile(path string) (journeymetrics.Ledger, error) { - data, err := os.ReadFile(path) - if err != nil { - return journeymetrics.Ledger{}, err - } - var ledger journeymetrics.Ledger - if err := json.Unmarshal(data, &ledger); err != nil { - return journeymetrics.Ledger{}, err - } - return ledger, nil -} diff --git a/cmd/spacedock-release/ledger_diff_test.go b/cmd/spacedock-release/ledger_diff_test.go deleted file mode 100644 index 8d2f25f9f..000000000 --- a/cmd/spacedock-release/ledger_diff_test.go +++ /dev/null @@ -1,99 +0,0 @@ -package main - -import ( - "encoding/json" - "os" - "path/filepath" - "testing" - - "github.com/spacedock-dev/spacedock/internal/journeymetrics" -) - -func writeLedgerFile(t *testing.T, path string, ledger journeymetrics.Ledger) { - t.Helper() - data, err := json.Marshal(ledger) - if err != nil { - t.Fatal(err) - } - if err := os.WriteFile(path, data, 0o644); err != nil { - t.Fatal(err) - } -} - -func ledgerDiffFixtureScenario(id string, turns int) journeymetrics.ScenarioLedgerEntry { - return journeymetrics.ScenarioLedgerEntry{ - ScenarioID: id, - Source: "live-harness", - Observations: []journeymetrics.Record{ - {ScenarioID: id, Runtime: "claude", Model: "claude-sonnet-4-6", Turns: turns, Outcome: journeymetrics.Outcome{Status: "passed"}}, - }, - } -} - -// TestLedgerDiffCommandAcceptsExactlyOneAddedScenario is AC-4's CLI-level proof -// that the pre-upload safeguard (release.DiffAddedScenarios) is actually wired -// into the `ledger-diff` subcommand the backfill invokes before any `gh release -// upload --clobber`, not just exercised as an internal package function. -func TestLedgerDiffCommandAcceptsExactlyOneAddedScenario(t *testing.T) { - dir := t.TempDir() - originalPath := filepath.Join(dir, "original.json") - rebuiltPath := filepath.Join(dir, "rebuilt.json") - writeLedgerFile(t, originalPath, journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ - ledgerDiffFixtureScenario("gate-guardrail", 5), - }}) - writeLedgerFile(t, rebuiltPath, journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ - ledgerDiffFixtureScenario("gate-guardrail", 5), - ledgerDiffFixtureScenario("shallow-boot-window", 3), - }}) - - if code := ledgerDiff([]string{originalPath, rebuiltPath, "--added", "shallow-boot-window"}); code != 0 { - t.Fatalf("ledgerDiff exit = %d, want 0 for a rebuild that only adds the named scenario", code) - } -} - -// TestLedgerDiffCommandRejectsMutatedExistingScenario proves the CLI surfaces a -// non-zero exit — the signal the backfill's upload step must gate on — when the -// rebuild alters a pre-existing historical scenario. -func TestLedgerDiffCommandRejectsMutatedExistingScenario(t *testing.T) { - dir := t.TempDir() - originalPath := filepath.Join(dir, "original.json") - rebuiltPath := filepath.Join(dir, "rebuilt.json") - writeLedgerFile(t, originalPath, journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ - ledgerDiffFixtureScenario("gate-guardrail", 5), - }}) - writeLedgerFile(t, rebuiltPath, journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ - ledgerDiffFixtureScenario("gate-guardrail", 6), // mutated - ledgerDiffFixtureScenario("shallow-boot-window", 3), - }}) - - if code := ledgerDiff([]string{originalPath, rebuiltPath, "--added", "shallow-boot-window"}); code == 0 { - t.Fatal("ledgerDiff exit = 0 for a rebuild that mutated a pre-existing scenario; want non-zero") - } -} - -// TestLedgerDiffCommandRejectsMissingArgs proves a miswired invocation is a -// usage error rather than a silent pass that would let an upload proceed -// unguarded. -func TestLedgerDiffCommandRejectsMissingArgs(t *testing.T) { - if code := ledgerDiff(nil); code == 0 { - t.Fatalf("ledgerDiff exit = 0 with no arguments; want non-zero") - } - if code := ledgerDiff([]string{"only-one-arg.json"}); code == 0 { - t.Fatalf("ledgerDiff exit = 0 with a single positional argument; want non-zero") - } -} - -// TestLedgerDiffCommandRejectsMissingFile proves an unreadable ledger path fails -// loud instead of comparing against an empty ledger. -func TestLedgerDiffCommandRejectsMissingFile(t *testing.T) { - dir := t.TempDir() - rebuiltPath := filepath.Join(dir, "rebuilt.json") - writeLedgerFile(t, rebuiltPath, journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ - ledgerDiffFixtureScenario("gate-guardrail", 5), - }}) - - code := ledgerDiff([]string{filepath.Join(dir, "does-not-exist.json"), rebuiltPath, "--added", "shallow-boot-window"}) - if code == 0 { - t.Fatalf("ledgerDiff exit = 0 with a missing original ledger file; want non-zero") - } -} diff --git a/cmd/spacedock-release/main.go b/cmd/spacedock-release/main.go index 1fccc2c1a..6de13bbe4 100644 --- a/cmd/spacedock-release/main.go +++ b/cmd/spacedock-release/main.go @@ -23,21 +23,22 @@ import ( // spacedock-release bump-calendar // spacedock-release dev-preversion // spacedock-release journey-delta --metrics-dir --pr -// spacedock-release shallow-boot-window-record --model --out -// spacedock-release ledger-diff --added [,...] // spacedock-release e2e-gate // // stamp-version rewrites each manifest's top-level `version` to the release // version (AC-4). bump-calendar advances the marketplace plugin entry's calendar // key to today's `0.0.YYYYMMDDNN` (AC-2d). Both 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`. journey-delta renders and posts (via a sticky -// `gh pr comment --edit-last`) the per-PR journey-cost delta against the -// previously published release ledger's latest-by-captured_at baseline per -// scenario/model. shallow-boot-window-record and ledger-diff are the AC-4 -// release-ledger backfill's extraction and pre-upload safeguard steps, invoked -// standalone against archived data by whoever performs the backfill (a -// captain-flagged manual procedure, not a CI step). e2e-gate is the +// 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 @@ -61,11 +62,7 @@ func main() { case "journey-costs": os.Exit(journeyCosts(os.Args[2:])) case "journey-delta": - os.Exit(journeyDelta(os.Args[2:], ghPostComment)) - case "shallow-boot-window-record": - os.Exit(shallowBootWindowRecord(os.Args[2:])) - case "ledger-diff": - os.Exit(ledgerDiff(os.Args[2:])) + os.Exit(journeyDelta(os.Args[2:], ghFindComment, ghPostComment)) case "e2e-gate": os.Exit(runE2EGate(os.Args[2:], ghRunListForCommit)) case "manifest-tag-gate": @@ -366,8 +363,6 @@ Usage: spacedock-release dev-preversion spacedock-release journey-costs --metrics-dir --out spacedock-release journey-delta --metrics-dir --pr - spacedock-release shallow-boot-window-record --model --out - spacedock-release ledger-diff --added [,...] spacedock-release e2e-gate spacedock-release manifest-tag-gate [ ...] spacedock-release notes diff --git a/cmd/spacedock-release/shallow_boot_window_record.go b/cmd/spacedock-release/shallow_boot_window_record.go deleted file mode 100644 index 9983070b9..000000000 --- a/cmd/spacedock-release/shallow_boot_window_record.go +++ /dev/null @@ -1,73 +0,0 @@ -// ABOUTME: `spacedock-release shallow-boot-window-record` extracts the AC-1 -// ABOUTME: boot-window observation from an archived stream, for the AC-4 backfill. -package main - -import ( - "fmt" - "os" - - "github.com/spacedock-dev/spacedock/internal/ensigncycle" - "github.com/spacedock-dev/spacedock/internal/journeymetrics" -) - -// shallowBootWindowRecord applies AC-1's extraction logic -// (ensigncycle.BuildShallowBootWindowRecord) to an ARCHIVED claude-stream.jsonl, -// standalone rather than at live-test time, and emits the resulting -// shallow-boot-window record into --out — the same mechanism the AC-4 backfill -// uses to add the observation to a historical run's already-recovered metrics -// dir before that run's ledger is rebuilt. -func shallowBootWindowRecord(args []string) int { - if len(args) < 5 { - fmt.Fprintln(os.Stderr, "spacedock-release shallow-boot-window-record: need --model --out ") - return 2 - } - streamPath := args[0] - var model, outDir string - for i := 1; i < len(args); i++ { - switch args[i] { - case "--model": - i++ - if i >= len(args) { - fmt.Fprintln(os.Stderr, "spacedock-release shallow-boot-window-record: --model needs a value") - return 2 - } - model = args[i] - case "--out": - i++ - if i >= len(args) { - fmt.Fprintln(os.Stderr, "spacedock-release shallow-boot-window-record: --out needs a value") - return 2 - } - outDir = args[i] - default: - fmt.Fprintf(os.Stderr, "spacedock-release shallow-boot-window-record: unknown argument %q\n", args[i]) - return 2 - } - } - if streamPath == "" || model == "" || outDir == "" { - fmt.Fprintln(os.Stderr, "spacedock-release shallow-boot-window-record: need --model --out ") - return 2 - } - - data, err := os.ReadFile(streamPath) - if err != nil { - fmt.Fprintf(os.Stderr, "read %s: %v\n", streamPath, err) - return 1 - } - turns, err := journeymetrics.ParseClaudeTurns(data) - if err != nil { - fmt.Fprintf(os.Stderr, "parse claude turns: %v\n", err) - return 1 - } - record, err := ensigncycle.BuildShallowBootWindowRecord(turns, model) - if err != nil { - fmt.Fprintf(os.Stderr, "build shallow-boot-window record: %v\n", err) - return 1 - } - if err := journeymetrics.EmitRecord(outDir, record); err != nil { - fmt.Fprintf(os.Stderr, "emit record: %v\n", err) - return 1 - } - fmt.Printf("wrote shallow-boot-window record to %s (turns=%d, tokens=%+v)\n", outDir, record.Turns, record.Tokens) - return 0 -} diff --git a/cmd/spacedock-release/shallow_boot_window_record_test.go b/cmd/spacedock-release/shallow_boot_window_record_test.go deleted file mode 100644 index 4148cc8dd..000000000 --- a/cmd/spacedock-release/shallow_boot_window_record_test.go +++ /dev/null @@ -1,91 +0,0 @@ -package main - -import ( - "encoding/json" - "os" - "path/filepath" - "testing" - - "github.com/spacedock-dev/spacedock/internal/journeymetrics" -) - -const shallowBootWindowFixtureStream = `{"type":"system","subtype":"init","model":"claude-test-model"} -{"type":"assistant","message":{"id":"msg_1","model":"claude-test-model","usage":{"input_tokens":10,"output_tokens":5,"cache_creation_input_tokens":100,"cache_read_input_tokens":50},"content":[{"type":"tool_use","id":"toolu_1","name":"Bash","input":{"command":"echo hi"}}]}} -{"type":"assistant","message":{"id":"msg_2","model":"claude-test-model","usage":{"input_tokens":20,"output_tokens":8,"cache_creation_input_tokens":200,"cache_read_input_tokens":90},"content":[{"type":"text","text":"Hello!"}]}} -` - -// TestShallowBootWindowRecordCommandExtractsFromArchivedStream is AC-4's proof -// that the CLI applies AC-1's extraction logic (BuildShallowBootWindowRecord) to -// a standalone archived claude-stream.jsonl — the mechanism the release-ledger -// backfill uses instead of duplicating extraction logic. The fixture's greet -// turn is turns[1] (the only turn with no dispatch tool_use), so Turns == 2 and -// Tokens carries turns[1]'s full usage. -func TestShallowBootWindowRecordCommandExtractsFromArchivedStream(t *testing.T) { - streamPath := filepath.Join(t.TempDir(), "claude-stream.jsonl") - if err := os.WriteFile(streamPath, []byte(shallowBootWindowFixtureStream), 0o644); err != nil { - t.Fatal(err) - } - outDir := t.TempDir() - - if code := shallowBootWindowRecord([]string{streamPath, "--model", "claude-test-model", "--out", outDir}); code != 0 { - t.Fatalf("shallowBootWindowRecord exit = %d, want 0", code) - } - - entries, err := os.ReadDir(outDir) - if err != nil { - t.Fatal(err) - } - if len(entries) != 1 { - t.Fatalf("expected exactly one emitted record file, got %d: %+v", len(entries), entries) - } - data, err := os.ReadFile(filepath.Join(outDir, entries[0].Name())) - if err != nil { - t.Fatal(err) - } - var record journeymetrics.Record - if err := json.Unmarshal(data, &record); err != nil { - t.Fatal(err) - } - if record.ScenarioID != "shallow-boot-window" { - t.Fatalf("ScenarioID = %q, want shallow-boot-window", record.ScenarioID) - } - if record.Turns != 2 { - t.Fatalf("Turns = %d, want 2", record.Turns) - } - want := journeymetrics.TokenTotals{Input: 20, Output: 8, CacheCreation: 200, CacheRead: 90, Total: 318} - if record.Tokens != want { - t.Fatalf("Tokens = %+v, want %+v", record.Tokens, want) - } -} - -// TestShallowBootWindowRecordCommandRejectsMissingArgs proves a miswired -// invocation (missing the required stream/--model/--out) is a usage error, not a -// silent no-op that would leave the AC-4 backfill believing extraction ran. -func TestShallowBootWindowRecordCommandRejectsMissingArgs(t *testing.T) { - if code := shallowBootWindowRecord(nil); code == 0 { - t.Fatalf("shallowBootWindowRecord exit = 0 with no arguments; want non-zero") - } -} - -// TestShallowBootWindowRecordCommandRejectsMissingStreamFile proves an -// unreadable stream path fails loud instead of emitting an empty/zero record. -func TestShallowBootWindowRecordCommandRejectsMissingStreamFile(t *testing.T) { - code := shallowBootWindowRecord([]string{filepath.Join(t.TempDir(), "does-not-exist.jsonl"), "--model", "claude-test-model", "--out", t.TempDir()}) - if code == 0 { - t.Fatalf("shallowBootWindowRecord exit = 0 with a missing stream file; want non-zero") - } -} - -// TestShallowBootWindowRecordCommandRejectsStreamWithNoAssistantTurns proves a -// stream that produced no assistant turns at all (nothing to extract a -// boot-window observation from) is rejected rather than silently skipped. -func TestShallowBootWindowRecordCommandRejectsStreamWithNoAssistantTurns(t *testing.T) { - streamPath := filepath.Join(t.TempDir(), "claude-stream.jsonl") - if err := os.WriteFile(streamPath, []byte(`{"type":"system","subtype":"init","model":"claude-test-model"}`+"\n"), 0o644); err != nil { - t.Fatal(err) - } - code := shallowBootWindowRecord([]string{streamPath, "--model", "claude-test-model", "--out", t.TempDir()}) - if code == 0 { - t.Fatalf("shallowBootWindowRecord exit = 0 on a stream with no assistant turns; want non-zero") - } -} diff --git a/internal/ensigncycle/journey_metrics_live_test.go b/internal/ensigncycle/journey_metrics_live_test.go index c941b4192..471afe6b7 100644 --- a/internal/ensigncycle/journey_metrics_live_test.go +++ b/internal/ensigncycle/journey_metrics_live_test.go @@ -89,7 +89,7 @@ func emitShallowBootWindowMetrics(t *testing.T, stream string, model string) { if err != nil { t.Fatalf("parse Claude turns for shallow-boot-window: %v", err) } - record, err := BuildShallowBootWindowRecord(turns, model) + 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) } diff --git a/internal/ensigncycle/shallow_boot_measure_unit_test.go b/internal/ensigncycle/shallow_boot_measure_unit_test.go index cc655adc7..25c7f4c0b 100644 --- a/internal/ensigncycle/shallow_boot_measure_unit_test.go +++ b/internal/ensigncycle/shallow_boot_measure_unit_test.go @@ -145,7 +145,14 @@ func TestParserExtractsTeamCallsFromRealHangCapture(t *testing.T) { // 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. +// 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)) @@ -156,9 +163,17 @@ func TestBuildShallowBootWindowRecord(t *testing.T) { 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) + record, err := BuildShallowBootWindowRecord(turns, model, claudeCodeVersion, resolvedModel) if err != nil { t.Fatalf("BuildShallowBootWindowRecord: %v", err) } @@ -173,6 +188,20 @@ func TestBuildShallowBootWindowRecord(t *testing.T) { 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. @@ -227,10 +256,14 @@ func TestShallowBootMeasureSignalsAreIndependent(t *testing.T) { 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"); err != nil { + 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) } // Formerly failed only the spike check: a pre-greet dispatch turn carrying the @@ -242,10 +275,14 @@ func TestShallowBootMeasureSignalsAreIndependent(t *testing.T) { 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 { + 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. @@ -257,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 index a20229ce1..ae082f502 100644 --- a/internal/ensigncycle/shallow_boot_window_record.go +++ b/internal/ensigncycle/shallow_boot_window_record.go @@ -49,16 +49,55 @@ func greetTurnIndex(turns []journeymetrics.ClaudeTurn) int { 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 both the former -// ceiling signal (Context() = input+cache_read+cache_creation) and the former -// pre-greet-spike signal (cache_creation) from this one recorded observation. +// 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 CLI can apply the SAME extraction logic to archived streams, // standalone, rather than duplicating it. -func BuildShallowBootWindowRecord(turns []journeymetrics.ClaudeTurn, model string) (journeymetrics.Record, error) { +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") } @@ -75,7 +114,11 @@ func BuildShallowBootWindowRecord(turns []journeymetrics.ClaudeTurn, model strin Host: "claude", Model: model, }, journeymetrics.BehaviorResult{Passed: true}, journeymetrics.Observation{ - Turns: greet + 1, - Tokens: turns[greet].Usage, + 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 a6f8847f4..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) @@ -137,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 04563a616..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,29 +122,41 @@ 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 @@ -130,34 +165,49 @@ type Record struct { 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"` - RunID string `json:"run_id,omitempty"` - RunURL string `json:"run_url,omitempty"` - CapturedAt string `json:"captured_at,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) { @@ -166,36 +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, - RunID: r.RunID, - RunURL: r.RunURL, - CapturedAt: r.CapturedAt, + 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 index f03a55573..80b4e1c36 100644 --- a/internal/release/journey_delta_workflow_test.go +++ b/internal/release/journey_delta_workflow_test.go @@ -1,7 +1,6 @@ package release import ( - "strings" "testing" "gopkg.in/yaml.v3" @@ -40,27 +39,3 @@ func TestRuntimeLiveWorkflowJourneyDeltaJobHasPRCommentPermission(t *testing.T) t.Fatal("runtime-live-e2e.yml journey-delta-comment job does not invoke the journey-delta CLI") } } - -// TestRuntimeLiveWorkflowJourneyDeltaJobRejectsMissingPermission is the -// adversarial twin: stripping the job's permissions block must trip the guard, -// proving the check is load-bearing rather than vacuously true. -func TestRuntimeLiveWorkflowJourneyDeltaJobRejectsMissingPermission(t *testing.T) { - live := readWorkflow(t, "runtime-live-e2e.yml") - const permBlock = " permissions:\n contents: read\n actions: read\n pull-requests: write\n" - if strings.Count(live, permBlock) != 1 { - t.Fatalf("expected exactly 1 occurrence of the journey-delta-comment permissions block, found %d", strings.Count(live, permBlock)) - } - adversarial := strings.Replace(live, permBlock, "", 1) - - var doc struct { - Jobs map[string]struct { - Permissions map[string]string `yaml:"permissions"` - } `yaml:"jobs"` - } - if err := yaml.Unmarshal([]byte(adversarial), &doc); err != nil { - t.Fatalf("parse adversarial workflow: %v", err) - } - if doc.Jobs["journey-delta-comment"].Permissions["pull-requests"] == "write" { - t.Fatal("mutation did not actually remove the permissions block") - } -} diff --git a/internal/release/journey_workflow_test.go b/internal/release/journey_workflow_test.go index 984e000d9..14b73f881 100644 --- a/internal/release/journey_workflow_test.go +++ b/internal/release/journey_workflow_test.go @@ -731,6 +731,57 @@ func TestReleaseDownloadStepGuardRejectsFlatCopyRegression(t *testing.T) { } } +// 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) + } +} + // 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 index 04677c71f..7c2d918c8 100644 --- a/internal/release/journeydelta.go +++ b/internal/release/journeydelta.go @@ -116,44 +116,56 @@ func ComputeJourneyDeltas(baseline journeymetrics.Ledger, current []journeymetri return deltas } -// journeyDeltaCommentMarker is stamped into every rendered comment. It has no -// behavioral role in THIS process — the sticky-update behavior comes from `gh -// pr comment --edit-last` editing the poster's own last comment — but it lets a -// human (or a future automated check) identify the comment's origin at a -// glance. -const journeyDeltaCommentMarker = "" +// 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, each cell an exact (PR value - baseline value) delta. +// 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(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 Δ | Tokens Δ | Cost Δ (USD) | Baseline |\n") - b.WriteString("| --- | --- | --- | --- | --- | --- | --- |\n") + b.WriteString("| Scenario | Runtime | Model | Turns Δ | Cache Read Δ | Cache Creation Δ | Tokens Δ (total) | Cost Δ (USD) | Baseline |\n") + b.WriteString("| --- | --- | --- | --- | --- | --- | --- | --- | --- |\n") for _, d := range deltas { - baseline := "none (new observation)" - switch { - case d.HasBaseline && d.BaselineRunURL != "": + if !d.HasBaseline { + fmt.Fprintf(&b, "| %s | %s | %s | 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) - case d.HasBaseline: - baseline = "latest published" } - fmt.Fprintf(&b, "| %s | %s | %s | %+d | %+d | %+.4f | %s |\n", - d.ScenarioID, d.Runtime, d.Model, d.TurnsDelta, d.TokensDelta.Total, d.CostDeltaUSD, baseline) + fmt.Fprintf(&b, "| %s | %s | %s | %+d | %+d | %+d | %+d | %+.4f | %s |\n", + d.ScenarioID, d.Runtime, d.Model, d.TurnsDelta, d.TokensDelta.CacheRead, d.TokensDelta.CacheCreation, d.TokensDelta.Total, d.CostDeltaUSD, baseline) } return b.String() } -// JourneyDeltaCommentArgs returns the `gh pr comment` argv the AC-3 step -// invokes to post (or update) the PR delta comment. --edit-last edits the -// poster's own last comment on the PR instead of appending a new one on every -// push; --create-if-none falls back to creating the first comment when none -// exists yet, so the FIRST post on a PR still lands. -func JourneyDeltaCommentArgs(prNumber, bodyFile string) []string { - return []string{"pr", "comment", prNumber, "--body-file", bodyFile, "--edit-last", "--create-if-none"} +// 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 index b9df310cc..777531fb6 100644 --- a/internal/release/journeydelta_test.go +++ b/internal/release/journeydelta_test.go @@ -160,7 +160,7 @@ func TestRenderJourneyDeltaCommentIncludesExactDeltasAndMarker(t *testing.T) { }, } body := RenderJourneyDeltaComment(deltas) - if !strings.HasPrefix(body, journeyDeltaCommentMarker) { + 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"} { @@ -170,14 +170,51 @@ func TestRenderJourneyDeltaCommentIncludesExactDeltasAndMarker(t *testing.T) { } } -// TestJourneyDeltaCommentArgsUseStickyEditLast proves the gh argv this AC-3 step -// invokes always carries --edit-last --create-if-none, on every call — the -// mechanism that makes repeated pushes update ONE comment instead of appending -// a new one each time. -func TestJourneyDeltaCommentArgsUseStickyEditLast(t *testing.T) { - args := JourneyDeltaCommentArgs("42", "/tmp/comment.md") +// 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{"pr comment", "42", "--body-file /tmp/comment.md", "--edit-last", "--create-if-none"} { + 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) } diff --git a/internal/release/ledgerdiff.go b/internal/release/ledgerdiff.go deleted file mode 100644 index e5459cecd..000000000 --- a/internal/release/ledgerdiff.go +++ /dev/null @@ -1,67 +0,0 @@ -// ABOUTME: Structural pre-upload safeguard for the AC-4 release-ledger backfill — -// ABOUTME: proves a rebuilt ledger equals the original plus exactly the added scenarios. -package release - -import ( - "encoding/json" - "fmt" - "slices" - "sort" - - "github.com/spacedock-dev/spacedock/internal/journeymetrics" -) - -// DiffAddedScenarios is the AC-4 pre-upload safeguard: it proves a rebuilt -// journey-cost ledger equals the backed-up original ledger's scenarios array -// PLUS EXACTLY the named added scenario ids, with every pre-existing scenario -// byte-for-byte unchanged. Rebuilding via `journey-costs` re-normalizes every -// historical record (normalizeRecord restamps schema_version and recomputes -// token totals on every record it reads), so a rebuild quirk could otherwise -// silently mutate already-published historical data that has nothing to do -// with the backfill. It returns a descriptive error on ANY deviation — a -// changed, missing, or unexpectedly-extra scenario — so the caller can abort -// BEFORE an upload instead of publishing a corrupted ledger. -func DiffAddedScenarios(original, rebuilt journeymetrics.Ledger, wantAdded ...string) error { - originalByID := scenarioByID(original) - rebuiltByID := scenarioByID(rebuilt) - - for id, origEntry := range originalByID { - rebuiltEntry, ok := rebuiltByID[id] - if !ok { - return fmt.Errorf("scenario %q present in the original ledger is MISSING from the rebuilt ledger", id) - } - origJSON, err := json.Marshal(origEntry) - if err != nil { - return fmt.Errorf("marshal original scenario %q: %w", id, err) - } - rebuiltJSON, err := json.Marshal(rebuiltEntry) - if err != nil { - return fmt.Errorf("marshal rebuilt scenario %q: %w", id, err) - } - if string(origJSON) != string(rebuiltJSON) { - return fmt.Errorf("scenario %q changed by the rebuild (a rebuild must only ADD the backfilled scenario, never alter an existing one) — original:\n%s\nrebuilt:\n%s", id, origJSON, rebuiltJSON) - } - } - - var extra []string - for id := range rebuiltByID { - if _, ok := originalByID[id]; !ok { - extra = append(extra, id) - } - } - sort.Strings(extra) - wantSorted := append([]string(nil), wantAdded...) - sort.Strings(wantSorted) - if !slices.Equal(extra, wantSorted) { - return fmt.Errorf("rebuilt ledger added scenarios %v, want exactly %v", extra, wantSorted) - } - return nil -} - -func scenarioByID(ledger journeymetrics.Ledger) map[string]journeymetrics.ScenarioLedgerEntry { - out := make(map[string]journeymetrics.ScenarioLedgerEntry, len(ledger.Scenarios)) - for _, entry := range ledger.Scenarios { - out[entry.ScenarioID] = entry - } - return out -} diff --git a/internal/release/ledgerdiff_test.go b/internal/release/ledgerdiff_test.go deleted file mode 100644 index 01637a0f2..000000000 --- a/internal/release/ledgerdiff_test.go +++ /dev/null @@ -1,110 +0,0 @@ -package release - -import ( - "strings" - "testing" - - "github.com/spacedock-dev/spacedock/internal/journeymetrics" -) - -func fixtureScenario(id string, turns int) journeymetrics.ScenarioLedgerEntry { - return journeymetrics.ScenarioLedgerEntry{ - ScenarioID: id, - Source: "live-harness", - Observations: []journeymetrics.Record{ - {ScenarioID: id, Runtime: "claude", Model: "claude-sonnet-4-6", Turns: turns, Outcome: journeymetrics.Outcome{Status: "passed"}}, - }, - } -} - -// TestDiffAddedScenariosAcceptsExactlyOneAddedScenario is AC-4's primary safety -// proof: a rebuilt ledger that equals the original PLUS exactly one new -// shallow-boot-window entry, with every pre-existing scenario byte-identical, -// must pass. -func TestDiffAddedScenariosAcceptsExactlyOneAddedScenario(t *testing.T) { - original := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ - fixtureScenario("gate-guardrail", 5), - fixtureScenario("shallow-boot", 3), - }} - rebuilt := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ - fixtureScenario("gate-guardrail", 5), - fixtureScenario("shallow-boot", 3), - fixtureScenario("shallow-boot-window", 3), - }} - if err := DiffAddedScenarios(original, rebuilt, "shallow-boot-window"); err != nil { - t.Fatalf("expected the exactly-one-addition case to pass, got: %v", err) - } -} - -// TestDiffAddedScenariosRejectsMutatedExistingScenario is AC-4's proof that a -// rebuild quirk mutating a pre-existing historical scenario (e.g. normalizeRecord -// restamping schema_version or recomputing token totals differently) is caught, -// not silently republished. -func TestDiffAddedScenariosRejectsMutatedExistingScenario(t *testing.T) { - original := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ - fixtureScenario("gate-guardrail", 5), - }} - rebuilt := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ - fixtureScenario("gate-guardrail", 6), // mutated: Turns 5 -> 6 - fixtureScenario("shallow-boot-window", 3), - }} - err := DiffAddedScenarios(original, rebuilt, "shallow-boot-window") - if err == nil { - t.Fatal("expected the mutated pre-existing scenario to be rejected") - } - if !strings.Contains(err.Error(), "gate-guardrail") { - t.Fatalf("error does not name the mutated scenario: %v", err) - } -} - -// TestDiffAddedScenariosRejectsMissingExistingScenario proves a rebuild that -// silently DROPPED a pre-existing scenario is caught. -func TestDiffAddedScenariosRejectsMissingExistingScenario(t *testing.T) { - original := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ - fixtureScenario("gate-guardrail", 5), - fixtureScenario("shallow-boot", 3), - }} - rebuilt := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ - fixtureScenario("shallow-boot", 3), - fixtureScenario("shallow-boot-window", 3), - }} - err := DiffAddedScenarios(original, rebuilt, "shallow-boot-window") - if err == nil { - t.Fatal("expected the dropped pre-existing scenario to be rejected") - } - if !strings.Contains(err.Error(), "gate-guardrail") { - t.Fatalf("error does not name the missing scenario: %v", err) - } -} - -// TestDiffAddedScenariosRejectsUnexpectedExtraScenario proves a rebuild that -// added something OTHER than (or in addition to) the expected scenario is -// caught, rather than treated as a benign superset. -func TestDiffAddedScenariosRejectsUnexpectedExtraScenario(t *testing.T) { - original := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ - fixtureScenario("gate-guardrail", 5), - }} - rebuilt := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ - fixtureScenario("gate-guardrail", 5), - fixtureScenario("shallow-boot-window", 3), - fixtureScenario("mystery-scenario", 1), - }} - if err := DiffAddedScenarios(original, rebuilt, "shallow-boot-window"); err == nil { - t.Fatal("expected an unexpected extra scenario beyond the named addition to be rejected") - } -} - -// TestDiffAddedScenariosRejectsMissingExpectedAddition proves a rebuild that did -// NOT actually add the expected scenario (e.g. the extraction silently no-opped) -// is caught rather than passing vacuously. -func TestDiffAddedScenariosRejectsMissingExpectedAddition(t *testing.T) { - original := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ - fixtureScenario("gate-guardrail", 5), - }} - rebuilt := journeymetrics.Ledger{Scenarios: []journeymetrics.ScenarioLedgerEntry{ - fixtureScenario("gate-guardrail", 5), - }} - if err := DiffAddedScenarios(original, rebuilt, "shallow-boot-window"); err == nil { - t.Fatal("expected a rebuild that did not add the expected scenario to be rejected") - } -} From cb5b0cf98e50d984bcd3734b44403f17f274228d Mon Sep 17 00:00:00 2001 From: Spike Test Date: Fri, 3 Jul 2026 11:39:47 +0800 Subject: [PATCH 6/7] C1: close cross-step wiring gap between Locate and Post journey-delta steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validation's cycle-3 adversarial audit found that reverting ONLY the "Post the journey-cost delta PR comment" step's --metrics-dir back to the old broken hardcoded path left the full suite green — nothing verified the Post step stayed wired to the "Locate this run's journey metrics" step's actual output directory. Same class of gap Cycle 1 found: a step's own mechanism tested in isolation, the wiring between two steps not. Adds TestJourneyDeltaLocateAndPostStepsShareMetricsDir, which extracts both steps' real run: blocks and derives each step's directory value from the actual script text (not a hardcoded assumption), then asserts they match. Verified it catches the exact adversarial mutation validation performed. Also folds in the two minors from the same review: fixes a stale "backfill CLI" doc-comment reference in shallow_boot_window_record.go left over from the AC-4 TRIM (should read "runbook"), and adds TestRenderJourneyDeltaCommentShowsTokenClassBreakdown, which checks the PR comment's Cache Read Δ / Cache Creation Δ table cells land in the correct (non-swapped) columns — verified it catches a reversion to the old Turns/Tokens-total/Cost table shape. --- .../ensigncycle/shallow_boot_window_record.go | 6 +-- internal/release/journey_workflow_test.go | 39 +++++++++++++++ internal/release/journeydelta_test.go | 48 +++++++++++++++++++ 3 files changed, 90 insertions(+), 3 deletions(-) diff --git a/internal/ensigncycle/shallow_boot_window_record.go b/internal/ensigncycle/shallow_boot_window_record.go index ae082f502..19218cf4b 100644 --- a/internal/ensigncycle/shallow_boot_window_record.go +++ b/internal/ensigncycle/shallow_boot_window_record.go @@ -1,5 +1,5 @@ // ABOUTME: Boot-window telemetry extraction shared by the live shallow-boot -// ABOUTME: scenario test and the release-ledger backfill CLI (AC-1/AC-4). +// ABOUTME: scenario test and the release-ledger backfill runbook (AC-1/AC-4). package ensigncycle import ( @@ -95,8 +95,8 @@ func preGreetPeakCacheCreation(turns []journeymetrics.ClaudeTurn, greet int) int // 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 CLI can apply the SAME extraction logic to archived streams, -// standalone, rather than duplicating it. +// 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") diff --git a/internal/release/journey_workflow_test.go b/internal/release/journey_workflow_test.go index 14b73f881..213711519 100644 --- a/internal/release/journey_workflow_test.go +++ b/internal/release/journey_workflow_test.go @@ -782,6 +782,45 @@ func TestJourneyDeltaLocateMetricsStepFindsNestedJourneyMetricsFiles(t *testing. } } +// 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_test.go b/internal/release/journeydelta_test.go index 777531fb6..b9b7b0688 100644 --- a/internal/release/journeydelta_test.go +++ b/internal/release/journeydelta_test.go @@ -170,6 +170,54 @@ func TestRenderJourneyDeltaCommentIncludesExactDeltasAndMarker(t *testing.T) { } } +// 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) | Baseline | (empty) | + if len(cells) != 11 { + t.Fatalf("data row has %d cells, want 11 (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) + } +} + // 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 From d89c8f7d042b13e4c466aa09830fa68ea9217159 Mon Sep 17 00:00:00 2001 From: Spike Test Date: Fri, 3 Jul 2026 15:22:13 +0800 Subject: [PATCH 7/7] Add wallclock/duration delta to per-PR journey-delta comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend JourneyDelta with DurationMSDelta, computed in ComputeJourneyDeltas from the existing journeymetrics.Record.DurationMS field, and render it as a new Duration Δ (ms) column in RenderJourneyDeltaComment. --- internal/release/journeydelta.go | 22 ++++++----- internal/release/journeydelta_test.go | 57 +++++++++++++++++++++++++-- 2 files changed, 66 insertions(+), 13 deletions(-) diff --git a/internal/release/journeydelta.go b/internal/release/journeydelta.go index 7c2d918c8..850e2f9d2 100644 --- a/internal/release/journeydelta.go +++ b/internal/release/journeydelta.go @@ -62,11 +62,12 @@ type JourneyDelta struct { ScenarioID string Runtime string Model string - HasBaseline bool - BaselineRunURL string - TurnsDelta int - TokensDelta journeymetrics.TokenTotals - CostDeltaUSD float64 + HasBaseline bool + BaselineRunURL string + TurnsDelta int + DurationMSDelta int64 + TokensDelta journeymetrics.TokenTotals + CostDeltaUSD float64 } // ComputeJourneyDeltas computes, for every scenario/runtime/model the current @@ -94,6 +95,7 @@ func ComputeJourneyDeltas(baseline journeymetrics.Ledger, current []journeymetri 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, @@ -138,11 +140,11 @@ func RenderJourneyDeltaComment(deltas []JourneyDelta) string { 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) | Baseline |\n") - b.WriteString("| --- | --- | --- | --- | --- | --- | --- | --- | --- |\n") + 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) | none (new observation) |\n", + 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 } @@ -150,8 +152,8 @@ func RenderJourneyDeltaComment(deltas []JourneyDelta) string { if d.BaselineRunURL != "" { baseline = fmt.Sprintf("[latest published](%s)", d.BaselineRunURL) } - fmt.Fprintf(&b, "| %s | %s | %s | %+d | %+d | %+d | %+d | %+.4f | %s |\n", - d.ScenarioID, d.Runtime, d.Model, d.TurnsDelta, d.TokensDelta.CacheRead, d.TokensDelta.CacheCreation, d.TokensDelta.Total, d.CostDeltaUSD, baseline) + 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() } diff --git a/internal/release/journeydelta_test.go b/internal/release/journeydelta_test.go index b9b7b0688..c7b0d942b 100644 --- a/internal/release/journeydelta_test.go +++ b/internal/release/journeydelta_test.go @@ -200,9 +200,9 @@ func TestRenderJourneyDeltaCommentShowsTokenClassBreakdown(t *testing.T) { 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) | Baseline | (empty) | - if len(cells) != 11 { - t.Fatalf("data row has %d cells, want 11 (header shape changed?): %q", len(cells), 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]) @@ -218,6 +218,57 @@ func TestRenderJourneyDeltaCommentShowsTokenClassBreakdown(t *testing.T) { } } +// 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