Pull request overview

This PR adds an array-api-strict test backend (as a CPU-only proxy for CuPy-style device/host conversion failures) and introduces pytest tooling to triage “array-API escapes” and manage per-backend expected failures.

Changes:

• Add array-api-strict as a selectable test array backend (including non-default device placement) plus a dedicated tox env / CI job.
• Add triage tooling: escape-site traceback deduping, per-backend backend_xfail/backend_skip markers, and optional logging of numpy coercion “escapes”.
• Document the new backend and tooling in docs and release notes, and mark known limitations as backend-specific xfails in relevant tests.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Critical review

Summary

The mechanism design is solid in the places people usually get wrong — outcome vs conclusion with continue-on-error, cross-workflow artifact download (run-id + github-token), the workflows: ["CI"] name match, checks: write/actions: read permissions, __file__-anchored baseline path surviving tox changedir, line-number-free baseline keys, and a missing-baseline-file that fails loud rather than silent. Those are all correct.

The real problems are in the tooling's failure modes and the CI plumbing's edge cases: the baseline regeneration mode can silently destroy the baseline file, enforcement silently stops enforcing if one env var is missing, and the strict-status pipeline conflates infrastructure failure with "expected test failure" (or reports nothing at all). Plus a changelog pointing at PRs that don't exist, a stale jax xfail that never fires, and the ratchet workflow — the heart of the PR — is undocumented in docs/array_api.rst.

All reproductions below were run against the PR head (b564834) in a fresh py3.12 venv (jax 0.10.2, dask 2026.7.1).

Bugs (confirmed, with reproductions)

B1. CCDPROC_WRITE_ESCAPE_BASELINE=1 silently wipes or truncates the baseline — ccdproc/tests/_escape_triage.py:412-414, 170-206

_write_escape_baseline() rewrites the file from _observed_library_sites(), which reads _ESCAPE_LOG_COUNTS — populated only by the _log_array_escapes fixture in conftest.py, which returns immediately unless CCDPROC_LOG_ARRAY_ESCAPES is truthy (conftest.py:266). Write mode neither enables the logger nor checks that it ran. The file header says only "Regenerate with CCDPROC_WRITE_ESCAPE_BASELINE=1 (preserves tags)."

Two failure modes:

• Follow the header's instruction literally (no CCDPROC_LOG_ARRAY_ESCAPES, or on the default numpy backend): observed set is empty → the entire baseline is replaced with the header plus # (no library escapes observed this run) (_escape_triage.py:192-194).
• Even with everything set correctly, running a subset of tests drops every baseline entry for code the subset didn't exercise — "preserves tags" preserves tags only for sites still observed.

Verified live:

CCDPROC_WRITE_ESCAPE_BASELINE=1 python -m pytest ccdproc/tests/test_ccdproc.py::test_create_deviation -q
# → 7 passed in 0.95s; array_escape_baseline.txt reduced from 13 entries to
#   "# (no library escapes observed this run)"  (git diff: 1 insertion, 13 deletions)

Suggested fix: refuse to write when the logger is inactive (if not _LOG_ARRAY_ESCAPES: raise UsageError(...)) and warn (or refuse) when the new site set is smaller than the existing baseline / empty.

B2. Enforcement silently no-ops without CCDPROC_LOG_ARRAY_ESCAPES — _escape_triage.py:415-416, 371-377

CCDPROC_ENFORCE_ESCAPE_BASELINE and CCDPROC_LOG_ARRAY_ESCAPES are coupled only in the tox enforce factor, not in code. Set enforce without the logger and _observed_library_sites() is empty → _new_escapes() is empty → session passes, and the terminal summary prints the reassuring "No foreign-array escapes observed (numpy backend?); baseline not checked." A CI config that copies only the enforce var gets a permanently-green, non-enforcing gate.

Verified live (deleted the combiner.py average_combine baseline line to simulate a new escape):

CCDPROC_ENFORCE_ESCAPE_BASELINE=1 CCDPROC_ARRAY_LIBRARY=dask python -m pytest ccdproc/tests/test_combiner.py -q
# → 74 passed, exit 0: "No foreign-array escapes observed (numpy backend?); baseline not checked."
CCDPROC_LOG_ARRAY_ESCAPES=1 CCDPROC_ENFORCE_ESCAPE_BASELINE=1 CCDPROC_ARRAY_LIBRARY=dask python -m pytest ccdproc/tests/test_combiner.py -q
# → exit 1: "NEW escapes not in baseline (1) -- these fail the session: + combiner.py  average_combine  numpy.asarray"

Note the message even says "numpy backend?" while running on dask — the diagnostic itself misleads. Suggested fix: in pytest_sessionfinish (or at import), error out if ENFORCE_BASELINE and not _LOG_ARRAY_ESCAPES.

B3. Changelog cites nonexistent PRs — CHANGES.rst:8,12

The two new entries are tagged [#937] and [#939]; neither PR exists. Both should be [#942]. Also, the new section heading is bare Unreleased, where this repo's convention is x.y.z (unreleased).

B4. Hardcoded artifact name breaks the moment the expected-failures matrix grows — ci_tests.yml:172-176, strict_status.yml:32,41

ci-tests-expected-failures is deliberately a matrix, but the artifact name is the constant strict-job-outcome. upload-artifact v4+ (v6.0.0 here) errors when an artifact of the same name already exists in the run — it is no longer last-write-wins. Add a second include: row and its upload fails with "an artifact with this name already exists". strict_status.yml likewise downloads the one fixed name and reads a single file, so the whole reporting path is structurally single-entry. Inert today (one row), guaranteed breakage on the first extension.

Suggested fix: name: strict-job-outcome-${{ matrix.name }}, download by pattern in strict_status.yml, iterate; or document that the job must stay single-entry.

B5. Infra failures are reported as "expected test failure" — or produce no check at all — ci_tests.yml:158-176, strict_status.yml:36

Two related holes:

• Conflation: steps.tests.outcome == 'failure' for any nonzero tox exit — env-creation failure, array-api-strict>=2.0 failing to resolve, tox internal error — not just failing tests. All get labeled ":warning: tests failed (expected until the remaining array-API bugs are fixed)" and a grey neutral check. A genuinely broken strict environment is rebranded as routine.
• Silent gap: "Record test outcome" and "Upload test outcome" have no if: always(). If an earlier step fails ("Install base dependencies"), they're skipped → no artifact → strict_status.yml's download fails (tolerated) → "Create check run" is skipped → no strict check is posted for that commit at all.

Note the naive fix interacts badly: with bare if: always(), a skipped tests step writes skipped to the file, which strict_status.yml maps to passed === false → "failing (expected)". The pipeline needs a third state ("infra error", e.g. conclusion: failure on the check run), or at minimum a comment acknowledging the limitation.

B6. Check-run name encodes the outcome — strict_status.yml:47

py313-strict: passing vs py313-strict: failing (expected) are two different checks to GitHub. Consequences: it can never be a branch-protection required check; the check "identity" flips names across pushes as the outcome changes; anything keyed on a stable check name breaks. If informational-only is the intent, say so in a comment — better, use a stable name: 'py313-strict' and vary only conclusion and output.title.

B7. The jax xfail on test_generator_ccds_without_unit is stale — the test XPASSes — ccdproc/tests/test_image_collection.py:~386

The reason says "ccds() on the jax backend does not raise ValueError when the files have no unit" — but that ValueError comes from fits_ccddata_reader at image_collection.py:960, before the backend conversion block (:963-973), so the backend cannot affect whether it raises. Verified empirically:

CCDPROC_ARRAY_LIBRARY=jax python -m pytest ccdproc/tests/test_image_collection.py -q -rX
# → 76 passed, 1 xpassed (this test) — same result in isolation and full-module runs

The marker is wrong (or was recorded against an environment quirk that no longer exists) and, because backend_xfail maps to strict=False, it XPASSes silently forever — a live demonstration of D5 below. Remove the marker; if it originally failed for someone, the failure was a different exception (e.g. a warning promoted to error), not the stated mechanism. (The other backend_xfails — astroscrappy/scipy/reproject/block_reduce — all sit on genuine numpy-only boundaries; this is the one outlier.)

Bugs (plausible — mechanism verified, occurrence depends on conditions)

P1. Duplicate strict checks on same-repo PRs — ci_tests.yml:3-13, strict_status.yml

on: push (all branches) + pull_request both fire for a branch pushed to the main repo; the concurrency group keys on github.ref, which differs (refs/heads/… vs refs/pull/N/merge), so neither run cancels the other. Two completed "CI" runs → two workflow_run events → two checks.create calls on the same head_sha → two (possibly contradictory, since the merge-ref run can differ) strict checks on the PR. Fix: gate strict_status.yml on github.event.workflow_run.event == 'pull_request' (which also stops noise checks from schedule/push-to-main runs).

P2. Baseline keys use frame function names → fragile across Python versions and comprehensions — _escape_triage.py (record_escape_log keying on frame.name)

An escape inside a genexpr/lambda keys as <genexpr>/<lambda> on every version; list comprehensions key as <listcomp> on py<3.12 but as the enclosing function on 3.12+ (PEP 709). The enforce CI job is py312; the documented refresh is "locally", unpinned. A contributor regenerating on 3.11 produces keys that mismatch CI → spurious NEW-escape failure + spurious stale entry. Fix: document a Python floor for regeneration, or normalize <listcomp>-style names to the enclosing frame.

P3. Escape logging is not thread-safe; the reentrancy guard is process-global — conftest.py (_ReentrancyGuard, shared across all three wrappers)

Under dask's threaded scheduler, thread A inside a wrapped np.asarray sets the guard; a concurrent escape on thread B sees active and is skipped entirely; and the defaultdict[key] += 1 first-insert isn't atomic. Both drop real escapes (ratchet false-negatives), never false alarms. Fix: threading.local() guard + a lock around the tally.

Design limitations that should at least be documented

D1. The logger only sees Python-level np.asarray/np.asanyarray/np.ma.asanyarray module-attribute calls. C-level coercions inside scipy/astroscrappy/reproject (exactly the BOUNDARY deps the baseline enumerates) and any from numpy import asarray bound before the patch are invisible — the ratchet has structural false negatives. The module docstring ("catch silent conversion back to numpy") oversells it.

D2. xdist silently defeats the ratchet. _ESCAPE_LOG_COUNTS is a per-process global; with pytest -n, workers accumulate and the controller sees nothing → "baseline not checked", green. Not live today (no -n in tox.ini) — worth an assert or comment before someone adds it.

D3. Strict-job device mismatch noise. Fixture data is created on device1 (pytest_fixtures.py), but inline xp.ones(...)/xp.asarray(...) in test bodies land on the default device; array-api-strict raises on cross-device ops. Part of the 109 strict failures are harness artifacts, not ccdproc bugs — diluting the signal the job exists to produce. Worth a helper (or fixture-provided make_array) that pins the device.

D4. py313-strict installs only extras = test + array-api-strict — no alldeps (tox.ini), so numpy-boundary paths through scipy/reproject/astroscrappy aren't exercised there. Probably intentional; one comment line would prevent someone "fixing" it.

D5. strict=False backend_xfails never flag stale markers. Correct choice given xfail_strict = true globally, but a fixed backend bug becomes a silent XPASS forever (see B7). A note in the marker docs ("prune on XPASS") is enough.

Refactoring suggestions

R1. _escape_triage.py is three modules in a trenchcoat: (1) failure-triage grouping (pytest_runtest_makereport + _ESCAPE_SITES report), (2) live escape-log tally + reporting, (3) baseline load/write/enforce. Meanwhile the monkeypatching lives in conftest.py but its helpers (locate_escape_site, record_escape_log) live here, creating a conftest↔module ping-pong. Suggest _escape_log.py (patching + recording + summary) and _escape_ratchet.py (load/write/enforce/report); "triage" would then honestly name what's left.

R2. Hook registration via from ._escape_triage import pytest_sessionfinish # noqa (conftest.py:27-33) works and is commented, but pytest_plugins = ["ccdproc.tests._escape_triage"] is the idiomatic, less-fragile spelling (the noqa'd imports are load-bearing and an overzealous linter autofix could silently remove them).

R3. Implicit contract between the two workflows — the outcome file format (success/failure text) is shared between ci_tests.yml and strict_status.yml with no shared definition; a one-line schema comment in both places (or writing JSON with an explicit state: field, which also solves B5's third state) would harden it.

R4. tox factor coupling: enforce sets both CCDPROC_LOG_ARRAY_ESCAPES and CCDPROC_ENFORCE_ESCAPE_BASELINE — currently the only place the B2 invariant is maintained. After fixing B2 in code, keep the factor as the single source of the pairing. Also: nothing explains that the CI strict/enforce envs deliberately omit the test factor so they run from the source tree (not .tmp/{envname}); a future "normalization" to py312-test-… would silently change import paths.

Documentation

docs/array_api.rst documents the strict backend, CCDPROC_TRIAGE_ESCAPES, CCDPROC_LOG_ARRAY_ESCAPES, and the markers — but has zero mention of the ratchet (grep -iE 'baseline|enforce|ratchet' docs/array_api.rst → nothing). Missing, in rough priority order:

1. Regenerating the baseline — that it requires all three of CCDPROC_WRITE_ESCAPE_BASELINE=1 + CCDPROC_LOG_ARRAY_ESCAPES=1 + a non-numpy CCDPROC_ARRAY_LIBRARY, over the full suite, in the source tree. Today that's split between a tox.ini comment and the baseline-file header, and the header's one-liner is the B1 footgun.
2. What to do when the ratchet fires on your PR (py312-alldeps-dask-enforce goes red): read the "NEW escapes" section of the log, either migrate the call site or (deliberately) add a baseline line.
3. How to read the strict CI signal: the ubuntu-py313-strict check is always green (carve-out); the real outcome is the grey py313-strict: failing (expected) / green py313-strict: passing check (only after this PR merges, since workflow_run executes from the default branch), plus the run's step summary. Nothing user-facing explains the grey check.
4. CCDPROC_ENFORCE_ESCAPE_BASELINE and CCDPROC_WRITE_ESCAPE_BASELINE aren't listed alongside the other env vars at all.

Verified non-issues (so nobody re-litigates them)

Backend-name spellings in all backend_xfail/backend_skip marks match _normalize_backend_name (no silent never-applies typos); markers registered for --strict-markers; no filterwarnings loosening; no weakened assertions anywhere in the test diff; action SHAs/versions all real and current; cross-workflow artifact download inputs correct; outcome (not conclusion) correctly read under continue-on-error; session.exitstatus = 1 in pytest_sessionfinish genuinely fails the run; baseline path is __file__-anchored (changedir-safe); missing baseline file fails loud; _env_truthy handles unset/"0"/"true".

Suggested fix priority

1. B1 + B2 (one-line guards; they undermine the tool's core promise) and B3 (trivial).
2. B5 + B6 (strict-status robustness: third state, stable check name, if: always() with skipped-handling).
3. B4 + P1 (artifact-per-matrix-name; gate workflow_run on pull_request).
4. B7 (delete the stale jax marker).
5. Docs section (regeneration + ratchet-fired + grey-check).
6. P2/P3/D1–D5/R1–R4 as follow-ups or comments.

🤖 Generated with Claude Code