Skip to content

VersionCheck: compare base branch instead of registry; drop @assert - #65

Merged
mtfishman merged 1 commit into
mainfrom
mf/versioncheck-base-branch
Apr 21, 2026
Merged

mtfishman merged 1 commit into
mainfrom
mf/versioncheck-base-branch

Conversation

@mtfishman

Copy link
Copy Markdown
Member

Summary

VersionCheck previously pulled the latest registered version from ITensorRegistry and asserted registered_version < current_version. That has a design gap: between a substantive PR merging (with a version bump) and the new version being registered, any number of further substantive PRs pass without bumping the version because registered_version is still the old value.

Concrete example seen on ITensor/ITensorGaussianMPS.jl#24:

State Registry main Project.toml PR Project.toml
Before #23 0.1.13 0.1.13 —
#23 merged (not yet registered) 0.1.13 0.1.14 —
#24 opened 0.1.13 0.1.14 0.1.14 (no bump)

Old check: 0.1.13 < 0.1.14 → pass ✅. Reviewer intuition: ❌, the PR didn't move the version forward.

Fix

Compare the current branch's Project.toml version against the same file on the PR's base branch. Answers the question reviewers actually have ("did this PR move the version forward relative to main"), removes the registration-timing dependency, and matches how General-registry tooling thinks about the check.

Implementation

  • New Fetch base branch step (actions/checkout@v4 is shallow on PR head only; the base branch isn't otherwise available): git fetch --depth=1 origin "$GITHUB_BASE_REF".
  • Drop julia-actions/julia-buildpkg from this workflow — the new check only needs TOML (stdlib), not registry resolution.
  • Use TOML.parse + VersionNumber + a real if / error("…") rather than @assert, so the failure produces a clean Julia error rather than an AssertionError. The error message spells out the base ref and both versions for actionable feedback.
  • Skip gracefully when the base Project.toml is missing (initial-commit edge case) or when neither file has a version field. Error if only the PR removed the version field.

The localregistry input becomes unused but is kept (with an updated description) for backward compatibility with existing callers in ITensorPkgSkeleton-generated workflows.

Test plan

  • CI passes on this PR (substantive change, version field on both base and head — should report a clean pass message).
  • Future PR opened that touches a substantive file but doesn't bump Project.toml version reports a clear failure message naming both versions.
  • Workflow-only PRs (e.g. .github/**-only changes) still skip via the classify-pr action.

🤖 Generated with Claude Code

The previous check pulled the latest registered version from
ITensorRegistry and asserted current > registered. This had a design
gap: between a substantive PR merging (with a version bump) and the
new version actually being registered, any number of further
substantive PRs would pass without bumping the version, because
"registered" was still the older value.

Switch to comparing the current branch's Project.toml version
against the same file on the PR's base branch. That answers the
question reviewers actually have ("did this PR move the version
forward relative to main"), removes the registration-timing
dependency, and matches how related ecosystems (e.g.
JuliaRegistries) intuit the check.

Implementation:
- Add a "Fetch base branch" step (the default actions/checkout@v4 is
  shallow on PR head only, so the base branch isn't otherwise
  available) that does git fetch --depth=1 origin "$GITHUB_BASE_REF".
- Drop julia-actions/julia-buildpkg from this workflow — the new check
  only needs TOML (stdlib), not registry resolution.
- Use TOML.parse + VersionNumber + a real if/error rather than @Assert,
  so the failure produces a clean Julia error message rather than an
  AssertionError. The error spells out the base ref and both versions.
- Skip gracefully when the base Project.toml is missing (initial-commit
  edge case) or when neither file has a version field. Error if only
  the PR removed the version field (regression).

The localregistry input becomes unused but is kept (with an updated
description) for backward compatibility with existing callers in
ITensorPkgSkeleton-generated workflows.

Tracked in
ITensorDevelopmentPlans/Projects/ITensorActions/fixes_and_improvements.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mtfishman
mtfishman merged commit fe4cdb3 into main Apr 21, 2026
1 check passed
@mtfishman
mtfishman deleted the mf/versioncheck-base-branch branch April 21, 2026 19:35
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.

1 participant