fix(bench): repair eight latent defects that made the Julia gate unpassable by hyperpolymath · Pull Request #28 · hyperpolymath/MetaManifold-WebUI

hyperpolymath · 2026-09-21T15:26:31Z

Why

The Julia job (Julia 1.12.5 / ubuntu-24.04) is the only gate on this repo
carrying no continue-on-error at job or step level — verified by
awk 'NR>=78 && NR<=445 && /continue-on-error/' returning nothing. It is also
the gate blocking upstream JoshuaJewell/MetaManifold-WebUI#6.

The original blocker (test/integration/test_server.jl:44) is fixed and
verified: run 35614055917 shows Server smoke tests by name and
1727 passed with no Fail column. What that fix uncovered was a second wall of
failures sitting behind it, in the bench scripts.

The trap: serial unmasking

The test job runs seven bench scripts sequentially. The first one to fail
aborts the step and hides every later one. bench/duckdb_aggregation
alone held six broken calls, each visible only once the previous was fixed —
so CI could surface at most one defect per ~21-minute cycle. Running the suite
locally exposed all of them in a single sitting.

Every one of these calls failed the first time it ever ran. The bench scripts
were only recently wired into CI, and code that is never executed cannot
drift-check itself. The unit tests have been calling the same functions
correctly the whole time.

The eight fixes

Corrected against the definitions in src/analysis/analysis.jl and against the
unit tests, which call every one of these functions correctly.

script	function	defect
`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 × 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 no method accepts (`:312`)
`permanova_nmds`	`alpha_boxplot`	`Vector{Any}` matched neither concrete method, and `metric=` is not a keyword of either (`:734`, `:840`)
`analysis_config`	—	built on BenchmarkTools, which was in neither `Project.toml` nor `Manifest.toml`

bench/analysis_config/benchmark.jl is structurally built on BenchmarkTools
(@benchmarkable, BenchmarkGroup, save/load/median) 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, no existing package version changed (git diff -U0 Manifest.toml | grep '^-version' → 0 lines).

One trap the resolve set, caught by mutation

Pkg.resolve also rewrote the Manifest's julia_version from 1.12.5 to
1.12.6 — the julia this laptop 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 shipping it would have turned a second
gate red. Restored by hand (Pkg only warns on that field; it does not gate) and
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, 95/95.

Concurrency (second commit)

cancel-in-progress: true applied to every event. main was merged to at
14:25, 14:28 and 14:43 on 2026-09-21; each run was cancelled 17–18 min into a
~31 min job by the next push, so three consecutive commits produced no test
verdict of any kind. The red everyone was reading as "CI is broken" was, for
those commits, no result at all.

Cancelling stays correct for a pull_request — a verdict on a superseded head is
worthless — so the flag is conditioned on the event rather than removed.

⚠ This does not rescue upstream #6, whose head is this fork's main: its
runs are cancelled by the head update itself, not by this setting. Only letting
main settle for ~31 min does that.

Verification

All seven bench scripts run clean locally, twice each — once creating a
baseline, once comparing against it, which is the path CI takes.
Tracked baselines for the five original categories were not modified by
any run.
bench/analysis_config/baseline.json is deliberately not committed: the
other five came from whichever host produced them, and adding a sixth from a
laptop would make this machine's timings the estate reference. CI regenerates
it per run and exits 0, matching the workflow's own statement that the deltas
are informational.
Repo hygiene is enforcing on this PR (a pull_request reads
github.repository as the BASE repo, which is this fork). All three scripts
run clean locally: check-spdx OK 265 files, check-format OK 311 files,
check-lint OK. Both commit subjects pass the conventional-commit regex.
test_install_pins.jl: 95 passed / 95 total.

🤖 Generated with Claude Code

https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm

…assable 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm

`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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm

coderabbitai · 2026-09-21T15:26:41Z

Warning

Review limit reached

Next included review available in 2 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: b89f4e0b-c873-4ad1-a33e-ea8b3759e2ce

📥 Commits

Reviewing files that changed from the base of the PR and between 57c6db5 and 60d1e1b.

📒 Files selected for processing (7)

.github/workflows/ci.yml
.gitignore
Manifest.toml
Project.toml
bench/duckdb_aggregation/benchmark.jl
bench/permanova_nmds/benchmark.jl
bench/table_loading/benchmark.jl

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

sonarqubecloud · 2026-09-21T15:27:14Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

…ssable (#28) ## Why The Julia job (`Julia 1.12.5 / ubuntu-24.04`) is the only gate on this repo carrying no `continue-on-error` at job or step level — verified by `awk 'NR>=78 && NR<=445 && /continue-on-error/'` returning nothing. It is also the gate blocking upstream `JoshuaJewell#6`. The original blocker (`test/integration/test_server.jl:44`) is **fixed and verified**: run `35614055917` shows `Server smoke tests` by name and `1727 passed` with no Fail column. What that fix uncovered was a second wall of failures sitting behind it, in the bench scripts. ## The trap: serial unmasking The `test` job runs seven bench scripts sequentially. The first one to fail aborts the step and **hides every later one**. `bench/duckdb_aggregation` alone held six broken calls, each visible only once the previous was fixed — so CI could surface at most one defect per ~21-minute cycle. Running the suite locally exposed all of them in a single sitting. Every one of these calls failed *the first time it ever ran*. The bench scripts were only recently wired into CI, and code that is never executed cannot drift-check itself. The unit tests have been calling the same functions correctly the whole time. ## The eight fixes Corrected against the definitions in `src/analysis/analysis.jl` and against the unit tests, which call every one of these functions correctly. | script | function | defect | |---|---|---| | `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 × 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 no method accepts (`:312`) | | `permanova_nmds` | `alpha_boxplot` | `Vector{Any}` matched neither concrete method, and `metric=` is not a keyword of either (`:734`, `:840`) | | `analysis_config` | — | built on BenchmarkTools, which was in neither `Project.toml` nor `Manifest.toml` | `bench/analysis_config/benchmark.jl` is structurally built on BenchmarkTools (`@benchmarkable`, `BenchmarkGroup`, save/load/median) 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, **no existing package version changed** (`git diff -U0 Manifest.toml | grep '^-version'` → 0 lines). ### One trap the resolve set, caught by mutation `Pkg.resolve` also rewrote the Manifest's `julia_version` from 1.12.5 to **1.12.6** — the julia this laptop 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 shipping it would have turned a *second* gate red. Restored by hand (Pkg only warns on that field; it does not gate) and 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, 95/95. ## Concurrency (second commit) `cancel-in-progress: true` applied to every event. `main` was merged to at 14:25, 14:28 and 14:43 on 2026-09-21; each run was cancelled 17–18 min into a ~31 min job by the next push, so **three consecutive commits produced no test verdict of any kind**. The red everyone was reading as "CI is broken" was, for those commits, no result at all. Cancelling stays correct for a `pull_request` — a verdict on a superseded head is worthless — so the flag is conditioned on the event rather than removed. ⚠ This does **not** rescue upstream #6, whose head *is* this fork's `main`: its runs are cancelled by the head update itself, not by this setting. Only letting `main` settle for ~31 min does that. ## Verification - All seven bench scripts run clean locally, **twice each** — once creating a baseline, once comparing against it, which is the path CI takes. - Tracked baselines for the five original categories were **not** modified by any run. - `bench/analysis_config/baseline.json` is deliberately **not** committed: the other five came from whichever host produced them, and adding a sixth from a laptop would make this machine's timings the estate reference. CI regenerates it per run and exits 0, matching the workflow's own statement that the deltas are informational. - Repo hygiene is **enforcing** on this PR (a `pull_request` reads `github.repository` as the BASE repo, which is this fork). All three scripts run clean locally: `check-spdx` OK 265 files, `check-format` OK 311 files, `check-lint` OK. Both commit subjects pass the conventional-commit regex. - `test_install_pins.jl`: 95 passed / 95 total. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm

hyperpolymath and others added 2 commits September 21, 2026 16:25

hyperpolymath merged commit 1f1821d into main Sep 21, 2026
3 of 4 checks passed

hyperpolymath deleted the fix/bench-api-drift branch September 21, 2026 15:28

Navigation Menu

Uh oh!

fix(bench): repair eight latent defects that made the Julia gate unpassable - #28

fix(bench): repair eight latent defects that made the Julia gate unpassable#28
hyperpolymath merged 2 commits into
mainfrom
fix/bench-api-drift

hyperpolymath commented Sep 21, 2026

Uh oh!

coderabbitai Bot commented Sep 21, 2026

Review limit reached

Uh oh!

sonarqubecloud Bot commented Sep 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

hyperpolymath commented Sep 21, 2026

Why

The trap: serial unmasking

The eight fixes

One trap the resolve set, caught by mutation

Concurrency (second commit)

Verification

Uh oh!

coderabbitai Bot commented Sep 21, 2026

Review limit reached

Uh oh!

sonarqubecloud Bot commented Sep 21, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant