From 8ca60c4d8c34d8bfecfb0615b337552d9ccf1eff Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Mon, 21 Sep 2026 16:25:50 +0100 Subject: [PATCH 1/2] fix(bench): repair seven API-drift calls that made the Julia gate unpassable Every bench script is run by the `test` job, which carries no `continue-on-error` at job or step level, so any one of them failing is fatal to `Julia 1.12.5 / ubuntu-24.04` -- the only honest gate on this repo and the one blocking upstream PR #6. All of these calls were unreachable until the bench steps were wired into CI, so each failed the first time it ever ran. They failed SERIALLY: the first script aborted the step, masking the next, so CI could only reveal one per ~21-minute cycle. All seven were instead run locally under julia 1.12.6 until clean, twice each -- once creating a baseline and once comparing against it, which is the path CI takes. Six of the seven fixes are in one file. Corrected against the definitions in `src/analysis/analysis.jl` and against the unit tests, which call every one of these functions correctly: table_loading filtered_df 7 args -> 4 (:130) duckdb_aggregation aggregate_by_taxon 4 args -> 6 (:141) duckdb_aggregation venn_taxa_present invented a `[g1, g2]` group-list parameter no method has ever taken; a Venn is assembled by calling it once per group (:161) duckdb_aggregation bar_chart passed (labels, counts, names); counts belongs third, and the matrix is (segments x samples) per :403 and test_analysis.jl:35 duckdb_aggregation taxa_bar_chart arguments 2 and 3 transposed (:475) duckdb_aggregation alpha_chart passed a 5th `groups` argument that no method accepts (:312) permanova_nmds alpha_boxplot `Vector{Any}` matched neither concrete method, and `metric=` is not a keyword of either. This file's own header says it measures "alpha_boxplot with significance", so `annotate_significance=true` is what was meant (:734, :840) `bench/analysis_config/benchmark.jl` failed for a different reason: it is built on BenchmarkTools (`@benchmarkable`, `BenchmarkGroup`, save/load/median) but BenchmarkTools was in neither Project.toml nor Manifest.toml, and CI never installs it -- so that step has never once succeeded since it was wired in. Pinned rather than fetched at CI time, because an unpinned network fetch would defeat the committed Manifest. The resolve is purely additive: BenchmarkTools 1.8.0 plus the Profile stdlib, with no existing package version changed. Its baseline.json is deliberately NOT committed. The other five baselines are tracked, but those numbers came from whichever host produced them; adding a sixth from a laptop would make this machine's timings the reference. CI regenerates it per run and exits 0, which matches the workflow's own statement that the deltas are informational. `bench/results/` is a run artifact and is now ignored alongside the existing `frontend/bench/results/` rule. The resolve also rewrote `julia_version` in the Manifest from 1.12.5 to 1.12.6 -- the julia this laptop actually runs, despite mise.toml pinning 1.12.5. That line is asserted by test/unit/test_install_pins.jl:112 against config/defaults/tool_versions.yml, so leaving it would have turned a second gate red. It is restored to 1.12.5 by hand; Pkg only warns on that field disagreeing with the running julia, it does not gate. Verified by mutation: with 1.12.6 the suite reports 94 pass / 1 fail at :112 ("1.12.5" == "1.12.6"), with 1.12.5 it reports 95/95. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm --- .gitignore | 1 + Manifest.toml | 15 ++++++++-- Project.toml | 2 ++ bench/duckdb_aggregation/benchmark.jl | 43 ++++++++++++++++++++++----- bench/permanova_nmds/benchmark.jl | 16 ++++++++-- bench/table_loading/benchmark.jl | 11 +++++-- 6 files changed, 73 insertions(+), 15 deletions(-) diff --git a/.gitignore b/.gitignore index fde8c7f..eac41e5 100644 --- a/.gitignore +++ b/.gitignore @@ -265,6 +265,7 @@ docs/* frontend/tests/results/ frontend/tests/coverage/ frontend/bench/results/ +bench/results/ # translation temp files diff --git a/Manifest.toml b/Manifest.toml index 409711d..dbfb54e 100644 --- a/Manifest.toml +++ b/Manifest.toml @@ -2,7 +2,7 @@ julia_version = "1.12.5" manifest_format = "2.0" -project_hash = "e7c84cb4c927ffec8c0134c4675cef074e79ac93" +project_hash = "5b8f89666567516def011d4b7c2b6a7d0010a274" [[deps.AliasTables]] deps = ["PtrArrays", "Random"] @@ -27,6 +27,12 @@ version = "1.11.0" uuid = "2a0f44e3-6c83-55bd-87e4-b1978d98bd5f" version = "1.11.0" +[[deps.BenchmarkTools]] +deps = ["Compat", "JSON", "Logging", "PrecompileTools", "Printf", "Profile", "Statistics", "UUIDs"] +git-tree-sha1 = "9670d3febc2b6da60a0ae57846ba74670290653f" +uuid = "6e4b80f9-dd63-53aa-95a3-0cdb28fa8baf" +version = "1.8.0" + [[deps.BitFlags]] git-tree-sha1 = "0691e34b3bb8be9307330f88d1a3c3f25466c24d" uuid = "d1d4a3ce-64b1-5f1a-9ba4-7e7e69966f35" @@ -416,7 +422,7 @@ uuid = "c8ffd9c3-330d-5841-b78e-0817d7145fa1" version = "2.28.1010+0" [[deps.MetaManifold]] -deps = ["CSV", "DBInterface", "DataFrames", "Dates", "Downloads", "DuckDB", "HTTP", "JSON3", "Logging", "OrderedCollections", "Oxygen", "PackageCompiler", "RCall", "Random", "SHA", "UUIDs", "XLSX", "YAML"] +deps = ["CSV", "DBInterface", "DataFrames", "Dates", "Downloads", "DuckDB", "HTTP", "JSON3", "Logging", "OrderedCollections", "Oxygen", "PackageCompiler", "RCall", "Random", "SHA", "Statistics", "UUIDs", "XLSX", "YAML"] path = "." uuid = "ea959a01-5458-4cf1-8f0d-4ed45446c396" version = "0.0.0" @@ -551,6 +557,11 @@ deps = ["Unicode"] uuid = "de0858da-6303-5e67-8744-51eddeeeb8d7" version = "1.11.0" +[[deps.Profile]] +deps = ["StyledStrings"] +uuid = "9abbd945-dff8-562f-b5e8-e1ebf5ef1b79" +version = "1.11.0" + [[deps.PtrArrays]] git-tree-sha1 = "4fbbafbc6251b883f4d2705356f3641f3652a7fe" uuid = "43287f4e-b6f4-7ad1-bb20-aadabca52c3d" diff --git a/Project.toml b/Project.toml index f4e2835..6ec2068 100644 --- a/Project.toml +++ b/Project.toml @@ -2,6 +2,7 @@ name = "MetaManifold" uuid = "ea959a01-5458-4cf1-8f0d-4ed45446c396" [deps] +BenchmarkTools = "6e4b80f9-dd63-53aa-95a3-0cdb28fa8baf" CSV = "336ed68f-0bac-5ca0-87d4-7b16caf5d00b" DBInterface = "a10d1c49-ce27-4219-8d33-6db1a4562965" DataFrames = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0" @@ -23,6 +24,7 @@ XLSX = "fdbf4ff8-1666-58a4-91e7-1b58723a45e0" YAML = "ddb6d928-2868-570f-bddf-ab3f9cf99eb6" [compat] +BenchmarkTools = "1.8.0" CSV = "0.10" DataFrames = "1.8" PackageCompiler = "2.2.5" diff --git a/bench/duckdb_aggregation/benchmark.jl b/bench/duckdb_aggregation/benchmark.jl index b9dccca..f752cde 100644 --- a/bench/duckdb_aggregation/benchmark.jl +++ b/bench/duckdb_aggregation/benchmark.jl @@ -36,27 +36,53 @@ function _create_mock_db(n_samples::Int=20, n_features::Int=1000) end function bench_aggregate_by_taxon(con, sample_cols) - @elapsed aggregate_by_taxon(con, "merged", sample_cols, "Genus") + # `aggregate_by_taxon(con, table, sample_cols, rank, where_clause, where_params)` + # -- six arguments. This previously passed four, omitting the trailing filter + # pair. `src/analysis/analysis.jl:141` has required all six since the function + # was introduced, and `test/unit/test_analysis_duckdb.jl` calls it that way. + # The call was unreachable until the bench steps were wired into CI, so it + # failed the moment it first ran. An empty filter benchmarks the unfiltered + # aggregation, which is what the header comment says this measures. + @elapsed aggregate_by_taxon(con, "merged", sample_cols, "Genus", "", []) end function bench_venn_taxa_present(con, sample_cols) # Split samples into 2 groups g1 = sample_cols[1:div(length(sample_cols),2)] g2 = sample_cols[div(length(sample_cols),2)+1:end] - @elapsed venn_taxa_present(con, "merged", sample_cols, [g1, g2], "Genus") + # `venn_taxa_present(con, table, sample_cols, rank_col, where_clause, where_params)` + # returns the taxa present in ONE sample set (`src/analysis/analysis.jl:161`). + # It has never accepted a list of groups: the previous call passed `[g1, g2]` + # as a fourth argument in a five-argument form that matches no method. A Venn + # is assembled by calling it once per group, which is what this now measures. + @elapsed begin + venn_taxa_present(con, "merged", g1, "Genus", "", []) + venn_taxa_present(con, "merged", g2, "Genus", "", []) + end end function bench_bar_chart() - labels = ["GroupA", "GroupB", "GroupC"] + # `bar_chart(segment_labels, sample_names, counts; top_n, ...)` -- the counts + # matrix is (segments x samples), as `src/analysis/analysis.jl:403` (column + # totals are per-sample) and `test/unit/test_analysis.jl:35` ("2 taxa x 2 + # samples") both establish. The previous call passed (labels, counts, names), + # putting the matrix in the `sample_names` position, so no method matched. + # The 100x3 matrix means 100 segments (taxa) across 3 samples (groups), so the + # taxon vector is the segment labels and the group vector the sample names. + segment_labels = ["Taxon$i" for i in 1:100] + sample_names = ["GroupA", "GroupB", "GroupC"] counts = rand(100, 3) * 1000 - @elapsed bar_chart(labels, counts, ["Taxon$i" for i in 1:100], top_n=20) + @elapsed bar_chart(segment_labels, sample_names, counts, top_n=20) end function bench_taxa_bar_chart() labels = ["Taxon$i" for i in 1:50] counts = rand(50, 10) * 100 sample_names = ["Sample$i" for i in 1:10] - @elapsed taxa_bar_chart(labels, counts, sample_names, top_n=20) + # `taxa_bar_chart(taxon_labels, sample_names, counts; ...)` -- arguments 2 and + # 3 were transposed here. The 50x10 matrix is already (taxa x samples), which + # is the orientation the function wants; only the call order was wrong. + @elapsed taxa_bar_chart(labels, sample_names, counts, top_n=20) end function bench_alpha_chart() @@ -64,8 +90,11 @@ function bench_alpha_chart() richness = rand(50:500, 20) shannon = rand(1.0:0.1:5.0, 20) simpson = rand(0.5:0.01:0.99, 20) - groups = [rand(["Control", "Disease"]) for _ in 1:20] - @elapsed alpha_chart(sample_names, richness, shannon, simpson, groups) + # `alpha_chart(sample_names, richness, shannon, simpson)` takes exactly four + # arguments (`src/analysis/analysis.jl:312`); it has no grouping parameter, and + # the `groups` vector built here was never consumed by any method. Grouped + # alpha display is `alpha_boxplot`'s job, benchmarked in permanova_nmds. + @elapsed alpha_chart(sample_names, richness, shannon, simpson) end function run_benchmarks(; n_samples=20, n_features=1000, reps=5) diff --git a/bench/permanova_nmds/benchmark.jl b/bench/permanova_nmds/benchmark.jl index c789aaf..a179ac6 100644 --- a/bench/permanova_nmds/benchmark.jl +++ b/bench/permanova_nmds/benchmark.jl @@ -50,15 +50,25 @@ function bench_normalise_counts(n::Int=100, n_features::Int=1000) end function bench_alpha_boxplot(n_groups::Int=3, n_per_group::Int=10) - groups = [] + # Two defects, both latent until the bench steps were wired into CI. + # + # 1. `groups = []` is a `Vector{Any}`, which matches NEITHER `alpha_boxplot` + # method (`src/analysis/analysis.jl:734` and `:840` both dispatch on a + # concrete `Vector{Tuple{...}}`). The elements pushed below are already the + # right 5-tuple shape, so annotating the container is all that is needed. + # 2. `metric=` is not a keyword of either method. The accepted keywords are + # show_points, annotate_significance, pairwise_brackets, paired_samples and + # significance_test. This file's own header says it measures "alpha_boxplot + # with significance", so `annotate_significance=true` is what was meant -- + # and it exercises more of the function than the default would. + groups = Tuple{String, Vector{String}, Vector{Int}, Vector{Float64}, Vector{Float64}}[] for g in 1:n_groups sample_names = ["Group$(g)_Sample$(i)" for i in 1:n_per_group] - counts = rand(50:500, n_per_group) shannon_vals = rand(1.0:0.1:5.0, n_per_group) simpson_vals = rand(0.5:0.01:0.99, n_per_group) push!(groups, ("Group$g", sample_names, collect(1:n_per_group), shannon_vals, simpson_vals)) end - @elapsed alpha_boxplot(groups, metric="shannon") + @elapsed alpha_boxplot(groups, annotate_significance=true) end function bench_nmds_chart(n::Int=20) diff --git a/bench/table_loading/benchmark.jl b/bench/table_loading/benchmark.jl index 2953006..8f37e80 100644 --- a/bench/table_loading/benchmark.jl +++ b/bench/table_loading/benchmark.jl @@ -46,8 +46,13 @@ function bench_filtered_counts(con, sample_cols, table::String="merged") @elapsed filtered_counts(con, table, sample_cols, "", []) end -function bench_filtered_df(con, sample_cols, table::String="merged") - @elapsed filtered_df(con, table, sample_cols, "", [], 1, 100) +function bench_filtered_df(con, table::String="merged") + # `filtered_df(con, table, where_clause, where_params)` -- four arguments. + # This previously passed seven (sample_cols + an offset/limit pair), a + # paginated signature that has never existed on any commit: the function has + # taken these four arguments since `2987464`. The call was unreachable until + # the bench steps were wired into CI, so it failed the moment it first ran. + @elapsed filtered_df(con, table, "", []) end function bench_taxonomy_levels(con, table::String="merged") @@ -72,7 +77,7 @@ function run_benchmarks(; n_samples=20, n_features=1000, reps=5) for _ in 1:reps push!(results["sample_columns"], bench_sample_columns(con)) push!(results["filtered_counts"], bench_filtered_counts(con, sample_cols)) - push!(results["filtered_df"], bench_filtered_df(con, sample_cols)) + push!(results["filtered_df"], bench_filtered_df(con)) push!(results["taxonomy_levels"], bench_taxonomy_levels(con)) push!(results["taxon_column"], bench_taxon_column()) end From 60d1e1bd15dcb5870787903efe085ae4d557b498 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Mon, 21 Sep 2026 16:25:50 +0100 Subject: [PATCH 2/2] fix(ci): queue `main` pushes instead of cancelling them `cancel-in-progress: true` applied to every event. The Julia job runs ~31 min, and `main` was merged to at 14:25, 14:28 and 14:43 on 2026-09-21. Each run was cancelled 17-18 min in by the next push, so three consecutive commits to `main` produced no test verdict of any kind -- the suite was never once allowed to finish. The red that everyone was reading as "CI is broken" was, for those commits, no result at all. Cancelling remains correct for a pull_request: a verdict on a superseded head is worthless. So the flag is now conditioned on the event rather than removed. This does not rescue an upstream PR whose head keeps moving -- JoshuaJewell#6's head IS this fork's `main`, so its runs are cancelled by the head update itself, not by this setting. Only letting `main` settle for ~31 min does that. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm --- .github/workflows/ci.yml | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a424232..b1f0103 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,7 +9,21 @@ on: concurrency: group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true + # A push to `main` QUEUES; a pull_request head update still cancels. + # + # Measured 2026-09-21: the Julia job takes ~31 min, and `main` was merged to at + # 14:25, 14:28 and 14:43. Every run was cancelled 17-18 min in by the next one, + # so three consecutive commits to `main` produced NO test verdict at all -- + # not a pass, not a failure, nothing. Cancelling is right for a PR, where a + # verdict on a superseded head is worthless; it is wrong for `main`, where each + # commit is a thing we actually want a recorded answer about. + # + # Trade-off, stated plainly: pushes to `main` now run serially, so a burst of + # N merges takes N x ~31 min to drain. That is the cost of getting an answer. + # This does NOT rescue an upstream PR whose head keeps moving (e.g. + # JoshuaJewell#6, whose head IS this fork's `main`) -- only letting `main` + # settle for ~31 min does that. + cancel-in-progress: ${{ github.event_name != 'push' }} jobs: # Estate hygiene gates (standards/RSR alignment): cheap, run alongside the