Skip to content

test: enable mypy typing checks for test/components/samplers/ - #12433

Open
sainikhiljuluri wants to merge 1 commit into
deepset-ai:mainfrom
sainikhiljuluri:test/enable-mypy-samplers
Open

test: enable mypy typing checks for test/components/samplers/#12433
sainikhiljuluri wants to merge 1 commit into
deepset-ai:mainfrom
sainikhiljuluri:test/enable-mypy-samplers

Conversation

@sainikhiljuluri

Copy link
Copy Markdown

Related Issues

Relates to #10396

Proposed Changes

Adds test/components/samplers/ to the types mypy allowlist, continuing the effort in #10396 to type-check our test files. The directory contains only __init__.py and test_top_p.py, so this covers it completely.

Both baseline errors were the same one:

test/components/samplers/test_top_p.py:68: error: Value of type variable "SupportsRichComparisonT" of "sorted" cannot be "float | None"  [type-var]
test/components/samplers/test_top_p.py:87: error: Value of type variable "SupportsRichComparisonT" of "sorted" cannot be "float | None"  [type-var]
Found 2 errors in 1 file (checked 2 source files)

Document.score is float | None, which isn't orderable, so sorted([doc.score for doc in docs]) is rejected. I narrowed with if doc.score is not None — the idiom already used for this exact expression in test_multi_query_text_retriever.py:149,191, test_multi_query_embedding_retriever.py:279 and test_multi_query_text_retriever_async.py:70,178.

Why one site gets an extra assertion and the other doesn't

The filter on its own would have quietly weakened test_run. The unfiltered sorted() was incidentally enforcing "every document has a score" by raising TypeError, and filtering throws that away. Concretely, if a document lost its score, three scores would filter to two, and sorted_scores[:2] would still match the two filtered documents — every assertion in the test would pass. So test_run also asserts the narrowed list kept every document, which restores exactly the invariant the TypeError used to provide.

test_run_top_p_1 needs no such guard: the pre-existing assert len(docs_filtered) == len(docs) above it already ties both sides to the input length, so a dropped score makes the lists different lengths and fails the comparison.

I also considered # type: ignore[type-var]. It's shorter, but AGENTS.md asks to avoid type: ignore where possible and warn_unused_ignores = true makes ignores a maintenance liability. assert all(s is not None for s in scores) doesn't work — it doesn't narrow the comprehension's element type, and mypy still errors.

How did you test it?

hatch run test:types                      # Success: no issues found in 2 source files
hatch run test:unit test/components/samplers/   # 14 passed
  • Behaviour is unchanged: all 14 tests pass both with and without this change. I verified the revert actually applied before trusting the result.
  • Ran the full types scope (449 source files) to confirm nothing else regressed.
  • ruff format --check and ruff check are clean.

No release note: this is a tests-only change, matching the precedent in #12327, #12394 and #12333.

Checklist


Note

This contribution was AI-assisted (Claude Code). The analysis, the mutation testing behind the assertion argument above, and the verification commands were run and reviewed before submitting.

Adds test/components/samplers/ to the mypy allowlist, continuing the effort to
type-check our test files.

Both errors were `sorted()` over `[doc.score for doc in docs]`, which mypy
rejects because `Document.score` is `float | None` and so is not orderable.
Narrowed with `if doc.score is not None`, the idiom already used in the
retriever and joiner tests.

The first site also asserts the narrowed list keeps every document. Without that,
the filter would have quietly weakened the test: the unfiltered `sorted()` used
to catch a missing score via TypeError, and three documents filtered to two would
still match the `[:2]` slice the assertion compares against. The second site
needs no such guard because the pre-existing `len(docs_filtered) == len(docs)`
assertion above it already ties both sides to the input length.

Behaviour is unchanged - all 14 tests pass with and without this change.

Relates to deepset-ai#10396
@sainikhiljuluri
sainikhiljuluri requested a review from a team as a code owner August 22, 2026 07:01
@sainikhiljuluri
sainikhiljuluri requested review from davidsbatista and removed request for a team August 22, 2026 07:01
@vercel

vercel Bot commented Aug 22, 2026

Copy link
Copy Markdown

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

A member of the Team first needs to authorize it.

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.

1 participant