Skip to content

test(storyboard): refactor cascade-skip-on-skip to (skip_reason × peer_shape × topology) matrix (#1548)#1587

Merged
bokelley merged 3 commits intomainfrom
bokelley/issue-1548-cascade-skip-matrix-refactor
May 8, 2026
Merged

test(storyboard): refactor cascade-skip-on-skip to (skip_reason × peer_shape × topology) matrix (#1548)#1587
bokelley merged 3 commits intomainfrom
bokelley/issue-1548-cascade-skip-matrix-refactor

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 8, 2026

Summary

Refactors test/lib/storyboard-cascade-skip-on-skip.test.js from prose-driven scenarios (one named test per scenario, ~2000 lines, 35 tests) to a fixture-table matrix keyed by the three orthogonal dimensions of cascade behavior:

skip_reason     ∈ { missing_tool, missing_test_controller, not_applicable, real_failure }
peer_shape      ∈ { sole_stateful, peer_passes, peer_fails, peer_substitute_declared, peer_substitute_missing }
phase_topology  ∈ { same_phase, downstream_phase }

PR #1545 fixed an asymmetric blind spot — the sole-stateful-step exemption applied to not_applicable (#1146) but not to missing_tool. The parallel nodejs-testing-expert review of #1545 traced the cause to test architecture: prose-driven scenarios hide empty cells in the implicit cube until production hits one. This refactor makes those cells structurally visible.

What's in the new file

  • 24 matrix rows — every historical cascade-behavior scenario is now a named matrix row tagged with its (skip_reason, peer_shape, phase_topology) triple. Test names carry the tag prefix so failures point straight at the cube cell.
  • 1 control row — the no-skip-occurs sanity check.
  • 9 parse-time validation rowspeer_substitutes_for / depends_on malformed-input rejection assertions.
  • 1 parity-invariant test — the loop PR fix(storyboard): sole stateful step missing_tool no longer trips cascade #1545 introduced for {missing_tool, not_applicable} × sole_stateful × downstream_phase. Add new exemption-family reasons here when they land.
  • 1 coverage-holes assertion — partitions the 40-cell cube into covered / impossible / tracked / empty. Fails CI when a new axis value is introduced and left uncovered.

Total: 36 tests pass (was 35 — added the coverage assertion).

Matrix coverage map

Bucket Count Notes
Covered by ≥1 row 11 dedup'd from 24 rows; multiple rows can target the same cell with different fixtures
Structurally impossible 4 real_failure × peer_substitute_*peer_substitutes_for rescues skips, not failures
Tracked elsewhere or in follow-up 25 mostly missing_test_controller × * (covered in storyboard-controller-seeding.test.js) and real_failure × peer_passes/peer_fails × *_phase (covered in storyboard-runner.test.js)
Empty (uncovered, untracked) 0 the assertion at the bottom fails if this is ever > 0

Reproduce locally: MATRIX_REPORT=1 node --test test/lib/storyboard-cascade-skip-on-skip.test.js prints the full breakdown.

Net line shrink

Lines
Before 2143
After 1256
Delta −887 (−41%)

Shrink comes from extracting the repeated fakeAgent setup, storyboardWith[Phases] boilerplate, and 35 near-identical runStoryboard invocations into three small helpers (step, runWith, plus the existing storyboardWith[Phases]). No assertions dropped — every original assert.* call has a 1:1 successor in a row's assert(result) callback.

What's NOT in this PR

References

  • adcp-client#1548 — this issue
  • adcp-client#1545 — bug + fix that surfaced the test-architecture gap
  • adcp-client#1146 — original not_applicable exemption
  • adcp#4053 — spec coordination for the exemption rule

Test plan

  • npm run format:check clean
  • node --test test/lib/storyboard-cascade-skip-on-skip.test.js — 36 / 36 pass
  • npm test — 8253 / 8256 pass (3 pre-existing skipped, 0 new failures)
  • Coverage assertion fires correctly when a row is removed (manually verified the partition by inspecting MATRIX_REPORT=1 output)

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com

bokelley and others added 2 commits May 8, 2026 06:57
…uarantees (#1384)

Sweep follow-up to #1327 for typed handler entrypoints in
`src/lib/server/decisioning/`. Audited each ctx field surfaced via
`RequestContext` against the dispatch reality in
`runtime/to-context.ts` + `runtime/from-platform.ts`.

Found one mismatch: both `CreativeBuilderPlatform` and
`CreativeAdServerPlatform` declared `listCreativeFormats` twice — once
correctly with `NoAccountCtx<TCtxMeta>` and once with `Ctx<TCtxMeta>`
(claiming `account` non-optional). TS overload resolution preserved the
narrow at the implementation site, so adopters were not silently
exposed to a runtime crash, but the duplicate was a footgun for
adopters reading the interface and a tripwire for future refactors
that might delete the "wrong" one.

Removed the `Ctx`-typed declarations on both interfaces, folded the
precedence note into the surviving `NoAccountCtx` declaration on
`CreativeBuilderPlatform`, and added regression locks in
`decisioning.type-checks.ts` mirroring the `previewCreative` pattern
from #1327.

The rest of the audit (`ctx.agent`, `ctx.ctxMetadata`, `ctx.recipes`,
`ctx.handoffToTask`, `ctx.state.*`, `ctx.resolve.*`) all match dispatch
guarantees. `ctx.authInfo`, `ctx.emitWebhook`, and
`ctx.publishStatusChange` are not on `RequestContext` (legacy
`HandlerContext` or module-level exports) — out of scope.
…r_shape × topology) matrix (#1548)

Refactors test/lib/storyboard-cascade-skip-on-skip.test.js from
prose-driven scenarios (~2000 lines, 35 named tests) to a fixture-table
matrix keyed by the three orthogonal dimensions of cascade behavior:

  skip_reason     ∈ { missing_tool, missing_test_controller,
                      not_applicable, real_failure }
  peer_shape      ∈ { sole_stateful, peer_passes, peer_fails,
                      peer_substitute_declared, peer_substitute_missing }
  phase_topology  ∈ { same_phase, downstream_phase }

PR #1545 fixed an asymmetric blind spot in cascade behavior — the
sole-stateful-step exemption applied to `not_applicable` (#1146) but
not to `missing_tool`. The parallel review traced the cause to test
architecture: prose-driven scenarios make empty cells in the implicit
cube invisible until production hits one. This refactor surfaces those
cells structurally.

Each row is one fixture run end-to-end. A coverage-holes assertion at
the bottom of the file partitions the 40-cell cube into covered (11),
structurally impossible (4), tracked-elsewhere (25), and empty (0).
Adding a new axis value or dimension fails the assertion until the
cube is repartitioned — so the next asymmetric blind spot lands as a
visible coverage gap, not a production bug.

Preserves all 35 historical assertions (24 matrix rows + 1 control row +
9 parse-time validation rows + 1 parity invariant). Net line shrink
~41% (2143 → 1256) from extracting the repeated fakeAgent setup +
storyboard scaffold into helpers.

Test-only change. No runner behavior change. Closes #1548.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two follow-ups from PR #1587 review:

1. The four `real_failure × peer_{passes,fails} × {same,downstream}_phase`
   cells previously cited a non-existent `storyboard-runner.test.js`. They
   are now first-class MATRIX_ROWS, exercising the within-phase cascade
   immediately-trips behavior (peer that runs after a real failure is
   itself cascade-skipped) and the cross-phase carry-through. Total rows
   grow from 36 to 40.

2. The vague `tracked_in_issue_1548_followup` markers (eleven cells) are
   retargeted at #1589, filed today, which lists the open contract
   questions per cell (substitute-rescue applicability for `not_applicable`,
   real-failure-wins ordering for `missing_tool` peers, and same/downstream
   asymmetry mirrors).

`MATRIX_REPORT=1` now shows: covered=15, impossible=4, tracked=21, empty=0.
@bokelley bokelley merged commit 7a2f05c into main May 8, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant