Skip to content

Commit b4f2b30

Browse files
chuenchen309claudedavidsbatista
authored
fix: clear stale socket references on neighbors in remove_component (#11971)
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: David S. Batista <dsbatista@gmail.com>
1 parent 1f3dcad commit b4f2b30

3 files changed

Lines changed: 134 additions & 6 deletions

File tree

haystack/core/pipeline/base.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,15 @@ def remove_component(self, name: str) -> Component:
468468
", ".join(n for n in self.graph.nodes),
469469
) from exc
470470

471+
# Remove this component's name from its neighbors' sockets before the edges are gone,
472+
# otherwise the surviving components are left holding dangling references to it.
473+
for _, _, edge_data in self.graph.in_edges(name, data=True):
474+
sender_socket = edge_data["from_socket"]
475+
sender_socket.receivers = [r for r in sender_socket.receivers if r != name]
476+
for _, _, edge_data in self.graph.out_edges(name, data=True):
477+
receiver_socket = edge_data["to_socket"]
478+
receiver_socket.senders = [s for s in receiver_socket.senders if s != name]
479+
471480
# Delete component from the graph, deleting all its connections
472481
self.graph.remove_node(name)
473482

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
fixes:
3+
- |
4+
Fixed ``Pipeline.remove_component`` leaving dangling references to the removed
5+
component on the sockets of its neighboring components. Previously, removing a
6+
component reset only its own sockets, so a surviving neighbor kept the removed
7+
component's name in its input socket's ``senders`` (or output socket's
8+
``receivers``). This corrupted introspection and validation: ``Pipeline.inputs()``
9+
hid a now-unconnected mandatory input, and feeding that input directly could
10+
raise a spurious "already connected" error. The removed component's name is now
11+
stripped from its neighbors' sockets as well.

test/core/pipeline/test_pipeline_base.py

Lines changed: 114 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -254,28 +254,136 @@ def test_remove_component_removes_component_and_its_edges(self):
254254
component_2 = component_class("Type2")()
255255
component_3 = component_class("Type3")()
256256
component_4 = component_class("Type4")()
257+
isolated = component_class("Isolated")()
257258

258259
pipe.add_component("1", component_1)
259260
pipe.add_component("2", component_2)
260261
pipe.add_component("3", component_3)
261262
pipe.add_component("4", component_4)
263+
pipe.add_component("isolated", isolated)
262264

263265
pipe.connect("1", "2")
264266
pipe.connect("2", "3")
265267
pipe.connect("3", "4")
266268

267269
pipe.remove_component("2")
270+
# Removing a component with no connections at all is a safe no-op over its (empty) edge sets.
271+
removed_isolated = pipe.remove_component("isolated")
268272

269273
assert sorted(pipe.graph.nodes) == ["1", "3", "4"]
270274
assert sorted([(u, v) for (u, v) in pipe.graph.edges()]) == [("3", "4")]
275+
assert removed_isolated.__haystack_added_to_pipeline__ is None # type: ignore[attr-defined]
276+
277+
def test_remove_component_middle_of_chain_removal(self):
278+
"""Removing a component that is both a downstream and an upstream neighbor cleans up both sides."""
279+
pipe = PipelineBase()
280+
producer_class = component_class("Producer", input_types={}, output_types={"value": int})
281+
middle_class = component_class("Middle", input_types={"value": int}, output_types={"value": int})
282+
consumer_class = component_class("Consumer", input_types={"value": int})
283+
pipe.add_component("producer", producer_class())
284+
pipe.add_component("middle", middle_class())
285+
pipe.add_component("consumer", consumer_class())
286+
pipe.connect("producer.value", "middle.value")
287+
pipe.connect("middle.value", "consumer.value")
288+
289+
pipe.remove_component("middle")
290+
291+
producer_socket = pipe.get_component("producer").__haystack_output__._sockets_dict["value"] # type: ignore[attr-defined]
292+
consumer_socket = pipe.get_component("consumer").__haystack_input__._sockets_dict["value"] # type: ignore[attr-defined]
293+
# Both the upstream and the downstream neighbor must be cleaned of the removed component.
294+
assert producer_socket.receivers == []
295+
assert consumer_socket.senders == []
296+
# With the stale sender gone, the now-unconnected mandatory input is exposed again.
297+
assert pipe.inputs() == {"consumer": {"value": {"type": int, "is_mandatory": True}}}
298+
# Feeding it directly must not raise a spurious "already connected" error.
299+
pipe.validate_input({"consumer": {"value": 5}})
300+
301+
def test_remove_component_fan_out(self):
302+
"""Removing a sender that fans out to multiple receivers cleans up every receiver, not just one."""
303+
pipe = PipelineBase()
304+
producer_class = component_class("Producer", output_types={"value": int})
305+
consumer_a_class = component_class("ConsumerA", input_types={"value": int})
306+
consumer_b_class = component_class("ConsumerB", input_types={"value": int})
307+
pipe.add_component("producer", producer_class())
308+
pipe.add_component("consumer_a", consumer_a_class())
309+
pipe.add_component("consumer_b", consumer_b_class())
310+
pipe.connect("producer.value", "consumer_a.value")
311+
pipe.connect("producer.value", "consumer_b.value")
312+
313+
pipe.remove_component("producer")
314+
315+
consumer_a_socket = pipe.get_component("consumer_a").__haystack_input__._sockets_dict["value"] # type: ignore[attr-defined]
316+
consumer_b_socket = pipe.get_component("consumer_b").__haystack_input__._sockets_dict["value"] # type: ignore[attr-defined]
317+
# Every receiver of the removed sender's output must be cleaned, not just the first one.
318+
assert consumer_a_socket.senders == []
319+
assert consumer_b_socket.senders == []
320+
321+
def test_remove_component_fan_in(self):
322+
"""Removing a receiver fed by multiple senders cleans up every sender, not just one."""
323+
pipe = PipelineBase()
324+
producer_a_class = component_class("ProducerA", output_types={"value": int})
325+
producer_b_class = component_class("ProducerB", output_types={"value": int})
326+
consumer_class = component_class("Consumer", input_types={"value": list[int]})
327+
pipe.add_component("producer_a", producer_a_class())
328+
pipe.add_component("producer_b", producer_b_class())
329+
pipe.add_component("consumer", consumer_class())
330+
pipe.connect("producer_a.value", "consumer.value")
331+
pipe.connect("producer_b.value", "consumer.value")
332+
333+
pipe.remove_component("consumer")
334+
335+
producer_a_socket = pipe.get_component("producer_a").__haystack_output__._sockets_dict["value"] # type: ignore[attr-defined]
336+
producer_b_socket = pipe.get_component("producer_b").__haystack_output__._sockets_dict["value"] # type: ignore[attr-defined]
337+
# Every sender of the removed receiver must be cleaned, not just the first one.
338+
assert producer_a_socket.receivers == []
339+
assert producer_b_socket.receivers == []
340+
341+
def test_remove_component_variadic_multi_sender(self):
342+
"""Removing one sender of a variadic receiver only strips itself, leaving the other senders intact."""
343+
pipe = PipelineBase()
344+
producer_a_class = component_class("ProducerA", output_types={"value": int})
345+
producer_b_class = component_class("ProducerB", output_types={"value": int})
346+
consumer_class = component_class("Consumer", input_types={"value": list[int]})
347+
pipe.add_component("producer_a", producer_a_class())
348+
pipe.add_component("producer_b", producer_b_class())
349+
pipe.add_component("consumer", consumer_class())
350+
pipe.connect("producer_a.value", "consumer.value")
351+
pipe.connect("producer_b.value", "consumer.value")
352+
353+
consumer_socket = pipe.get_component("consumer").__haystack_input__._sockets_dict["value"] # type: ignore[attr-defined]
354+
assert consumer_socket.is_variadic
355+
assert sorted(consumer_socket.senders) == ["producer_a", "producer_b"]
356+
357+
pipe.remove_component("producer_a")
358+
359+
# Only the removed sender is stripped; the surviving sender must remain in the list.
360+
assert consumer_socket.senders == ["producer_b"]
361+
362+
def test_remove_component_multiple_connections_to_same_neighbor(self):
363+
"""Removing a component with several parallel connections to the same neighbor cleans up all of them."""
364+
pipe = PipelineBase()
365+
producer_class = component_class("Producer", output_types={"first": int, "second": int})
366+
consumer_class = component_class("Consumer", input_types={"first": int, "second": int})
367+
pipe.add_component("producer", producer_class())
368+
pipe.add_component("consumer", consumer_class())
369+
pipe.connect("producer.first", "consumer.first")
370+
pipe.connect("producer.second", "consumer.second")
371+
372+
pipe.remove_component("producer")
373+
374+
consumer_first_socket = pipe.get_component("consumer").__haystack_input__._sockets_dict["first"] # type: ignore[attr-defined]
375+
consumer_second_socket = pipe.get_component("consumer").__haystack_input__._sockets_dict["second"] # type: ignore[attr-defined]
376+
# Both parallel connections between the same pair of components must be cleaned up.
377+
assert consumer_first_socket.senders == []
378+
assert consumer_second_socket.senders == []
271379

272380
def test_remove_component_allows_you_to_reuse_the_component(self):
273381
pipe = PipelineBase()
274-
Some = component_class("Some", input_types={"in": int}, output_types={"out": int})
382+
some = component_class("Some", input_types={"in": int}, output_types={"out": int})
275383

276-
pipe.add_component("component_1", Some())
277-
pipe.add_component("component_2", Some())
278-
pipe.add_component("component_3", Some())
384+
pipe.add_component("component_1", some())
385+
pipe.add_component("component_2", some())
386+
pipe.add_component("component_3", some())
279387
pipe.connect("component_1", "component_2")
280388
pipe.connect("component_2", "component_3")
281389
component_2 = pipe.remove_component("component_2")
@@ -289,9 +397,9 @@ def test_remove_component_allows_you_to_reuse_the_component(self):
289397
}
290398

291399
pipe2 = PipelineBase()
292-
pipe2.add_component("component_4", Some())
400+
pipe2.add_component("component_4", some())
293401
pipe2.add_component("component_2", component_2)
294-
pipe2.add_component("component_5", Some())
402+
pipe2.add_component("component_5", some())
295403

296404
pipe2.connect("component_4", "component_2")
297405
pipe2.connect("component_2", "component_5")

0 commit comments

Comments
 (0)