test: enable mypy typing checks for test/components/retrievers - #12481
test: enable mypy typing checks for test/components/retrievers#12481ShousenZHANG wants to merge 2 commits into
Conversation
|
@ShousenZHANG is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
- Purpose: extend repository-wide
mypycoverage totest/components/retrievers/(incremental step of #10396) by fixing typing issues in the retriever test modules without changing test intent/assertions.
Changes:
- Add
test/components/retrievers/to thehatch run test:typesmypy target list. - Make retriever test doubles and parametrized tests mypy-friendly via explicit return types/annotations and narrowly-scoped
# type: ignore[...]where needed. - Tighten score-ordering assertions with
score is not Nonechecks before comparing sort order.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
pyproject.toml |
Adds test/components/retrievers/ to the mypy types script target set. |
test/components/retrievers/test_text_embedding_retriever.py |
Refines score ordering assertion to satisfy mypy (and adds score is not None guard). |
test/components/retrievers/test_text_embedding_retriever_async.py |
Async equivalent typing + score ordering assertion adjustments. |
test/components/retrievers/test_multi_retriever.py |
Adds return types for local components/test doubles; updates ordering asserts to satisfy mypy. |
test/components/retrievers/test_multi_query_text_retriever_async.py |
Adds narrow ignore for protocol/type mismatch in async test double wiring. |
test/components/retrievers/test_multi_query_embedding_retriever_async.py |
Adds score None guard + ordering typing fix; narrow ignore for protocol/type mismatch. |
test/components/retrievers/test_in_memory_embedding_retriever.py |
Adds narrow ignores for method monkeypatching and dynamic store class typing. |
test/components/retrievers/test_in_memory_bm25_retriever.py |
Adds narrow ignores for method monkeypatching and annotates pipeline tests for mypy. |
test/components/retrievers/test_filter_retriever.py |
Adds narrow ignore for monkeypatch; simplifies a pipeline call to avoid mypy friction. |
test/components/retrievers/test_filter_retriever_async.py |
Removes an annotation that confused mypy while keeping runtime behavior the same. |
test/components/retrievers/test_auto_merging_retriever.py |
Renames rebound variables to keep stable types across assertions. |
test/components/retrievers/test_auto_merging_retriever_async.py |
Async equivalent variable rebinding fix for mypy. |
Suppressed comments (3)
test/components/retrievers/test_multi_retriever.py:237
scoresis inferred aslist[float | None], which forces# type: ignore[type-var]on the ordering assertion. You can avoid the ignore while still failing the test if any score isNoneby building alist[float]with an explicit loop and per-document assertion.
scores = [doc.score for doc in result["documents"]]
assert all(score is not None for score in scores)
assert scores == sorted(scores, reverse=True) # type: ignore[type-var]
test/components/retrievers/test_multi_retriever.py:452
scoresis inferred aslist[float | None], which forces# type: ignore[type-var]on the ordering assertion. You can avoid the ignore while still failing the test if any score isNoneby building alist[float]with an explicit loop and per-document assertion.
assert all(doc.score is not None for doc in result["documents"])
scores = [doc.score for doc in result["documents"]]
assert scores == sorted(scores, reverse=True) # type: ignore[type-var]
test/components/retrievers/test_multi_retriever.py:519
scoresis inferred aslist[float | None], which forces# type: ignore[type-var]on the ordering assertion. You can avoid the ignore while still failing the test if any score isNoneby building alist[float]with an explicit loop and per-document assertion.
scores = [doc.score for doc in result["documents"]]
assert all(score is not None for score in scores)
assert scores == sorted(scores, reverse=True) # type: ignore[type-var]
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| docs = result["documents"] | ||
| assert all(doc.score is not None for doc in docs) | ||
| scores = [doc.score for doc in docs] | ||
| assert scores == sorted(scores, reverse=True) # type: ignore[type-var] |
| docs = result["documents"] | ||
| assert all(doc.score is not None for doc in docs) | ||
| scores = [doc.score for doc in docs] | ||
| assert scores == sorted(scores, reverse=True) # type: ignore[type-var] |
| docs = result["documents"] | ||
| assert all(doc.score is not None for doc in docs) | ||
| scores = [doc.score for doc in docs] | ||
| assert scores == sorted(scores, reverse=True) # type: ignore[type-var] |
| assert all(doc.score is not None for doc in result["documents"]) | ||
| scores = [doc.score for doc in result["documents"]] | ||
| assert scores == sorted(scores, reverse=True) | ||
| assert scores == sorted(scores, reverse=True) # type: ignore[type-var] |
Coverage report
This PR does not seem to contain any modification to coverable code. |
|
Good call — done in b17cb41, and it removed all seven Each site now collects the scores in an explicit loop: docs = result["documents"]
scores: list[float] = []
for doc in docs:
assert doc.score is not None
scores.append(doc.score)
assert scores == sorted(scores, reverse=True)
That takes the suppressions in this PR from 20 down to 13, and the remaining ones are all Re-verified: |
Adds the directory to the `types` target and clears the 78 errors that surfaced, without changing what any test asserts: - In the auto-merging retriever tests, `docs` was bound first to the list of input documents, then to the splitter's dict result, then to the retriever's. Give each value its own name (`docs`, `split_docs`, `merged`) so mypy can keep one type per variable. This removes 40 of the errors on its own. - Annotate the mock retrievers' `run` / `run_async` and the parametrised pipeline tests, which were missing parameter and return types. - Assert that every document has a score before ordering them, then keep a narrow `# type: ignore[type-var]` on the `sorted` call: `Document.score` is `float | None` and mypy does not narrow through the list comprehension. - Add narrow ignores where a test double is passed where the component asks for the real protocol type, and where a method is monkeypatched.
Follows the review suggestion: collecting the scores in an explicit loop with a per-document assertion narrows the list to `list[float]`, so the ordering assertions no longer need `# type: ignore[type-var]`. The runtime check is also sharper, since a failure now names the document whose score is missing.
b17cb41 to
ba03755
Compare
|
Rebased — this was conflicting against Re-verified on the rebased branch: Worth flagging for whoever picks up the next increment: with |
Related Issues
Proposed Changes:
test/components/retrievers/is not in the repository mypy target, so its 16 test modules are not checked byhatch run test:types. This is the next disjoint increment for #10396, claimed in this comment. It does not overlapfetchers/(#12435), the only increment still in flight.The baseline was 78 errors across 11 modules. Nothing here changes what a test asserts:
call-overload/assignmentintest_auto_merging_retriever.pyand its async twin.docswas bound three times in the same test — first to the inputlist[Document], then to the dict returned byHierarchicalDocumentSplitter.run, then to the retriever's result — so mypy had no single type for it. Fixed by naming the three valuesdocs,split_docsandmerged. No suppressions were needed for this group.no-untyped-defon the mock retrievers'run/run_asyncand on parametrised pipeline tests that took unannotated fixtures.type-varonsorted(scores, ...), whereDocument.scoreisfloat | None. I addedassert all(doc.score is not None for doc in docs)before the ordering assertion and kept a narrow# type: ignore[type-var]on thesortedcall, since mypy doesn't narrow through the list comprehension. I deliberately avoided filtering theNones out ofscores, because that would let a document with no score slip past the ordering check instead of failing it.arg-type/ 4method-assignwhere a test double is passed where the component asks for the real protocol type, or a method is monkeypatched onto a store. Narrow ignores.Per AGENTS.md I kept
type: ignoreto the cases where it is necessary — the largest group (the auto-merging tests) is fixed structurally instead.mypy --warn-unused-ignoresreports no unused suppressions.How did you test it?
Red/green against the repository configuration:
mypy test/components/retrievers/reported 78 errors in 11 files.Success: no issues found in 16 source files.hatch run test:types->Success: no issues found in 479 source files(461 before).hatch run test:unit test/components/retrievers/-> 194 passed, 30 deselected.hatch run fmt-check-> clean across the repo (6785 files).Notes for the reviewer
The renaming in the auto-merging tests is the part worth a look. Three assertions there were checking
docs["documents"]afterdocshad been rebound to the retriever result, so they now readmerged["documents"]; the objects being asserted on are the same ones as before, and both tests still pass. Theassert all(doc.score is not None ...)lines are new checks rather than relaxations.No release note: this is test-only, which AGENTS.md excludes from the release-note requirement.
This PR was fully generated with an AI assistant. I have reviewed the changes and run the relevant tests.
Checklist