Skip to content

QA: run_qa v1.6 form + ExplicitImports#1497

Merged
ChrisRackauckas merged 3 commits into
SciML:masterfrom
ChrisRackauckas-Claude:qa-run_qa-v1.6
Jul 3, 2026
Merged

QA: run_qa v1.6 form + ExplicitImports#1497
ChrisRackauckas merged 3 commits into
SciML:masterfrom
ChrisRackauckas-Claude:qa-run_qa-v1.6

Conversation

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Contributor

Ignore until reviewed by @ChrisRackauckas.

Brings Catalyst's quality-assurance onto the SciMLTesting run_qa v1.6 form with ExplicitImports enabled. Catalyst had no Aqua/JET and only a hand-rolled 3-check ExplicitImports test, so this is a from-scratch QA group (mode: scratch).

What changed

  • New test/qa group (test/qa/qa.jl + test/qa/Project.toml) wired through the SciMLTesting harness. It runs Aqua, JET, and the full 6-check ExplicitImports suite in its own sub-environment, resolving released SciMLTesting 1.6.0 from the registry (no dev-from-branch). test/qa/Project.toml keeps Aqua as a direct dep (so test_ambiguities' child process can find it once :ambiguities is un-broken) and JET (weakdep, turned on by using JET); ExplicitImports comes transitively from SciMLTesting.
  • Wired into the existing GROUP runner: runtests.jl gains an activate_qa_env() + GROUP == "QA" branch (mirroring the existing activate_extensions_env()), and QA is added to the Test.yml matrix. The QA group is intentionally excluded from GROUP=All (and therefore from the Downgrade lane).
  • Removed the superseded test/miscellaneous_tests/explicit_imports.jl and the now-unused root ExplicitImports test dependency.

Fixes surfaced by enabling QA

  • import Base: (==)import Base: == — the redundant parentheses were JET's only top-level error; JET now reports 0.
  • export CartesianGridReJCartesianGridRej — a typo'd, undefined export.
  • Added [compat] for the stdlibs LinearAlgebra and SparseArrays.

ExplicitImports / QA findings

Hard-passing checks (run as normal @test):

  • no_implicit_imports, no_stale_explicit_imports (both with allow_unanalyzable = (Catalyst, Catalyst.PhysicalScale) for the PhysicalScale enum), and all_explicit_imports_via_owners.

Tracked-broken (preserved via aqua_broken / ei_broken, tracked in #1496):

  • aqua_broken: ambiguities (55 method ambiguities), unbound_args (the GridLattice = Union{Array{Bool,N}, CartesianGridRej{N,T}} alias leaves T unbound on the Array branch), undefined_exports (Variable is reexported via @reexport using ModelingToolkitBase but undefined upstream).
  • ei_broken: all_qualified_accesses_via_owners, all_qualified_accesses_are_public, all_explicit_imports_are_public — Catalyst pervasively imports/accesses internal (non-public) names of ModelingToolkitBase, Symbolics, SymbolicUtils, SciMLBase, DiffEqBase, JumpProcesses, Latexify, Graphs and Base (~200 names). These go green as the upstream packages mark names public; the issue notes they can be narrowed to per-name ignore lists meanwhile.

deps_compat runs with check_extras = false (the test-only [extras] carry no declared bounds).

Verification

Verified locally against released SciMLTesting 1.6.0 via the full CI path (Pkg.test("Catalyst") with GROUP=QA), on Julia 1.10 and 1.11:

Test Summary:     | Pass  Broken  Total     Time
Quality Assurance |   11       6     17
     Testing Catalyst tests passed

0 Fail / 0 Error.

🤖 Generated with Claude Code

ChrisRackauckas and others added 2 commits June 25, 2026 07:48
Add a `test/qa` group wired through the SciMLTesting run_qa v1.6 harness,
running Aqua, JET, and the full ExplicitImports check suite in its own
`test/qa` sub-environment (resolves SciMLTesting 1.6.0 from the registry).

This supersedes the hand-rolled `test/miscellaneous_tests/explicit_imports.jl`
(3 EI checks), which is removed along with the now-unused root ExplicitImports
test dependency.

Fixes surfaced by enabling QA:
- `import Base: (==)` -> `import Base: ==` (the redundant parentheses were the
  sole JET top-level error; JET now reports 0).
- `export CartesianGridReJ` -> `CartesianGridRej` (typo'd, undefined export).
- Add [compat] for the stdlibs LinearAlgebra and SparseArrays.

Tracked-broken (SciML#1496), preserved via run_qa markers:
- aqua_broken: ambiguities (55), unbound_args (GridLattice union leaves T
  unbound), undefined_exports (`Variable` reexported from ModelingToolkitBase
  but undefined upstream).
- ei_broken: all_qualified_accesses_via_owners,
  all_qualified_accesses_are_public, all_explicit_imports_are_public
  (pervasive use of upstream internal/non-public names).

The three passing EI checks (no_implicit_imports, no_stale_explicit_imports,
all_explicit_imports_via_owners) run as hard tests; the first two pass
`allow_unanalyzable = (Catalyst, Catalyst.PhysicalScale)`. deps_compat runs
with `check_extras = false` (test-only extras carry no declared bounds).

QA group: 11 pass, 6 broken, 0 fail/error on Julia 1.10 and 1.11.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
JET 0.11.5 crashes with `UndefRefError: access to undefined reference`
(in `JET._unique!` / `analyze_and_report_package!`) when analysing under
the Julia 1.13.0-rc1 `Compiler.jl`, so the new `test/qa` JET check breaks
the `Tests (pre, QA)` lane while passing on lts and 1 (1.10/1.12).

JET is a SciMLTesting weak dependency loaded via `using JET` precisely so
it can be omitted on Julia versions it does not support yet; guard that
load behind `VERSION < v"1.13-"` so Aqua and ExplicitImports still run on
the prerelease lane and the JET check returns once a JET release analyses
cleanly on 1.13. Tracked in SciML#1496.

Verified locally: JET=0.11.5 + Julia 1.13.0-rc1 reproduces the
`UndefRefError` crash in `report_package(Catalyst; mode=:typo)`; the
guarded `test/qa/qa.jl` still runs the full QA green on Julia 1.10 LTS
(Quality Assurance | Pass 11 | Broken 6 | Total 17).

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ChrisRackauckas-Claude

Copy link
Copy Markdown
Contributor Author

CI triage (own-check failures)

Pushed ea8fb947 fixing the one conversion-introduced, fixable failure; the rest are pre-existing/infra.

Fixed — Tests (pre, QA) (conversion-introduced): the new test/qa JET check crashes on the pre lane (Julia 1.13.0-rc1) with UndefRefError: access to undefined reference inside JET._unique! / analyze_and_report_package!. JET 0.11.5 does not analyse cleanly under the 1.13 prerelease Compiler.jl. Since JET is a SciMLTesting weak dep loaded via using JET specifically so it can be skipped on unsupported Julia versions, I guarded the load behind @static if VERSION < v"1.13-". Aqua + ExplicitImports still run on pre; the JET check returns once a JET release works on 1.13.

  • Verified locally: JET=0.11.5 + Julia 1.13.0-rc1 reproduces the exact crash in report_package(Catalyst; mode=:typo); the guarded qa.jl still runs the full QA green on Julia 1.10 LTS (Quality Assurance | Pass 11 | Broken 6 | Total 17).

Pre-existing, not touched:

  • Tests (1/lts/pre, Modeling)@test_broken Unexpected Pass at test/reactionsystem_core/reactionsystem.jl:1150 (getmetadata(complete(rs2), MiscSystemData, …), issue complete swaps order of metadata in ReactionSystems #1353). The test is byte-identical on master and is driven purely by the resolved ModelingToolkitBase version (CI resolves v1.46.0, which fixed the upstream metadata-ordering bug). Unrelated to the QA conversion — a fresh master push resolves the same MTKBase and hits the identical flip. Should be flipped to @test and complete swaps order of metadata in ReactionSystems #1353 closed in a separate change.
  • Tests (1, QA) and Extensions Tests (pre) — both The self-hosted runner lost communication with the server (infra runner-loss), not code. QA passes on lts identically (locally + CI); a re-run should pass.
  • build — the Documentation workflow (pre-existing fleet docs noise), out of scope for the QA conversion.

The new run_qa JET check passes on the lts lane (JET 0.9) but fails on the
Julia 1 lane (JET 0.11 on Julia 1.12), which reports 5 pre-existing latent
findings that JET 0.9 does not flag and that the old qa.jl never ran (Catalyst
had no JET check). All 5 reproduce byte-identical on master:

  - metadata_entries (src/dsl.jl:436) and reaction (src/chemistry_functionality.jl:400):
    wrong variable names interpolated into error(...) strings on already-error paths.
  - clipboard x2 (src/latexify_recipes.jl:65, 161): clipboard (InteractiveUtils) is
    not imported; only reachable when Latexify.COPY_TO_CLIPBOARD is set.
  - V (src/reactionsystem.jl:78): the unparameterised NetworkProperties() @kwdef
    keyword constructor references the type parameter V; that bare constructor is
    never called (only NetworkProperties{Int, SymbolicT}() is).

Gate jet_broken on VERSION >= 1.12 so the JET check stays a hard test_package on
lts (where JET reports nothing and jet_broken would flip it to an Unexpected Pass)
and becomes a documented @test_broken on the lanes whose JET surfaces the findings.
Tracked in SciML#1496.

Verified locally on Julia 1.12.6 (released SciMLTesting 1.7.0, JET 0.11.5):
Quality Assurance 10 pass / 7 broken / 0 fail / 0 error (was 10 pass / 1 fail / 6
broken). lts behaviour is unchanged (jet_broken evaluates to false = the prior default).

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
@ChrisRackauckas-Claude

Copy link
Copy Markdown
Contributor Author

CI triage follow-up — Tests (1, QA) (Julia 1.12) was still red

The pre-lane JET-skip fix (ea8fb94) did not address the Tests (1, QA) lane, which was still failing on Julia 1.12.6. Diagnosed and fixed in 82a5f0b.

Root cause (conversion-surfaced, pre-existing/latent): run_qa runs JET.test_package(Catalyst; mode = :typo) as a hard check. On the lts lane JET resolves to 0.9.18 and reports nothing (the JET test passes). On the Julia 1 lane JET resolves to 0.11.5, which reports 5 typo-mode findings that 0.9 does not flag and that the old hand-rolled QA (Aqua-less, JET-less) never ran. All 5 reproduce byte-identical on master:

name location nature
metadata_entries src/dsl.jl:436 undefined name interpolated into an error(...) string (should be metadata_i)
reaction src/chemistry_functionality.jl:400 undefined name interpolated into an error(...) string (should be rx)
clipboard ×2 src/latexify_recipes.jl:65, :161 clipboard (InteractiveUtils) not imported; only reachable when Latexify.COPY_TO_CLIPBOARD is set
V src/reactionsystem.jl:78 unparameterised NetworkProperties() @kwdef ctor references the type param V; bare ctor is never called

Fix: jet_broken gated on VERSION >= v"1.12-". The lts lane keeps the hard test_package (where it passes; a global jet_broken=true would flip lts to an Unexpected Pass since JET 0.9 reports nothing), and the Julia 1 lane marks the JET check @test_broken. The 5 findings are documented in #1496 for a separate source fix; report_package auto-flags an Unexpected Pass once they are fixed.

Verified locally on Julia 1.12.6 (released SciMLTesting 1.7.0, JET 0.11.5):
Quality Assurance | 10 pass | 7 broken | 0 fail | 0 error (was 10 pass / 1 fail / 6 broken). lts behaviour is unchanged — jet_broken evaluates to false there, identical to the prior default.

Ignore until reviewed by @ChrisRackauckas.

@ChrisRackauckas-Claude ChrisRackauckas-Claude marked this pull request as ready for review July 3, 2026 00:36
@ChrisRackauckas

Copy link
Copy Markdown
Member

Will let this slide for now, but what the QA demonstrates here is that this package probably has the most QA issues. For now, to get it merging with the rest of the crew, things are just being flagged as broken so CI passes. But those will need to get addressed as they are direct tests as to the fragility of the package maintenance.

@ChrisRackauckas ChrisRackauckas merged commit 4296aca into SciML:master Jul 3, 2026
14 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants