Skip to content

fix: validate runtime SentenceWindowRetriever window size - #12183

Merged
julian-risch merged 2 commits into
deepset-ai:mainfrom
vinkiYu:fix/sentence-window-runtime-window-validation
Jul 29, 2026
Merged

fix: validate runtime SentenceWindowRetriever window size#12183
julian-risch merged 2 commits into
deepset-ai:mainfrom
vinkiYu:fix/sentence-window-runtime-window-validation

Conversation

@vinkiYu

@vinkiYu vinkiYu commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Summary

This is the standalone SentenceWindowRetriever part requested by the maintainer in #12167.

  • Preserve an explicitly provided runtime window_size=0 instead of falling back to the constructor value.
  • Reject runtime zero and negative window sizes consistently in un() and un_async().
  • Reuse one _validate_window_size() helper from the constructor and both execution paths.
  • Update the synchronous and asynchronous regression tests, remove dead constructor-test calls, and document the async validation behavior.

Tests

  • python -m py_compile passed for the implementation and both test files.
  • git diff --check passed.
  • Hatch is not installed in the available environment, so hatch run release-note and the full Hatch test suite could not be run. The release note was added in the repository's standard Reno YAML format.

Relationship to #12167

The FilterRetriever change from #12167 is intentionally not included because {} is not a valid Haystack filter dict and changing its meaning would be a breaking behavior change.

@vinkiYu
vinkiYu requested a review from a team as a code owner July 29, 2026 02:41
@vinkiYu
vinkiYu requested review from anakin87 and removed request for a team July 29, 2026 02:41
@vercel

vercel Bot commented Jul 29, 2026

Copy link
Copy Markdown

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

A member of the Team first needs to authorize it.

@anakin87
anakin87 requested review from julian-risch and removed request for anakin87 July 29, 2026 08:15
@anakin87

Copy link
Copy Markdown
Member

@julian-risch could you please take a look?

@github-actions github-actions Bot added the type:documentation Improvements on the docs label Jul 29, 2026
@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  haystack/components/retrievers
  sentence_window_retriever.py
Project Total  

This report was generated by python-coverage-comment-action

Address review feedback on the runtime window_size validation:

- Reword the release note from the user's perspective and add an
  `upgrade` section, since passing `window_size=0` at runtime now raises
  instead of silently using the constructor value.
- Regenerate the release note filename with `hatch run release-note` so
  it carries reno's hash suffix like every other note.
- Revert the unrelated rename of `test_init_with_invalid_window_size_parameter`.
- Add sync and async tests asserting that omitting `window_size` uses the
  constructor value, covering the other half of the changed line.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 29, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
haystack-docs Ignored Ignored Preview Jul 29, 2026 10:21am

Request Review

@julian-risch julian-risch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you @vinkiYu ! I applied some smaller and the PR looks good to me now. Congratulations on your first PR merged into Haystack!

@julian-risch
julian-risch enabled auto-merge (squash) July 29, 2026 10:23
@julian-risch julian-risch changed the title fix: validate runtime SentenceWindowRetriever window size fix!: validate runtime SentenceWindowRetriever window size Jul 29, 2026
@julian-risch
julian-risch disabled auto-merge July 29, 2026 10:23
@julian-risch
julian-risch enabled auto-merge (squash) July 29, 2026 10:23
@julian-risch
julian-risch merged commit 984fc43 into deepset-ai:main Jul 29, 2026
24 checks passed
@vinkiYu vinkiYu changed the title fix!: validate runtime SentenceWindowRetriever window size fix: validate runtime SentenceWindowRetriever window size Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic:tests type:documentation Improvements on the docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants