[ty] Fix divergent tuple unpacking when RHS sequence is the cycle head by charliermarsh · Pull Request #23812 · astral-sh/ruff

charliermarsh · 2026-03-08T18:26:06Z

Summary

This PR adds a fast path in Unpacker::unpack that, for assignment unpacking with a fixed-length sequence RHS, structurally matches the LHS targets against the RHS subexpressions and pulls their types directly from the already-inferred ExpressionInference.

Closes astral-sh/ty#2872.

astral-sh-bot · 2026-03-08T18:27:50Z

Typing conformance results

No changes detected ✅

Current numbers

The percentage of diagnostics emitted that were expected errors held steady at 85.05%. The percentage of expected errors that received a diagnostic held steady at 78.05%. The number of fully passing files held steady at 63/132.

astral-sh-bot · 2026-03-08T18:29:12Z

`mypy_primer` results

Changes were detected when running on open source projects

sympy (https://github.com/sympy/sympy)
- sympy/logic/tests/test_boolalg.py:963:35: error[unsupported-operator] Unary operator `+` is not supported for object of type `BooleanTrue | BooleanFalse | Divergent`
+ sympy/logic/tests/test_boolalg.py:963:35: error[unsupported-operator] Unary operator `+` is not supported for object of type `BooleanTrue | BooleanFalse | Zero | One | Any`
- sympy/logic/tests/test_boolalg.py:964:35: error[unsupported-operator] Unary operator `-` is not supported for object of type `BooleanTrue | BooleanFalse | Divergent`
+ sympy/logic/tests/test_boolalg.py:964:35: error[unsupported-operator] Unary operator `-` is not supported for object of type `BooleanTrue | BooleanFalse | Zero | One | Any`
- sympy/logic/tests/test_boolalg.py:965:39: error[invalid-argument-type] Argument to function `abs` is incorrect: Expected `SupportsAbs[Unknown]`, found `BooleanTrue | BooleanFalse | Divergent`
+ sympy/logic/tests/test_boolalg.py:965:39: error[invalid-argument-type] Argument to function `abs` is incorrect: Expected `SupportsAbs[Unknown]`, found `BooleanTrue | BooleanFalse | Zero | One | Any`
- sympy/logic/tests/test_boolalg.py:967:39: error[invalid-argument-type] Argument to function `__new__` is incorrect: Expected `str | Buffer | SupportsInt | SupportsIndex | SupportsTrunc`, found `BooleanTrue | BooleanFalse | Divergent`
+ sympy/logic/tests/test_boolalg.py:967:39: error[invalid-argument-type] Argument to function `__new__` is incorrect: Expected `str | Buffer | SupportsInt | SupportsIndex | SupportsTrunc`, found `BooleanTrue | BooleanFalse | Zero | One | Any`

astral-sh-bot · 2026-03-08T18:29:14Z

Memory usage report

Memory usage unchanged ✅

oconnor663 · 2026-03-10T06:44:48Z

crates/ty_python_semantic/src/types/unpacker.rs

+                    .insert(target.into(), value_inference.expression_type(value_expr));
+                true
+            }
+            ast::Expr::Starred(_) => false,


You could delete this arm?

oconnor663 · 2026-03-10T06:48:06Z

crates/ty_python_semantic/src/types/unpacker.rs

+    fn unpack_fixed_sequence_from_inference(
+        &mut self,
+        targets: &[ast::Expr],
+        values: Option<&[ast::Expr]>,


A little awkward for this to be an Option just so this function can short-circuit. Probably cleaner to remove the Option here, and then have the caller short circuit instead if they have nothing to give us.

oconnor663 · 2026-03-10T06:55:26Z

crates/ty_python_semantic/src/types/unpacker.rs

+        // `self.targets` when a later element returns `false`. This is harmless:
+        // the fallback `unpack_inner` path will overwrite any partial entries.
+        targets.iter().zip(values).all(|(target, value_expr)| {
+            self.unpack_assignment_sequence_from_inference(target, value_expr, value_inference)


Is this for wacky recursive cases like a, (b, c) = 1, (2, 3)? That's probably worth a comment. (Looks like there's already a "Nested tuple with unpacking" test case.)

Oh wait no even the base case a, b = 1, 2 ends up recursing to handle the individual expr::Names?

oconnor663 · 2026-03-10T07:16:05Z

crates/ty_python_semantic/src/types/unpacker.rs

+    ///
+    /// When the RHS sequence is the cycle head, its normalized sequence type can collapse
+    /// element-level unions like `None | Divergent` to `Divergent`. Reusing the already-inferred
+    /// subexpression types lets unpacking converge as if each element had been queried directly.


If my understanding here is still up-to-date, this is because recursive_type_normalized likes to collapse recursive structures down to a single level of nesting. (Or I maybe I should say "no nesting"?) So like the classic infinitely growing tuple from an assignment like x = (0, x) gets collapsed to (Literal[0], Divergent), which does make sense. But unfortunately in our unpacking assignments like a, b, c = (foo(), bar(), baz()), that RHS follows the same rule in cyclic cases and gets collapsed to something like (Divergent, Divergent, Divergent), so we lose all information about a, b, and c. We know that the outer/literal tuple structure itself doesn't participate in any recursion, but recursive_type_normalized doesn't know that.

Anyway, if that sounds right, maybe add something like that to the comment here?

oconnor663 · 2026-03-10T07:19:20Z

crates/ty_python_semantic/src/types/unpacker.rs

-            infer_expression_types(self.db(), value.expression(), TypeContext::default())
-                .expression_type(value.expression().node_ref(self.db(), self.module()));
+        let value_inference =
+            infer_expression_types(self.db(), value.expression(), TypeContext::default());


I really want to understand this part of the change better. If what I said about recursive_type_normalized below is right, we're trying to dodge inferring the RHS as a whole, but it seems like infer_expression_types is still going to aggressively do that. Clearly this works because it's passing tests. So...is there some subtlety here about exactly what ends up being the cycle head? Or am I just wrong that infer_expression_types does everything greedily?

I think the key is that it returns ExpressionInference, not the type of the outer RHS node, which stores a type for every subexpression. So we then use value_inference.expression_type(child_expr) for each fixed element rather than unpacking from value_inference.expression_type(value_expr).

If you look at ExpressionInference::expression_type, it's just a hash map lookup, because all the inference is already done by the time infer_expression_types returns. (Or maybe I'm totally wrong...)

I'm learning that it does store an entry for every child expression separately before it builds the outer tuple... The outer tuple can lose its shape, but the child entries stay precise, I think?

Yes, ok, Carl and I were discussing the same thing here on the plane :) So the key is that the whole tuple expression is still as gutted as it always was, but we're no longer going through that tuple expression to get the individual elements. Instead we're reaching for the values of the element expressions directly, and those are ... "one level less gutted" ... which ends up being enough for the cycle to stabilize at a useful value.

Wow this sounds like a great flight!

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

crates/ty_python_semantic/src/types/unpacker.rs

oconnor663 · 2026-03-11T02:21:51Z

crates/ty_python_semantic/src/types/unpacker.rs

+                true
+            }
+            ast::Expr::List(ast::ExprList { elts, .. })
+            | ast::Expr::Tuple(ast::ExprTuple { elts, .. }) => {


The recursion here is somewhat mirroring the recursion done in the regular unpack_inner codepath. My instinct was to unify them, though when I ask Claude to do it the result is maybe only a bit simpler, so I could see doing it either way. I'll ask you to pick :)

Co-authored-by: Jack O'Connor <oconnor663@gmail.com>

charliermarsh added the ty Multi-file analysis & type inference label Mar 8, 2026

charliermarsh marked this pull request as ready for review March 9, 2026 16:44

charliermarsh requested review from AlexWaygood, carljm, dcreager, ibraheemdev and sharkdp as code owners March 9, 2026 16:44

astral-sh-bot bot assigned oconnor663 Mar 9, 2026

astral-sh-bot bot requested a review from oconnor663 March 9, 2026 16:44

charliermarsh added the bug Something isn't working label Mar 9, 2026

carljm removed their request for review March 9, 2026 18:10

oconnor663 reviewed Mar 10, 2026

View reviewed changes

charliermarsh and others added 4 commits March 10, 2026 08:45

Fixed divergent tuple unpacking

fe5fdba

Simplify

b400a9c

Update unpacking test to expect literal types for list literal unpacking

80ae5ba

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Nits

d42b093

charliermarsh force-pushed the charlie/cycle-head branch from 6545ee2 to d42b093 Compare March 10, 2026 13:02

charliermarsh requested a review from oconnor663 March 10, 2026 13:02

oconnor663 approved these changes Mar 11, 2026

View reviewed changes

Update crates/ty_python_semantic/src/types/unpacker.rs

ab86a5f

Co-authored-by: Jack O'Connor <oconnor663@gmail.com>

charliermarsh enabled auto-merge (squash) March 11, 2026 12:40

charliermarsh merged commit 22f63ec into main Mar 11, 2026
50 checks passed

charliermarsh deleted the charlie/cycle-head branch March 11, 2026 12:46

Conversation

charliermarsh commented Mar 8, 2026

Summary

Uh oh!

astral-sh-bot bot commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

No changes detected ✅

Uh oh!

astral-sh-bot bot commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

astral-sh-bot bot commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Memory usage report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oconnor663 Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

astral-sh-bot bot commented Mar 8, 2026 •

edited

Loading

astral-sh-bot bot commented Mar 8, 2026 •

edited

Loading

`mypy_primer` results

astral-sh-bot bot commented Mar 8, 2026 •

edited

Loading

oconnor663 Mar 10, 2026 •

edited

Loading