-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix/fvh boost issue #15604
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix/fvh boost issue #15604
Conversation
5d143b8 to
05ad5ac
Compare
…tting via ./gradlew tidy
7cb0e18 to
e9d31cd
Compare
|
Hi @dweiss @dsmiley @jainankitk , |
jainankitk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @yangfeng123456 for this contribution. While the change itself looks fine to me, I am wondering if branch_10_3 is the right place for this. 10.4 is already being discussed, so it should be part of main and branch_10_x
Thanks for the feedback! I’ll close this PR and resubmit the fix based on main as suggested. |
Thanks for the suggestion earlier. I’ve resubmitted the change based on main here: #15635 |
Summary
This PR fixes a small issue in FastVectorHighlighter where boost values were not fully preserved when overlapping phrases were merged (#15442).
The change is limited to the boost accumulation logic, and a unit test is added to cover the case.
Motivation
While investigating FastVectorHighlighter behavior with overlapping phrase queries, I noticed that part of the boost information could be lost during phrase merging.
This can lead to highlight scoring that does not fully reflect the original query boosts.
This mainly affects legacy Elasticsearch and Solr setups that still rely on FVH.
Implementation
Preserve boost values when merging overlapping WeightedPhraseInfo instances
Add a small null-safety guard around TokenGroup boost handling
Add a focused test reproducing the issue
Risks
The change is small and limited to FVH internals, and should be safe to backport to 10.3.