-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Fix ProfileScorer.getChildren() to expose wrapped scorer in scorer tree #20607
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
base: main
Are you sure you want to change the base?
Fix ProfileScorer.getChildren() to expose wrapped scorer in scorer tree #20607
Conversation
📝 WalkthroughWalkthroughThis PR modifies ProfileScorer to expose the wrapped scorer as a child through the getChildren() method, which now returns a singleton list containing a ChildScorable with "PROFILED" relationship instead of delegating to the wrapped scorer's children. Includes corresponding test coverage and changelog documentation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: Martin Gaievski <[email protected]>
c5a7642 to
6fddaa4
Compare
|
❌ Gradle check result for 6fddaa4: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
Looks like rarely failing test, not related to this change, passes in my local environment |
|
❌ Gradle check result for 6fddaa4: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Description
ProfileScorer.getChildren()currently delegates directly to the wrapped scorer'sgetChildren(),which skips the
ProfileScorerlevel in the scorer tree. This makes the wrapped scorer invisible toany code traversing the scorer hierarchy via the standard Lucene
Scorable.getChildren()API.This change makes
ProfileScorerexpose the wrapped scorer as a child with the"PROFILED"relationship label, allowing plugins and custom collectors to discover wrapped scorers through
profiling wrappers using standard tree traversal — without requiring reflection.
Changes
ProfileScorer.java: ChangedgetChildren()fromreturn scorer.getChildren()toreturn Collections.singletonList(new ChildScorable(scorer, "PROFILED"))ProfileScorerTests.java: AddedtestGetChildren_exposesWrappedScorerAsChild()to verifythe new behavior
Related Issues
Resolves #20548
PR #20549 added
getWrappedScorer()— a public method onProfileScorerthat returns the wrapped scorer. This was intended to let neural-search unwrapProfileScorerto findHybridQueryScorer.When implementing the neural-search side, we discovered that
getWrappedScorer()alone doesn't solve the problem becauseProfileScoreris a package-privatefinal class. Even thoughgetWrappedScorer()is apublicmethod, Java's access control system blocksMethod.invoke()from external modules (plugins) on methods of non-public classes. The plugin code gets:This means the neural-search plugin had to use
method.setAccessible(true)+@SuppressForbiddento callgetWrappedScorer()via reflection — defeating the purpose of having a clean public API.Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.