[ty] Fix loop-header reachability cycles in conditional unpacking (#24006)

charliermarsh · web-flow · commit bc7cf1434dea · 2026-03-23T00:22:44.000Z
## Summary Closes astral-sh/ty#3057. Closes astral-sh/ty#3104.
diff --git a/crates/ty_python_semantic/resources/mdtest/loops/while_loop.md b/crates/ty_python_semantic/resources/mdtest/loops/while_loop.md
@@ -383,6 +383,34 @@ while random():
 x
 ```
 
+### Statically unreachable `del` branches don't poison cyclic loopback
+
+This creates a loop-header cycle through `if x`, but the `del x` branch should still disappear once
+the loopback type settles:
+
+```py
+def random() -> bool:
+    return False
+
+x = 1
+while random():
+    if x:
+        x = 1
+    else:
+        del x
+    reveal_type(x)  # revealed: Literal[1]
+```
+
+Comparison-guarded `del` branches should also disappear once the loopback type settles:
+
+```py
+x = 1
+while x < 10:
+    if x == 4:
+        del x
+    reveal_type(x)  # revealed: Literal[1]
+```
+
 ### Bindings in a loop are possibly-unbound after the loop
 
 ```py
@@ -441,6 +469,43 @@ while random():
     reveal_type(y)  # revealed: Literal[1, 2]
 ```
 
+### Monotonic widening can keep stale loopback bindings reachable
+
+```py
+def random() -> bool:
+    return False
+
+x = 0
+while random():
+    # TODO: This should reveal `Literal[0]`.
+    reveal_type(x)  # revealed: Literal[0, 2]
+    if x == 1:
+        x = 2
+```
+
+### Conditional unpacking and loop exits converge normally
+
+This reduced example from issue #3057 used to panic with "too many cycle iterations":
+
+```py
+def fetch(req) -> tuple:
+    return (True, None)
+
+def paginate():
+    bookmark = None
+    while True:
+        if bookmark is None:
+            req = None
+        else:
+            req = bookmark
+        ok, next_bookmark = fetch(req)
+        if not ok:
+            return
+        bookmark = next_bookmark
+        if bookmark is None or bookmark == 0:
+            break
+```
+
 ### Loop bodies that are guaranteed to execute at least once
 
 TODO: We should be able to see when a loop body is guaranteed to execute at least once. However,
diff --git a/crates/ty_python_semantic/src/place.rs b/crates/ty_python_semantic/src/place.rs
@@ -1181,7 +1181,6 @@ fn loop_header_reachability_impl<'db>(
     let loop_header = get_loop_header(db, loop_header_definition.loop_token());
     let place = loop_header_definition.place();
 
-    let mut has_defined_bindings = false;
     let mut deleted_reachability = Truthiness::AlwaysFalse;
     let mut reachable_bindings = FxIndexSet::default();
 
@@ -1202,7 +1201,6 @@ fn loop_header_reachability_impl<'db>(
                     def, definition,
                     "loop headers only include bindings from within the loop"
                 );
-                has_defined_bindings = true;
                 reachable_bindings.insert(ReachableLoopBinding {
                     definition: def,
                     narrowing_constraint: live_binding.narrowing_constraint,
@@ -1221,7 +1219,6 @@ fn loop_header_reachability_impl<'db>(
     }
 
     LoopHeaderReachability {
-        has_defined_bindings,
         deleted_reachability,
         reachable_bindings,
     }
@@ -1230,8 +1227,6 @@ fn loop_header_reachability_impl<'db>(
 /// Result of [`loop_header_reachability`]: pre-computed reachability info for loop-back bindings.
 #[derive(Debug, Clone, PartialEq, Eq, salsa::Update, get_size2::GetSize)]
 pub(crate) struct LoopHeaderReachability<'db> {
-    /// Whether any reachable loop-back binding is a defined binding.
-    pub(crate) has_defined_bindings: bool,
     pub(crate) deleted_reachability: Truthiness,
     /// Reachable loop-back bindings that are not `del`s.
     pub(crate) reachable_bindings: FxIndexSet<ReachableLoopBinding<'db>>,
@@ -1247,7 +1242,6 @@ impl<'db> LoopHeaderReachability<'db> {
         reachable_bindings.extend(self.reachable_bindings);
 
         LoopHeaderReachability {
-            has_defined_bindings: self.has_defined_bindings,
             deleted_reachability: self.deleted_reachability,
             reachable_bindings,
         }
@@ -1392,7 +1386,7 @@ fn place_from_bindings_impl<'db>(
                 // that none of them loop-back. In that case short-circuit, so that we don't
                 // produce an `Unknown` fallback type, and so that `Place::Undefined` is still a
                 // possibility below.
-                if !loop_header.has_defined_bindings {
+                if loop_header.reachable_bindings.is_empty() {
                     return None;
                 }
             } else {
