Skip to content

Fix VersionRange.intersect broadening for local-version other#949

Merged
radoering merged 4 commits into
python-poetry:mainfrom
dimbleby:intersect-bug
Jun 8, 2026
Merged

Fix VersionRange.intersect broadening for local-version other#949
radoering merged 4 commits into
python-poetry:mainfrom
dimbleby:intersect-bug

Conversation

@dimbleby

@dimbleby dimbleby commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

VersionRange.intersect(Version) has a special case introduced in #579 to handle '>=X+local ∩ public_X' by broadening to '[X+local, next_patch(X))', because per PEP 440 a public-version Version constraint '==X' matches all local-tagged variants of X.

The broadening must not fire when 'other' is itself a local-tagged version: '==X+local' matches only the literal point X+local, never a sibling local-tagged variant. For an exclusive lower bound '>X+local ∩ ==X+local' the broadening wrongly returned the non-empty range '(X+local, next_patch(X))' instead of EmptyConstraint, which downstream caused Poetry's solver to derive contradictory punctured-range terms and crash in partial_solution.satisfier with a '[BUG] ... is not satisfied' RuntimeError.

Skip the broadening when 'other.is_local()'; whether the literal point falls in the range is then decided entirely by the existing 'self.allows(other)' check above.

Resolves: python-poetry#

  • Added tests for changed code.
  • Updated documentation for changed code.

dimbleby and others added 2 commits June 6, 2026 19:10
VersionRange.intersect(Version) has a special case introduced in python-poetry#579
to handle '>=X+local ∩ public_X' by broadening to '[X+local,
next_patch(X))', because per PEP 440 a public-version Version constraint
'==X' matches all local-tagged variants of X.

The broadening must not fire when 'other' is itself a local-tagged
version: '==X+local' matches only the literal point X+local, never a
sibling local-tagged variant.  For an exclusive lower bound
'>X+local ∩ ==X+local' the broadening wrongly returned the non-empty
range '(X+local, next_patch(X))' instead of EmptyConstraint, which
downstream caused Poetry's solver to derive contradictory
punctured-range terms and crash in partial_solution.satisfier with a
'[BUG] ... is not satisfied' RuntimeError.

Skip the broadening when 'other.is_local()'; whether the literal point
falls in the range is then decided entirely by the existing
'self.allows(other)' check above.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="tests/constraints/version/test_version_range.py" line_range="738-747" />
<code_context>
     assert isinstance(result, EmptyConstraint)


+def test_intersect_with_local_version_other_does_not_broaden_exclusive_min() -> None:
+    """Regression test: ``>0.21.0+cpu,<0.22.0 ∩ ==0.21.0+cpu`` must be
+    empty. Previously returned ``>0.21.0+cpu,<0.21.1`` because the
+    ``>=X+local ∩ public_X`` broadening fired for local ``other`` too.
+    """
+    excluded_point = Version.parse("0.21.0+cpu")
+    upper = Version.parse("0.22.0")
+
+    exclusive = VersionRange(
+        excluded_point, upper, include_min=False, include_max=False
+    )
+    assert isinstance(exclusive.intersect(excluded_point), EmptyConstraint)
+
+    # Inclusive-lower case still returns the literally-equal point.
+    inclusive = VersionRange(
+        excluded_point, upper, include_min=True, include_max=False
+    )
+    assert inclusive.intersect(excluded_point) == excluded_point
+
+    # Original motivating case (``>=X+local ∩ public X``) still broadens.
</code_context>
<issue_to_address>
**suggestion (testing):** Consider also asserting the symmetric intersection direction for local-version exclusivity.

To better protect against regressions, consider also exercising the symmetric calls (where applicable), e.g. `excluded_point.intersect(exclusive)` and `excluded_point.intersect(inclusive)`, if they share the same logic.

If `Version.intersect(VersionRange)` is part of the supported API and expected to mirror `VersionRange.intersect(Version)`, these assertions would verify empty vs. non-empty behavior is consistent regardless of operand order. Otherwise, a brief comment noting that only the current direction is supported would clarify the intent.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread tests/constraints/version/test_version_range.py
@radoering radoering merged commit 19dfc92 into python-poetry:main Jun 8, 2026
34 of 35 checks passed
@dimbleby dimbleby deleted the intersect-bug branch June 8, 2026 16:21
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