Skip to content

Conversation

phananh1010
Copy link
Owner

@phananh1010 phananh1010 commented Oct 6, 2025

Single commit with tree=16aecaca5333692fff65a3d71818c0d6de287e37^{tree}, parent=eb3bc2f2929209461802884ea01f77d3b3fcf96a. Exact snapshot of upstream PR head. No conflict resolution attempted.

Summary by CodeRabbit

  • New Features
    • Restored the “Linear retriever with the multi-field query format” example in the retrievers documentation.
  • Documentation
    • Clarified limitation: Until 9.2, multi-field queries only work with single-index searches.
    • Updated examples list to reflect available multi-field query usage.
  • Tests
    • Updated RRF retriever tests to cover non-default ranking parameters.

BASE=eb3bc2f2929209461802884ea01f77d3b3fcf96a
HEAD=16aecaca5333692fff65a3d71818c0d6de287e37
Branch=main
@coderabbitai
Copy link

coderabbitai bot commented Oct 6, 2025

Walkthrough

Documentation note updated to specify timeframe for multi-index support and an example reference restored. A unit test for RRF retriever now uses a non-default rank_constant (double the default) and updates the related comment; no functional code changes.

Changes

Cohort / File(s) Summary
Documentation updates
docs/reference/elasticsearch/rest-apis/retrievers.md
Updated limitation wording to “Until 9.2…” for multi-field queries and restored the “Linear retriever with the multi-field query format” example reference.
Test adjustments
x-pack/plugin/rank-rrf/src/test/java/.../RRFRetrieverBuilderTests.java
In testMultiIndexMultiFieldsParamsRewrite, set rank_constant to DEFAULT_RANK_CONSTANT * 2 and adjusted the comment to note non-default window size and rank_constant.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my ears at docs aligned,
A note of time now clearly signed.
Tests hop forth with constants new,
Double steps in RRF’s view.
With tidy trails and carrots bright,
This merge will land just right. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The title “Mirror upstream elastic#134980 for AI review (snapshot of HEAD tree)” focuses on the meta action of syncing with an upstream PR rather than summarizing the actual content changes, making it too generic for a reviewer to understand what functionality or documentation was modified at a glance. Please update the title to clearly and concisely describe the key changes introduced, for example “Update retrievers docs for multi-field queries in 9.2 and adjust RRF retriever tests for custom rank_constant.”
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch eval/pr-134980-diff

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eb3bc2f and e47cc8c.

📒 Files selected for processing (2)
  • docs/reference/elasticsearch/rest-apis/retrievers.md (1 hunks)
  • x-pack/plugin/rank-rrf/src/test/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderTests.java (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
x-pack/plugin/rank-rrf/src/test/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderTests.java (1)
x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilder.java (1)
  • RRFRetrieverBuilder (54-354)
🔇 Additional comments (3)
x-pack/plugin/rank-rrf/src/test/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderTests.java (1)

448-454: LGTM! Test enhancement aligns comment with test parameters.

The changes correctly update both the comment and the rank_constant value to test with a doubled (rather than halved) constant. This maintains test coverage for non-default values while ensuring the comment accurately describes the test scenario. The assertion logic validates that the rank constant is preserved through rewriting, so the specific non-default value (120 vs 30) doesn't affect test validity.

docs/reference/elasticsearch/rest-apis/retrievers.md (2)

236-236: LGTM! Timeframe clarification improves user guidance.

The updated limitation note now specifies that multi-index support will be available starting in version 9.2, providing users with a clear timeline for when this feature will be available.


242-242: LGTM! Example reference restored for completeness.

The addition of the linear retriever example reference complements the RRF example on line 241, providing users with complete documentation of the multi-field query format examples.


Comment @coderabbitai help to get the list of available commands and usage tips.

@phananh1010 phananh1010 closed this Oct 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant