Repository navigation
Upstream/docs community - #163
Merged
Merged
Conversation
analysis.differential.method selects nb_glm (the default, unchanged) or clr_lm: multiplicative zero replacement (Martin-Fernandez et al. 2003, the zCompositions::multRepl operator) over every observed taxon, the centred log-ratio, then stats::lm per taxon with BH over the fitted taxa. min_prevalence is applied after the transform, so a filtered taxon still shapes every sample's CLR. Replacement keeps each sample total and the ratios among its observed taxa exactly, checked at runtime, and refuses with a reason rather than guessing. The result carries an effect key so the volcano plot, table and CSV label a CLR difference as such, not as a fold change, and reports how many zeros were replaced and the largest share of a sample they hold. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qbn8ECdtai37Nzdyf5so3D
Address review on #25: - Apply min_prevalence before zero replacement. Zeros are now replaced only for the taxa that are tested, so rare taxa no longer put imputed values into every sample, and the CLR is taken over the same taxa. - Replace zeros with zCompositions::cmultRepl (GBM, the count-data method) through RCall, in place of the Julia multiplicative replacement, which is removed with its tests. z.warning = 1 and z.delete = FALSE stop cmultRepl from dropping rows or columns. A taxon seen in fewer than 2 samples is refused with a message naming the min_prevalence that excludes it; a missing zCompositions raises ZCompositionsUnavailable (503 from the route). - Test each taxon with Welch's t-test (t.test, var.equal = FALSE) in place of the pooled-variance lm; each group needs 2 samples. - Pin zCompositions 1.6.2 and its new dependencies (NADA, truncnorm, survival) in renv.lock; check it in CI alongside MASS. - Report the median as well as the maximum imputed share per sample. - Tests: parity with direct cmultRepl and Welch t.test calls (with a pooled-variance control), prevalence-before-replacement, and a sparse fixture with rare taxa that is refused unfiltered and runs once min_prevalence is set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The method now tests each taxon with Welch's t-test rather than a linear model, so the config value, UI options and labels say so. clr_lm was never on main, so no alias is kept; a test checks it is refused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CI installs zCompositions, so the ZCompositionsUnavailable path and the route's 503 were never run there. The new testset unloads the package and drops its library from .libPaths() for its duration, checks the error and its restore hint, checks the route answers 503 r_unavailable, and restores the library path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
CI installs zCompositions into R's base library, which .libPaths() cannot drop, so the testset took its "cannot be hidden; SKIPPED" branch on every CI run and the absence path (ZCompositionsUnavailable, its showerror, and the route's 503 r_unavailable) was never executed there. The testset now binds a requireNamespace in R's global environment that reports zCompositions missing and defers to base::requireNamespace for every other package. The production probe is evaluated in globalenv, so it sees the package as absent wherever the library lives. The binding is removed in a finally block and the test checks it is gone. The skip branch for an unhideable library is no longer needed. Checked: deleting the route's ZCompositionsUnavailable => 503 line makes this testset fail (3 failed, 1 errored); restored, test_differential.jl passes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
The "not a valid composition" refusal after cmultRepl was the PR's one uncovered line: cmultRepl never returns such output on an honest input, so no end-to-end test reaches it. Move the check into _check_compositions, which replace_zeros calls unchanged, and test it in pure Julia with malformed output: a replaced zero left at 0, a NaN, a row that does not sum to 1, a failure in the second sample, and a dropped column. Weakening the guard fails 3 of the 6 assertions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
bhAdjust mirrors bh_adjust step for step (stable sort as lexicographic (p, index) order, backwards running minimum, clamp, scatter-back). Proved under --safe --without-K with no postulates: length, R-BH-1, R-BH-2a, R-BH-2b, R-BH-2c (ties included) and R-BH-2d (envelope). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JPmNukJT5PUzQ9d74mhHhj
…tionals Proportions sum to 1 and lie in [0,1]; a non-negative column with a positive count has a positive total; centring by any positive constant keeps factors positive and ordered; the median of a non-empty list of positive ratios is positive. Geometric means, exp and log are not rational and are abstracted as an arbitrary positive divisor. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JPmNukJT5PUzQ9d74mhHhj
The audit refuses postulates, holes, check-disabling pragmas and flags, modules without --safe/--without-K, and modules unreachable from MetaManifold/All.agda. Module headers now cite bh_adjust, tss_factors, rle_factors and _median in src/analysis/differential.jl as D1 wrote them, including the descending-sort tie order and the 1.0-seeded running minimum. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JPmNukJT5PUzQ9d74mhHhj
proofs/agda/_build/ is regenerated by every type-check and is specific to the Agda version that wrote it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JPmNukJT5PUzQ9d74mhHhj
Adds one job, "Agda proofs", which installs Agda 2.6.4.3 and agda-stdlib 2.1 from Debian trixie in a digest-pinned image, runs the axiom audit, and type-checks proofs/agda/MetaManifold/All.agda. The existing test job is unchanged. docs/formal/agda-bh-scaling-proofs.md maps each theorem to the function in src/analysis/differential.jl and to the D1 test that exercises it, and lists what is not proved (geometric means, scale invariance, floating point) and why. docs/* is otherwise kept local, so .gitignore gains a single !docs/formal/ exception. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JPmNukJT5PUzQ9d74mhHhj
Move docs/formal/agda-bh-scaling-proofs.md to proofs/agda/README.md and drop the .gitignore exception that published docs/formal/, so docs/ stays local as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JPmNukJT5PUzQ9d74mhHhj
A taxon with no reads in every sample of one group drives the glm.nb group coefficient to infinity: the fit still reports convergence, with no warning and theta inside its bounds, while the standard error grows without limit and the Wald p-value collapses towards 1. Such a taxon was reported as `ok`, entered the Benjamini-Hochberg family and was drawn as non-significant - the taxa found in only one condition, often the most interesting ones, were published as negative findings. Measured on 4 vs 4 samples, reference all zero, contrast 50/80/120/30: estimate 25.5 (log2 FC ~37), SE 12812, p = 0.998, converged. The fit loop now checks each taxon for a group in which it has no reads and reports it as `failed`, naming the group, with no estimate and no p-value, so it stays out of the multiple-testing family as the module already does for every fit it cannot trust. Test: a taxon absent from group A is refused with its reason, the BH family is the other taxa only, and a control shows the direct glm.nb call converges with p > 0.9. Disabling the check fails 5 assertions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`_alpha_significance` returned `(nothing, empty DataFrame)` in three cases that are not findings: R/vegan absent, the groups sharing no sample IDs for a paired test, and R's own `tryCatch` yielding `NA_real_` for the statistic. A genuine result in which no pair reached significance has the same shape, so each of those three was rendered the way a null result is rendered: the pairwise panel drew no brackets, which is exactly how "nothing significant" looks. `AlphaSignificance` makes `status` the primary field: `:computed` is the only status whose numbers may be read as evidence; `:r_unavailable`, `:no_paired_samples` and `:test_failed` carry a reader-facing `reason`. A panel whose pairwise tests did not run now carries an explicit "not run" notice, and the omnibus caption says "not run" with the reason instead of "n/a" beside a test name. A busy R runtime is unchanged: `RBusyError` still propagates to the server, which reports the chart as unavailable, and the existing `@test_throws` assertion stands. Tests: a not-computed result is asserted unequal to a computed null, the notice appears only when the test did not run (with a computed-null positive control), and each reason gets distinct caption wording. Mutants killed before this was believed: making `_not_computed` report `:computed` fails 12 assertions plus 1 error; restoring the silent early return in `_add_pairwise_annotations!` fails the notice test. Ported from f23a1d5 (fork issue #31), adapted to upstream's RBusyError design. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6qhB21oLqSPK261cRy5R8
Runs _alpha_significance through R with every sample in one group, so kruskal.test errors and R's tryCatch returns NA: the result must be :test_failed with no omnibus p-value. A two-group positive control shows the same path computes a p-value when it can. Covers the ismissing branch Codecov reported as missed; skips loudly without R/vegan. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
`_read_databases` and `_read_primers` both answer `json_error(400, ...)`: the route's own verdict that a malformed `config/databases.yml` or `config/primers.yml` is an operator-fixable input, not a server fault. Logging that at `@error` with a full backtrace reports a routine bad file as if the server had crashed, and the backtrace pointed into YAML.jl's parser rather than at the offending line, so it carried no diagnostic value either. The reason string is kept -- `reason=sprint(showerror, e)` is what actually names the YAML defect. Only the backtrace goes. Deliberately NOT changed: the three 500-class sites, which stay at `@error` with their backtraces -- src/server/routes/analysis_config.jl:144 src/server/routes/annotations.jl:318 src/server/routes/composition.jl:370 A 500 is the route saying it did not expect this, and there the backtrace is the only thing separating a data problem from a code bug. Dropping it would be muting a diagnostic rather than fixing one. `analysis_config.jl` already draws this line itself: its `ArgumentError` branch returns 400 with no logging at all, while everything else logs at `@error` and returns 500. `src/server/routes/pipeline.jl:520` is untouched -- it is the silent-degradation shape tracked in #31, not a severity question. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm (cherry picked from commit e2147d5)
Covers the _read_primers catch branch Codecov reported as missed, and asserts the log level at both unreadable-config sites, so a return to @error fails the suite rather than only the coverage figure. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
Closes #38. The `test` job was named `Julia ${{ matrix.julia-version }} / ${{ matrix.os }}`, and that string is what branch ruleset 23793298 requires as a status check. A ruleset matches a required check by display name. Bumping either pin renamed the check, and a renamed check does not report at all -- it does not fail, it is simply absent, and an absent required check can never be satisfied. Every open pull request would have deadlocked until an admin bypassed the rule. The trigger was an ordinary version bump: the single most routine edit this repository gets. Both halves of the name interpolated, so the OS pin carried the same fault as the Julia pin. Measured, by parsing three hypothetical bumps through both versions of the file: matrix before after 1.12.5 / ubuntu-24.04 "Julia 1.12.5 / ubuntu-24.04" "Julia tests" 1.12.6 / ubuntu-24.04 "Julia 1.12.6 / ubuntu-24.04" "Julia tests" 1.13.0 / ubuntu-26.04 "Julia 1.13.0 / ubuntu-26.04" "Julia tests" The versions are not hidden, only moved off the identifier: they remain in `strategy.matrix` and in the `Set up Julia` step's log. `test/unit/test_install_pins.jl` gains a reflexive testset asserting that NO job name in ci.yml interpolates anything. It is written over every job rather than over the one job we require today, because a rule enforced at each door in turn is a rule the next door escapes. Verified by killing the mutant: against the unedited ci.yml the assertion fails on exactly one job of three (97 pass / 1 fail), and passes 98/98 after the rename. The job ID `test` is unchanged, so `needs: [test]` and the existing `ci["jobs"]["test"]` assertions are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm (cherry picked from commit 8a098d2)
#39 renamed this job to `Julia tests` and claimed the required status check was therefore version-stable. It was not, and the claim was never measured. MEASURED on PR #39 head 776e340, after that change had landed: Julia tests (1.12.5, ubuntu-24.04) <- what GitHub actually posted Julia tests <- what the ruleset requires GitHub appends the matrix combination to a job's posted check name whenever the `name:` does not itself reference the matrix. The old interpolating name suppressed the suffix by accident; replacing it with a static string turned the suffix on. A 1x1 matrix is still a matrix, so both pins stayed embedded in the check name and the required context `Julia tests` was never reported at all -- so `main` has been sitting in precisely the deadlock issue #38 describes, with #39 itself merged only by an admin bypass. The matrix selected exactly one combination and bought nothing, so it is removed rather than worked around: `runs-on: ubuntu-24.04` and the setup-julia `version: "1.12.5"` are literals, each keeping the reasoning that used to sit beside the matrix entry. The non-matrix `repo-hygiene` job was the control -- it has always posted its `name:` verbatim. Why the guard did not catch it: `test_install_pins.jl` asserted that no job name interpolates anything -- a property of the YAML `name:` field -- while the consumer, the ruleset, matches the rendered check-run name. The guard asked a different question than its consumer, so it passed on a broken fix. It now asserts BOTH necessary conditions, over every job in the file: the name interpolates nothing AND the job has no `strategy.matrix`. Run against the previous commit that assertion fails on `test` alone (5 pass / 1 fail), so it discriminates rather than blanket-failing. `CI installs what is pinned` made the same claims about the same two values and has been repointed at where they now live, locating the setup step by its `uses:` rather than by index. Mutating either literal fails it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm (cherry picked from commit 6c29b53)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6qhB21oLqSPK261cRy5R8
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n delete
H2: _config_value_error only type-checked string keys, so
PATCH /api/v1/config {"dada2.output.fasta_prefix": "../../tmp/pwn"} was
stored, and DADA2 joins that stem onto its Tables directory
(dada2.jl:148-150, chimera.jl, dada2_functions.r), writing outside the
run. The three dada2.output.*_prefix keys and cutadapt.r1_suffix /
r2_suffix (H8: spliced into a \Q...\E regex in tools.jl _mate_regex, so a
"\E" escaped the quoting) must now be safe names (Validation.is_safe_name,
letters, digits and . _ -).
The remote block was only partly gated: remote.staging_dir (interpolated
into the remote login shell's command), remote.stages and remote.threads
were stored unchecked, although validate.jl says such values are refused
at the write gate. The remote-block checks of _validate_pipeline_cfg are
extracted into _validate_remote! (and _staging_dir_errors!) and exposed
to the gate as Validation.remote_value_error, so the gate runs
validation's own checks rather than a copy; staging_dir is checked even
before a host is named. remote.host and remote.rscript keep the gate's
existing stricter check. Validation behaviour of a full config is
unchanged.
H3: DELETE .../results/tables/{table} had no reserved-name guard, so it
deleted merged.csv (the merge sentinel) and dropped the merged table.
Save and delete now share _PIPELINE_TABLES (merged, merged_otu,
merged_cdhit, cluster_membership); deleting one returns 400
reserved_name and touches nothing.
Tests: test/unit/test_write_gates.jl (76 assertions): each bad value
through PATCH /api/v1/config returns 400 with config/pipeline.yml
byte-identical; good values are stored and pass _validate_remote!;
DELETE of each pipeline table returns 400 with its CSV and DuckDB table
intact, while a user table still deletes. Planted control (gate
additions and delete guard removed): 26 pass / 46 fail / 4 error; with
the fix 76/76. test_write_gates + test_validation + test_remote_stages +
test_report_funnel + test_sample_reads + test_routes: 528/528 (Julia
1.12.6).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Covers the _staging_dir_errors! branch for a missing or non-string staging_dir, the line Codecov reported as missed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm
…e terms (#35) adonis2 in the pinned vegan 2.7-3 defaults to by = NULL: one test of the whole model. run_permanova read row 1 as the group term and a comment called the rows sequential terms, so the reported R2/F/p mixed group and run and the per-term list held a single "Model" row. adonis2 is now called with by = "terms"; the top-level result names its term (group). With permutations restricted within individuals (how(blocks = ...)), permute enumerates every permutation when there are fewer than 999: six individuals of two samples give 63, so the smallest attainable p is 1/64. That count was never reported. It is now (`permutations`, `min_p_value`), read from the control adonis2 used. A term constant within every block - group, when each individual belongs to one group - never has its labels exchanged by within-block permutation, so the p-value vegan prints for it does not test a group difference (measured: 0.03125 with a real effect, 0.0625 on noise; it is not fixed at 1). Its p-value is withheld with the reason; R2 and F are still reported, and terms that vary within blocks (run) are tested as before. Tests (test_permanova.jl, against direct vegan::adonis2 calls with the same seed): per-term names, R2, F and p match by = "terms"; 999 permutations unblocked; 63 permutations and floor 1/64 blocked; group's p withheld with its reason while run's p and F match vegan. 184/184 with test_determinism. Reverting by = "terms" and the refusal: 8 fail, 4 error. Frontend shows the reason and the permutation count; typecheck clean, 27/27. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Joshua Jewell <developer@joshuajewell.dev>
What was wrong (review F1, F2, F3, F6):
- F1: the analysis tabs stay mounted, and DiversityPanel (alpha, NMDS,
PERMANOVA), VennPanel, DifferentialPanel and TaxaCompositionChart kept
their last result after the runs, dataset, aggregate flag, rank or chart
options that produced it changed. A request still in flight for the old
inputs was stored on arrival under the new ones, and a failed rerun in
VennPanel, DiversityPanel and TaxaCompositionChart left the previous
result on screen.
- F2: failures read as findings. VennPanel and DifferentialPanel caught each
rank lookup as [] and so showed "No taxonomy rank shared by the selected
runs"; discoverResultsTables caught as [] and AnalysisWorkspace showed
"no results table shared by the selected runs"; DiversityPanel turned a
failed capabilities check into "NMDS and PERMANOVA need R with vegan".
- F3: useApi kept the previous data when a request failed, so DataTable
showed rows for the old filters under the new ones next to the error.
- F6: the TaxaCompositionChart rank effect had no cancelled flag, so a slow
response for an earlier run or table could overwrite the current ranks.
What changed:
- Each panel clears its result when its request inputs change and tags
each request with a counter (the useRef pattern already used in
FigureBuilder and phylogeny/common), dropping responses, errors and
loading resets from superseded requests. A failed rerun clears the old
result.
- New sharedRanks() helper (used by Venn and Differential) rejects when any
run's ranks cannot be read; the panels show "Could not read the taxonomy
ranks: <error>" instead of the empty-intersection message.
- discoverResultsTables() now rejects on failure. New
useSharedResultsTablesState() returns { options, error } and
AnalysisWorkspace shows the error; useSharedResultsTables() keeps its
signature for its other callers. CompositionPanel, which calls
discoverResultsTables directly, now catches and toasts the error.
- DiversityPanel reports a failed capabilities check as an error, not as
"R unavailable".
- useApi drops data on error unless the failing request is a re-run of the
very request that produced it (same fetcher), so polling refetches that
fail transiently (RunView, Layout) keep data for the same inputs while a
request for new inputs never shows the old data.
- TaxaCompositionChart rank effect gets a cancelled flag.
Tests (bun test src): 36 pass, 0 fail (27 before, 9 new) in useApi.test.ts,
sharedRanks.test.ts, annotationShared.test.ts. bun run typecheck clean.
Planted control: restoring the old behaviour (useApi keeping data on every
error, the [] catch in discoverResultsTables, a per-run [] catch in
sharedRanks) gave 33 pass, 3 fail, one per planted defect. The component
reset and request-counter logic has no automated test: the frontend has no
DOM or React rendering test setup.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
H1: the CORS middleware accepted any Origin whose hostname was localhost, 127.0.0.1 or ::1, whatever its scheme and port, and reflected it with PUT/PATCH/DELETE allowed. Any page on another localhost port (e.g. a dev server on :3000) could drive the whole API, including a text/plain POST that creates data. Origins are now checked against an explicit set: the server's own bound port spelled with localhost and 127.0.0.1 (plus the bound host itself when it is a specific name), the Vite dev server origin (localhost/127.0.0.1:5173, which proxies /api and /files without rewriting Origin), and any extra origins in JULIA_METAMANIFOLD_ALLOWED_ORIGINS. Requests with no Origin pass as before; the Host check is unchanged. H4: serve() was called with Oxygen's defaults, so /docs, /docs/schema and the metrics dashboard were registered by DocsMiddleware, which Oxygen composes outside the app middleware, i.e. outside the Host/Origin checks. Nothing in the frontend or docs uses them; they are now off (_SERVE_OPTIONS: docs=false, metrics=false). H5: report uploads keep arbitrary bytes for allowed extensions and served .svg inline as image/svg+xml, so a script-bearing SVG opened directly ran in the app origin. The report file route, and /files/ when it resolves into a study's report/ directory, now send X-Content-Type-Options: nosniff and Content-Security-Policy "default-src 'none'; style-src 'unsafe-inline'; sandbox". Every /files/ response gets nosniff; pipeline output (MultiQC HTML, PDFs) gets no CSP so it keeps working. <img> rendering in the report panel is unaffected. Tests: test/unit/test_server_security.jl (74 assertions) drives requests in-process through the app middleware and through the handler Oxygen's serve() builds from _SERVE_OPTIONS (setupmiddleware, no socket). Planted control: restoring the old hostname-only origin check, docs and metrics on, and dropping the upload headers gives 39 pass / 35 fail; with the fix 74/74. test_server_security + test_routes + test_report_funnel + test_trees: 366/366 pass (Julia 1.12.6). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With web/dist built (as in CI), unknown paths fall through to the SPA catch-all and answer 200 with index.html. The docs-disabled test now accepts that exact body as well as a non-200 status; the openapi/swagger/ redoc body checks are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013aSu89DNALjTYHBvA6FcoM
…old about Review on #33: `apiBase` was mentioned only in a comment in client.ts and JULIA_METAMANIFOLD_ALLOWED_ORIGINS only in the environment-variable table, so anyone serving the frontend from another address, or reaching the server through an SSH tunnel on another local port, would get 403 with nothing pointing at the setting. Add a "Split deployment" subsection under Deployment: what to set on each side, that origins match exactly (scheme and port included), what a page on an unlisted origin sees, and that a tunnel on a different port is a different origin. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqBtj4KbxVrGMBVZ4gNFUi
…t the vsearch stage's peak depends on order A MemoryProbe (src/core/memory_probe.jl), off unless a pipeline.yml sets occupancy_probe.enabled: true. When on, each attested stage gains an `occupancy` record in attestation.yml: - julia_allocated_bytes: the Base.gc_num() delta over the stage; - per spawned task: the sampled peak VmRSS of the child processes it ran, and the kernel's VmHWM for each; - child_peak_rss_kib: the largest sum of the stage's children's VmRSS at one sampling instant. It reads child memory from /proc/<pid>/status at a 10 ms interval, not Sys.maxrss, which is the Julia process's own lifetime peak. With the flag off, nothing starts and a stage record has exactly its earlier keys, so attestations are byte-identical. The default stays commented out in config/defaults/pipeline.yml, because the run config is embedded in attestation.yml. test/occupancy_disagreement_test.jl runs the vsearch stage's two searches (ASV and OTU) on a generated fixture in three orders: ASV then OTU, OTU then ASV, and concurrently, as the full pipeline spawns them. It asserts: - the hits are identical in every order; - the Julia allocation agrees within 1%; - the concurrent stage peak exceeds the serial one. The fixture is 1000 references, 300 ASV and 150 OTU queries, the smallest that kept the two searches resident together for at least 15 samples in every repeat. vsearch runs with --threads 1 here: with several threads its output order varies, and _sort_hits_by_query! then does a different amount of work, which moved the allocation by up to 12%. Cherry-picked from hyperpolymath/MetaManifold-WebUI bf68ba1 (#106), unchanged except the end-of-file block in config/defaults/pipeline.yml, which now follows differential.replacement_delta. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015pMjy981ie74uCttyYDanC
…tested helper
Codecov reported this PR's patch coverage at 94.70%. 7 lines in
src/server/routes/pipeline.jl were missed: the vsearch searches wrapped in
MemoryProbe.task, in _run_full_pipeline and in _run_stage. No test runs
either function, so those lines, like the vsearch calls they replaced, had
never been executed. A further line in src/core/memory_probe.jl was missed:
the watch!(_) fallback.
- _vsearch_searches(asv, otu; concurrent) holds both shapes of the stage:
- concurrent, as the full pipeline runs them, both waited for before a
failure is rethrown;
- one after the other, as a single-stage re-run does.
_vsearch_stage(search, project, asvs, otus, db; concurrent) builds the
two calls. Each call site is now that one line.
- test/occupancy_disagreement_test.jl now runs its ASV-then-OTU and
concurrent orders through _vsearch_searches. It measures the pipeline's
own code, not a copy of it. New tests cover:
- both shapes' return values and task labels;
- _vsearch_stage with a stub search;
- that a failed concurrent ASV search is rethrown only after the OTU
search finishes;
- that watch! ignores a handle that is neither a Process nor a
ProcessChain, so the probe cannot fail the stage it measures.
One behaviour change. A single-stage vsearch re-run used to return only
the OTU search's result, or nothing when there was no OTU search. Its
provenance record therefore left out the ASV taxonomy file. It now
returns both results, as the full pipeline does, so both are attested.
Patch coverage, computed locally with CoverageTools 1.4.2 as CI processes
it, is now 156/159 (98.11%):
- memory_probe.jl: 137/137;
- pipeline.jl: 16/19.
The 3 lines left are the call into _vsearch_stage and the OTU read, inside
_run_full_pipeline and _run_stage, which no test runs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015pMjy981ie74uCttyYDanC
…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>
… community files The README gains a Documentation section pointing at the wiki in metadatastician/MetaManifold-Evidence (docs/wikis), and a Differential Abundance section documenting the tab end to end: the two-condition selection semantics, rank-list scope, aggregation, the Unclassified pooling after the read-count bounds and exclude_categories, the per-taxon negative-binomial GLM (MASS::glm.nb with a size-factor offset, no pseudocounts, Wald test, Benjamini-Hochberg over the taxa that produced a p-value), where the size factors come from, and where the code and the route live. Settings cross-reference the existing Configuring analysis anchor. Also adds the community files GitHub surfaces by convention: a bug report and feature request issue template (both telling reporters to search first and to raise fork-specific issues on the fork), a pull request template carrying the PR checklist, and the Contributor Covenant code of conduct. Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe <vibe@mistral.ai>
…ings in the README Both CodeRabbit findings from #143, accepted; the PR merged before the fixes could ride on it, so they land here. - The bug report form required R and browser versions from everyone: a command-line reporter has no browser to name, and a web-interface reporter often cannot know the server's R version, so both fields forced placeholder answers. They are now optional, each with the wording that says when it applies; Julia stays required, since every reporter runs the Julia server. - The differential-abundance README section described only the default nb_glm method and listed only offset and min_prevalence, presenting the analysis as NB-only; clr_welch and replacement_delta have been in the pipeline config and code since the upstream syncs. The section now documents method and replacement_delta, notes that offset applies to nb_glm only, and states clr_welch's effect as a CLR difference, not a fold change. The wording follows config/defaults/pipeline.yml's own comments for the same keys. Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe <vibe@mistral.ai>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 59 minutes. View limit details
Autofix is paused · Paused
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
enabled auto-merge (squash)
October 15, 2026 22:49
|
Autofix was enabled. Check current status in the Coding task. |
13 conflicted files, all the fork's sync-policy set (CI pinning, docstrings, occupancy work, tests). Fork main's versions win: this branch's docs content was already absorbed there via #143/#145, and its older upstream state is superseded. The net diff against main is CODE_OF_CONDUCT.md (community files this PR adds, not yet on main) and upstream's vsearch entry-point test not yet absorbed. Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe <vibe@mistral.ai>
|
hyperpolymath
disabled auto-merge
October 15, 2026 23:15
hyperpolymath
enabled auto-merge (squash)
October 15, 2026 23:15
hyperpolymath
disabled auto-merge
October 15, 2026 23:21
hyperpolymath
disabled auto-merge
October 15, 2026 23:21
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.
Summary
Closes #
Type of change
How has this been verified?
Tested with Julia
…, R…and bun….What this leaves out
Checklist
main(behind_byis0; see CONTRIBUTING.md) and has no merge commits frommain.julia --project=. test/runtests.jl(andcd frontend && bun run testfor frontend changes) and they pass.LICENSE,CITATION.cffand the licence and acknowledgement sections ofREADME.mdunchanged.Notes for reviewers