Repository navigation
test(pipeline): run the vsearch stage through the server's entry points - #113
Merged
hyperpolymath merged 1 commit intoOct 14, 2026
Merged
Conversation
The refactor left 3 lines in src/server/routes/pipeline.jl uncovered, and said so in its own message: the call into _vsearch_stage and the OTU read, inside _run_full_pipeline and _run_stage, which no test runs. This closes that. The reason no test ran them is that none of them called the server's own entry points. test/integration/test_pipeline.jl built its run directories by calling the stages by hand, which is not what pressing a stage in the UI does: the UI calls _run_full_pipeline and _run_stage, and those two calls are the only callers of _vsearch_stage outside the occupancy test. The new testset re-runs the vsearch stage both ways, on the runs the loop left behind: - run_A through _run_stage(..., "vsearch", ...): the serial shape; - run_B through _run_full_pipeline(...): the concurrent shape. Each vsearch output is removed first. A stage whose outputs are current skips before its work begins, and a stage that skips records nothing, so a re-run that skipped would test the skip guard instead of the stage. The stages before vsearch still skip on their own guards, so neither call redoes the DADA2 work. The probe is switched on for both, at the narrowest level of the config cascade - the run's own pipeline.yml, which is the way the defaults file says to switch it on, the config editor offering only keys that have a factory default. occupancy_probe sits in no stage's hashed section, so turning it on marks nothing stale. What the test asserts is what each run records: the vsearch stage, and that stage's occupancy, one labelled task per search. Every line the refactor added to src/server/routes/pipeline.jl is now executed by the suite: 19 of 19, where it was 16. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
hyperpolymath
added a commit
that referenced
this pull request
Oct 15, 2026
…ts (#113) Brings the fix for the Codecov report on upstream PR #39 (JoshuaJewell#39) onto #39's head branch. Codecov's last report on #39 flags 3 lines in `src/server/routes/pipeline.jl` — the file's patch coverage is 84.21%, the PR's 98.11%: | line | code | |---|---| | 300 | `_vsearch_stage(vsearch, project, asvs, otus, dbs["$(db_name)_vsearch"]; concurrent=true)` in `_run_full_pipeline` | | 474 | `otus = swarm_enabled ? _oturesult_from_disk(run_dir) : nothing` in `_run_stage` | | 475 | `_vsearch_stage(vsearch, project, asvs, otus, dbs["$(db_name)_vsearch"]; concurrent=false)` in `_run_stage` | No test ran them, because no test called the server's own entry points: `test/integration/test_pipeline.jl` builds its run directories by calling the stages by hand, while the UI goes through `_run_full_pipeline` and `_run_stage` — the only two callers of `_vsearch_stage` outside `test/occupancy_disagreement_test.jl`. This adds a testset that re-runs the vsearch stage both ways, on the runs the integration loop leaves behind: `run_A` through `_run_stage` (serial), `run_B` through `_run_full_pipeline` (concurrent). Each vsearch output is removed first, since a stage whose outputs are current skips before doing any work and a stage that skips records nothing; the stages before vsearch still skip on their own guards, so neither call redoes the DADA2 work. Both calls run with `occupancy_probe.enabled` set at the run's own config level, and the test asserts what each run records: the vsearch stage, and its occupancy with one labelled task per search. Merging this (squash is fine) puts one commit on `feat/occupancy-probe`, so #39's head carries it and Codecov recomputes the report for the new head. It is based on #39's head (`4b32184`) rather than on `main`, so the PR does not drag the fork's other `main`-only work into #39. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
hyperpolymath
added a commit
that referenced
this pull request
Oct 15, 2026
The two things run_permanova decides for itself rather than taking from vegan, proved: the reported min_p_value (the 1/(B+1) floor, never raised by more permutations) and which terms have their p-value withheld (_varies_within_any_block, exactly when no within-block permutation exchanges the term's labels). Rebuilt for this fork's main: the branch this PR carried was based on upstream's history with three superseded commits whose content main already absorbed (#106, #112, #113), so it conflicted and no workflow could post a check. The proof file is byte-identical to the branch's; the import in All.agda, the README's PERMANOVA section and theorem-to-code map entries, and the Agda job's comment are re-applied onto main's side of each file (both sides were identical, so the diff applies clean). Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe <vibe@mistral.ai>
5 of 13 tasks
hyperpolymath
added a commit
that referenced
this pull request
Oct 15, 2026
<!-- SPDX-License-Identifier: CC-BY-SA-4.0 Copyright (c) Jonathan D.A. Jewell <j.d.a.jewell@open.ac.uk> --> ## Summary Adds `proofs/agda/MetaManifold/Permanova.agda`: machine-checked proofs of the two things `run_permanova` in `src/analysis/analysis.jl` decides for itself rather than taking from vegan — the reported `min_p_value` (the `1/(B+1)` floor, never raised by more permutations, and attained) and which terms have their p-value withheld (`_varies_within_any_block`, exactly when no within-block permutation exchanges the term's labels). With the import in `All.agda` (so the axiom audit reaches it), the README's PERMANOVA section and theorem-to-code map entries, and the Agda CI job's comment. The Julia side this branch originally carried (the CLR method chain, the vsearch helper, the server entry-point tests) reached main refined through the upstream syncs — as #112, #113 and the occupancy line — so nothing of it is re-landed here. ## Branch repair The branch as first opened was built on upstream's history and carried three superseded commits, so it was `CONFLICTING` against main — which is also why no workflow posted a check: with no merge ref, GitHub cannot start a `pull_request` workflow at all, and only the external scanners ran. The branch is now a single commit rebased onto current main; every content change is byte-identical to the original rebase commit (`e9bb4e2`, whose conflict resolution already kept main's side of every touched file — both sides were identical, so the diff applies clean). ## Type of change - [ ] 🐛 Bug fix (non-breaking change that fixes an issue) - [ ] ✨ New feature (non-breaking change that adds functionality) - [ ] 💥 Breaking change (would change existing behaviour) - [ ] 🕳️ Soundness fix (fixes a checker/proof false-negative) - [ ] 📖 Documentation - [ ] 🧹 Refactor / tech debt (behaviour-preserving) - [ ] ⚡ Performance - [x] 🔧 Build / CI / tooling (a new machine-checked proof module and its CI wiring; no runtime code changes) ## How has this been verified? - CI on this PR: the **Agda proofs** job type-checks `MetaManifold/All.agda` (which now imports Permanova) with the pinned Agda 2.6.4.3 + agda-stdlib 2.1, and the **Julia tests** job runs the full unit suite including `test/unit/test_permanova.jl`, the code the theorems model. - The proof file is byte-identical to `e9bb4e2`'s, whose theorems were checked there; the only re-applied deltas are the `All.agda` import, the README entries and the ci.yml comment, each applied mechanically from that commit's diff. ## Checklist - [ ] My commits are **signed** (`git commit -S`). — the signing key's passphrase was not available in the session that produced this commit - [x] I ran the project's own checks/tests locally and they pass. — the Julia pin tests and root-shape suite pass locally; the Agda type-check is CI's pinned toolchain, which is authoritative here - [x] New files carry the correct `SPDX-License-Identifier` (code/config `MPL-2.0`, prose `CC-BY-SA-4.0`); I did not relicense existing files. — `Permanova.agda` carries `AGPL-3.0-only`, matching every other file under `proofs/agda/` - [x] Docs are updated, and no public claim now overstates what the code does. — the proofs README gains the PERMANOVA section and theorem-to-code map entries - [x] I have not introduced a soundness hole (or I have flagged where I might have). — no postulates, no holes; `--safe --without-K` as everywhere else in the tree ## Notes for reviewers Nothing here changes runtime behaviour; the risk surface is the Agda job's type-check and the axiom audit. If this unit is later proposed upstream, it should be re-cut fresh from upstream `main` per the absorption register's runbook, not replayed from this fork branch. Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe <vibe@mistral.ai> Co-authored-by: Mistral Vibe <vibe@mistral.ai>
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
1 participant
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.
Brings the fix for the Codecov report on upstream PR #39 (JoshuaJewell#39) onto #39's head branch.
Codecov's last report on #39 flags 3 lines in
src/server/routes/pipeline.jl— the file's patch coverage is 84.21%, the PR's 98.11%:_vsearch_stage(vsearch, project, asvs, otus, dbs["$(db_name)_vsearch"]; concurrent=true)in_run_full_pipelineotus = swarm_enabled ? _oturesult_from_disk(run_dir) : nothingin_run_stage_vsearch_stage(vsearch, project, asvs, otus, dbs["$(db_name)_vsearch"]; concurrent=false)in_run_stageNo test ran them, because no test called the server's own entry points:
test/integration/test_pipeline.jlbuilds its run directories by calling the stages by hand, while the UI goes through_run_full_pipelineand_run_stage— the only two callers of_vsearch_stageoutsidetest/occupancy_disagreement_test.jl.This adds a testset that re-runs the vsearch stage both ways, on the runs the integration loop leaves behind:
run_Athrough_run_stage(serial),run_Bthrough_run_full_pipeline(concurrent). Each vsearch output is removed first, since a stage whose outputs are current skips before doing any work and a stage that skips records nothing; the stages before vsearch still skip on their own guards, so neither call redoes the DADA2 work. Both calls run withoccupancy_probe.enabledset at the run's own config level, and the test asserts what each run records: the vsearch stage, and its occupancy with one labelled task per search.Merging this (squash is fine) puts one commit on
feat/occupancy-probe, so #39's head carries it and Codecov recomputes the report for the new head. It is based on #39's head (4b32184) rather than onmain, so the PR does not drag the fork's othermain-only work into #39.