Repository navigation
fix: resolve PR #219 conflicts and address CladeCumulus & validated-statistics findings - #228
Closed
hyperpolymath wants to merge 13 commits into
Closed
hyperpolymath wants to merge 13 commits into
hyperpolymath wants to merge 13 commits into
Conversation
Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
…nifold-webui Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
…nifold-webui Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
…nifold-webui Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
|
Warning Review limit reachedYou'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 33 minutes. View limit details
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. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Fork governance
🟣 critical · commit
Hypatia scan: 133 finding(s)
panic-attack assail: 19 finding(s)
pons asinorum: 10 finding(s)
rhodibot (RSR compliance): not_run — enrolled, but no report-back was received
panicbot (panic recovery, error handling): not_run — enrolled, but no report-back was received
echidnabot (formal verification: proofs/): not_run — enrolled, but no report-back was received
seambot (integration health: API routes): not_run — enrolled, but no report-back was received
finishingbot (release readiness): not_run — enrolled, but no report-back was received
accessibilitybot (WCAG on the React UI): not_run — enrolled, but no report-back was received
glambot (presentation quality): not_run — enrolled, but no report-back was received
cipherbot: not_applicable — no cryptography is implemented in this repository
sustainabot: 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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b501c01c25
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
hyperpolymath
enabled auto-merge (squash)
October 23, 2026 07:54
hyperpolymath
disabled auto-merge
October 23, 2026 13:50
hyperpolymath
enabled auto-merge (squash)
October 23, 2026 13:51
|
Add Carrot credits or activate Agent usage billing to use Autofix |
This was referenced Oct 23, 2026
Owner
Author
Resolution: this branch is fully superseded by
|
| File | Status on main |
|---|---|
.github/workflows/ci.yml |
richer per-assertion digest of failing validated-statistics tests + log tail |
src/analysis/clade_cumulus.jl |
PhyloNode.edge_number already used in _graft_jplace_queries; _reaggregate_after_move already adds rk.subtree_pd + the node's own branch, and rebuilds emptied clades from direct_count |
src/server/routes/clade_cumulus.jl |
with_results_db and String(strip(...)) already in place; like-weight renormalisation deliberately not applied (see #236/#237: unplaced mass is reported as unplaced_reads) |
src/server/routes/validated_statistics.jl |
whole-body raw-token validation after ijson_read, and streamed bounded rows instead of a materialised DataFrame - both supersets of this branch |
test/unit/* |
main's suites are much larger; the one test this branch had that main lacked ("Nested-clade drag-and-drop counts each active branch once") is restored in #244 |
App findings
- Codex P2 - "Keep leaf subtree PD consistent after a move": this one was
still open againstmain, and it was right._reaggregate_after_move
credits an active leaf with its own incoming branch whilebuild_node
hard-coded0.0, so validating a proposal changed the PD of tips whose
topology had not moved. Fixed in fix: resolve PR #228, unify leaf subtree PD, and answer malformed JSON with 400 #244: both builders now derive a leaf's PD
fromclade_faith_pd, with a regression test that asserts the node-local
invariant and compares untouched tips across a proposal. - CodeRabbit: the review run on this PR was rate-limited ("Review limit
reached") and the Autofix request returned "Add Carrot credits or activate
Agent usage billing", so no new CodeRabbit findings were produced against
this head. Its earlier findings on fix: restore validated statistics and CladeCumulus CI #219 (the nested-clade double count, the
results.duckdbconnection handling, the count-token checking) are all
addressed onmain, and the double-count fix now has a test. - SonarCloud: quality gate passed (0 new issues, 0 hotspots).
- panic-attack: the
InputBoundaryfinding was real - 21 unguarded
JSON3.read(String(req.body))sites turned a malformed body into a 500.
They now go throughServer._request_jsonand answer 400. The
DynamicCodeExecutionfindings are false positives (RCall.revalevaluates
R source strings, not Juliaeval). - pons asinorum: the
unreachable-after-jumphits point at plain
arrow-function bodies, and thestring-concat-in-loophits at
parts.push(...)/parts.join(''). No code change warranted. - Hypatia: 43 criticals are all
cicd_rules/banned_language_fileon
frontend/src/**/*.ts, an upstream-authored React/TypeScript frontend. That
rule's only carve-outs are narrow path prefixes (bindings/typescript/,
.d.ts,vite.config.ts, ...) and it cannot be suppressed repo-side, so
migrating it is an estate-level AffineScript project rather than a fix for
this fork. The scan stays advisory here by design (scripts/check-governance-contract.sh). - DEED: green, and reproduced locally with the exact linter the reusable
lane uses (deed_lint.jsathyperpolymath/standards@75bd5994). The
stdlib Python port beside it is older and rejects#u8; the JS linter that
CI runs accepts it, so the deed should not be "fixed" to satisfy the port.
Recommendation
Close this pull request as superseded. The conflict resolution is recorded in
#244 (merge commit 77376ef, tree identical to main) together with the
Codex P2 fix and the input-boundary hardening, so nothing from this branch is
lost by closing it.
Thanks @coderabbitai and @chatgpt-codex-connector - the subtree-PD finding in
particular was a real inconsistency that survived several rounds of review on
main.
|
To use Codex here, create an environment for this repo. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -u
for number in 228 244; do
gh pr view "$number" --repo hyperpolymath/MetaManifold-WebUI \
--json number,state,url,title,baseRefName,headRefName,mergeCommit,mergedAt
done
fd -t f '^(clade_cumulus|validated_statistics|test_clade_cumulus|test_validated_statistics|test_exact_summaries)\.jl$'
for file in src/analysis/clade_cumulus.jl src/server/routes/clade_cumulus.jl src/server/routes/validated_statistics.jl test/unit/test_clade_cumulus.jl test/unit/test_validated_statistics.jl test/unit/test_exact_summaries.jl; do
if [ -f "$file" ]; then
ast-grep outline "$file" --items all 2>/dev/null | head -60
rg -n -C 4 'clade_faith_pd|subtree_pd|with_results_db|unplaced_reads|_statistics_body|_reject_float_count_tokens|MAX_CELLS|Nested-clade|untouched|VS_SERVER|Main.SV' "$file" | head -120
fi
done
for file in .github/workflows/ci.yml scripts/check-governance-contract.sh; do
if [ -f "$file" ]; then
printf '\n--- %s ---\n' "$file"
sed -n '1,220p' "$file"
fi
doneLength of output: 27888
I confirmed that I have not independently verified the tree equality, I have not closed You are interacting with an AI system. |
Owner
Author
|
Superseded by #242, which is now fully green and mergeable as |
auto-merge was automatically disabled
October 23, 2026 16:45
Pull request was closed
hyperpolymath
added a commit
that referenced
this pull request
Oct 23, 2026
## Change and reason Supersedes #219 and the earlier reconciliation attempt #228. The validated-statistics and CladeCumulus changes on those stale branches have already landed through later work on `main`; conflict comparison showed that taking either stale side would now revert newer strict I-JSON, streamed `BigInt`, read-only DuckDB, tree-rooting, and unplaced-read fixes. This branch is therefore based directly on current `main` and carries only changes that are still needed. - **CladeCumulus:** active non-root tips now use the same node-local PD convention before and after drag-and-drop reaggregation. Regression tests cover every fixture tip before and after an accepted move, plus exact nested-clade totals (0.60 and 1.40) that reject parent-side double-counting. This addresses the remaining Codex finding on #228 and CodeRabbit's aggregation finding on #219. - **CI:** the full Julia job has a 60-minute hang bound, and validated-statistics diagnostics no longer use `|| true`; the captured test exit status remains authoritative. - **Frontend quality:** SVG path fragments are batched and joined once rather than repeatedly concatenated, and `PaneEditor` uses the direct hook dependency. - **Security and conformance:** adds the missing security policy and EditorConfig, plus pinned OpenSSF Scorecard and full-history secret-scanning workflows. Scorecard retains least-privilege default-branch SARIF upload without a self-referential PR alert. The repository gitleaks config backports two upstream rules for biological-data false positives and allowlists only ten exact public schema/callback identifiers—no path or commit exclusions. The existing DEED lane is conformant and unchanged. ## Validation - `npx --yes bun@1.3.10 install --frozen-lockfile` - `npx --yes bun@1.3.10 run typecheck` - `npx --yes bun@1.3.10 test src` — **72 passed, 0 failed** (200 assertions) - `npx --yes bun@1.3.10 run build` - Parsed every workflow as YAML and every inline `run` block with `bash -n` - `bash scripts/check-governance-contract.sh` — all 20 checks passed - `python3 test/references/check_high_precision_references.py --check test/unit/test_high_precision.jl` - `bash scripts/lint-sonar-reliability.sh` - `git diff --check` - Reproduced the legacy gitleaks rules over all 188 reachable commits: 211,428 biological/generated-code false positives reduced to zero; synthetic AWS, Facebook, and generic credential canaries still produced exactly three findings - Fresh-head `Secret history and working tree / gitleaks` — **passed** with the CI-pinned gitleaks 8.18.4 - [CI run 39986326202](https://github.com/hyperpolymath/MetaManifold-WebUI/actions/runs/39986326202) on `ed24c9c` — **passed**: Julia 1.12.5 tests, validated statistics, frontend build/tests, R/RCall checks, high-precision references, benchmarks, Agda, and both occupancy lanes ## Deliberate review triage - No baseline or exclusion was added to hide Hypatia findings. - TypeScript remains the settled frontend stack and the independent Python numerical oracles remain in place; the banned-language findings are policy debt, not vulnerabilities introduced here. - XML namespace `http://www.w3.org/...` strings were not changed to HTTPS: they are identifiers, not network requests. - Existing licence headers were left unchanged as `CONTRIBUTING.md` requires. - No placeholder CodeQL, Hypatia, mirror, or AI-manifest files were added. CodeQL default setup and the Hypatia governance lane are already active; external mirroring and the legacy A2ML-to-DEED migration require explicit owner configuration/decisions. ## Scope - [x] Does not alter dependencies, lockfiles, generated assets, or vendored files - [x] Secrets, local paths, and run outputs are not committed - [x] Commits include `Signed-off-by:` (Developer Certificate of Origin) --------- Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Owner
Author
Independent audit and supersession of #228Closing this stale, conflicting PR as superseded, not merging it and not claiming all outstanding work is on main.
Security/governance follow-upThe earlier claim that #243 also carries the independent K9/DEED/Hypatia lanes, pinned full-history secret scanning with redacted reporting and boundary tests, least-privilege workflows, and dependency/security tooling from the #227 audit. Local contract/mutation/logging/scanner tests and Julia syntax parsing passed after this update. Julia/R are not installed locally; runtime validation is not claimed. #243 was subsequently rebased with explicit user permission and merged at 16:54:51 UTC as No other contributor's branch was rewritten, no checks were waived, and no stale count-loading or placement-normalisation code was restored. The leaf-PD work has landed through #242 and the security follow-up through #243. The immediate CI repair is #247; malformed-JSON follow-up remains in #244. #228 was already closed by the time the explicit close command ran; its closed/unmerged state is confirmed. Latest validation / repair status#247 ( |
hyperpolymath
added a commit
that referenced
this pull request
Oct 23, 2026
## Scope Security/governance follow-up to #227. All four Codex findings on that old PR already landed in #229/#234 and follow-ups; I answered/resolved its threads and closed it as superseded, rather than merging a conflicting branch that could undo newer fixes. CodeRabbit's remaining notices are account limits/billing, not code questions. ## Changes - Add pinned, least-privileged full-history/working-tree secret scanning and default-branch Scorecard publication; no inherited secrets or new alert suppressions. - Add Dependabot, SECURITY.md, editor settings and a repository-specific AI manifest without changing existing licensing/architecture. - Add independent pinned K9 conformance and its artifact to the PR verdict. The current empty corpus is explicitly grey, not a semantic pass. Missing semantic evidence fails closed. Keep DEED syntax and advisory Hypatia as separate evidence families. - Bound Julia CI at 90 minutes and preserve producer **and** log-writer failures. Add 8 negative/mutation tests and 4 exit-combination tests. - Refresh the bun lockfile within current major ranges for real security advisories, notably DOMPurify, Vite and transitive build dependencies; raise minimum Vite to 6.4.5 and Router DOM to 6.30.6. This is why the lockfile changes. No generated assets are included. - Audit high/critical frontend dependencies in CI. ## Verification Bun **1.3.10**: frozen install, TypeScript checking, **72 frontend tests / 200 assertions**, production build, and high/critical audit pass. Full audit reduced from **31 advisories (1 critical, 13 high, 14 moderate, 3 low) to 2 moderate Router advisories**. This is not a zero-vulnerability claim. Canonical pinned DEED lint, all 39 governance contract checks, 8 weakening mutations, 4 pipeline-exit combinations, unique-key YAML parsing, all embedded shell syntax, Sonar reliability lint, proof axiom audit (7 modules), and git diff checks pass locally. Julia/R/Agda/Nickel are not installed in this sandbox; hosted CI must supply those results. No local runtime/semantic pass is claimed. ## Hosted wrapper fixes The first run found two issues in the pinned standards wrappers. Fork-local copies retain the same immutable standards validator/renderer/config sources and all gates, but fix the terminal optional-log copy for an empty K9 corpus and upgrade Gitleaks from 8.18.4 to 8.30.1 (upstream release-asset digest). The old rules falsely matched historical DNA/FASTQ and schema labels; no history exclusions or local rule suppressions were added. A synthetic AWS credential must still fail in CI. Three K9 copy cases and a credential-sensitivity test also pass locally. Hosted checks are rerunning. ## Remaining / intentionally not done See `docs/release-notes/2026-10-23-governance-security.md` for the source Hypatia run and complete triage. Two moderate React Router advisories need a v7 migration review. Estate language bans conflict with the settled React/TS stack and independent Python reference oracles; W3C SVG namespace URIs are not HTTP requests. Do not fix those by rewriting the architecture or breaking namespaces. No configured mirror target exists. Existing upstream licensing headers remain protected. Fleet bots without report-backs remain not_run. No fake conformance reports have been added. The integration returns 403 for code-scanning/Dependabot alert listing and the sandbox blocks Actions artifact/log storage hosts; no alerts have been dismissed. Published grids/check APIs are still readable. Scorecard's default-branch publication can only be verified after landing; K9 has no committed contracts and therefore cannot claim substantive contract coverage yet. ## PR #228 security follow-up (`1be406d`) Independent verification found that the recorded conflict-resolution tree for #228 is identical to main; #228 is being closed as superseded, not conflict-merged. Its valid leaf-PD finding is already implemented in **unmerged** #242/#244 and is not duplicated here. #242's integration run 39986326202 passed all six jobs; #244's Julia validation remains pending at review time. The R-evaluation warning was not dismissed as a language-detector false positive. This follow-up validates metadata column identifiers before they enter PERMANOVA's parsed R formula and validates package names before provenance interpolates them into R source. Expressions, quotes/escapes, controls and reserved formula terms are covered by regression cases. Plain identifiers preserve the existing formula and scientific tests; non-identifier metadata columns must be renamed, as documented in the function contract. Local verification for the new commit: 39 governance assertions, 8 weakening mutations, 3 K9 copy cases, 4 CI logging cases, scanner-boundary tests, complete working-tree/history shell scanning, git diff checks and syntax parsing of all four changed Julia files pass. **Syntax parsing is not execution**: the new Julia/R regressions await hosted CI. This does not certify all remaining R call sites or dismiss outstanding advisory findings. ### Main advanced during review #242 has now merged as `b656d2a99e53267108d51ad87d7eaee13222d368` (16:46:42 UTC), after its full integration suite passed. The leaf-PD fix is therefore on main. #228 is confirmed closed/unmerged. This PR (#243) now has conflicts with the newly updated main and is **not merge-ready**; no main-merge commit or unrequested history rewrite has been made. The new R-boundary/scanner tests are running at `1be406d`; earlier passing checks must not be mistaken for validation of a future rebased head. --------- Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
hyperpolymath
added a commit
that referenced
this pull request
Oct 23, 2026
…N with 400 (#244) ## Change and reason Three things, in the order the linked review asked for them. ### 1. PR #228's merge conflicts, resolved (and shown to be a no-op) PR #228 (`arena/7805092e`) branched from `8df1041`; all six files it touched have since been rewritten on `main` by #231, #233, #234, #236, #237 and #239. Resolving the six conflicted files in favour of `main` produces a tree that is **byte-identical** to `main` (`git diff --stat origin/main` is empty), so the merge commit records the resolution and changes no content. Concretely: * `ci.yml` - main publishes a per-assertion digest of every failing validated-statistics test plus a log tail; #228's awk windows are superseded. * `src/analysis/clade_cumulus.jl` - main already reads `PhyloNode.edge_number` in `_graft_jplace_queries`, already sums `rk.subtree_pd` plus the node's own branch, and additionally recomputes an emptied clade from `direct_count`. * `src/server/routes/clade_cumulus.jl` - main already uses `with_results_db` and `String(strip(...))`, and deliberately does **not** renormalise placement like-weights by the retained total: `parse_jplace` already rejects ratios outside [0, 1], a multi-candidate placement already partitions its mass, and dividing a lone 0.6 candidate by its own total would move the 0.4 the tool left unplaced onto the reported edge (#236/#237 report it as `unplaced_reads` instead). * `src/server/routes/validated_statistics.jl` - main validates raw tokens for the whole body after `ijson_read` proves it I-JSON, and streams bounded rows instead of materialising a DataFrame. Both are strict supersets. * `test/unit/*` - main's suites are far larger; the one test #228 had that main lacked ("Nested-clade drag-and-drop counts each active branch once") is restored here. ### 2. Codex P2 on PR #228: leaf subtree PD `_reaggregate_after_move` credits an active leaf with its own incoming branch, so a parent can sum `subtree_pd` alone and count each active edge once. The ordinary builder (`build_node`) hard-coded `0.0` for leaves, so validating a drag-and-drop changed the reported PD of leaves whose topology had not changed and made the proposed tree's metrics incomparable with the original. Both builders now take a leaf's PD from `clade_faith_pd` - the function the internal-node branch already used - so the conventions cannot drift again. Internal nodes and inactive tips are unchanged. ### 3. panic-attack `InputBoundary`: malformed bodies are 400s 35 unguarded `JSON3.read(String(req.body))` call sites across eight route files raised a JSON3 parse error out of the handler, so a truncated or hostile body produced a 500. Every route now parses through `Server._request_json`, which throws the existing `BadRequest`. The helper copies `req.body`, because `String(::Vector{UInt8})` can consume the buffer. Rescuing that turned out to need one more step, which CI caught: Oxygen builds its chain as `custom(serializer(router))` and its serializer catches every handler exception and answers 500 before our middleware runs, so `_bad_request_middleware` never saw the throw. Oxygen's error mapper is the hook inside the serializer, and it is already a dispatch point (`handle_error(::ValidationError)` answers 400). Extending it for `BadRequest` keeps this server's `{"error", "message"}` shape and makes the test's `status == 400` true. ### 4. pons asinorum: tree path building is linear The six `string-concat-in-loop` hits in `TreeShapes.tsx` were real: every node appended to four shared strings inside a `useMemo` over the whole layout, which is quadratic in the node count on a view that renders every node. The fragments are collected per node and joined once; the emitted order is unchanged. ### 5. panic-attack `ProofDrift` and a guarded benchmark baseline The `@test x isa Y` assertions the rule flagged were real tautologies: they passed for any string. The occupancy pair now asserts the text the code emits (`measured 201 KiB above the predicted 101 KiB`) and the evidence round trip asserts the ISO-8601 UTC stamp `save_document` writes. The advanced zero-handling benchmark parsed its baseline outside any guard, so a truncated baseline surfaced as a JSON3 failure from inside the timing loop; it now names the file it could not read. ## Validation - `cd frontend && npx tsc --noEmit -p .` - clean. - `cd frontend && npx vite build` - clean (chunk-size warning only, pre-existing). - `cd frontend && bun test src` - **72 pass, 0 fail** (11 files). - `bash scripts/check-governance-contract.sh` - **all 20 checks passed**. - `bash scripts/lint-sonar-reliability.sh` - clean. - `bash -n` on every `scripts/*.sh` - clean. - All four workflows parse as YAML; the DEED lints clean with the pinned `deed_lint.js` from `hyperpolymath/standards@75bd5994` (see below). - **Not run locally:** the Julia suite (no Julia runtime in this environment; the two changed Julia files are covered by CI's `Julia tests` job). ## Hypatia and K9/DEED conformance * **DEED** - `MetaManifold-WebUI_chora.deed` lints clean with the exact linter the reusable lane runs (`bun 1-formats/deed/tools/deed_lint.js`). Note the stdlib Python port in the same directory is older and rejects `#u8`; the JS linter the workflow uses accepts it, so the deed is correct as written. * **K9** - this repository has no `.machine_readable/contractiles/` registry or Mustfile, and `governance.yml` does not call `k9-contractile.yml`, so the K9 contractile lane is not in scope here. What *was* wrong is that the deed declared `:report-backs ".gitbot-fleet/report-backs/"` while the directory did not exist; it now does, with the envelope contract documented, and the grid keeps rendering those members orange until a real report-back lands. * **Hypatia** - the 43 critical findings are all `cicd_rules/banned_language_file` for `frontend/src/**/*.ts`. That rule is a repo-side-unfixable policy finding: TypeScript's only carve-outs are narrow path prefixes (`bindings/typescript/`, `.d.ts`, `vite.config.ts`, ...), none of which cover an upstream-authored React frontend, and the rule cannot be suppressed from the repository. Migrating upstream's frontend to AffineScript is an estate project, not a change this fork can make. * **panic-attack / pons asinorum** - most remaining findings are stale or false positives at `main`: `api/events.ts` and `tree/model.ts` already wrap `JSON.parse` in try/catch (hardened in #239), the `DynamicCodeExecution` hits are `RCall.reval` (R source strings, not Julia `eval`), and the `unreachable-after-jump` hits point at plain arrow-function bodies. The two classes that were real - unguarded request-body parsing and quadratic path building - are fixed above. The grid's residual after this PR: Hypatia 45 critical (all `banned_language_file` on upstream TypeScript), 61 high, 35 medium; panic-attack 6 high (all `RCall.reval` false positives), 7 medium; pons asinorum 2 medium (`unreachable-after-jump` on a `useMemo` return and a `useEffect` cleanup, both false positives). ## Scope - [x] Focused on the linked review plus conformance evidence - [x] Does not alter dependencies, lockfiles, generated assets, or vendored files - [x] Secrets, local paths, and run outputs are not committed ## Contribution checks - [x] I followed CONTRIBUTING.md - [x] My commits include `Signed-off-by:` (DCO) and the licence is unchanged --------- Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change and reason
Resolves the merge conflicts on PR #219 (
fix: restore validated statistics and CladeCumulus CI) againstmainand addresses all review and CI findings raised by apps on PR #219:src/analysis/clade_cumulus.jl,src/server/routes/clade_cumulus.jl):read_label_and_quoted(capture_nhx=..., capture_edge=...)andPLACEMENT_WEIGHT_SUM_TOL = 0.05frommainwith PR fix: restore validated statistics and CladeCumulus CI #219'sPhyloNode.edge_numberfixes in_graft_jplace_queriesandPlacementCandidate.like_weightin/api/v1/phylo/validate-jplace._reaggregate_after_moveso nested-clade drag-and-drop counts each active non-root branch once (sub_pd += rk.subtree_pdplus incoming branch on active non-root clades) matchingclade_faith_pd.results.duckdbviawith_results_db(readonly=true) in_load_run_table_dfand/api/v1/studies/{study}/runs/{run}/clade-cumulus/sourceswhile filteringduckdb_tables() WHERE NOT internaland convertingSubStringroute parameters (tree_file,table) toStringfor_valid_tree_file/_valid_name.src/server/routes/validated_statistics.jl):_statistics_body(req)via_reject_float_count_tokens(raw)so integral decimal/exponent tokens (1.0,1e0) fail closed beforeJSON3.read._validated_duckdb_countsby checking theSELECT COUNT(*) AS nrow/cell budget (MAX_CELLS) before materialising raw rows into aDataFrame, then aggregating per-feature counts inBigInt.test/unit/test_clade_cumulus.jl,test/unit/test_exact_summaries.jl,test/unit/test_validated_statistics.jl,.github/workflows/ci.yml):LoadError: invalid assignment to constant Main.SVintest/unit/test_clade_cumulus.jlby bindingSV = MetaManifold.Server(non-const, matching the other test files included inMain) andconst VS_SERVER = MetaManifold.Serverintest/unit/test_validated_statistics.jl.DuckDB.DB(CHECKPOINT+DBInterface.close!(con)+DBInterface.close!(db)) before computingbefore_db_shaso read-only CladeCumulus routes leaveresults.duckdbbyte-for-byte identical.Run validated-statistics tests first.Validation
git diff --check.python3 test/references/check_high_precision_references.py --check test/unit/test_high_precision.jl..github/workflows/ci.ymlshell step syntax withbash -n.arena/7805092e-metamanifold-webui.Scope
Contribution checks
CONTRIBUTING.mdandCODE_OF_CONDUCT.mdSigned-off-by:(Developer Certificate of Origin) or I agree to license my contribution under the project license (AGPL-3.0-only)