Repository navigation
fix: parametric fits run again; CI names failing assertions - #65
Merged
Merged
Conversation
arena-ai-coding-agent
Bot
requested a review
from hyperpolymath
as a code owner
September 25, 2026 21:44
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
hyperpolymath
approved these changes
Sep 25, 2026
arena-ai-coding-agent
Bot
changed the title
hyperpolymath
added a commit
that referenced
this pull request
Sep 25, 2026
## Why With #65's annotations, the red Julia job on `main` is readable for the first time. Run 36193210557 names 11 failures and 20 errors, **all in `test/unit/test_estimation.jl`**. The Scaling suite (#16) passes. There are three causes: 1. **Every parametric fit came back `not_run`.** R writes the fits with `write.csv(..., na = "NA")`, and Julia read them with `CSV.File(path)`, whose default `missingstring` is `""`. Any numeric column containing an `NA` was read as `String31`, and `_num` then threw `MethodError: no method matching isfinite(::String31)`. The catch-all turned that into `status = :not_run`, which produced the cascade of `KeyError`s (`n_tested`, `n_failed`, `correction`, `t_up`) and `0 == 3` results in five testsets. 2. **`glmGamPoi` never reached its #21 refusal.** `AdvancedConfig` lower-cases `dispersion_method` and then checks it against `VALID_DISPERSION_METHODS`, which spells it `glmGamPoi`. So `nb_config(dispersion = "glmGamPoi")` was refused as an unknown name. This is the same class of bug as the `tss`/`TSS` one fixed in #62. 3. **The "no placeholder statistics" scan flagged comments.** Comments in `estimation.jl` and `Execution.jl` quoted the removed code literally (`hash(taxon_id)`). ## What - `estimation.jl`: `R_NA_STRINGS = ["NA", ""]` is passed as `missingstring` to both `CSV.File` reads. `_refused_dispersion` looks up `REFUSED_DISPERSION` case-insensitively, and the error message uses the canonical name. - `AnalysisConfig.jl`: the dispersion compare is now lower-case against `lowercase.(VALID_DISPERSION_METHODS)`. - The comments are reworded, and the source-scan test is unchanged and still strict. - `test_estimation.jl`: three extra spellings (`glmgampoi`, `GLMGAMPOI`, ` glmGamPoi `) must each hit the #21 refusal. - `CHANGELOG.md` entry. ## Verified / not verified - spdx, format and blob-hygiene gates are clean. - There is no Julia runtime in the authoring sandbox, so **this PR's CI run is the first execution**. Diagnosis 1 comes from the `reason` string in the annotation. Once the fits run, some numeric assertions that were never reached may surface in turn. If so, the annotations will name them. Co-authored-by: hyperpolymath <6759885+hyperpolymath@users.noreply.github.com>
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
1 participant
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.
Why
Julia tests on
mainhave been red since #60, and the reason was never visible: the job log is served from a blob host that some environments cannot reach, and the one annotation the step posted (the last 6000 bytes of output) was truncated by the checks API at 4096 bytes — which cut it off exactly before the failing testset. Fore26d6f2(#63) all it shows is thatScaling — TSS, CSS, RSS/TMM and size factors (issue #16)has a failure.What
The "Run tests" step, on failure only, now posts:
::errorannotation per failing assertion (the Test.jl block starting atTest Failed at/Error During Test at, up to 60 lines / 3500 bytes, max 8), andTest Summaryrows whose Fail or Error column is non-empty (found by the header's column positions), plus theERROR:line.If no failure block is found (e.g. a load error), the old tail annotation is posted as a fallback. The test command and its exit status are unchanged.
Verified
Test Failed, oneError During TestwithGot exception outside of a @test, nested summary) and produced exactly two failure annotations and a summary with only the failing rows.Not claimed
This does not fix the red test. It makes the failure readable so the next PR can fix it rather than guess. This PR's own CI run should fail the same way as
mainand name the assertion.