Repository navigation
Conversation
Fixes deepset-ai#13378 Co-authored-by: toobatehreem <51713056+toobatehreem@users.noreply.github.com>
Contributor
|
@Cha-Imaa is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
This branch has not been deployed
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.
Fixes #13378
Related Issues
Componentprotocol #9329 (introduced typing support and adjusted the Component protocol)Cause
The
Componentprotocol declaredrunasdef run(self, *args: Any, **kwargs: Any) -> Mapping[str, Any]. The typing spec says a(*args: Any, **kwargs: Any)signature should be treated like...("accepts any arguments"), and mypy, Pyright and Pyrefly's default preset follow that rule.Pyrefly's
--preset strictenablesstrict-callable-subtyping, which turns the rule off. Under that setting, a component whoseruntakes named parameters (for example,run(self, documents: list[Document])) no longer matches the protocol, so everyPipeline.add_component(...)call fails withbad-argument-type.Fix
In
haystack/core/component/component.py, type checkers and the runtime now get different definitions ofrun(anif TYPE_CHECKING:split):Callable[..., Mapping[str, Any]]. A literal...means "any signature" in every checker, including Pyrefly withstrict-callable-subtyping. It is a property rather than a plainrun: Callable[...]annotation because a plain annotation declares a settable attribute, and mypy rejects classes whoserunis a (read-only) method.runstays a regular method, unchanged. A property would makeruna non-method protocol member, andissubclass(SomeClass, Component)would then raiseTypeErroron the@runtime_checkableprotocol.isinstance()/issubclass()checks, the@componentdecorator, and pipelines therefore behave exactly as before.Other changes
test/core/component/test_component.py: Added anissubclass(MockComponent, Component)assertion. It guards the runtime half of the fix: if someone later "simplifies"runinto a plain property, this test fails.test/core/pipeline/test_pipeline_base.py:test_add_invalid_component_namepassed the classFakeComponentwhere an instance is expected. The old(*args, **kwargs)signature let mypy accept a class object; the new protocol correctly flags it, so the test now passesFakeComponent(). The test still checks the same name validation.How did you test it?
Issue reproduction
Reproduced the issue with the example from #13378 (a pipeline with
DocumentWriterplus a custom@component):pyrefly check --preset strict(Pyrefly 1.4.0) reportsbad-argument-typeon everyadd_componentcall.Type checking
hatch run test:types: No issues found in 566 source files.Unit tests
hatch run test:unit(full unit suite, Python 3.11, Windows): 6966 passed, 1 failed, 49 skipped.The single failure is
test/dataclasses/test_byte_stream.py::test_from_file_path_guess_mime_type[spam.csv-...]. It is Windows-specific and unrelated: Windows' registry maps.csvtoapplication/vnd.ms-excelinstead oftext/csv.Lint and formatting
hatch run fmt-check), codespell and the release-note backtick check pass on the changed files.Notes for the reviewer
if TYPE_CHECKING:block incomponent.py. The comment above it explains why both branches are needed.add_component("name", MyComponent)), which previously type-checked silently.Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:and added!in case the PR includes breaking changes.