Repository navigation
ci: PR gate runs scenarios, affected specs and the oracle from one debug build; add main.yml on push to main - #370
Merged
Merged
Conversation
jsam
force-pushed
the
fm/test-t05
branch
2 times, most recently
from
October 5, 2026 18:14
f5621ae to
81f8fc6
Compare
…bug build; main.yml on push to main (#339) - Tests job: make unit-test (unit, integration, scenario and oracle tests), then the snapshot specs the PR affects against the target/debug binaries that build left (run_snapshot_tests.sh --debug --skip-build --affected), then the per-binary timing table in the job summary. - run_snapshot_tests.sh: --debug (target/debug binaries, dev-profile build) and --affected REF. The file-to-category map moves here from test-affected.sh, which now delegates; leaf modules keep a narrow mapping and every other engine change runs the whole corpus (about 1.5 min on 4 cores in debug), so an unmapped module costs time, not coverage. - Both SDK jobs' filters add ws-protocol/, docs/spec/ and src/protocol/. - main.yml on push to main: every spec, oracle at 50 seeds, js-test-live, python-sdk-live, python-test-live and release e2e-reactive with samples. - CONTRIBUTING and TESTING.md describe the three tiers.
…from #362) Folds #362's runner changes into the PR gate: a -f filter (or an --affected category set) that matches nothing fails instead of passing vacuously, so a renamed spec cannot drop out of its gate, and --profile picks the binaries' cargo profile (dev = target/debug; --debug is its shorthand).
…d main.yml dispatch
…. The operator traced it to runner starvation: GitHub-hosted ubuntu-latest jobs waited more than 15 minutes for a runner and were cancelled. The invariant is that every job in the PR gate runs on blacksmith-4vcpu-ubuntu-2404, so none of them waits on the hosted pool. In .github/workflows/ci.yml, the four jobs still on `runs-on: ubuntu-latest` now use `runs-on: blacksmith-4vcpu-ubuntu-2404`: changes (Detect changes), the supply-chain job, the Python SDK job and the JS SDK job. Every runs-on in ci.yml is now blacksmith-4vcpu-ubuntu-2404. Other workflows are outside the PR gate and were left alone. The file still parses as YAML. No docs mention ubuntu-latest, so none needed updating. The PR-description owner should add: "PR gate jobs all run on blacksmith-4vcpu, because hosted ubuntu-latest runners starved and the jobs were cancelled
jsam
marked this pull request as ready for review
October 5, 2026 20:59
This was referenced Oct 5, 2026
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.
Closes #290: the
.debugplan specs it names now run on every PR that touches the engine (the PR gate runs the whole spec corpus for any engine change). #362 was closed as superseded. Its empty-filter guard is folded in here, applied only to explicit-ffilters.PR gate runners: every PR-gate job now runs on
blacksmith-4vcpu-ubuntu-2404. The hostedubuntu-latestjobs (Detect changes, supply chain, and the Python and JS SDK jobs) waited 15+ minutes for a free runner and were cancelled, which held up the whole gate.Timing on the final head (
Tests, with #358's scenarios now included): 10m04 job wall. The 3 GB cargo cache downloaded at about 40 MB/s on this run, versus about 610 MB/s on the earlier run, so the restore took 2m36 instead of 34 s. Cargo test took 6m14 (1m31 compile; 95 binaries, 4,387 tests; scenarios 12 s, oracle 19 s), and the full spec corpus took 59 s. With a normal cache restore the job is about 8 minutes, against the 10-minute budget.Intent
please leave this query batching functionliaty just refactor things so its clear and optimized for this ... auto materialize needs to be default mode of operation where rules get deployed as views and everything else is a query .. make the docs true with the correct systems design ... this is not accceptable state for production readiness (Context: implement #339 exactly as written.)
Issue #339, "Testing: PR gate restructuring and
main.yml(T-05)", as written: Part of the test strategy (captain, 2026-10-05: scenarios that test the changes together; cleaner, faster suites; TESTING.md). Epic #305. Scope:ci.yml:Testsjob runscargo test --workspace --all-features(now including scenarios), thenscripts/run_snapshot_tests.sh --debug --affected <merge-base>, then prints the per-binary timing table; newJS SDKjob (path filter addsws-protocol/**,docs/spec/**,src/protocol/**to both SDK jobs); new.github/workflows/main.ymlon push to main: full spec corpus, oracle 50 seeds,js-test-live,python-sdk-live,python-test-live, releasee2e-reactive.run_snapshot_tests.sh --debugusestarget/debugbinaries. Acceptance: a PR touchingsrc/finishesTestsin under 10 min wall onblacksmith-4vcpu; the three #290 specs fail the firstmain.ymlrun (then are fixed by #290); a PR touching onlypackages/inputlayer-jsruns the JS job and nothing Rust.What Changed
PR gate (
ci.yml): theTests (fast)job is nowTests. It makes one debug build and runsmake unit-test(unit, integration, scenario and oracle tests), with the output saved to a log. It then runsrun_snapshot_tests.sh --debug --skip-build --affected HEAD^1against thetarget/debugbinaries, and it adds a per-binary timing table to the job summary using the newscripts/test-timings.sh. Both SDK jobs now also run whenws-protocol/**,docs/spec/**orsrc/protocol/**change. The Rust path filter now also covers the two scripts.New
.github/workflows/main.yml: runs on every push tomain. It runs every snapshot spec, the differential oracle at 50 seeds, the JS and Python SDK live suites (js-test-live,python-sdk-live,python-test-live) ande2e-reactivein release.run_snapshot_tests.shgains three changes:--debugbuilds and runs thetarget/debugbinaries.--affected REFholds the file-to-category map. Leaf modules such as the vector/HNSW and temporal code map to specific categories, a changed spec runs its own category, and any other engine source, manifest, protocol or ontology-client change runs every spec.-ffilter that matches no spec now fails the run instead of passing with nothing tested.test-affected.shnow just calls--affected. CONTRIBUTING, TESTING.md and perf-gate/README.md describe the three CI tiers: PR gate, main and full suite.Risk Assessment
✅ Low: The fix round applies exactly the four operator decisions. It removes
--profileand keeps--debug, which sets the dev profile andtarget/debug. It dropsworkflow_dispatchfrom main.yml and updates the CONTRIBUTING text. It addsontology-client/*to the every-spec arm and to the TESTING.md mapping. In--affectedmode it keeps only categories that still have runnable specs, using the same find criteria as the runner, and exits 0 when none remain, while an explicit-fthat matches nothing still fails. Traced cases (a deleted category, a category with only_pending_specs, an ontology-client-only change, the AFFECTED_ALL path) all behave correctly, and no new defects were found.Testing
I built the debug server and client in an isolated target dir and drove
run_snapshot_tests.shfrom a throwaway clone. The full spec corpus passed on the debug binaries alone (1152 passed, 0 failed, about 74s).--affectedpicked exactly the expected categories: 29_temporal for a temporal_ops edit, 31_lsh for a spec edit (where it caught the broken spec), and every category for an ontology-client edit. It exited 0 for a JS-only change, a deleted category and a pending-only category. An explicit no-match-f, a bad ref, and--affectedcombined with-feach fail clearly. Without--debugthe script still uses release.test-timings.shrenders a correct markdown table. I parsed the workflow YAML and evaluated the path filters statically. That is not a live run, so the workflow trigger and routing scenario stays untested. Two things need a real GitHub Actions run and could not be done locally: the workflow behavior, including the under-10-minute wall time on blacksmith-4vcpu, and the live SDK and e2e jobs in main.yml. These are CLI and CI changes with nothing visual, so the evidence is CLI transcripts. The clone and the build output have been removed.Evidence: Full spec corpus on debug binaries
Evidence: --affected runs: temporal, ontology-client, explicit -f no-match, default release
Evidence: --affected spec-only change runs only its category
Evidence: --affected early exits and guards
Evidence: Per-binary timing table
Evidence: Workflow triggers and path-filter checks (static)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 5 issues found → auto-fixed ✅
scripts/run_snapshot_tests.sh:451- The--affectedmap leaves outontology-client/, so a PR that only changes that crate runs no snapshot specs, even though it is compiled into the server binary.inputlayer-ontology-clientis a normal dependency in the root Cargo.toml (line 110), and src/protocol/handler.rs calls it on the server path, e.g.registry::validate_component("ontology name", name)at handler.rs:5269. Example: a PR that only edits ontology-client/src/registry.rs passes the ci.yml rust path filter (ci.yml:51), soTestsruns.affected_categoriesthen reaches no case arm, AFFECTED is empty, and the script prints "No test-relevant changes" and exits 0. Spec coverage of that server behavior is silently skipped. This contradicts the function's own comment that 'any other change to the engine's sources or build runs the whole corpus, so a module missing from this list costs time rather than coverage', and the CONTRIBUTING:371 / TESTING.md:307 claim that every spec runs 'when the engine's sources, manifests ... change'. The same gap applies toontology-client/Cargo.tomlandws-protocol/Cargo.toml(only the rootCargo.tomlis matched;ws-protocol/*already covers the latter). Fix: addontology-client/*to the run-all arm and list it in TESTING.md:307.scripts/run_snapshot_tests.sh:627- In--affectedmode, a PR that deletes a whole spec category, or adds a category containing only_pending_or_helperspecs, fails the PR gate.examples/iql/*/*maps each changed path to its category directory (line 453) without checking that the directory still has runnable specs. The filter/\(23_foo\)/then matches no file, and the new 'No tests match filter' check exits 1. This is realistic: the corpus already has gaps where categories were retired (03, 05, 23, 26, 37, 38). Example:git rm -r examples/iql/42_consistency_pack→ Tests fails with 'No tests match filter: /(42_consistency_pack)/'. Smallest fix: in the --affected branch, keep only categories that still contain runnable specs and take the existing 'No test-relevant changes' exit when none remain. The no-match failure should apply only to a user-supplied-f. See also the simplification finding on that check.scripts/run_snapshot_tests.sh:469- Simplification: issue Testing: PR gate restructuring andmain.yml(T-05) #339 asks only forrun_snapshot_tests.sh --debug(use the target/debug binaries). The general--profile NAMEoption, with its profile-to-directory mapping and TESTING.md row, comes from ci: gate the .debug query-plan snapshot specs on every PR #362 per the commit message; neither CI workflow uses it. No Testing: PR gate restructuring andmain.yml(T-05) #339 requirement needs it. Recommend removing--profileand keeping only--debug(a debug/release switch).scripts/run_snapshot_tests.sh:625- Simplification: failing a run whose-ffilter matches no spec is ci: gate the .debug query-plan snapshot specs on every PR #362 scope (commit f1d97f0, 'from ci: gate the .debug query-plan snapshot specs on every PR #362'), not Testing: PR gate restructuring andmain.yml(T-05) #339, which asks for the change to be implemented exactly as written. The check is also what turns the deleted-category case above into a gate failure. Recommend removing it from this change, or at least not applying it to the filter that--affectedgenerates..github/workflows/main.yml:15- Simplification: issue Testing: PR gate restructuring andmain.yml(T-05) #339 specifies main.yml 'on push to main'. The addedworkflow_dispatchtrigger, which runs on any ref and is documented in CONTRIBUTING, goes beyond that. Remove it unless manual runs are wanted.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
CARGO_TARGET_DIR=/tmp/nm-01M46-target cargo build --bin inputlayer-client --bin inputlayer-server(debug profile, isolated target dir)./scripts/run_snapshot_tests.sh --debug --skip-build(full corpus, debug only, no release build present)./scripts/run_snapshot_tests.sh --debug --skip-build --affected HEADwith src/temporal_ops.rs edited (only 29_temporal ran)--affected HEADwith a spec in 31_lsh edited (only 31_lsh ran; the broken spec failed the run)--affected HEADwith ontology-client/src/lib.rs edited (every category selected)--affected HEADwith a JS-only change, a deleted category 42, and a new category holding only a pending spec (all exit 0 with 'No runnable specs affected')./scripts/run_snapshot_tests.sh --debug --skip-build -f no_such_spec_zzz(exit 1 'No tests match filter')--affectedcombined with-f(rejected);--affected nosuchref123(error exit 1)./scripts/test-affected.shon a clean tree (delegates to --affected and exits 0)./scripts/run_snapshot_tests.sh --skip-build -f 29_temporalwithout --debug (looks for release binaries)./scripts/test-timings.shon a cargo test log with passing, failing, crashed and doc-test binaries, and with no argumentParsed ci.yml and main.yml with PyYAML and checked the dorny path filters against sample paths: JS-only, src/protocol, ws-protocol, docs/spec, scripts (static check only, not a live CI run)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.