Skip to content

Conversation

@mridula-s109
Copy link
Contributor

What does this PR do?

  • Removed the mute entry for the YAML REST test.
  • Fixed the failing test in [950_pinned_interaction.yml] .
  • Addresses this github issue.

How was this verified?

  • Ran the test locally using:
    ./gradlew :x-pack:plugin:rank-rrf:yamlRestTest \
      -Dtests.rest.testpaths=x-pack/plugin/rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/rrf/950_pinned_interaction.yml

@mridula-s109 mridula-s109 requested review from a team, Mikep86, Copilot and kderusso July 1, 2025 16:59
@mridula-s109 mridula-s109 self-assigned this Jul 1, 2025
@mridula-s109 mridula-s109 added auto-backport Automatically create backport pull requests when merged Team:SearchOrg Meta label for the Search Org (Enterprise Search) :SearchOrg/Relevance Label for the Search (solution/org) Relevance team v8.19.0 v9.1.0 Team:Search - Relevance The Search organization Search Relevance team labels Jul 1, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/search-eng (Team:SearchOrg)

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/search-relevance (Team:Search - Relevance)

This comment was marked as outdated.

@elasticsearchmachine
Copy link
Collaborator

Hi @mridula-s109, I've created a changelog YAML for you.

Copy link
Member

@kderusso kderusso left a comment

Choose a reason for hiding this comment

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

Agree with Mike on the changelog entry. Thanks for resolving the issue!

@mridula-s109 mridula-s109 added the >test Issues or PRs that are addressing/adding tests label Jul 2, 2025
@mridula-s109 mridula-s109 requested review from a team as code owners July 2, 2025 13:24
@github-actions
Copy link
Contributor

github-actions bot commented Jul 2, 2025

🔍 Preview links for changed docs:

🔔 The preview site may take up to 3 minutes to finish building. These links will become live once it completes.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 2, 2025

🔍 Preview links for changed docs

More links …

@mridula-s109 mridula-s109 force-pushed the SEARCH-1080-yaml-test-failure-rrf-with-pinned-retriever-as-a-sub-retriever branch from a4503f4 to 9125b38 Compare July 2, 2025 13:26
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR un-mutes the 950_pinned_interaction.yml test and fixes its assertions by removing the default retriever, using match_none for the nested standard retriever, and updating the expected hits.

  • Unmutes the failing YAML REST test for pinned sub-retriever
  • Removes the initial standard retriever and replaces its nested query with match_none
  • Updates assertions to expect only two pinned documents

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
x-pack/plugin/rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/rrf/950_pinned_interaction.yml Removed default retriever, replaced nested query with match_none, and updated hit assertions
muted-tests.yml Removed mute entry for rrf with pinned retriever as a sub-retriever test
Comments suppressed due to low confidence (2)

x-pack/plugin/rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/rrf/950_pinned_interaction.yml:86

  • [nitpick] Consider adding a brief comment explaining why match_none is used here to clarify that the default retriever should return no documents.
                          match_none: {}

x-pack/plugin/rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/rrf/950_pinned_interaction.yml:88

  • [nitpick] The test no longer verifies hit scores; consider re-adding an assertion for the expected scores or ordering to ensure scoring behavior is still correct.
  - match: { hits.total.value: 2 }

@mridula-s109 mridula-s109 requested a review from Mikep86 July 2, 2025 14:01
Copy link
Member

@kderusso kderusso left a comment

Choose a reason for hiding this comment

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

LGTM

@mridula-s109 mridula-s109 enabled auto-merge (squash) July 2, 2025 14:19
Copy link
Contributor

@Mikep86 Mikep86 left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for the iterations!

@mridula-s109 mridula-s109 merged commit 73d6063 into elastic:main Jul 2, 2025
32 checks passed
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.19 Commit could not be cherrypicked due to conflicts
9.1 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 130402

mridula-s109 added a commit to mridula-s109/elasticsearch that referenced this pull request Jul 3, 2025
…er` (elastic#130402)

* fixed failing pinned retriever test

* Update docs/changelog/130402.yaml

* Delete docs/changelog/130402.yaml

* Worked on the PR comments

* Fixed the Match_none

* space format

* space format
mridula-s109 added a commit to mridula-s109/elasticsearch that referenced this pull request Jul 15, 2025
…er` (elastic#130402)

* fixed failing pinned retriever test

* Update docs/changelog/130402.yaml

* Delete docs/changelog/130402.yaml

* Worked on the PR comments

* Fixed the Match_none

* space format

* space format
# Conflicts:
#	muted-tests.yml
mridula-s109 added a commit to mridula-s109/elasticsearch that referenced this pull request Jul 15, 2025
…er` (elastic#130402)

* fixed failing pinned retriever test

* Update docs/changelog/130402.yaml

* Delete docs/changelog/130402.yaml

* Worked on the PR comments

* Fixed the Match_none

* space format

* space format
# Conflicts:
#	muted-tests.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged backport pending >bug :SearchOrg/Relevance Label for the Search (solution/org) Relevance team Team:Search - Relevance The Search organization Search Relevance team Team:SearchOrg Meta label for the Search Org (Enterprise Search) >test Issues or PRs that are addressing/adding tests v8.19.0 v9.1.0 v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants