test: integrate pyhs3 with HS3TestSuite#283
Conversation
📝 WalkthroughWalkthroughChangesThe PR adds pinned HS3TestSuite conformance testing, a pyhs3 backend adapter, result and known-failure validation, audit tooling, CI execution with report artifacts, test dependencies, and testing documentation. HS3TestSuite Conformance
Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #283 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 39 39
Lines 2917 2917
Branches 349 349
=========================================
Hits 2917 2917 ☔ View full report in Codecov by Harness. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.github/workflows/ci.yml:
- Around line 140-142: Update the primary actions/checkout step in the CI job to
set persist-credentials to false, matching the existing external-repository
checkout configuration; leave the separate action hash-pinning concern
unchanged.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 79734c13-8e04-4b97-a4de-b157062145e2
📒 Files selected for processing (11)
.github/workflows/ci.yml.pre-commit-config.yamldocs/testing.rstpixi.tomlpyproject.tomltests/hs3testsuite/known_failures.jsontests/hs3testsuite/pin.jsontests/hs3testsuite_pyhs3_backend.pytests/test_hs3testsuite.pytests/test_hs3testsuite_backend.pytools/audit_hs3testsuite.py
| - uses: actions/checkout@v7 | ||
| with: | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Set persist-credentials: false on the primary checkout too.
This job's second checkout (145-150, external HS3TestSuite) already sets persist-credentials: false, but the first checkout of the PR's own repo (140-142) doesn't, despite this job also pulling in a third-party repository into the same workspace. actions/checkout v7 now stores persisted credentials outside .git/config, which reduces the practical exposure, but matching the sibling step's setting is still cheap defense-in-depth.
🔒 Proposed fix
- uses: actions/checkout@v7
with:
fetch-depth: 0
+ persist-credentials: falseSeparately, zizmor also flags actions/checkout@v7, setup-pixi@v0.10.0, and actions/upload-artifact@v7 here as unpinned-to-hash — this simply mirrors the existing convention used throughout the rest of this workflow file (e.g. lines 24, 98, 122), so it's better addressed as a repo-wide hash-pinning initiative rather than only in this new job.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/checkout@v7 | |
| with: | |
| fetch-depth: 0 | |
| - uses: actions/checkout@v7 | |
| with: | |
| fetch-depth: 0 | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 140-142: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 140-140: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/ci.yml around lines 140 - 142, Update the primary
actions/checkout step in the CI job to set persist-credentials to false,
matching the existing external-repository checkout configuration; leave the
separate action hash-pinning concern unchanged.
Source: Linters/SAST tools
1 participant
Summary
This PR integrates pyhs3 with the external HS3TestSuite, providing a direct comparison against the frozen RooFit reference results stored in the suite.
The integration uses a new backend adapter owned and maintained by pyhs3.
Motivation
ROOT already runs HS3TestSuite against RooFit. Adding the same suite to pyhs3 makes it possible to:
Implementation
This PR adds:
JSON, Markdown, and JUnit compatibility reports.
The adapter loads each HS3 workspace with pyhs3, performs the requested structure checks, constructs the corresponding likelihood, and evaluates the twice-delta-NLL scans using public PyTensor APIs.
Current compatibility
The pinned suite contains 56 checks across 19 fixtures. Current pyhs3 results are:
Expected failures currently cover unsupported HS3 functions and distributions, one multidimensional evaluation issue, and one numerical mismatch. The expected failures are strict: if one starts passing, CI fails until its ledger entry is deliberately removed. CI also fails if a known failure changes stage—for example, if a numerical mismatch regresses into a workspace-import failure.
Pinning and update policy
HS3TestSuite is pinned to:
9d04e321ae6fddd283a35507f14ecf852eb7df61
The pin is intentionally not updated automatically. Updating it requires reviewing changes to:
A candidate revision can be audited without changing the tracked pin:
pixi run -e py312 python tools/audit_hs3testsuite.py \ --suite-root /path/to/HS3TestSuite \ --output-dir /tmp/pyhs3-hs3-auditThe resulting reports identify the candidate commit and manifest digest while leaving normal CI pin enforcement unchanged.
Testing
Validated locally with:
Results:
HS3TestSuite integration: 36 harness passes and 25 strict xfails
Raw HS3 compatibility: 31 passes and 25 known failures
Adapter unit tests: 8 passes
Ruff and configuration validation pass
Documentation doctests pass
Summary by CodeRabbit
New Features
Documentation
Bug Fixes