diff --git a/.gitignore b/.gitignore index 54a15cc6..f072065a 100644 --- a/.gitignore +++ b/.gitignore @@ -262,6 +262,7 @@ docs/* !docs/type-system/ !docs/types/ !docs/reproducibility.md +!docs/owner-review-2026-09-25.md # Test + benchmark run artifacts (not the committed fixtures/baselines) frontend/tests/results/ diff --git a/CHANGELOG.md b/CHANGELOG.md index a70b2156..51961d9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,21 @@ types, tests, infrastructure, and alignment. ## [Unreleased] +### Fixed — the ILR path stops substituting a basis it was not asked for (2026-09-25) + +- **Unimplemented ILR bases are refused rather than substituted.** The configuration + accepted `phylogenetic`, `sequential_binary_partition` and `balance_dendrogram`, and + the execution path warned and computed the default Helmert basis instead. An ILR + balance is only interpretable under the basis that defined it, so substituting a + basis computes numbers that mean something other than what the analyst asked for. + The three deferred bases (`DEFERRED_ILR_BASIS`) are refused at construction and in + `prepare_analysis_table`. +- **Balances are labelled as balances.** An ILR table of $n$ taxa has $n-1$ balances, + not $n$ features. The rows are relabelled `balance_1`..`balance_n-1`, + `diagnostics.checks["ilr"]` records the basis, definition, input taxa count and + taxa order, and the all-zero-taxa healing block no longer restores taxon labels onto + balance rows. + ### Fixed — `method = "TSS"` was refused by the layer that claimed to have shipped it (2026-09-25) - **The allowed-normalisation table held the three names in a different case from the diff --git a/docs/owner-review-2026-09-25.md b/docs/owner-review-2026-09-25.md new file mode 100644 index 00000000..58ef6773 --- /dev/null +++ b/docs/owner-review-2026-09-25.md @@ -0,0 +1,110 @@ + +# NOTE TO JOSHUA: OWNER REVIEW (25 SEPTEMBER 2026) + +# HELLO JOSHUA! PLEASE READ THIS FIRST. + +This document is written for you, Joshua Jewell, owner and creator of **MetaManifold-WebUI**. +It explains what has been happening on the **hyperpolymath** fork of your repository, what was changed, why it was changed, and what choices you have now. +It is written in plain, friendly English so that anyone—including a smart 14-year-old—can follow along without getting lost in technical jargon. + +--- + +## 1. WORD GLOSSARY + +Before we start, here are nine simple words you will see throughout this note: + +- **Project (or Codebase):** The collection of computer files, scripts, and notes that make up MetaManifold-WebUI. +- **Fork:** A copy of your original project created under a different account (`hyperpolymath`) where new work, bug fixes, and testing happen without changing your original files. +- **Commit:** A saved snapshot of changes to files, like saving a new draft of a document with a note explaining what was updated. +- **Pull Request (PR):** An invitation to take changes made on a branch or fork and add them back into a main project. +- **Merge:** Accepting a Pull Request and combining its saved changes into the project's main branch. +- **Issue:** A post on GitHub used to report a bug, request a new feature, or ask a question. +- **Open vs. Closed:** An "Open" issue is something currently being tracked or planned. A "Closed" issue means the specific discussion or fix is finished. +- **Test:** A short automated check that runs the code with sample data and verifies that the output matches known mathematics. +- **CI (Continuous Integration):** A robot in the cloud (GitHub Actions) that automatically runs every test whenever code is pushed, showing a green checkmark if all tests pass or a red X if something broke. + +--- + +## 2. WHAT HAPPENED IN FIVE SENTENCES + +1. You built MetaManifold-WebUI as a visual workbench for microbiome analysis, with the original code last updated on 21 July 2026. +2. The team on the `hyperpolymath` fork set up strict tests and discovered that some analysis screens were showing realistic-looking fake numbers rather than running real statistical calculations. +3. We removed all invented numbers, connected real statistical models (`MASS::glm.nb` and linear regressions), and added exact offset calculations for library size and normalisation (TSS, CSS, TMM, and RLE). +4. Whenever a method or package was missing from the locked environment, the software was taught to say "Not Implemented" and refuse to guess, protecting researchers from publishing false biology. +5. All original code remains safe and untouched in your repository, and four pull requests are ready and waiting for your review. + +--- + +## 3. NINE THINGS THAT NEED EXPLANATION + +### 1. Copy-vs-Original Divergence +Your original project (`JoshuaJewell/MetaManifold-WebUI`) has stayed exactly as you left it on 21 July 2026. Meanwhile, on the `hyperpolymath` fork, dozens of improvements, automated tests, mathematical verifications, and safety checks were added. This means the fork has moved ahead significantly while keeping your core vision intact. + +### 2. Four Pull Requests Waiting Since September +Four pull requests were opened against your upstream repository in September 2026: +- PR #7: Stipple and Vue interface migration. +- PR #8: Fix for the 1x1 matrix edge case. +- PR #10: Baseline benchmarks and CI setup. +- PR #11: Pinned download retries for reliable setup. +These are ready whenever you want to inspect or merge them. + +### 3. Old Screens Invented Numbers — Deleted, With a Guard Test +Earlier versions of the execution harness had placeholder code that computed p-values from `hash(taxon_id)`. Because the numbers came from the letters in the microbe's name and not from the sequencing counts, they looked believable but were completely meaningless. We deleted that placeholder code completely, replaced it with real statistical models, and added a test that fails automatically if fake numbers are ever reintroduced. + +### 4. One Capital Letter Reddened CI ("tss" vs "TSS") +During work on normalisation offsets, the configuration saved the method name in lowercase (`"tss"`), while the compatibility list checked for uppercase (`"TSS"`). That single mismatched letter caused valid runs to be rejected and turned CI red. We fixed both sides to compare in lowercase, and CI now accepts all standard spellings. + +### 5. Closed Does Not Mean Done For Deferred Notes +When issues on the fork were closed after reaching milestone boundaries, that did not mean every conceivable feature was built. Deferred ideas (such as phylogenetic tree calculations or complex multinomial models) are carefully recorded in `docs/milestones/02-deferred-issues.md` so that future researchers know exactly what remains for future versions. + +### 6. Two Issues Must Stay Open +Two issues must never be closed prematurely: +- **Issue #1 (The Julia Statistics Layer):** The overarching umbrella for exact, validated biological statistics. +- **Issue #2 (The Symbolic Mathematics Engine):** Kept with a large **BLOCKED** sign. Symbolic formula manipulation must wait until the core numerical statistics are proven solid on real biological data. + +### 7. Refusals Are a Feature, Not a Failure +If a user asks for an algorithm that is not installed or inputs data that violates mathematical rules (like asking for log of zero or running a negative binomial model with no offset), the software now halts immediately with an explicit error. Saying "No, this cannot be computed safely" protects scientists from writing erroneous papers. + +### 8. The Locked Package List (`renv.lock`) +To ensure anyone in the world gets the exact same results today and five years from now, all R packages are pinned in `renv.lock`. Packages not in the lockfile (such as `metagenomeSeq` or `glmGamPoi`) are not used or faked. When requested, the software clearly states that the package is absent from the lockfile rather than guessing. + +### 9. Nothing Was Deleted From Your Original Project +Not a single file, commit, or branch on `JoshuaJewell/MetaManifold-WebUI` was overwritten or lost. Your repository remains 100% intact, exactly where you left it on 21 July 2026. + +--- + +## 4. FIVE-MINUTE PR-REVIEW WALKTHROUGH + +When you have five minutes to review the pull requests sent to your repository, follow this quick checklist: + +1. **Check PR #11 ("Ci/retry pinned downloads"):** + - Look at `.github/workflows/ci.yml`. + - It simply adds retry logic so that temporary network glitches during package downloads do not fail the build. Safe and straightforward. +2. **Check PR #8 ("Fix/38 drop the 1x1 matrix"):** + - Look at the test and matrix manipulation lines. + - It prevents single-element matrices from crashing array operations. +3. **Check PR #10 ("Feat/baseline benchmarks ci"):** + - Look at `frontend/bench/` and the benchmark script. + - It records runtimes so future pull requests can check if they made the UI slower. +4. **Check PR #7 ("Stipple/Vue migration"):** + - This is the largest PR. It modernises the reactive UI components and brings frontend dependencies into lockfile compliance. + - Run the frontend tests with `bun test` to confirm everything stays green. + +--- + +## 5. FOUR OPTIONS FOR THE FUTURE OF THE FORK + +Here are four ways you can proceed, depending on your time and goals: + +- **Option A (Full Merge & Reconnection):** + Review and merge the open PRs into your repository, then fast-forward your main branch to incorporate the verified statistics and test suite from `hyperpolymath`. +- **Option B (Selective Cherry-Picking):** + Accept the infrastructure and bug-fix PRs (#8, #10, #11), while keeping statistical and UI changes in the fork until you have time to examine them in depth. +- **Option C (Dual-Track / Research Fork):** + Leave your original repository as the historical v1 milestone, and let `hyperpolymath/MetaManifold-WebUI` serve as the active development engine for papers and production deployments. +- **Option D (Handover / Archive Notice):** + Add a short note in your README pointing researchers to the `hyperpolymath` fork for active maintenance, while you retain full credit as the original creator and architect. + +Whatever you decide, thank you for creating MetaManifold! Your work gave this entire effort its foundation. diff --git a/src/analysis/AnalysisConfig.jl b/src/analysis/AnalysisConfig.jl index 5f812c4f..224f81dc 100644 --- a/src/analysis/AnalysisConfig.jl +++ b/src/analysis/AnalysisConfig.jl @@ -98,6 +98,7 @@ const ZERO_POLICY_STRINGS = Dict{String,ZeroPolicy}( const VALID_DISPERSION_METHODS = ("parametric", "local", "mean", "pooled", "glmGamPoi") const VALID_ZERO_HANDLING = ("pseudocount", "multiplicative_replacement", "bayesian_multiplicative", "refuse") const VALID_ILR_BASIS = ("default", "phylogenetic", "sequential_binary_partition", "balance_dendrogram") +const DEFERRED_ILR_BASIS = ("phylogenetic", "sequential_binary_partition", "balance_dendrogram") # Method names are canonicalised to lower case by `NormalizationConfig`, so the allowed # names are held in the same case and compared in it. They were not always: the entries # for TSS/CSS/RSS arrived upper case while the constructor stored `"tss"`, so every one of @@ -172,7 +173,9 @@ struct NormalizationConfig throw(ArgumentError("zero_policy='refuse' is mathematically invalid for CLR/ILR (log(0) undefined). Refusing even with DANGER token. Use pseudocount or multiplicative_replacement.")) end if method_clean == "ilr" - if !isnothing(ilr_basis) && !(ilr_basis in VALID_ILR_BASIS) + if !isnothing(ilr_basis) && (ilr_basis in DEFERRED_ILR_BASIS) + throw(ArgumentError("ilr_basis '$ilr_basis' is not implemented (deferred, see GitHub issue #20). Only 'default' (Helmert sequential binary partition) is implemented. Refusing meaningless substitution.")) + elseif !isnothing(ilr_basis) && !(ilr_basis in VALID_ILR_BASIS) throw(ArgumentError("ilr_basis must be one of $(join(VALID_ILR_BASIS, ", ")) (got '$ilr_basis')")) end else # clr @@ -960,12 +963,12 @@ function context_help(field_path::String) "normalization.ilr_basis" => """ ILR basis (only for ILR, meaningless otherwise) - - default: default ILR basis from compositions package - - phylogenetic: phylogenetic tree based balances (requires tree, deferred) - - sequential_binary_partition: SBP from user-provided partition (deferred) - - balance_dendrogram: balance dendrogram + - default: Helmert-style sequential binary partition (first taxon vs rest, second vs rest, etc., creating n-1 balances) + - phylogenetic: NOT IMPLEMENTED (deferred, see GitHub issue #20) + - sequential_binary_partition: NOT IMPLEMENTED (deferred, see GitHub issue #20) + - balance_dendrogram: NOT IMPLEMENTED (deferred, see GitHub issue #20) - Refuses meaningless use for non-ILR methods. See JSON schema enum and Nickel. + Refuses unimplemented bases and meaningless use for non-ILR methods. See JSON schema enum and Nickel. """, "correction.method" => """ Multiple testing correction — BH mandatory in v1, hard-stop DANGER banner on overrides diff --git a/src/analysis/Execution.jl b/src/analysis/Execution.jl index 2376bcad..18458a0e 100644 --- a/src/analysis/Execution.jl +++ b/src/analysis/Execution.jl @@ -1056,6 +1056,7 @@ function prepare_analysis_table( # decides only what the response is and which offset it gets. transform_method = lowercase(strip(config.normalization.method)) prepared = copy(counts_after_zero) + filtered_taxa_ids_before_ilr = copy(filtered_taxa_ids) offset = nothing scaling = nothing count_response = config.method == AnalysisConfig.NB_GLM @@ -1169,8 +1170,8 @@ function prepare_analysis_table( # For simplicity, we return CLR for now with warning that exact ILR basis deferred, and set prepared to clr_table # Real implementation would use `compositions::ilr` or `philr` or custom - if config.normalization.ilr_basis in ("phylogenetic", "sequential_binary_partition", "balance_dendrogram") - @warn "ILR basis $(config.normalization.ilr_basis) is deferred (see GitHub issue 05-ilr-basis-phylogenetic-sbp). Currently using default basis with warning." + if !isnothing(config.normalization.ilr_basis) && (config.normalization.ilr_basis in AnalysisConfig.DEFERRED_ILR_BASIS) + throw(ArgumentError("ILR basis '$(config.normalization.ilr_basis)' is not implemented (deferred, see GitHub issue #20). Refusing to substitute default Helmert basis.")) end # For default basis, we can compute ILR as: ilr = V^T * clr where V is n x (n-1) orthonormal basis @@ -1194,9 +1195,7 @@ function prepare_analysis_table( end prepared = ilr_table - # Note: prepared now has n_taxa-1 rows, not n_taxa — for ILR, number of balances = n_taxa-1 - # For consistency, we keep taxa_ids as balances? For stub, we create new taxa_ids for balances - # But for simplicity, we keep prepared as ilr_table and adjust taxa_ids later + filtered_taxa_ids = ["balance_$i" for i in 1:(n_taxa - 1)] elseif transform_method == "presence_absence" prepared = Float64.(counts_after_zero .> 0) @@ -1236,6 +1235,14 @@ function prepare_analysis_table( checks["scaling"] = Scaling.factor_checks(scaling) append!(warnings, scaling.warnings) end + if transform_method == "ilr" + checks["ilr"] = OrderedDict{String,Any}( + "basis" => something(config.normalization.ilr_basis, "default"), + "definition" => "Helmert-style sequential binary partition (balance_i = sqrt(i/(i+1)) * (mean(log(x_1..x_i)) - log(x_{i+1})))", + "taxa_in" => length(filtered_taxa_ids_before_ilr), + "taxa_order" => filtered_taxa_ids_before_ilr + ) + end checks["all_zero_taxa"] = OrderedDict{String,Any}( "all_zero_taxa_indices" => all_zero_taxa_indices, "all_zero_taxa_count" => length(all_zero_taxa_indices), @@ -1292,7 +1299,7 @@ function prepare_analysis_table( try (healed_counts, healed_taxa_ids, heal_t, _) = heal_all_zero_taxa( filtered_counts, - filtered_taxa_ids, + transform_method == "ilr" ? filtered_taxa_ids_before_ilr : filtered_taxa_ids, dp, effective_epsilon; indices=all_zero_taxa_indices @@ -1302,10 +1309,13 @@ function prepare_analysis_table( # For ILR, prepared has n-1 rows, so need to handle differently — for stub, skip if ILR if transform_method != "ilr" prepared = prepared[keep_rows, :] + filtered_taxa_ids = healed_taxa_ids end - filtered_taxa_ids = healed_taxa_ids filtered_counts = healed_counts else + if transform_method != "ilr" + filtered_taxa_ids = healed_taxa_ids + end filtered_counts = healed_counts end append!(healings, heal_t) diff --git a/test/unit/test_scaling.jl b/test/unit/test_scaling.jl index 97c1e7b3..ad591935 100644 --- a/test/unit/test_scaling.jl +++ b/test/unit/test_scaling.jl @@ -289,13 +289,18 @@ end @testset "prepare_analysis_table keeps counts and hands over the offset" begin + # 4 samples: prepare_analysis_table requires >= 2*min_samples_per_group samples + table_4 = [10.0 20.0 40.0 20.0; + 20.0 20.0 10.0 30.0; + 30.0 60.0 50.0 40.0] + ids_4 = ["s1", "s2", "s3", "s4"] adv = AnalysisConfig.AdvancedConfig(min_prevalence = 0.0, min_abundance = 0.0, min_samples_per_group = 2) expectations = [ - ("TSS", "tss", tss_factors(table_a).factors), - ("CSS", "css", css_factors(table_a; quantile = 0.75).factors), - ("RSS", "rss", tmm_factors(table_a).factors), - ("size_factors", "size_factors", rle_factors(table_a).factors), + ("TSS", "tss", tss_factors(table_4).factors), + ("CSS", "css", css_factors(table_4; quantile = 0.75).factors), + ("RSS", "rss", tmm_factors(table_4).factors), + ("size_factors", "size_factors", rle_factors(table_4).factors), ] for (method_name, kind, expected) in expectations config = AnalysisConfig.AnalysisConfig( @@ -303,17 +308,17 @@ normalization = AnalysisConfig.NormalizationConfig(method = method_name), advanced = adv, created_by = "test_scaling") (prepared, diagnostics, manifest, sample_ids, _) = - Execution.prepare_analysis_table(config, table_a; - sample_ids = ids_a, + Execution.prepare_analysis_table(config, table_4; + sample_ids = ids_4, taxa_ids = ["t1", "t2", "t3"], drop_policy = "drop") - @test prepared == table_a # the count response is untouched + @test prepared == table_4 # the count response is untouched @test manifest.offset ≈ log.(expected) atol = 1e-10 @test diagnostics.checks["scaling"]["kind"] == kind @test manifest.provenance["scaling"]["kind"] == kind @test manifest.provenance["scaling"]["offset_sha256"] isa String @test length(manifest.provenance["scaling"]["offset_sha256"]) == 64 - @test sample_ids == ids_a + @test sample_ids == ids_4 end # `none` is not "no offset": for a count model it is the plain log library size, @@ -323,10 +328,10 @@ normalization = AnalysisConfig.NormalizationConfig(method = "none"), advanced = adv, created_by = "test_scaling") (_, diag_none, manifest_none, _, _) = Execution.prepare_analysis_table( - config_none, table_a; sample_ids = ids_a, taxa_ids = ["t1", "t2", "t3"], + config_none, table_4; sample_ids = ids_4, taxa_ids = ["t1", "t2", "t3"], drop_policy = "drop") @test diag_none.checks["scaling"]["kind"] == "tss" - @test manifest_none.offset ≈ log.(tss_factors(table_a).factors) atol = 1e-10 + @test manifest_none.offset ≈ log.(tss_factors(table_4).factors) atol = 1e-10 # A run that declares `relative` has no offset, and does not get one by accident. config_rel = AnalysisConfig.AnalysisConfig( @@ -334,12 +339,59 @@ normalization = AnalysisConfig.NormalizationConfig(method = "relative"), advanced = adv, created_by = "test_scaling") (_, diag_rel, manifest_rel, _, _) = Execution.prepare_analysis_table( - config_rel, table_a; sample_ids = ids_a, taxa_ids = ["t1", "t2", "t3"], + config_rel, table_4; sample_ids = ids_4, taxa_ids = ["t1", "t2", "t3"], drop_policy = "drop") @test isnothing(manifest_rel.offset) @test !haskey(diag_rel.checks, "scaling") end + @testset "deferred ILR bases are refused at construction and in prepare_analysis_table" begin + for basis in AnalysisConfig.DEFERRED_ILR_BASIS + @test_throws ArgumentError AnalysisConfig.NormalizationConfig( + method = "ilr", ilr_basis = basis) + end + # "default" basis is accepted + norm_ok = AnalysisConfig.NormalizationConfig(method = "ilr", ilr_basis = "default") + @test norm_ok.ilr_basis == "default" + end + + @testset "ILR relabels rows to balance_1..n-1 with diagnostics" begin + table_ilr = [ + 10.0 20.0 30.0 40.0; + 20.0 30.0 40.0 50.0; + 30.0 40.0 50.0 60.0; + 40.0 50.0 60.0 70.0 + ] + sample_ids_ilr = ["s1", "s2", "s3", "s4"] + taxa_ids_ilr = ["t1", "t2", "t3", "t4"] + + norm_ilr = AnalysisConfig.NormalizationConfig(method = "ilr", pseudocount = 0.5) + adv = AnalysisConfig.AdvancedConfig(min_prevalence = 0.0, min_abundance = 0.0, min_samples_per_group = 2) + config_ilr = AnalysisConfig.AnalysisConfig( + method = "ilr_lm", + formula = "~ group", + metadata_columns = ["group"], + normalization = norm_ilr, + advanced = adv, + created_by = "test_ilr" + ) + + (prep_ilr, diag_ilr, manifest_ilr, sids_ilr, tids_ilr) = Execution.prepare_analysis_table( + config_ilr, table_ilr; + sample_ids = sample_ids_ilr, + taxa_ids = taxa_ids_ilr, + drop_policy = "drop" + ) + + @test size(prep_ilr, 1) == 3 + @test tids_ilr == ["balance_1", "balance_2", "balance_3"] + @test haskey(diag_ilr.checks, "ilr") + @test diag_ilr.checks["ilr"]["basis"] == "default" + @test diag_ilr.checks["ilr"]["taxa_in"] == 4 + @test diag_ilr.checks["ilr"]["taxa_order"] == taxa_ids_ilr + @test occursin("Helmert", diag_ilr.checks["ilr"]["definition"]) + end + # ------------------------------------------------------------------ # The R transcription # @@ -391,10 +443,10 @@ RCall.globalEnv[:scaling_A] = table_a RCall.reval(r_reference_script) ( - tss = convert(Vector{Float64}, RCall.reval("r_tss")), - css = convert(Vector{Float64}, RCall.reval("r_css")), - tmm = convert(Vector{Float64}, RCall.reval("r_tmm")), - rle = convert(Vector{Float64}, RCall.reval("r_rle")), + tss = RCall.rcopy(RCall.reval("r_tss")), + css = RCall.rcopy(RCall.reval("r_css")), + tmm = RCall.rcopy(RCall.reval("r_tmm")), + rle = RCall.rcopy(RCall.reval("r_rle")), reference = RCall.rcopy(RCall.reval("r_ref")), ) end