Skip to content

fix: SuperComponent input filter must use identity, not equality#11624

Open
Aarkin7 wants to merge 1 commit into
deepset-ai:mainfrom
Aarkin7:fix/super-component-delegate-default-identity
Open

fix: SuperComponent input filter must use identity, not equality#11624
Aarkin7 wants to merge 1 commit into
deepset-ai:mainfrom
Aarkin7:fix/super-component-delegate-default-identity

Conversation

@Aarkin7

@Aarkin7 Aarkin7 commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Proposed Changes:

SuperComponent.run and SuperComponent.run_async filtered out the internal _delegate_default sentinel with value != _delegate_default. That calls __ne__ on every user-provided input, which for numpy arrays, pandas DataFrames/Series, and torch tensors returns an element-wise array, and the dict-comprehension's boolean context then raises ValueError: The truth value of ... is ambiguous. So passing any array-shaped input (a routine ML pipeline thing) failed before the wrapped pipeline ever ran.

Switched both call sites to is not _delegate_default. That's an identity check on a sentinel class — what the filter actually meant, and it never touches __ne__ on user values.

How did you test it?

  • Added two unit tests in TestSuperComponentDelegateDefaultFiltering covering the sync (run) and async (run_async) paths with a real numpy.ndarray input.
  • hatch run test:unit test/core/super_component/, all 29 tests pass (27 pre-existing + 2 new).
  • hatch run test:unit test/core/, 1587 passed, no regressions.
  • hatch run test:types haystack/core/super_component/super_component.py: clean.
  • hatch run fmt-check: clean.

Notes for the reviewer

  • Two-line behavioural change, repeated in run (line 122) and run_async (line 150). I kept a one-line # is not, not != comment above each so the next reader doesn't "simplify" it back.
  • Semantic equivalence for ordinary values: _delegate_default is a class object with no __eq__ override, so for any value that doesn't overload __ne__, != and is not produce the same result. Behaviour changes only for (a) array-like values that overload __ne__ to return non-scalar (the fix), and (b) the pathological case of a user class whose __eq__ returns True for everything (previously silently dropped, now correctly kept, strict improvement).
  • Release note added under releasenotes/notes/.

Checklist

  • I have read the contributors guidelines and the code of conduct.
  • I have updated the related issue with new insights and changes.
  • I have added unit tests and updated the docstrings.
  • I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
  • I have documented my code.
  • I have added a release note file, following the contributors guidelines.
  • I have run pre-commit hooks and fixed any issue.

`value != _delegate_default` calls __ne__ on every input, which crashes on numpy / pandas / torch values (element-wise compare -> ambiguous truth). Switch to `is not` so the sentinel check stays an identity check.
@Aarkin7 Aarkin7 requested a review from a team as a code owner June 14, 2026 18:01
@Aarkin7 Aarkin7 requested review from bogdankostic and removed request for a team June 14, 2026 18:01
@vercel

vercel Bot commented Jun 14, 2026

Copy link
Copy Markdown

@Aarkin7 is attempting to deploy a commit to the deepset Team on Vercel.

A member of the Team first needs to authorize it.

@sjrl

sjrl commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

@bogdankostic for some additional context we did run into a problem like this before in our Pipeline class here #8873

"""

def test_run_accepts_numpy_ndarray_input(self):
np = pytest.importorskip("numpy")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We have numpy in our test deps so we shouldn't need this and can just import numpy as normal.

@sjrl

sjrl commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

@bogdankostic I can take over the review for this

@sjrl sjrl removed the request for review from bogdankostic June 18, 2026 07:02
@sjrl sjrl self-assigned this Jun 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants