Skip to content

fix(reranker): support omitting top_n#7199

Merged
mudler merged 3 commits intomudler:masterfrom
mkhludnev:omit-rerank
Nov 9, 2025
Merged

fix(reranker): support omitting top_n#7199
mudler merged 3 commits intomudler:masterfrom
mkhludnev:omit-rerank

Conversation

@mkhludnev
Copy link
Copy Markdown
Contributor

Description

This PR fixes the issue introduced in #7025

Notes for Reviewers

#7025 introduced handling result cropping by top_n. However, I believe the most users just omit top_n, just because don't bother to count len(documents). So, I'm afraid releasing #7025 causes a trouble for many users. @mudler, beg your pardon.
This PR let users to omit top_n or send top_n=0 meaning all docs.

One thing about tests, since tey bring up the backend every time, isn't it worth to loop three alt requests in the single test method?

Signed commits

  • [v] Yes, I signed my commits.

Signed-off-by: Mikhail Khludnev <mkhl@apache.org>
@netlify
Copy link
Copy Markdown

netlify bot commented Nov 8, 2025

Deploy Preview for localai ready!

Name Link
🔨 Latest commit eec1f59
🔍 Latest deploy log https://app.netlify.com/projects/localai/deploys/6910b05b2fe2450008889c8d
😎 Deploy Preview https://deploy-preview-7199--localai.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Signed-off-by: Mikhail Khludnev <mkhl@apache.org>
Signed-off-by: Mikhail Khludnev <mkhludnev@users.noreply.github.com>
request = backend_pb2.RerankRequest(
query="I love you",
documents=["I hate you", "I really like you"],
top_n=0 #
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

otherwise we can better handle top_n REST API param

notice https://api.jina.ai/redoc#tag/rerank/operation/rank_v1_rerank_post

defaults to the length of documents

@mudler
Copy link
Copy Markdown
Owner

mudler commented Nov 9, 2025

#7025 introduced handling result cropping by top_n. However, I believe the most users just omit top_n, just because don't bother to count len(documents). So, I'm afraid releasing #7025 causes a trouble for many users. @mudler, beg your pardon. This PR let users to omit top_n or send top_n=0 meaning all docs.

One thing about tests, since tey bring up the backend every time, isn't it worth to loop three alt requests in the single test method?

Yes, that would make def. sense!

Copy link
Copy Markdown
Owner

@mudler mudler left a comment

Choose a reason for hiding this comment

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

Thanks for following up!

@mudler mudler merged commit 01cd58a into mudler:master Nov 9, 2025
31 checks passed
@mkhludnev
Copy link
Copy Markdown
Contributor Author

asserts #7284

@mudler mudler added the bug Something isn't working label Nov 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants