Skip to content

fix: respect falsy run() overrides in SentenceTransformersSimilarityRanker - #3855

Merged
bogdankostic merged 2 commits into
deepset-ai:mainfrom
keosung:fix-st-ranker-falsy-overrides
Sep 23, 2026
Merged

bogdankostic merged 2 commits into
deepset-ai:mainfrom
keosung:fix-st-ranker-falsy-overrides

Conversation

@keosung

@keosung keosung commented Aug 26, 2026 •

Copy link
Copy Markdown
Contributor

Related Issues

Proposed Changes:

run() resolved its overrides with or, which falls back on any falsy value, not just None. So run(scale_score=False) never disabled scaling, run(score_threshold=0.0) was replaced by the init value, and run(top_k=0) silently used the init top_k instead of raising the documented ValueError. This validates top_k before the fallback and switches the three fallbacks to None checks, the same change #3745 and #3746 made for the fastembed rankers. Behavior now matches the run() docstring, which already says "If set, overrides the value set at initialization."

How did you test it?

Added 3 regression tests, one per parameter. They fail on main and pass with the fix. Also verified with a real cross-encoder model that run(scale_score=False) now returns raw logits instead of sigmoid-scaled scores, and that the unit and integration suites both pass (23 + 3 tests).

Notes for the reviewer

Behavior change: run(top_k=0) now raises instead of silently using the init value, same as the fastembed rankers fix.

Checklist

@github-actions github-actions Bot added integration:sentence-transformers type:documentation Improvements or additions to documentation labels Aug 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Heads-up for maintainers

This PR is from a fork and touches integrations whose integration tests require API keys.
Those tests are skipped in CI because fork PRs don't have access to repo secrets for security reasons.

Affected integrations:

  • sentence_transformers

Please run the integration tests locally (hatch run test:integration inside each folder) before approving.

@github-actions

Copy link
Copy Markdown
Contributor

Coverage report (sentence_transformers)

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  integrations/sentence_transformers/src/haystack_integrations/components/rankers/sentence_transformers
  sentence_transformers_similarity.py
Project Total  

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

@keosung
keosung marked this pull request as ready for review August 30, 2026 22:56
@keosung
keosung requested a review from a team as a code owner August 30, 2026 22:56
@keosung
keosung requested review from bogdankostic and removed request for a team August 30, 2026 22:56

@bogdankostic bogdankostic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, thanks for the PR @keosung!

@bogdankostic
bogdankostic merged commit 108ca46 into deepset-ai:main Sep 23, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration:sentence-transformers type:documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Falsy run() overrides are silently ignored in SentenceTransformersSimilarityRanker

2 participants