From a3dec9a7d38f660bad0adf0eb9a1344226e603e8 Mon Sep 17 00:00:00 2001 From: Arena Agent Date: Fri, 23 Oct 2026 08:01:50 +0000 Subject: [PATCH 1/5] fix(cladistics): finish the edge_number rename and the duplicate include MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builds on #222, which fixed the graft query fields but left three sites: - `_graft_jplace_queries` still constructed `PhyloNode(..., u.edge_num, ...)` and read `u.edge_num` when matching placements to edges. The field is `edge_number`, so any graft threw `FieldError` before the tree was rebuilt ("Got exception outside of a @test" — no test line, which is why it read as an unrelated suite failure). - The diversity block in the route still summed `c.weight` over jplace candidates. `QueryCandidate` carries `like_weight`; reading `.weight` threw inside the handler, and with `show_errors=false` the POST answered a bare 500, so every assertion on the combined-mode payload failed with `KeyError: key :root not found`. - `routes/cladistics.jl` was included twice in server.jl, registering every /cladistics route twice. `cand.edge_num` stays: that is the `QueryCandidate` field. The undirected edge struct's `edge_no` and its `weight` are separate types. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com> --- src/analysis/clade_cumulus.jl | 6 +++--- src/server/routes/clade_cumulus.jl | 4 ++-- src/server/server.jl | 1 - 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/analysis/clade_cumulus.jl b/src/analysis/clade_cumulus.jl index 63cbdf24..92253778 100644 --- a/src/analysis/clade_cumulus.jl +++ b/src/analysis/clade_cumulus.jl @@ -1029,8 +1029,8 @@ function _graft_jplace_queries( function walk(u::PhyloNode)::PhyloNode rebuilt_kids = PhyloNode[walk(ch) for ch in u.children] - cur = PhyloNode(u.id, u.name, u.branch_length, u.support, u.edge_num, rebuilt_kids) - specs = u.edge_num === nothing ? nothing : get(by_edge, u.edge_num, nothing) + cur = PhyloNode(u.id, u.name, u.branch_length, u.support, u.edge_number, rebuilt_kids) + specs = u.edge_number === nothing ? nothing : get(by_edge, u.edge_number, nothing) (specs === nothing || isempty(specs)) && return cur # Order grafts from most distal (closest to `cur`, smallest `distal`) to most proximal. @@ -1048,7 +1048,7 @@ function _graft_jplace_queries( sub = PhyloNode(string("jplace_clade#", seq_auto[]), "", 0.0, 1.0, nothing, PhyloNode[sub, qtip]) end rem_len = max(0.0, total_len - prev_distal) - PhyloNode(sub.id, sub.name, rem_len, cur.support, cur.edge_num, sub.children) + PhyloNode(sub.id, sub.name, rem_len, cur.support, cur.edge_number, sub.children) end walk(base_root) diff --git a/src/server/routes/clade_cumulus.jl b/src/server/routes/clade_cumulus.jl index 0dd6918d..5e303e39 100644 --- a/src/server/routes/clade_cumulus.jl +++ b/src/server/routes/clade_cumulus.jl @@ -276,13 +276,13 @@ end jd_div = MetaManifold.CladeCumulus.parse_jplace(jplace_str) for q in jd_div.queries q.status === :valid && !isempty(q.candidates) || continue - tw = sum((c.weight for c in q.candidates); init = 0.0) + tw = sum((c.like_weight for c in q.candidates); init = 0.0) tw > 0.0 || continue multi = length(q.candidates) > 1 for (qname, _) in q.names for (ci, cand) in enumerate(q.candidates) tid = multi ? string(qname, "@edge", cand.edge_num, "#", ci) : qname - tip_weights[tid] = cand.weight / tw + tip_weights[tid] = cand.like_weight / tw end end end diff --git a/src/server/server.jl b/src/server/server.jl index ebf5abb6..eca368a0 100644 --- a/src/server/server.jl +++ b/src/server/server.jl @@ -118,7 +118,6 @@ module Server include(joinpath(@__DIR__, "routes", "phylogeny.jl")) include(joinpath(@__DIR__, "routes", "cladistics.jl")) include(joinpath(@__DIR__, "routes", "clade_cumulus.jl")) - include(joinpath(@__DIR__, "routes", "cladistics.jl")) ## Bad request middleware # Helpers deep in a route (e.g. `_req_group`) throw this to reject a request. From 1a6a25a1ebac77fa325681680d5861fdab75872a Mon Sep 17 00:00:00 2001 From: Arena Agent Date: Fri, 23 Oct 2026 08:02:44 +0000 Subject: [PATCH 2/5] test(cladistics): guard against duplicate route registration and stray `end` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both merge-damage classes that broke this branch were invisible to the tests that existed: - A duplicated `include` in server.jl registers every route in that file twice. A request test cannot see it: the second registration shadows the first and both answer. `test_routes.jl` now reads server.jl and asserts that each include path appears once and that both cladistics route files are present, then asserts that no route file declares the same method+path twice. - A dropped `module` opener leaves a bare `end`, which parses cleanly and only fails at lowering — so the "Parse Julia sources" gate passed while the whole suite died at include time. `scripts/check-julia-syntax.jl` now rejects a top-level `end` with the code unit it was found at. Also names `DuckDB` and `DBInterface` in routes/clade_cumulus.jl rather than inheriting them from results.jl, which happens to be included earlier. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com> --- scripts/check-julia-syntax.jl | 8 ++++++ src/server/routes/clade_cumulus.jl | 4 +++ test/unit/test_routes.jl | 39 ++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/scripts/check-julia-syntax.jl b/scripts/check-julia-syntax.jl index bc8a9d60..e214e466 100644 --- a/scripts/check-julia-syntax.jl +++ b/scripts/check-julia-syntax.jl @@ -20,6 +20,14 @@ for file in files issue = ex break end + # A stray `end` at top level parses cleanly and only fails at + # lowering (""end" at top level"), so this gate has to reject it + # explicitly: a dropped `module` opener is otherwise invisible here. + if ex === :end + issue = "top-level `end` at code unit $position: a block opener " * + "(module, function, begin) is missing or was removed" + break + end next_position > position || error("Julia parser made no progress at code unit $position") position = next_position diff --git a/src/server/routes/clade_cumulus.jl b/src/server/routes/clade_cumulus.jl index 5e303e39..c5ca9c58 100644 --- a/src/server/routes/clade_cumulus.jl +++ b/src/server/routes/clade_cumulus.jl @@ -11,6 +11,10 @@ # write only study-local sidecars under `projects/{study}/trees/.clade_cumulus/`. using DataFrames, OrderedCollections, SHA, Dates, JSON3 +# Named here rather than inherited from results.jl, which is included +# earlier: relying on a sibling file's `using` breaks when this file is +# evaluated on its own, and a missing DuckDB binding is a 500 at runtime. +using DuckDB, DBInterface const _CC_SIDECAR_MAX_BYTES = 5 * 1024 * 1024 diff --git a/test/unit/test_routes.jl b/test/unit/test_routes.jl index 62998944..4aa9d4d1 100644 --- a/test/unit/test_routes.jl +++ b/test/unit/test_routes.jl @@ -930,6 +930,45 @@ _saved_root = SV.ServerState._root[] end end + + ## Registration itself + # A duplicated `include` in server.jl registers every route in that file + # twice. No request test can see that: the second registration simply + # shadows the first, and both answer. These checks read the sources, so + # they fail on the merge that reintroduces a duplicate. + @testset "Registered routes" begin + src_dir = joinpath(@__DIR__, "..", "..", "src", "server") + server_src = read(joinpath(src_dir, "server.jl"), String) + # Every `include(joinpath(@__DIR__, "a", "b.jl"))` path, in order. + included = String[] + for m in eachmatch(r"include\(joinpath\(@__DIR__((?:,\s*\"[^\"]+\")+)\)\)", + server_src) + push!(included, join([c[2:end-1] for c in + eachmatch(r"\"[^\"]+\"", m.captures[1])], "/")) + end + @test !isempty(included) + # The duplicate that shipped in 8b76b71: routes/cladistics.jl twice. + # No request test can see this — the second registration shadows the + # first and both answer — so the invariant is checked on the sources. + @test allunique(included) + @test count(==("routes/cladistics.jl"), included) == 1 + @test count(==("routes/clade_cumulus.jl"), included) == 1 + + # And no route file declares the same method+path twice. + routes = Tuple{String,String}[] + for file in sort(readdir(joinpath(src_dir, "routes"); join = true)) + endswith(file, ".jl") || continue + for m in eachmatch(r"@(get|post|put|patch|delete)\s+(\"[^\"]+\")", + read(file, String)) + push!(routes, (m.captures[1], m.captures[2])) + end + end + @test !isempty(routes) + @test allunique(routes) + @test any(r -> occursin("/cladistics", r[2]), routes) + @test any(r -> occursin("/clade-cumulus", r[2]), routes) + end + end # Testsets above point the server at temporary roots. From de4e30d18c5dd791e3f58f600dcca12bd3970c6f Mon Sep 17 00:00:00 2001 From: Arena Agent Date: Fri, 23 Oct 2026 08:17:13 +0000 Subject: [PATCH 3/5] diag: surface the combine handler error (temporary) Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com> --- src/server/server.jl | 2 +- test/unit/test_clade_cumulus.jl | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/server/server.jl b/src/server/server.jl index eca368a0..e127c5fd 100644 --- a/src/server/server.jl +++ b/src/server/server.jl @@ -432,7 +432,7 @@ module Server _analysis_source_middleware, _file_middleware] # Oxygen's docs (/docs, /docs/schema) and metrics sit outside the middleware # above, so they would bypass the Host and Origin checks; nothing uses them. - const _SERVE_OPTIONS = (access_log=nothing, show_errors=false, docs=false, metrics=false) + const _SERVE_OPTIONS = (access_log=nothing, show_errors=true, docs=false, metrics=false) function start(; root=pwd(), host="127.0.0.1", port=8080, listen::Bool=true) global_logger(_SuppressEpipe(global_logger())) diff --git a/test/unit/test_clade_cumulus.jl b/test/unit/test_clade_cumulus.jl index 6e12a0c9..9ebda98e 100644 --- a/test/unit/test_clade_cumulus.jl +++ b/test/unit/test_clade_cumulus.jl @@ -330,6 +330,8 @@ echo "(MMTIP_000001:0.05,(MMTIP_000002:0.03,MMTIP_000003:0.04)0.910:0.12,MMTIP_0 "supportThreshold" => 0.85, "lwrThreshold" => 0.95, ))) + # TEMP diagnostic: surface the handler's own error text. + r_comb.status == 200 || error("combine failed: ", String(r_comb.body)) @test r_comb.status == 200 comb = JSON3.read(String(r_comb.body)) @test comb.root.cumulative_count ≈ 110.0 atol = 1e-9 From 29c30b1f5c38d8da2bcfda2720d9084ee23c69f7 Mon Sep 17 00:00:00 2001 From: Arena Agent Date: Fri, 23 Oct 2026 08:37:05 +0000 Subject: [PATCH 4/5] diag: replicate the combine handler in the test; drop the clashing const SV (temporary) Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com> --- test/unit/test_clade_cumulus.jl | 51 ++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/test/unit/test_clade_cumulus.jl b/test/unit/test_clade_cumulus.jl index 9ebda98e..a7298e11 100644 --- a/test/unit/test_clade_cumulus.jl +++ b/test/unit/test_clade_cumulus.jl @@ -14,7 +14,7 @@ const EP = MetaManifold.Epistemic const CC = MetaManifold.CladeCumulus const DM = MetaManifold.DiversityMetrics const PH = MetaManifold.Phylogeny -const SV = MetaManifold.Server +SV = MetaManifold.Server _cc_req(method::String, path::String, body::String = "") = SV.Oxygen.internalrequest( @@ -332,6 +332,55 @@ echo "(MMTIP_000001:0.05,(MMTIP_000002:0.03,MMTIP_000003:0.04)0.910:0.12,MMTIP_0 ))) # TEMP diagnostic: surface the handler's own error text. r_comb.status == 200 || error("combine failed: ", String(r_comb.body)) + # TEMP diag: run the handler's steps here so the exception text + # lands in the CI annotation instead of a bare 500. + let + df2, meta2, lerr2 = SV._load_run_table_df("StudyCC", "run1", "merged") + lerr2 === nothing || error("load: ", lerr2) + digs2 = OrderedDict{String,String}("duckdb:merged" => "x") + bod2 = JSON3.read(JSON3.write(Dict("treeSource" => "study_tree", + "treeFile" => "amplicon.nwk"))) + nwk2, jp2, tm2, terr2 = SV._resolve_tree_inputs_from_request("StudyCC", bod2, digs2) + terr2 === nothing || error("tree inputs: ", terr2) + cc2 = CC.build_clade_tree(df2; mode = :combined, newick = nwk2, + jplace_json = jp2, sample_cols = meta2.sample_cols, + support_threshold = 0.85, lwr_threshold = 0.95, + tree_method = tm2, max_asvs = 500, seed = 42) + d2 = CC.clade_node_to_dict(cc2.root) + pl2 = CC.to_plotly_figure(cc2.root) + tips2 = sort!(collect(CC._collect_tree_tips!(Set{String}(), cc2.phylo_root))) + idc2 = CC._detect_id_col(df2) + rbi2 = Dict{String,Int}() + for (ri2, r2) in enumerate(eachrow(df2)) + t2 = String(strip(string(r2[idc2]))) + haskey(rbi2, t2) || (rbi2[t2] = ri2) + end + tv2 = String[] + tr2 = Tuple{Int,Float64}[] + for t2 in tips2 + if haskey(rbi2, t2) + push!(tv2, t2); push!(tr2, (rbi2[t2], 1.0)) + end + end + mat2 = zeros(Float64, length(tv2), length(meta2.sample_cols)) + for (ci2, sc2) in enumerate(meta2.sample_cols) + cv2 = df2[!, sc2] + for (mi2, (ri2, w2)) in enumerate(tr2) + v2 = cv2[ri2] + mat2[mi2, ci2] = ismissing(v2) ? 0.0 : max(0.0, Float64(v2)) * w2 + end + end + pd2 = OrderedDict{String,Float64}(sc2 => CC.faith_pd(@view(mat2[:, ci2]), tv2, cc2.phylo_root) + for (ci2, sc2) in enumerate(meta2.sample_cols)) + uw2 = CC.unifrac_matrix(mat2, tv2, meta2.sample_cols, cc2.phylo_root; metric = :unweighted) + wr2 = CC.unifrac_matrix(mat2, tv2, meta2.sample_cols, cc2.phylo_root; metric = :weighted, normalized = false) + rcpt2 = EP.create_receipt(; study = "StudyCC", run = "run1", + source_digests = digs2, tool_records = cc2.tool_records, + parameters = OrderedDict{String,Any}("x" => 1), + warnings = cc2.warnings) + EP.receipt_dict(rcpt2) + error("diag reached the end: ", length(pd2), " ", size(uw2), " ", size(wr2), " ", length(tips2)) + end @test r_comb.status == 200 comb = JSON3.read(String(r_comb.body)) @test comb.root.cumulative_count ≈ 110.0 atol = 1e-9 From edfd02dc438865bb689d2478bc800398ef4e93ac Mon Sep 17 00:00:00 2001 From: Arena Agent Date: Fri, 23 Oct 2026 09:45:49 +0000 Subject: [PATCH 5/5] ci: rerun (runner lost communication) Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>