Drastically reduced number and run time of integration tests

[digicosmos86]

This PR reduces the number of integration tests by using smart combinations to maintain coverage and pruning unnecessary tests

Summary by CodeRabbit

• Bug Fixes

  • Invalid combinations of bounded priors and custom distributions now raise a clear ValueError.
  • Updated test fixture paths improve reliability for likelihood-related validation.

• Tests

  • Expanded integration coverage across sampling, variational inference, backends, model types, missing data, deadlines, and sampler validation.
  • Added coverage for default sampler behavior, post-processing, and unsupported black-box likelihood handling.
  • Streamlined slow-test execution and manual test triggering for more consistent verification.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 49f2dd33-2bb6-4421-a6f4-e88c58ebc3a6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

This PR refactors HSSM’s integration test suite to reduce runtime and duplication by replacing large Cartesian-product parameter grids with smaller pairwise covering arrays, while also tightening a prior-construction error into a proper runtime exception and updating tooling configuration.

Changes:

• Reworked multiple integration test matrices (MCMC, VI, choice-only, missing-data/deadline) from full grids into pairwise covering arrays; merged deadline + missing-data test files where behavior is controlled by a single mechanism flag.
• Changed Prior to raise ValueError (instead of relying on assert) when bounds and a custom dist are provided together; updated unit test expectations accordingly.
• Updated pyrefly versions/config and simplified slow-test workflow wiring.

Reviewed changes

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

Show a summary per file

File	Description
tests/unit/test_prior.py	Updates expected exception type for invalid Prior(bounds=..., dist=...) usage.
tests/unit/likelihoods/test_likelihoods.py	Fixes ONNX fixture path resolution to point at tests/fixtures.
tests/integration/test_vi.py	Replaces large VI grid with pairwise covering array; splits out blackbox rejection tests.
tests/integration/test_missing_data_vi.py	Merges missing-data and deadline VI coverage; switches to pairwise matrix + deterministic data injection.
tests/integration/test_missing_data_mcmc.py	Merges missing-data + deadline MCMC tests; replaces full grid with pairwise matrix + explicit rejected-combo tests.
tests/integration/test_missing_data_and_deadline_vi.py	Removes now-redundant deadline VI integration file (merged elsewhere).
tests/integration/test_missing_data_and_deadline_mcmc.py	Removes now-redundant deadline MCMC integration file (merged elsewhere).
tests/integration/test_mcmc.py	Replaces full sampler/likelihood/shape cross with pairwise coverage; adds default-sampler resolution test.
tests/integration/test_choice_only.py	Shrinks choice-only integration matrix via pairwise coverage; keeps analytical+jax path coverage.
src/hssm/prior.py	Converts bounds+dist conflict from assert to ValueError; adds/adjusts type annotations.
pyproject.toml	Bumps dev dependency requirement for pyrefly to >=1.2.0.
.pre-commit-config.yaml	Updates pyrefly hook rev to 1.2.0 and modifies hook configuration.
.github/workflows/run_tests.yml	Removes support for custom pytest args and simplifies test invocation.
.github/workflows/run_slow_tests.yml	Refactors slow-test workflow to delegate to run_tests.yml.

---

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

tests/integration/test_missing_data_mcmc.py (1)

178-196: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

match receives an unescaped literal message.

pytest.raises(match=...) treats the string as a regular expression. The pattern works here only because . matches any character. Wrap the expected text in re.escape so future edits to the message stay reliable.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/test_missing_data_mcmc.py` around lines 178 - 196, Update
test_deadline_requires_missing_data to pass the expected ValueError message
through re.escape before supplying it to pytest.raises(match=...), and add the
required re import if absent. Preserve the existing message text and exception
assertion.

tests/integration/test_choice_only.py (1)

89-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

sample is duplicated across the integration suites with a different return contract.

This copy returns the sampling result. The copies in tests/integration/test_mcmc.py (Line 72) and tests/integration/test_missing_data_mcmc.py (Line 88) return None. Move one helper into a shared module, such as tests/integration/conftest.py, and always return model.sample(...). This removes three near-identical bodies and one silent behavior difference. See the consolidated comment for the full set of duplicated helpers.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/test_choice_only.py` around lines 89 - 108, Move the
duplicated sample helper into the shared integration conftest module, preserving
its slice-specific step configuration and sampling arguments. Update
test_mcmc.py, test_missing_data_mcmc.py, and test_choice_only.py to reuse this
helper, and ensure every path returns model.sample(...) consistently.

tests/integration/test_mcmc.py (1)

232-303: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

The post-processing tests share one fit and depend on figure state.

Each test calls az.plot_trace_dist several times and then reads plt.gcf(). The figure count assertions pass only if no earlier figure remains current. Close figures between the assertions to keep the tests independent of the matplotlib global state.

♻️ Proposed cleanup for `test_post_processing_reg_v_a`

     az.plot_trace_dist(model.traces)
     fig = plt.gcf()
     assert len(fig.axes) // 2 == 8
+    plt.close("all")
 
     az.plot_trace_dist(model.traces, var_names=["~a"])
     fig = plt.gcf()
     assert len(fig.axes) // 2 == 8
+    plt.close("all")

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/test_mcmc.py` around lines 232 - 303, Update the plotting
assertions in test_post_processing_simple, test_post_processing_reg, and
test_post_processing_reg_v_a to close each generated matplotlib figure after
inspecting it, including between repeated az.plot_trace_dist calls. Ensure
figure cleanup prevents plt.gcf() from observing stale figures and keeps the
tests independent of global matplotlib state.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/integration/test_missing_data_mcmc.py`:
- Around line 59-69: Update prepare_data to normalize missing_indices into a
boolean NumPy mask for both modes, converting the random index selection in the
non-"opn" branch accordingly. Replace positional iloc assignment with
label-based data.loc[missing_mask, "rt"] assignment, preserving the existing
missing-value marker and avoiding dependence on column order.

---

Nitpick comments:
In `@tests/integration/test_choice_only.py`:
- Around line 89-108: Move the duplicated sample helper into the shared
integration conftest module, preserving its slice-specific step configuration
and sampling arguments. Update test_mcmc.py, test_missing_data_mcmc.py, and
test_choice_only.py to reuse this helper, and ensure every path returns
model.sample(...) consistently.

In `@tests/integration/test_mcmc.py`:
- Around line 232-303: Update the plotting assertions in
test_post_processing_simple, test_post_processing_reg, and
test_post_processing_reg_v_a to close each generated matplotlib figure after
inspecting it, including between repeated az.plot_trace_dist calls. Ensure
figure cleanup prevents plt.gcf() from observing stale figures and keeps the
tests independent of global matplotlib state.

In `@tests/integration/test_missing_data_mcmc.py`:
- Around line 178-196: Update test_deadline_requires_missing_data to pass the
expected ValueError message through re.escape before supplying it to
pytest.raises(match=...), and add the required re import if absent. Preserve the
existing message text and exception assertion.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9ff15e31-effe-4df8-9726-b6f3782b7615

📥 Commits

Reviewing files that changed from the base of the PR and between a8ff8f2 and cb29c1b.

📒 Files selected for processing (14)

• .github/workflows/run_slow_tests.yml
• .github/workflows/run_tests.yml
• .pre-commit-config.yaml
• pyproject.toml
• src/hssm/prior.py
• tests/integration/test_choice_only.py
• tests/integration/test_mcmc.py
• tests/integration/test_missing_data_and_deadline_mcmc.py
• tests/integration/test_missing_data_and_deadline_vi.py
• tests/integration/test_missing_data_mcmc.py
• tests/integration/test_missing_data_vi.py
• tests/integration/test_vi.py
• tests/unit/likelihoods/test_likelihoods.py
• tests/unit/test_prior.py

💤 Files with no reviewable changes (2)

• tests/integration/test_missing_data_and_deadline_mcmc.py
• tests/integration/test_missing_data_and_deadline_vi.py

[AlexanderFengler]

Heads-up on a merge-order collision with #1143 (scheduled drift detection) — flagging early since whichever of us merges second breaks the other, and both breakages are silent rather than loud.

#1143 adds a drift.yml that re-runs the existing gates on a weekly cron against a fresh dependency resolve of unchanged main. It consumes the same reusable workflows this PR reshapes, in two ways that stop working:

1. test_args is removed, and drift.yml passes it.

  fast:
    uses: ./.github/workflows/run_tests.yml
    with:
      test_args: "-o addopts= --timeout=360"

After this PR, run_tests.yml declares only run_slow_tests, so that call becomes an "invalid input" error and the job fails to start. Easy to fix on my side — but the reason the input was there is worth a decision: addopts still carries --exitfirst, which is right for PRs and wrong for drift triage. When an upstream release breaks us, the useful signal is the full blast radius (which suites broke, how many tests), not the first failure. Without a way to override addopts, a drift run tells us "something broke" and stops.

Three ways out, your call: keep a narrow pytest_args string input; move --exitfirst/--cov* out of the shared addopts into the PR-facing invocation; or accept truncated drift output and I'll drop the override.

2. The slow suite becomes unreachable from a scheduled run.

    if: >-
      github.event_name == 'workflow_dispatch' ||
      contains(github.event.head_commit.message, '[run slow]')

The github context inside a reusable workflow comes from the caller, so on the weekly cron github.event_name is schedule and github.event.head_commit doesn't exist — both operands are false and the job skips. The opt-in is exactly right for pushes; it just has no branch a scheduler can take.

The clean fix given this PR's own design is for drift.yml to stop calling run_slow_tests.yml and instead call run_tests.yml with run_slow_tests: true. That's strictly nicer than what I have today, so I'm happy to adopt it — I just can't do it until this lands. Alternatively add github.event_name == 'schedule' to the if.

Suggested order: merge this first, then I rebase #1143 onto it and adapt both jobs in one commit. If you'd rather I go first, say so and I'll instead add the drift-facing bits here as a follow-up.

Worth noting the safety net worked as designed while I was checking this: even if the slow job silently skipped, the spine's hssm-slow-suite freshness budget would go red after 17 days without a successful slow leg, because it evaluates ages per job rather than per run — a skipped job never refreshes its clock. So this would surface, just two weeks later than it should. Better to fix it at merge time.