Skip to content

Commit cb5b0cf

Browse files
author
Spike Test
committed
C1: close cross-step wiring gap between Locate and Post journey-delta steps
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.
1 parent 06094d5 commit cb5b0cf

3 files changed

Lines changed: 90 additions & 3 deletions

File tree

internal/ensigncycle/shallow_boot_window_record.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// ABOUTME: Boot-window telemetry extraction shared by the live shallow-boot
2-
// ABOUTME: scenario test and the release-ledger backfill CLI (AC-1/AC-4).
2+
// ABOUTME: scenario test and the release-ledger backfill runbook (AC-1/AC-4).
33
package ensigncycle
44

55
import (
@@ -95,8 +95,8 @@ func preGreetPeakCacheCreation(turns []journeymetrics.ClaudeTurn, greet int) int
9595
// silent alias-resolution change (e.g. "sonnet" moving to a new snapshot)
9696
// rather than the FO's own contract.
9797
// Exported (unlike the live-scenario test glue around it) so the AC-4 release-
98-
// ledger backfill CLI can apply the SAME extraction logic to archived streams,
99-
// standalone, rather than duplicating it.
98+
// ledger backfill runbook's throwaway `go run` script can apply the SAME
99+
// extraction logic to archived streams, standalone, rather than duplicating it.
100100
func BuildShallowBootWindowRecord(turns []journeymetrics.ClaudeTurn, model string, claudeCodeVersion string, resolvedModel string) (journeymetrics.Record, error) {
101101
if len(turns) == 0 {
102102
return journeymetrics.Record{}, fmt.Errorf("stream carried no assistant turns — nothing to record")

internal/release/journey_workflow_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -782,6 +782,45 @@ func TestJourneyDeltaLocateMetricsStepFindsNestedJourneyMetricsFiles(t *testing.
782782
}
783783
}
784784

785+
// TestJourneyDeltaLocateAndPostStepsShareMetricsDir closes the cross-step
786+
// wiring gap validation's cycle-3 adversarial audit found: the "Locate this
787+
// run's journey metrics" step's own mechanism is tested in isolation above,
788+
// but nothing previously verified that the "Post the journey-cost delta PR
789+
// comment" step's --metrics-dir argument stays wired to the Locate step's
790+
// actual output directory. A one-line regression reverting ONLY the Post
791+
// step's --metrics-dir back to the old hardcoded broken path left the full
792+
// suite green, because each step's script was only ever checked in isolation.
793+
// This derives BOTH directories from the REAL extracted step scripts (not a
794+
// hardcoded assumption on either side) and asserts they're the same value.
795+
func TestJourneyDeltaLocateAndPostStepsShareMetricsDir(t *testing.T) {
796+
live := readWorkflow(t, "runtime-live-e2e.yml")
797+
locateScript := extractStepRun(t, live, "Locate this run's journey metrics")
798+
postScript := extractStepRun(t, live, "Post the journey-cost delta PR comment")
799+
800+
locateDir := firstQuotedArg(t, locateScript, `mkdir -p "`)
801+
postDir := firstQuotedArg(t, postScript, `--metrics-dir "`)
802+
if locateDir != postDir {
803+
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)
804+
}
805+
}
806+
807+
// firstQuotedArg extracts the double-quoted value immediately following the
808+
// given prefix (e.g. `mkdir -p "`) in script, failing the test if the prefix
809+
// or its closing quote is not found.
810+
func firstQuotedArg(t *testing.T, script, prefix string) string {
811+
t.Helper()
812+
start := strings.Index(script, prefix)
813+
if start < 0 {
814+
t.Fatalf("script does not contain %q:\n%s", prefix, script)
815+
}
816+
start += len(prefix)
817+
end := strings.Index(script[start:], `"`)
818+
if end < 0 {
819+
t.Fatalf("unterminated quoted argument after %q:\n%s", prefix, script)
820+
}
821+
return script[start : start+end]
822+
}
823+
785824
// extractStepRun pulls a named step's `run: |` block out of a workflow document,
786825
// dedented to a runnable shell script, so tests exercise the EXACT script CI
787826
// runs rather than a hand-copied duplicate.

internal/release/journeydelta_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,54 @@ func TestRenderJourneyDeltaCommentIncludesExactDeltasAndMarker(t *testing.T) {
170170
}
171171
}
172172

173+
// TestRenderJourneyDeltaCommentShowsTokenClassBreakdown proves the comment
174+
// renders Cache Read Δ and Cache Creation Δ as their OWN columns, in the
175+
// correct (non-swapped) position — not collapsed into the Tokens Δ (total)
176+
// figure. cache_read and cache_creation differ ~12x in cost and meaning for a
177+
// boot metric, so folding them into one number hides the signal this Minor
178+
// exists to surface. Uses distinguishable, easily-confused-if-swapped values
179+
// (CacheRead=111, CacheCreation=222) and checks the actual table row's cell
180+
// positions, not just substring presence anywhere in the body (which would
181+
// not catch a column swap).
182+
func TestRenderJourneyDeltaCommentShowsTokenClassBreakdown(t *testing.T) {
183+
deltas := []JourneyDelta{
184+
{
185+
ScenarioID: "shallow-boot-window", Runtime: "claude", Model: "claude-sonnet-4-6",
186+
HasBaseline: true, BaselineRunURL: "https://github.com/spacedock-dev/spacedock/actions/runs/27931963802",
187+
TurnsDelta: 2, TokensDelta: journeymetrics.TokenTotals{CacheRead: 111, CacheCreation: 222, Total: 1034}, CostDeltaUSD: 0.15,
188+
},
189+
}
190+
body := RenderJourneyDeltaComment(deltas)
191+
192+
var row string
193+
for _, line := range strings.Split(body, "\n") {
194+
if strings.HasPrefix(line, "| shallow-boot-window") {
195+
row = line
196+
break
197+
}
198+
}
199+
if row == "" {
200+
t.Fatalf("comment body missing the data row:\n%s", body)
201+
}
202+
cells := strings.Split(row, "|")
203+
// | (empty) | Scenario | Runtime | Model | Turns Δ | Cache Read Δ | Cache Creation Δ | Tokens Δ (total) | Cost Δ (USD) | Baseline | (empty) |
204+
if len(cells) != 11 {
205+
t.Fatalf("data row has %d cells, want 11 (header shape changed?): %q", len(cells), row)
206+
}
207+
cacheReadCell := strings.TrimSpace(cells[5])
208+
cacheCreationCell := strings.TrimSpace(cells[6])
209+
tokensTotalCell := strings.TrimSpace(cells[7])
210+
if cacheReadCell != "+111" {
211+
t.Fatalf("Cache Read Δ cell = %q, want +111 (got %q for Cache Creation Δ) — columns may be swapped", cacheReadCell, cacheCreationCell)
212+
}
213+
if cacheCreationCell != "+222" {
214+
t.Fatalf("Cache Creation Δ cell = %q, want +222 (got %q for Cache Read Δ) — columns may be swapped", cacheCreationCell, cacheReadCell)
215+
}
216+
if tokensTotalCell != "+1034" {
217+
t.Fatalf("Tokens Δ (total) cell = %q, want +1034", tokensTotalCell)
218+
}
219+
}
220+
173221
// TestRenderJourneyDeltaCommentRendersNoBaselineAsNewNotSelfDelta proves a
174222
// scenario/model with no matching baseline observation renders "n/a (new)" in
175223
// its delta cells rather than a self-delta against an implicit zero baseline

0 commit comments

Comments
 (0)