fix: SuperComponent input filter must use identity, not equality#11624
Open
Aarkin7 wants to merge 1 commit into
Open
fix: SuperComponent input filter must use identity, not equality#11624Aarkin7 wants to merge 1 commit into
Aarkin7 wants to merge 1 commit into
Conversation
`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 is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
@bogdankostic for some additional context we did run into a problem like this before in our Pipeline class here #8873 |
sjrl
reviewed
Jun 18, 2026
| """ | ||
|
|
||
| def test_run_accepts_numpy_ndarray_input(self): | ||
| np = pytest.importorskip("numpy") |
Contributor
There was a problem hiding this comment.
We have numpy in our test deps so we shouldn't need this and can just import numpy as normal.
Contributor
|
@bogdankostic I can take over the review for this |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed Changes:
SuperComponent.runandSuperComponent.run_asyncfiltered out the internal_delegate_defaultsentinel withvalue != _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 raisesValueError: 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?
TestSuperComponentDelegateDefaultFilteringcovering the sync (run) and async (run_async) paths with a realnumpy.ndarrayinput.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
run(line 122) andrun_async(line 150). I kept a one-line# is not, not !=comment above each so the next reader doesn't "simplify" it back._delegate_defaultis a class object with no__eq__override, so for any value that doesn't overload__ne__,!=andis notproduce 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__returnsTruefor everything (previously silently dropped, now correctly kept, strict improvement).releasenotes/notes/.Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:and added!in case the PR includes breaking changes.