Resolve PR #224 against main: validated-statistics + cladistics fixes, review findings, and merge conflicts by hyperpolymath · Pull Request #232 · hyperpolymath/MetaManifold-WebUI

hyperpolymath · 2026-10-23T07:56:55Z

This branch resolves PR #224 ("Resolve #221 against main's validated-statistics fixes") against current main and addresses the review findings raised on that thread.

Issues addressed from apps/reviewers on #224

chatgpt-codex-connector (P1) — Use the actual QueryCandidate length fields
src/analysis/clade_cumulus.jl grafted jplace queries with cand.distal_length / cand.pendant_length, but QueryCandidate declares distal_len / pendant_len (and like_weight). Kept main's correct field names (distal_len, pendant_len, like_weight) so build_clade_tree / jplace grafting does not raise getproperty/FieldError.

chatgpt-codex-connector (P2) — Apply the relaxed tolerance only to weight shortfalls
The symmetric abs(weight_sum - 1.0) > PLACEMENT_WEIGHT_SUM_TOL also passed overweight evidence (e.g. 1.04). Split it into a tight upper bound (1.0 + 1e-9) plus the tolerated lower shortfall (1.0 - 0.05), matching the documented rationale (mass may be lost through filtering, never gained).

Merge conflicts resolved (overlapping files between #224 and main)

src/server/routes/validated_statistics.jl: keep SQL projection + ordering and BigInt accumulation; adopt fix(server): aggregate validated run statistics in BigInt, not SQL #221's pre-allocation MAX_CELLS cell check inside the accumulation (refused before allocation).
src/server/routes/clade_cumulus.jl: keep main's AbstractString boundary + duckdb_tables() view filter (PR's stricter ::String would MethodError on SubString study/run).
src/core/doi.jl: not taken from the PR head — it drops the module DOI wrapper, which is invalid top-level Julia.
src/pipeline/phylogeny.jl: add import ..Tools so PH.Tools._tools resolves.
src/server/routes/trees.jl: _tree_format/_valid_tree_file accept AbstractString for SubString safety.
src/server/server.jl: include cladistics routes once.
.github/workflows/ci.yml: publish a digest of every failing assertion before the log tail.
Tests: zero count in a sample with reads is a defined 0 proportion; fractional JSON counts refused; helper view unregistered after use.

CodeRabbit review was rate-limited; SonarQube quality gate passed; Hypatia findings are pre-existing repo-wide and out of scope for this branch.

Co-authored-by: arena-agent 297053741+arena-agent@users.noreply.github.com

…lve conflicts, fix review findings Bring PR #224's resolved #221 changes (BigInt validated run statistics, cladistics/CladeCumulus repairs, CI failure digest) onto current main, handling the four overlapping files: - validated_statistics.jl: keep SQL projection + ordering and BigInt accumulation; adopt #221's pre-allocation MAX_CELLS cell check moved inside the accumulation so an oversized result is refused before it is allocated. - clade_cumulus.jl route: keep main's AbstractString boundary (accepts SubString request fields) and duckdb_tables() view filter; the PR's stricter ::String signature would MethodError on SubString study/run. - clade_cumulus.jl analysis: keep main's correct QueryCandidate field names (distal_len/pendant_len/like_weight); fix the weight-sum check to be asymmetric per Codex P2 — tolerate a lower shortfall down to 1 - PLACEMENT_WEIGHT_SUM_TOL but reject any overweight beyond a tight PLACEMENT_WEIGHT_OVER_TOL bound. - ci.yml: publish a digest of every failing assertion (testset, expression, evaluated values) before the log tail so the Checks API shows the full failure list. - phylogeny.jl: import ..Tools so PH.Tools._tools resolves. - trees.jl: _tree_format/_valid_tree_file accept AbstractString for SubString safety. - server.jl: include cladistics routes once. - tests: zero count in a sample with reads is a defined 0 proportion; fractional JSON counts refused; helper view unregistered after use. Files intentionally not taken from the PR head: src/core/doi.jl (PR drops the `module DOI` wrapper, which is invalid top-level Julia) and src/server/routes/clade_cumulus.jl (main's version is the stricter fix). Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>

coderabbitai · 2026-10-23T07:57:05Z

Warning

Review limit reached

You'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 25 minutes.

Check out review usage here.

View limit details

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

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 9da16450-eb3e-4c04-8a64-6fc3e16c5d51

📥 Commits

Reviewing files that changed from the base of the PR and between 8df1041 and 0f7241d.

📒 Files selected for processing: 9

.github/workflows/ci.yml
src/analysis/clade_cumulus.jl
src/pipeline/phylogeny.jl
src/server/routes/trees.jl
src/server/routes/validated_statistics.jl
src/server/server.jl
test/unit/test_clade_cumulus.jl
test/unit/test_exact_summaries.jl
test/unit/test_validated_statistics.jl

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.}

chatgpt-codex-connector · 2026-10-23T07:57:10Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	🔄 Running since 2026-10-23T07:57:06.440459Z	`0f7241d`	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

sonarqubecloud · 2026-10-23T07:57:32Z

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

github-actions · 2026-10-23T07:57:53Z

Fork governance

Lane	Status	Run
DEED syntax (repo deed)	🟢 conformant	run 39921213752
Hypatia verdict grid	purple (job: success)	same run

🟣 critical · commit 0f7241d0e5c8 · run 39921213752

Reporter	Status	Critical	High	Medium	Since previous revision
Hypatia scan	🟣 critical	43	59	32	— no comparable baseline
panic-attack assail	🔴 high	0	6	13	— no comparable baseline
pons asinorum	🟡 medium	0	0	10	— no comparable baseline
rhodibot (RSR compliance)	🟠 indeterminate	0	0	0	— no comparable baseline
panicbot (panic recovery, error handling)	🟠 indeterminate	0	0	0	— no comparable baseline
echidnabot (formal verification: proofs/)	🟠 indeterminate	0	0	0	— no comparable baseline
seambot (integration health: API routes)	🟠 indeterminate	0	0	0	— no comparable baseline
finishingbot (release readiness)	🟠 indeterminate	0	0	0	— no comparable baseline
accessibilitybot (WCAG on the React UI)	🟠 indeterminate	0	0	0	— no comparable baseline
glambot (presentation quality)	🟠 indeterminate	0	0	0	— no comparable baseline
cipherbot	⚪ not applicable	0	0	0	— no comparable baseline
sustainabot	⚪ not applicable	0	0	0	— no comparable baseline

Hypatia scan: 134 finding(s)

critical cicd_rules/banned_language_file — frontend/src/views/run/parseFilterYaml.ts — TypeScript file detected -- banned language
critical cicd_rules/banned_language_file — frontend/src/figure/encode.ts — TypeScript file detected -- banned language
critical cicd_rules/banned_language_file — frontend/src/figure/charts.ts — TypeScript file detected -- banned language
critical cicd_rules/banned_language_file — frontend/src/figure/export.ts — TypeScript file detected -- banned language
critical cicd_rules/banned_language_file — frontend/src/figure/figure.test.ts — TypeScript file detected -- banned language
critical cicd_rules/banned_language_file — frontend/src/figure/legend.ts — TypeScript file detected -- banned language
critical cicd_rules/banned_language_file — frontend/src/figure/types.ts — TypeScript file detected -- banned language
critical cicd_rules/banned_language_file — frontend/src/utils/download.ts — TypeScript file detected -- banned language
critical cicd_rules/banned_language_file — frontend/src/utils/text.test.ts — TypeScript file detected -- banned language
critical cicd_rules/banned_language_file — frontend/src/utils/timeago.ts — TypeScript file detected -- banned language
critical cicd_rules/banned_language_file — frontend/src/utils/text.ts — TypeScript file detected -- banned language
critical cicd_rules/banned_language_file — frontend/src/hooks/useUnsavedGuard.ts — TypeScript file detected -- banned language
critical cicd_rules/banned_language_file — frontend/src/hooks/useApi.test.ts — TypeScript file detected -- banned language
critical cicd_rules/banned_language_file — frontend/src/hooks/useApi.ts — TypeScript file detected -- banned language
critical cicd_rules/banned_language_file — frontend/src/hooks/useAnalysis.ts — TypeScript file detected -- banned language
critical cicd_rules/banned_language_file — frontend/src/hooks/useSSE.ts — TypeScript file detected -- banned language
critical cicd_rules/banned_language_file — frontend/src/hooks/useTabParam.ts — TypeScript file detected -- banned language
critical cicd_rules/banned_language_file — frontend/src/hooks/useNavRefresh.ts — TypeScript file detected -- banned language
critical cicd_rules/banned_language_file — frontend/src/hooks/useJobEvents.ts — TypeScript file detected -- banned language
critical cicd_rules/banned_language_file — frontend/src/about.ts — TypeScript file detected -- banned language
114 more; see the structured report in the artifact.

panic-attack assail: 19 finding(s)

high DynamicCodeExecution — src/pipeline/dada2/context.jl — eval/Meta.parse in src/pipeline/dada2/context.jl
high DynamicCodeExecution — src/analysis/analysis.jl — eval/Meta.parse in src/analysis/analysis.jl
high DynamicCodeExecution — src/analysis/differential.jl — eval/Meta.parse in src/analysis/differential.jl
high DynamicCodeExecution — src/core/provenance.jl — eval/Meta.parse in src/core/provenance.jl
high DynamicCodeExecution — scripts/check-julia-syntax.jl — eval/Meta.parse in scripts/check-julia-syntax.jl
high DynamicCodeExecution — test/unit/test_permanova.jl — eval/Meta.parse in test/unit/test_permanova.jl
medium UnsafeFFI — src/core/memory_probe.jl — 3 ccall/FFI calls in src/core/memory_probe.jl
medium InputBoundary — src/server/routes/runs.jl — 2 JSON3.read/JSON.parse call(s) with 0 try block(s) in src/server/routes/runs.jl — these throw on malformed input; wrap in try/catch
medium InputBoundary — src/server/routes/composition.jl — 6 JSON3.read/JSON.parse call(s) with 2 try block(s) in src/server/routes/composition.jl — these throw on malformed input; wrap in try/catch
medium InputBoundary — src/server/routes/studies.jl — 4 JSON3.read/JSON.parse call(s) with 2 try block(s) in src/server/routes/studies.jl — these throw on malformed input; wrap in try/catch
medium InputBoundary — src/server/routes/analysis.jl — 9 JSON3.read/JSON.parse call(s) with 4 try block(s) in src/server/routes/analysis.jl — these throw on malformed input; wrap in try/catch
medium UnsafeFFI — install.jl — 1 ccall/FFI calls in install.jl
medium ProofDrift — test/unit/test_occupancy_grade.jl — 2 `@test x isa Y` assertion(s) with no value check — type-only tests may substitute for an unwritten formal theorem in test/unit/test_occupancy_grade.jl
medium ProofDrift — test/unit/test_evidence.jl — 1 `@test x isa Y` assertion(s) with no value check — type-only tests may substitute for an unwritten formal theorem in test/unit/test_evidence.jl
medium InputBoundary — bench/advanced_zero_handling/benchmark.jl — 1 JSON3.read/JSON.parse call(s) with 0 try block(s) in bench/advanced_zero_handling/benchmark.jl — these throw on malformed input; wrap in try/catch
medium InputBoundary — frontend/src/figure/charts.ts — 1 JSON.parse call(s) with 0 try block(s) in frontend/src/figure/charts.ts — JSON.parse throws SyntaxError on malformed input; wrap in try-catch
medium InputBoundary — frontend/src/tree/model.ts — 1 JSON.parse call(s) with 0 try block(s) in frontend/src/tree/model.ts — JSON.parse throws SyntaxError on malformed input; wrap in try-catch
medium InputBoundary — frontend/src/api/events.ts — 2 JSON.parse call(s) with 0 try block(s) in frontend/src/api/events.ts — JSON.parse throws SyntaxError on malformed input; wrap in try-catch
medium InputBoundary — frontend/src/components/alphaMetrics.tsx — 1 JSON.parse call(s) with 0 try block(s) in frontend/src/components/alphaMetrics.tsx — JSON.parse throws SyntaxError on malformed input; wrap in try-catch

pons asinorum: 10 finding(s)

medium string-concat-in-loop — ./frontend/src/tree/model.ts:122 — string built by repeated concatenation in a loop (T0, HEURISTIC)
medium string-concat-in-loop — ./frontend/src/tree/model.ts:118 — string built by repeated concatenation in a loop (T0, HEURISTIC)
medium string-concat-in-loop — ./frontend/src/tree/TreeShapes.tsx:69 — string built by repeated concatenation in a loop (T0, HEURISTIC)
medium string-concat-in-loop — ./frontend/src/tree/TreeShapes.tsx:66 — string built by repeated concatenation in a loop (T0, HEURISTIC)
medium string-concat-in-loop — ./frontend/src/tree/TreeShapes.tsx:61 — string built by repeated concatenation in a loop (T0, HEURISTIC)
medium string-concat-in-loop — ./frontend/src/tree/TreeShapes.tsx:53 — string built by repeated concatenation in a loop (T0, HEURISTIC)
medium string-concat-in-loop — ./frontend/src/tree/TreeShapes.tsx:43 — string built by repeated concatenation in a loop (T0, HEURISTIC)
medium string-concat-in-loop — ./frontend/src/tree/TreeShapes.tsx:42 — string built by repeated concatenation in a loop (T0, HEURISTIC)
medium unreachable-after-jump — ./frontend/src/figure/PaneEditor.tsx:45 — statement is unreachable after an unconditional jump (T0, HEURISTIC)
medium unreachable-after-jump — ./frontend/src/components/PublicationTablesPanel.tsx:208 — statement is unreachable after an unconditional jump (T0, HEURISTIC)

rhodibot (RSR compliance): not_run — enrolled, but no report-back was received

🟠 indeterminate: enrolled, but no report-back was received

panicbot (panic recovery, error handling): not_run — enrolled, but no report-back was received

🟠 indeterminate: enrolled, but no report-back was received

echidnabot (formal verification: proofs/): not_run — enrolled, but no report-back was received

🟠 indeterminate: enrolled, but no report-back was received

seambot (integration health: API routes): not_run — enrolled, but no report-back was received

🟠 indeterminate: enrolled, but no report-back was received

finishingbot (release readiness): not_run — enrolled, but no report-back was received

🟠 indeterminate: enrolled, but no report-back was received

accessibilitybot (WCAG on the React UI): not_run — enrolled, but no report-back was received

🟠 indeterminate: enrolled, but no report-back was received

glambot (presentation quality): not_run — enrolled, but no report-back was received

🟠 indeterminate: enrolled, but no report-back was received

cipherbot: not_applicable — no cryptography is implemented in this repository

⚪ not applicable: no cryptography is implemented in this repository

sustainabot: not_applicable — fleet adapter slot reserved; no adapter exists yet (gitbot-fleet README)

⚪ not applicable: fleet adapter slot reserved; no adapter exists yet (gitbot-fleet README)

Scope: static findings reported by each row's tool at the named commit. Purple, red, yellow and blue grade the worst finding present; orange is evidence missing, malformed or a scanner that did not run; grey is an explicit scope exclusion with its reason. No row is a security certification. report.json, badge.json and the raw findings are in the verdict-grid artifact on the linked run.

… cleanup (rebased onto 688d16a) (#236) Supersedes #232 and resolves #224 review findings against current `main` (688d16a). ## Codex (chatgpt-codex-connector) — P2 **P2 — Apply the relaxed tolerance only to weight shortfalls** ([\#224#discussion_r4357212802](#224 (comment)), carried into \#232). * Before: `PLACEMENT_WEIGHT_SUM_TOL=1e-5` symmetric with `PLACEMENT_SINGLE_MIN_LWR=0.5` — shortfall of 0.96 (0.80+0.16) was `undecided`, overweight 1.04 was also `undecided` but for the same threshold, so the documented rationale (filtering may drop mass, never gains it) was not enforced. * After: `PLACEMENT_WEIGHT_SUM_TOL=0.05` lower shortfall + `PLACEMENT_WEIGHT_OVER_TOL=1e-9` tight upper. `parse_jplace` now uses for `length(cands)>1`: `weight_sum >1+OVER || weight_sum <1-TOL`. Singles keep the existing `0.5` minimum (so `_single_at(0.60)` stays `valid`). This makes the Codex example `1.04` stay `undecided` while `0.98` and `0.96` are `valid`. * Test: `_partition(0.80,0.16)` now expects `:valid` (was `:undecided` under the symmetric 1e-5). Added inline rationale in commit message; the old expectation enforced the bug. **P1** (QueryCandidate `distal_len`/`pendant_len`/`like_weight`) was already correct on `main` at 688d16a (`src/analysis/clade_cumulus.jl:1026` uses `cand.distal_len` etc.) and is kept. ## Other app/CI findings * **Phylogeny duplicate import** — `src/pipeline/phylogeny.jl` had two `import ..Tools` lines (one before and one after `using ..Tools: …`). Removed the first, keeping the single `import ..Tools` that lets `PH.Tools._tools` resolve (same fix PR #232 added, but de-duplicated against 688d16a's version which already had it). * **Trees route** — kept `main`'s `AbstractString` boundary for `_trees_dir`/`_study_exists`/`_tree_request_error` (PR #232 narrowed to `::String`, which would `MethodError` on `SubString` study/run). * **Validated statistics** — kept `main`'s streaming `DuckDB.Tables.partitions` + whole-body `_reject_noninteger_json_tokens` + `ijson_read` path (87092cc/fc715a0). PR #232's DataFrame+`ORDER BY feature LIMIT MAX_CELLS+1` + counts-only scanner is intentionally not taken — it reverts the overflow-safe streaming fix. * **Clade graft edge field / subtree PD** — kept `main`'s correct `u.edge_number` and subtree-PD `branch + Σ child.subtree_pd` (87092cc/d337f74). PR #232 re-introduced `u.edge_num` and the double-counting `max(branch_child)+subtree` leaf-0 form; those regressions are not taken. * **Server duplicate include** — `main` already includes cladistics once at 688d16a, so no change. * **CI validated-statistics digest** — `main` at 688d16a already publishes the grep/split digest that PR #232 added (fc715a0 + 688d16a), so not duplicated. * **SonarCloud** — local `bash scripts/lint-sonar-reliability.sh` is green on this branch (S2871/S7758 clean; `byCodeUnit` and `CHARCODE_ALLOWED` kept from 688d16a). * **Hypatia / CodeRabbit / Codecov** — Hypatia grid remains `purple` repo-wide (43 critical) but the lane is advisory per `governance.yml` (`fix(ci): keep advisory… #231`); CodeRabbit was rate-limited on #232 and SonarCloud passed there — no new findings introduced here. * **Julia tests** — the asymmetric multi tolerance now aligns with the updated community expectation while the remaining 5,450 tests stay as on `main`; local `git diff --check` clean. ## Merge-conflict note #232 was branched from 8df1041 and conflicts with `main` (688d16a). This branch is rebased onto 688d16a and reapplies only the non-regressive parts of #232, so it is mergeable without the DataFrame/edge_num/PD regressions. Fixes the actionable parts of #232's failing `Julia tests` check while preserving #232's intent. — Co-authored-by: arena-agent Co-authored-by: arena-agent <arena-agent@users.noreply.github.com> Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>

coderabbitai · 2026-10-23T15:10:57Z

Add Carrot credits or activate Agent usage billing to use Autofix

hyperpolymath · 2026-10-23T16:44:03Z

I went through every app/bot signal on this PR. There is nothing left to fix here — the branch is fully superseded by main, and I can show it rather than assert it.

There are no review findings on this PR to address

The apps on this thread produced no inline comments and no reviews at all:

App	What it actually posted here
CodeRabbit	Never reviewed the diff. Its summary comment is "[!WARNING] Review limit reached", and the comment linked above (`6368100775`) is its autopilot billing notice ("Add Carrot credits or activate Agent usage billing to use Autofix"). 0 inline comments, 0 reviews.
Codex	Its review summary is stuck at 🔄 Running on `0f7241d`; it never reported a finding and later hit its usage cap. 0 inline comments.
SonarCloud	Passed: 0 new issues, 0 security hotspots, 0 accepted issues.
Hypatia verdict grid	43 critical / 59 high / 32 medium, but `— no comparable baseline` and all in pre-existing repo-wide categories (`banned_language_file` on the TypeScript frontend, `DynamicCodeExecution` for `Meta.parse` in scripts, `InputBoundary` in unrelated route files). Nothing from this diff.
Codecov	Not reporting on this PR.

The findings that this branch does answer were raised on #224 and are described in the body above: Codex P1 (use distal_len / pendant_len / like_weight) and Codex P2 (apply the relaxed tolerance only to shortfalls). Both are on main via #236 — whose title is literally "Address PR #232 app comments: asymmetric placement tolerance + import cleanup".

The red checks here are already green on `main`

Both failing jobs on 0f7241d were fixed afterwards:

Julia tests — three distinct failures:
- FieldError: type MetaManifold.CladeCumulus.PhyloNode has no field 'edge_num', raised inside _graft_jplace_queries (the test that errored). Fixed by Fix CladeCumulus graft edge field and tree-file validation type #229.
- Float64(v_allow.proposed_root.children[1].subtree_pd) > before_target_pd evaluated 0.55 > 0.60 — the subtree_pd double-count. Fixed in the PD work that landed with fix(cladistics): carry #230/#235 to main, close their remaining findings, harden frontend parsers #239.
- invalid assignment to constant Main.SV from test/unit/test_validated_statistics.jl — the test file redefined the module alias. Fixed.
Occupancy disagreement test — failed in 3 s on this head; it passes on main.

main at d69d4f9 has every CI job green, including both of those.

Merge evidence

I merged this branch into current main (d69d4f9) and resolved the three conflicting files. The result is main plus three comment lines:

#232 change	Where it already lives on `main`
`PLACEMENT_WEIGHT_OVER_TOL` + shortfall-only tolerance	`src/analysis/clade_cumulus.jl:55`, applied at `:1052`
validated-statistics failure digest in `ci.yml`	`:429` (same 3 000-byte chunks, same 8-chunk cap)
`_tree_format` / `_valid_tree_file` on `AbstractString`	`src/server/routes/trees.jl:20,22`
single `include` of `routes/cladistics.jl`	`src/server/server.jl:131`
`_validated_duckdb_counts` without SQL arithmetic	Superseded — `main` streams `DuckDB.StreamResult` partitions and bounds the matrix as features arrive, instead of `ORDER BY feature LIMIT MAX_CELLS + 1` over the whole table
`unregister_data_frame` after the replacement scan	`test/unit/test_clade_cumulus.jl:575`
fractional-JSON 422 case	`test/unit/test_validated_statistics.jl` — already present, so the merge would have duplicated it; dropped
zero-count proportions, helper-view cleanup	`test/unit/test_exact_summaries.jl`, `test_clade_cumulus.jl`

import ..Tools in src/pipeline/phylogeny.jl is the only change main does not carry, and it is not needed: nothing in src/ reaches Tools through Phylogeny, and the FastTree test reaches the registry as MetaManifold.Tools._tools with a comment explaining exactly that. Taking the import alone would make that comment false, so main's shape is kept.

The only survivor is the explanatory comment above unregister_data_frame (why the connection-local replacement scan must be unregistered), which is on arena/101b4aee-metamanifold-webui if you want it landed.

Recommendation: close this as superseded. Nothing in it is missing from main, and it cannot merge (cut from 8df1041, ~40 commits behind) without re-deriving what #236 and #239 already did.

hyperpolymath · 2026-10-23T16:45:50Z

Closing as superseded, per the merge evidence in the comment above: merging this branch onto main (d69d4f9) leaves main plus three comment lines, and every functional change here already lives on main via #236 and #239.

The one genuinely uncovered item I found while checking this — the zero-read half of the vacated-clade tip rule — is going into a follow-up PR rather than being re-landed here.

…ule (#246) The last uncovered piece of the CladeCumulus move logic, plus one comment that was carried across from #232. This is what remains after #230, #232, #235 and #239 were dispositioned — all four are now closed, with every functional change they made already on `main`. ## What this adds **A test for the second half of the tip rule.** `_reaggregate_after_move` treats a childless clade as a tip when it holds reads *or* when its rank is a sequence terminal: ```julia counts_as_tip = own_count > 0.0 || u.rank in ("tip", "ASV") ``` The emptied-wrapper test on `main` pins the first half — a clade vacated by a move stops being a tip. The second half had no test, so nothing would have caught it being dropped. The new case moves a `GenusB` terminal into a clade holding a read-bearing `GenusA` tip, a **zero-read** `GenusA` tip and an unassigned zero-read tip, and runs both `"tip"` and `"ASV"` ranks. The two monophyly assertions are the regression guard, and they fail in the right direction. With the rule intact `GenusA` has two tips, so neither reads `true`. Drop the rank half and the zero-read tip leaves the global per-taxon tip count, `GenusA` looks like a singleton, the read-bearing tip flips to `true`, and the zero-read tip gets no verdict at all (`nothing`) instead of `false`. Taxon purity is asserted as well, but deliberately not as the guard: it reads `0.5` either way, because the tie-break between two equally sized genera lands on the same number by a different route. That is noted in the test so a future reader does not treat it as the thing that catches the regression. Every expected value was derived by porting `_reaggregate_after_move` to a scratch script and running the fixture under both the intact and the weakened rule — not by reasoning about it in my head. That mattered: my first draft of the comment claimed purity would move to `2/3`, which the port showed was wrong. **One comment, from #232.** Above `DuckDB.unregister_data_frame` in the `.jplace` test: the registration is a connection-local replacement scan, so leaving it in the file makes the view unreadable for every later connection. This was the only line of #232 that had no equivalent on `main`. ## Validation * tree-sitter parses the touched file with no ERROR/MISSING node. * Julia is not installable in this environment (the pinned runtime lives on `julang-s3`, unreachable here, source-only on the GitHub mirror), so CI is the real check — the `Julia tests` job exercises exactly this file. I will follow it and fix anything it turns up. ## Correction on the record On #235 I closed with a claim that the "zero-read terminal" testset from a CodeRabbit autofix commit "could not have passed". That was wrong, and I have posted a correction there. It would have passed; the reason I reached a different outcome (taking `main`'s side in the merge) was that `main` already had the equivalent rule with the emptied-wrapper assertion — not that the test was broken. --------- Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>

hyperpolymath enabled auto-merge (squash) October 23, 2026 13:50

hyperpolymath mentioned this pull request Oct 23, 2026

Address PR #232 app comments: asymmetric placement tolerance + import cleanup (rebased onto 688d16a) #236

Merged

hyperpolymath closed this Oct 23, 2026

auto-merge was automatically disabled October 23, 2026 16:45
Pull request was closed

hyperpolymath mentioned this pull request Oct 23, 2026

test(cladistics): cover the zero-read half of the vacated-clade tip rule #246

Merged

Navigation Menu

Repository navigation

Resolve PR #224 against main: validated-statistics + cladistics fixes, review findings, and merge conflicts - #232

Resolve PR #224 against main: validated-statistics + cladistics fixes, review findings, and merge conflicts#232
hyperpolymath wants to merge 1 commit into
mainfrom
arena/2b6e1587-metamanifold-webui

hyperpolymath commented Oct 23, 2026

Uh oh!

coderabbitai Bot commented Oct 23, 2026 •

edited by hyperpolymath

Loading

Review limit reached

Uh oh!

chatgpt-codex-connector Bot commented Oct 23, 2026

Uh oh!

sonarqubecloud Bot commented Oct 23, 2026

Uh oh!

github-actions Bot commented Oct 23, 2026

Uh oh!

coderabbitai Bot commented Oct 23, 2026

Uh oh!

hyperpolymath commented Oct 23, 2026

Uh oh!

hyperpolymath commented Oct 23, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

hyperpolymath commented Oct 23, 2026

Issues addressed from apps/reviewers on #224

Merge conflicts resolved (overlapping files between #224 and main)

Uh oh!

coderabbitai Bot commented Oct 23, 2026 • edited by hyperpolymath Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

chatgpt-codex-connector Bot commented Oct 23, 2026

Codex Review Summary

Uh oh!

sonarqubecloud Bot commented Oct 23, 2026

Quality Gate passed

Uh oh!

github-actions Bot commented Oct 23, 2026

Fork governance

Uh oh!

coderabbitai Bot commented Oct 23, 2026

Uh oh!

hyperpolymath commented Oct 23, 2026

There are no review findings on this PR to address

The red checks here are already green on main

Merge evidence

Uh oh!

hyperpolymath commented Oct 23, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Oct 23, 2026 •

edited by hyperpolymath

Loading

The red checks here are already green on `main`