ci: run the scenario corpus in ladder-tests, and let corpus changes reach it - #473
Merged
Conversation
…each it Nothing in CI executed scripts/scenario/run_scenarios.py. The corpus could therefore be refused wholesale by a real server with every workflow green, and was: sixteen of sixteen ledger scenarios refused for five days and through a merge (morph#460), while drift-guard.yml's scenario-coverage job reported "ledger actions 18/18 dispatched, workflows 16/16" the whole time. That job parses the corpus; only running it can see a refusal. Three changes. `ladder-tests` now runs the corpus after its ctest step. It is the right job: drift-guard.yml deferred this because running the corpus needs the ladder_<rung>_server binaries and that workflow deliberately has no build, but `ladder-tests` already builds every one of them unconditionally (cmake/morph_add_rung.cmake). --build-dir names the build so no stale binary from another tree can be picked up, and --rung is passed once per corpus directory found on disk rather than left to the driver's default, so the set that runs is demonstrably the set that exists. A missing binary is not tolerated: run_scenarios.py exits 2, which is correct and is not softened. `scripts/scenario/` joins ci-path-regex. Without it a pull request touching only the corpus matched nothing and skipped `ladder-tests` altogether -- so the one gate that can catch a broken scenario would have been absent from exactly the changes most able to break one, which is morph#179's defect one directory over. check_rung_filters.sh now probes two corpus paths against the generated regex, behaviourally, and its self-test reintroduces the omission and requires the gate to catch it. A pre-flight check refuses a corpus this job could not have run. A scenarios/<name>/ directory with no RungSpec drops out of the driver's default set in silence, and a corpus for a rung this job configures no server for is only reported after a full Qt + ladder build. Both are this issue's own failure one level down, so both are checked on a bare checkout in seconds. This is what resolves the bank interaction (morph#470): examples/bank is deliberately not in examples/rungs.txt and its server comes from a local add_executable(), so when its corpus lands the check fails by name rather than letting five of six directories run and calling that success. drift-guard.yml's comment saying this run "is its own change" now points at the change instead of deferring it. Fixes #462 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C6uB9zxdSG8qp3VNAAyFvF
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Yaraslaut
added a commit
that referenced
this pull request
Sep 7, 2026
…expect it (#475) Master went red on the merge of #470 (bank's server and 22-scenario corpus) into #473's pre-flight, exactly as #473's own comment said it would: ::error::scripts/scenario/scenarios/bank/ is a corpus for 'bank', which is not listed in examples/rungs.txt -- this job configures no ladder_bank_server for it. Either list it there, or build its server in this job and extend this check to expect it. That is the gate working. A corpus with no server in this job would otherwise have run five of six directories and reported green, which is morph#462's defect one level down. So this takes the second branch the error names rather than weakening the check or adding bank to examples/rungs.txt -- the latter would enrol bank in wasm-ladder.yml's build loop, coverage.sh and codecov.yml's per-rung components, which is a scope decision that rungs.txt and examples/LADDER.md have already settled the other way. ladder-tests now configures -DMORPH_BUILD_BANK_EXAMPLE=ON. That option alone guards add_subdirectory(examples/bank) in the root CMakeLists.txt; examples/bank/CMakeLists.txt then declares ladder_bank_server inside its own if(MORPH_BUILD_QT), which this job already sets. Nothing new to install: the apt step already names Lightweight's libsqlite3-dev, libyaml-cpp-dev, libzip-dev, unixodbc-dev and libsqliteodbc (MORPH_BUILD_LADDER=ON fetches the same pinned ORM through examples/common) and libgl1-mesa-dev, and the server links only Qt6::Core and Qt6::WebSockets, both already installed. The Build step names the target after the all-target build -- a no-op when the flag is present, and "no rule to make target" when someone drops it, instead of a silent absence surfacing as the scenario step's exit 2 a Qt build later. The pre-flight gains a third arm, not a relaxation: a corpus that is neither a listed rung nor served by an add_executable(ladder_<name>_server) in examples/<name>/CMakeLists.txt still fails. The arm is keyed on that declaration rather than on the literal name "bank", so deleting the target makes the check fire again -- verified by commenting the add_executable() out and watching bank go red -- and a scratch scenarios/nonesuch/ still fails. scripts/scenario/README.md's "In CI" section said CI does not run the corpus because no workflow builds the servers. That has been false since #473; rewritten to describe the pre-flight, the whole-corpus run, and the scripts/scenario/ entry in the job's path filter. Verified locally with the job's own flags: ladder_bank_server builds, all six servers exist, `run_scenarios.py` over bank+bookmarks+kanban+ledger+pastebin+ polls reports "every scenario passed", ctest -L ladder -LE stress is 1026/1026, scripts/check_rung_filters.sh is 39/39, and `ladder_rungs.sh ci-path-regex` is byte-identical (neither of its inputs is touched). Claude-Session: https://claude.ai/code/session_01C6uB9zxdSG8qp3VNAAyFvF Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #462.
Nothing in CI ran
scripts/scenario/run_scenarios.py.grep -rn run_scenarios .github/workflows/returned exactly one hit and it was a comment saying so.The corpus could be refused wholesale by a real server with every workflow
green, and was — 16 of 16 ledger scenarios refused for five days and through a
merge (#460), while
drift-guard.yml'sscenario-coveragejob reportedledger actions 18/18 dispatched, workflows 16/16throughout. That job parsesthe corpus; only running it can see a refusal.
What this does
1.
ladder-testsruns the corpus, after itscteststep.That job is where it belongs, and
drift-guard.yml's own deferral comment sayswhy: running the corpus needs the
ladder_<rung>_serverbinaries, a build thatworkflow deliberately does not have.
ladder-testsalready builds every one ofthem unconditionally (
cmake/morph_add_rung.cmake:373-377), so the cost thecomment cited as the blocker is already paid there. The run is not moved into
drift-guard.yml, which stays fast and dependency-free.--build-dir build/gcc-debugnames the build so no stale binary from anothertree can be selected.
--rungis passed once per corpus directory found ondisk rather than left to the driver's default, so the set that runs is
demonstrably the set that exists.
A missing binary is not tolerated.
run_scenarios.pyexits 2 when a serveris absent and that is left exactly as it is.
2.
scripts/scenario/joinsci-path-regex, with the pin the issue askedfor.
Before this change a pull request touching only the corpus matched nothing and
skipped
ladder-testsentirely — so the gate would have been green on exactlythe pull requests most able to break the corpus.
check_rung_filters.shnowprobes two corpus paths against the generated regex (behavioural, like its
existing rung checks, so a pattern rewritten into one that matches nothing
fails rather than passing), and
test_check_rung_filters.shreintroduces theomission and requires the gate to catch it.
3. A pre-flight check refuses a corpus this job could not have run.
Two ways a corpus can drop out of the run without anything saying so: a
scenarios/<name>/directory with noRungSpec(the driver's default set isthe intersection of the two, so it is simply not in it), and a rung this job
builds no server for (loud —
exit 2— but only after a full Qt + ladderbuild). Both are this issue's own failure one level down, so both are checked
on a bare checkout, in seconds, before anything is installed or compiled.
4.
drift-guard.yml's comment saying this run "is its own change" nowpoints at the change rather than deferring it. No behavioural change there.
The bank interaction (#470), decided
#470 adds a 22-scenario bank corpus and a
ladder_bank_server. Bank isdeliberately not in
examples/rungs.txt— unnumbered prior art, its serverbuilt by a local
add_executable(), notmorph_add_rung().ladder-testsbuilds servers from
rungs.txt, so it will not build bank's; the driver'sdefault rung set is every rung with a
scenarios/<rung>/directory, so itwill include bank. A whole-corpus run in
ladder-testswould hit a missingladder_bank_serverand exit 2 — after a twenty-minute build.Chosen: run every corpus that exists on disk, and fail loudly up front on
any corpus this job does not build a server for. Not "run only what this job
builds" — that is the silent skip the issue exists to end, and it would have
quietly run five of six corpora the day #470 lands. Not "an explicit list in
the workflow" either, since a list is a second copy that can fall behind; the
list is derived from disk and the pre-flight is the check that it matches what
the job can actually run.
#470 touches both
scripts/scenario/**andexamples/rungs.txt, so once thislands it will trigger
ladder-testson its own pull request, not onlyafter merging. The job goes red in its first thirty seconds — before any
install or compile — with:
What will need doing when #470 lands (a follow-up on top of it, not part of
this PR — bank does not exist on
origin/master):ladder-tests—-DMORPH_BUILD_BANK_EXAMPLE=ONon that job's
Configurestep. Its apt step already installslibsqlite3-dev,libyaml-cpp-dev,libzip-dev,unixodbc-devandlibsqliteodbcfor the ladder's own Lightweight fetch, and its commentsalready cite
examples/bank/CMakeLists.txtas the reason. bank: give it a server, and a scenario corpus over all 41 actions #470 gatesadd_executable(ladder_bank_server ...)onMORPH_BUILD_QT, which this jobalready sets, so the option is the only missing piece;
examples/bank/CMakeLists.txtrather than fromexamples/rungs.txt— oneextra arm, not a relaxation: the requirement stays "name the thing that
builds this corpus's server, or fail".
That is deliberately a red build rather than a silent skip. Either the corpus
runs or CI says which one does not.
Evidence
Both runs are of
ladder-tests→ Run the scenario corpus against the builtservers, on the same tree, differing only in one scenario file.
Green — this branch, corpus as it stands
Application ladder, run 34060226868 — success73 files, 73 expected verdicts, against five real servers started by the step.
The pre-flight, from the same job, four steps earlier:
Red — #474, one scenario file reverted to the integer enum spelling
Application ladder, run 34060255730 — failure, and the only failing step in the job#474 is based on this branch and its entire diff is
scripts/scenario/scenarios/ledger/categorise-an-account.scenario, restored toits pre-
8a67ffe7form — the spelling #460 measured as refused.expected_quotefrom a realladder_ledger_server— the exact refusal thatsat unnoticed for five days and one merge. Every other step of that job
succeeded, including
Buildandctest, so the red is the corpus and nothingelse. Restoring the file is what makes it green: that restored file is what the
green run above ran.
The path filter, demonstrated
#474's diff is one file, under
scripts/scenario/, and nothing else(
gh pr view 474 --json files→scripts/scenario/scenarios/ledger/categorise-an-account.scenario (+6/-6)).On
mastertoday that matches no alternative inci-path-regex, soladder-testswould not have run at all — the job that just caught this wouldhave been skipped on the change that broke it. With this PR's entry it runs;
from that job's
Determine whether the ladder needs to runstep:and the job proceeded through every subsequent step. That the red run happened
at all is the path-filter proof: a corpus-only change reached the gate.
Locally, the same three paths against the old and new pattern:
Cost
Measured from the green run's own step timings (
morph-docker-3, warm caches):ladder-teststotal~24 s added to a 13 m 24 s job — about 3%. On the GitHub-hosted path,
where the same job last took 40 m 02 s with a cold cache
(run 33904711229),
the same 24 s is about 1%. It is a fixed cost: five servers started, 73
scenario files driven, no compilation.
The red run's failing step took 27 s, so the cost is the same whichever way it
goes — the gate does not get slow only when it has something to say.
One second-order cost, stated plainly.
ladder-sanitizersshares thisfilter by design (
scripts/ladder_rungs.shsays both steps must stayidentical, and
check_rung_filters.shenforces that both derive it), so acorpus-only pull request now also triggers the ASan ladder build — the largest
single job in the workflow. Giving the two jobs different patterns to avoid
that would reintroduce exactly the divergence that file exists to prevent, so
this PR does not do it.
Known follow-up, deliberately not done here
scripts/scenario/README.md's "In CI" section still says "What CI does notdo is run the corpus", which this PR makes false. #470 rewrites that exact
paragraph (
the five→the six ladder_<name>_server binaries), so editing ithere would be a guaranteed conflict on a file that PR owns. It is left to
whichever of the two lands second.
Verified locally
bash scripts/ladder_rungs.sh ci-path-regex, before and after — onealternative added, nothing else moved:
The pre-flight check's four failure modes, exercised against scratch copies of
the tree (the exact
run:script extracted fromci.yml):And the run step, on a tree with no build (
exit 2, not a skip):🤖 Generated with Claude Code
https://claude.ai/code/session_01C6uB9zxdSG8qp3VNAAyFvF