Repository navigation
Add exact descriptive statistics and independent pipeline evidence - #69
Open
hyperpolymath wants to merge 13 commits into
Open
hyperpolymath wants to merge 13 commits into
hyperpolymath wants to merge 13 commits into
Conversation
Adapt the statistics-only work from fork PRs #199, #202, #257 and #259 onto upstream main. Preserve integer/rational counts through the API, DuckDB, persistence and React display, and label MPFR metrics explicitly as approximations. Keep scientific review and release approval pending. Validate original pipeline CSVs with independent Python arithmetic and retain input, configuration and software provenance. Reject stale output, handle R's row-name CSV heading, and fix the integration prerequisite's null executable-path handling. Make evidence writers and mutation controls fail closed on incomplete or unrelated failures. Relates-to: #65
Taking abs of a minimum fixed-width signed integer can overflow before the decimal renderer reaches BigInt arithmetic. Widen the numerator first so the stated nearest-decimal, ties-away rule also holds at the Int8 through Int128 boundaries. All eleven added boundary assertions fail before this change and pass after it.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
hyperpolymath
marked this pull request as ready for review
November 7, 2026 19:47
Codex Review Summary: ❌ FailedThis 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. |
![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: ec6d5a8ef2
ℹ️ 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".
Trace all six example FASTQs to the original mothur archive and record the publisher's site-wide CC BY 4.0 notice alongside upstream's existing data-specific annotations. Correct the incomplete earlier account of the published terms while distinguishing the scope inference from a new grant. Provide separate scientific-review and maintainer decision forms, with revision-bound evidence, limitations and an explicit pending-review state. No permissions, reviewer identities or release approvals are invented. Documentation only; runtime implementation, data and licence declarations are unchanged. Relates to JoshuaJewell#65.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 203460c827
ℹ️ 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".
… boundaries Default to the merged pipeline table and accept the existing safe saved-table names without coercing non-string input. Report Float16 significand precision correctly. Add route, serialization, catalogue and failed-publication regressions, and retain raw LCOV as a CI artifact. Focused Julia 1.12.5 tests: 3389 assertions passed; full hosted CI remains required.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1c955b069
ℹ️ 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".
Validate nested filter types and fields before the legacy filter transport can silently ignore them. Refuse pipe-containing or empty categorical values it cannot preserve, and normalize cleared arrays/null fields to inactive filters. Regression tests cover actual row selection, malformed requests, bounds and cleared UI state. Focused route run exposed the cleared-array legacy behavior; the corrected composition passes its source-level smoke checks. Full hosted CI remains required.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28f0f398c4
ℹ️ 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".
Honor aggregation in the workspace, match subgroup prefixes at their delimiter, preserve count columns that collide with feature-label aliases, reject unknown text-filter columns, and refuse integer columns whose role is ambiguous under the shared metadata-name heuristic. Measure simulation errors at reference precision. Cover the regressions through persisted route results, MPFR state checks, and a rendered workspace interaction test, using a pinned React 18 development test renderer. Preserve the concurrent saved-table, precision metadata and filter-validation fixes. Scientific and release decisions remain with the reviewers.
The pinned Julia runtime converts BigFloat to Rational through an ambient-precision intermediate. Scope that conversion to the stored operand precision before decimal rounding, with 100-place regressions at lower ambient precisions. Add an exact threshold-straddling simulation control and reject non-string subgroup/rank fields. Preserve the concurrent scope, schema, simulation and rendered-workspace repairs in 1b45afd. Isolated precision/syntax controls passed; hosted CI must validate the combined revision.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce88ead742
ℹ️ 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".
Quote every identifier that reaches a filter predicate through the shared helper the projection already used, so a saved table column such as odd"sample cannot turn a filtered summary into malformed SQL. Refuse a BIGNUM column instead of omitting it: it is an integer the sample detector cannot name, exactly like the unsigned types the guard already covered, and an unbounded width cannot be read as an exact 64-bit count. Record and check the run group a summary was actually read from. A grouped run addressed without ?group= resolves to its directory\u2019s group, so filing the unresolved value misidentified the record\u2019s source and made the explicit query form fail to retrieve a record the implicit form had just created. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The new case asserted one mechanism (a 422 naming the type) for a boundary where the repository has not settled whether an unbounded integer is refused or carried under the existing count bounds. Test the thing that is not negotiable: a successful summary must not omit the column. Both outcomes now pass, and only a silent omission fails. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Correct the saved-table fixture and execute quoted-column filters with exact-count assertions. Retain diagnostic coverage on failed runs without weakening the test gate; add derived-record I/O and storage-limit checks. Follow-up to JoshuaJewell#69. Local frontend tests/build, independent reference controls, REUSE and CI report checks passed; SonarCloud and Semgrep passed on #312. Signed-off-by: hyperpolymath <6759885+hyperpolymath@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Follow-up to JoshuaJewell#69. Refresh only GitHub Actions pins to Node 24 releases already used on the maintained fork: checkout v7.0.1, setup-julia v3.0.3, Julia cache v3.5.0, cache v6.1.0, upload-artifact v7.0.2 and Codecov v7.1.1. Every pin is an immutable commit SHA verified against its release tag. Julia 1.12.5, R 4.5.0, bun 1.3.10 and external-tool pins are unchanged. YAML parses and the coverage-report shell fixture passes. The full upstream suite validates the upgraded actions after landing. --------- Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Merged
3 of 5 tasks
## Change and reason The `Julia tests` job fails on [JoshuaJewell#69](JoshuaJewell#69) at head `1e58411`: 6590 pass, 9 fail (run 42435863478). All 9 failures are in tests that [#312](#312) added without a Julia run. In each case the code is right and the test expectation is wrong. 1. **Quoted-column filters** (`test_validated_statistics.jl`, 6 failures). The response has one entry per sample. Each sample has its own count for the one remaining feature. The test read all four counts, and their sum, from the first sample. It now checks every sample by label: `2`, `20`, `200` and `"9007199254740993"`. 2. **Numeric count filter** (2 failures). The test sent `max` as `9007199254740993` and expected the 422 count-filter refusal. The request parser refuses an unsafe integer token first, with 400. The test now sends a safe bound for the 422 case, and it asserts the 400 for the unsafe token. 3. **Unreadable derived record** (`test_derived_analyses.jl`, 1 failure). The test read a directory and expected an I/O error. In Julia, `read` on a directory returns no bytes and raises nothing. The test now uses a Unix-domain socket path. Its open fails for every user, root included. No production code changes. This pull request targets the branch of upstream PR 69. A merge here updates that pull request. ## Validation Julia 1.12.5, from the repository root: ```bash julia --project=. test/runtests.jl test_derived_analyses.jl test_validated_statistics.jl ``` - At `1e58411`: 487 pass, 9 fail. These are the same 9 failures as in CI. - At this head: 528 pass, 0 fail. - I printed the real response for the filtered request. It has four samples with counts `[2]`, `[20]`, `[200]` and `["9007199254740993"]`. - I checked the directory behaviour directly: `read(directory)` returns `UInt8[]`, and `read(socket path)` raises `SystemError`. Not run: the full suite, the integration and server tests, and the frontend tests. No workflow runs on this pull request, because `ci.yml` on this branch starts only for pull requests into `main`. The upstream CI runs after the merge. ## Scope - The socket-path test also covers `src/analysis/derived_analyses.jl` line 79, which CI listed as a coverage gap. - Five uncovered patch lines remain from the CI list. They are not part of this fix: - `src/server/routes/validated_statistics.jl` 229 and 230: the catch around `_body_filter_params`. - `src/server/routes/validated_statistics.jl` 296: a float `time_limit_seconds`. - `src/analysis/derived_analyses.jl` 127: the lost hardlink race. - `src/canonical.jl` 220: this branch looks unreachable, because `repr` of a finite `Float64` always contains a `.`. - No lockfile change. ## Contribution checks - [x] I followed [CONTRIBUTING.md](https://github.com/JoshuaJewell/MetaManifold-WebUI/blob/main/CONTRIBUTING.md). - [x] This branch starts at the head of PR 69 (`1e58411`) and contains no merges from fork main. - [ ] Every commit is signed and shows as Verified. (The managed signing identity decides this.) - [ ] CI has run and passes. (No workflow runs here. See Validation.) - [x] Existing licensing, citation and acknowledgement text is unchanged. --- _Generated by [Claude Code](https://claude.ai/code/session_0136P7ZoTPXfxg1C85rVNx78)_ Co-authored-by: Claude <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.
Adds exact count totals and reduced rational proportions, plus explicitly approximate MPFR descriptive metrics, with bounded inputs, immutable derived records and a React interface. Independent Python Fraction/Decimal checks validate original pipeline count matrices and retain provenance.
Relates to #65. Keep #65 open for independent scientific review and JoshuaJewell's release decision. Issue #23 and ordination PR #260 are excluded. Existing inferential implementations and Julia/R/toolchain pins are unchanged. Review status remains
pending_review.Engineering review fixes
The branch preserves concurrent commits
d1c955b(default/saved tables, Float16 metadata and boundary coverage) and28f0f39(typed, lossless filter validation). Follow-up1b45afd:Aseparate fromAB.featureand internal-looking or quoted names.A pinned
react-test-renderer@18.3.1development dependency and its minimal bun lockfile additions support the rendered interaction regression. Production dependencies are unchanged. No generated outputs are committed.Concurrent follow-up
ce88eadpreserves stored BigFloat precision during decimal display, rejects non-string scope fields, and adds a threshold-straddling simulation regression. These changes are retained; their numeric-policy/route/evidence selection passes 631 assertions. Final follow-upa1076ecadds CONTRIBUTING-required credit to the two changed dependency manifests through REUSE metadata, preserving their existing licences and all runtime code. CI below targets the final revision.Validation
Tested with Julia 1.12.5, R 4.5.0 and bun 1.3.10. At
1b45afd: 672 focused route/boundary/adversarial/evidence assertions pass; all 68 frontend tests (206 assertions), the frontend build and REUSE pass. The broader statistics selection passed 3,486 assertions before the final concurrent-filter and reserved-name additions; the final CI runs the complete suite. The corrected simulation passed 200 exact and 40 precision replicates. The UI regression rejects restoration of the original aggregation defect.CI for the current signed revision is queued/running; earlier green runs are not substituted for this revision. GitHub verifies the new signature. The branch preserves reviewed history without merge commits.
Run:
Reference and evidence commands are in validation-evidence.md. CI retains generated matrices, independent receipts and raw Julia coverage. An earlier local full-suite differential fixture failure also reproduced on pristine upstream
89be534; no differential code or tests were changed here.Published data-use basis and reviewer decisions
The original mothur site has a CC BY 4.0 notice, and upstream already explicitly labels these FASTQs CC BY 4.0. All six decompressed repository inputs match the original archive. The data-use record documents original sources, hashes, attribution and the scope inference applying the site notice to its download. No dataset-specific permission correspondence is asserted.
The scientific and maintainer decision forms remain unfilled. The three MiSeq samples are time points from one mouse, supporting a limited count-only pipeline exercise, not representative biological validation or a PR2 taxonomy claim.
ExactCounts' Agda model evidence is documented without adding that dependency or claiming whole-application proof. The historical six-mutant campaign passed at
419f2fb; it is not represented as a campaign on this later revision. Neither CI, coverage nor bounded model proofs supplies scientific or release approval.