Skip to content

[LTR] Fix feature display order when using explain. #137671

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Nov 6, 2025
Merged

[LTR] Fix feature display order when using explain. #137671

merged 3 commits into from
Nov 6, 2025

Conversation

@afoucret
Copy link
Contributor

@afoucret afoucret commented Nov 6, 2025

When using the explain feature of LTR the feature should be displayed in the same order as declared in the model.

@afoucret afoucret added >bug auto-backport Automatically create backport pull requests when merged Team:Search - Relevance The Search organization Search Relevance team v9.0.9 v9.2.2 v8.19.8 v9.1.8 labels Nov 6, 2025
@elasticsearchmachine elasticsearchmachine added needs:triage Requires assignment of a team area label v9.3.0 and removed Team:Search - Relevance The Search organization Search Relevance team labels Nov 6, 2025
@afoucret afoucret added :Search Relevance/Search Catch all for Search Relevance Team:Search - Relevance The Search organization Search Relevance team labels Nov 6, 2025
@elasticsearchmachine elasticsearchmachine added the Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch label Nov 6, 2025
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Nov 6, 2025

Hi @afoucret, I've created a changelog YAML for you.

@elasticsearchmachine elasticsearchmachine removed needs:triage Requires assignment of a team area label Team:Search - Relevance The Search organization Search Relevance team labels Nov 6, 2025
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Nov 6, 2025

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

afoucret
afoucret commented Nov 6, 2025
View reviewed changes
...s/src/javaRestTest/java/org/elasticsearch/xpack/ml/integration/LearningToRankRescorerIT.java
}""");
var response = client().performRequest(request);
assertExplainExtractedFeatures(response, List.of("type_tv", "cost", "two"));
assertExplainExtractedFeatures(response, List.of("cost", "type_tv", "two"));
Copy link
Contributor Author

@afoucret afoucret Nov 6, 2025

Choose a reason for hiding this comment

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

ℹ️ The bug is here: we should check the order of feature in explain is the same as the one in the model definition.

@afoucret afoucret requested a review from a team as a code owner November 6, 2025 13:13
@afoucret afoucret force-pushed the ltr-explain-feature-order branch from c5eb6d8 to 32aa948 Compare November 6, 2025 13:14
markjhoy
markjhoy approved these changes Nov 6, 2025
View reviewed changes
Copy link
Contributor

@markjhoy markjhoy left a comment

Choose a reason for hiding this comment

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

Ah - good catch! LGTM

@afoucret afoucret enabled auto-merge (squash) November 6, 2025 13:55
mridula-s109
mridula-s109 approved these changes Nov 6, 2025
View reviewed changes
Copy link
Contributor

@mridula-s109 mridula-s109 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 catching and resolving the bug!

@afoucret afoucret merged commit cb11a71 into elastic:main Nov 6, 2025
34 checks passed
This was referenced Nov 6, 2025
Closed
Merged
Merged
@afoucret afoucret mentioned this pull request Nov 6, 2025
Merged
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Nov 6, 2025

💚 Backport successful

Status Branch Result
✅ 9.0
✅ 9.2
✅ 8.19
✅ 9.1

afoucret added a commit to afoucret/elasticsearch that referenced this pull request Nov 6, 2025
@afoucret
[LTR] Fix feature display order when using explain. (elastic#137671)
6bb034b
afoucret added a commit to afoucret/elasticsearch that referenced this pull request Nov 6, 2025
@afoucret
[LTR] Fix feature display order when using explain. (elastic#137671)
bc4e7dd
afoucret added a commit to afoucret/elasticsearch that referenced this pull request Nov 6, 2025
@afoucret
[LTR] Fix feature display order when using explain. (elastic#137671)
5541f21
afoucret added a commit to afoucret/elasticsearch that referenced this pull request Nov 6, 2025
@afoucret
[LTR] Fix feature display order when using explain. (elastic#137671)
61de513
szybia added a commit to szybia/elasticsearch that referenced this pull request Nov 6, 2025
@szybia
Merge remote-tracking branch 'upstream/main' into ilm-explain-invalid…
237d2f8 
…-json

* upstream/main:
  Mute org.elasticsearch.xpack.inference.action.filter.ShardBulkInferenceActionFilterBasicLicenseIT testLicenseInvalidForInference {p0=false} elastic#137691
  Mute org.elasticsearch.xpack.inference.action.filter.ShardBulkInferenceActionFilterBasicLicenseIT testLicenseInvalidForInference {p0=true} elastic#137690
  [LTR] Fix feature display order when using explain. (elastic#137671)
  Remove extra RemoteClusterService instances in unit test (elastic#137647)
  Fix `ComponentTemplatesFileSettingsIT.testSettingsApplied` (elastic#137669)
  Consolidates troubleshooting content into the "Returning semantic field embeddings in _source" section (elastic#137233)
  Update bundled JDK to 25.0.1 (elastic#137640)
  resolve indices for prefixed _all expressions (elastic#137330)
  ESQL: Add TopN support for exponential histograms (elastic#137313)
  allows field caps to be cross project (elastic#137530)
  ESQL: Add exponential histogram percentile function (elastic#137553)
  Wait for nodes to have downloaded databases in `GeoIpDownloaderIT` (elastic#137636)
  Tighten on when THROTTLE decision can be returned (elastic#136794)
  Mute org.elasticsearch.xpack.esql.qa.single_node.GenerativeMetricsIT test elastic#137655
  Add a test for two little known conditional processor paths (elastic#137645)
  Extract a common ORIGIN constant (elastic#137612)
  Remove early phase failure in batched (elastic#136889)
  Returning correct index mode from get data streams api (elastic#137646)
  [ML] Manage AD results indices (elastic#136065)
szybia added a commit to szybia/elasticsearch that referenced this pull request Nov 6, 2025
@szybia
Merge remote-tracking branch 'upstream/main' into ilm-explain-invalid…
2699fe0 
…-json

* upstream/main:
  Mute org.elasticsearch.xpack.inference.action.filter.ShardBulkInferenceActionFilterBasicLicenseIT testLicenseInvalidForInference {p0=false} elastic#137691
  Mute org.elasticsearch.xpack.inference.action.filter.ShardBulkInferenceActionFilterBasicLicenseIT testLicenseInvalidForInference {p0=true} elastic#137690
  [LTR] Fix feature display order when using explain. (elastic#137671)
  Remove extra RemoteClusterService instances in unit test (elastic#137647)
  Fix `ComponentTemplatesFileSettingsIT.testSettingsApplied` (elastic#137669)
  Consolidates troubleshooting content into the "Returning semantic field embeddings in _source" section (elastic#137233)
  Update bundled JDK to 25.0.1 (elastic#137640)
  resolve indices for prefixed _all expressions (elastic#137330)
  ESQL: Add TopN support for exponential histograms (elastic#137313)
  allows field caps to be cross project (elastic#137530)
  ESQL: Add exponential histogram percentile function (elastic#137553)
  Wait for nodes to have downloaded databases in `GeoIpDownloaderIT` (elastic#137636)
  Tighten on when THROTTLE decision can be returned (elastic#136794)
  Mute org.elasticsearch.xpack.esql.qa.single_node.GenerativeMetricsIT test elastic#137655
  Add a test for two little known conditional processor paths (elastic#137645)
  Extract a common ORIGIN constant (elastic#137612)
  Remove early phase failure in batched (elastic#136889)
  Returning correct index mode from get data streams api (elastic#137646)
  [ML] Manage AD results indices (elastic#136065)
elasticsearchmachine pushed a commit that referenced this pull request Nov 6, 2025
@afoucret
[LTR] Fix feature display order when using explain. (#137671) (#137685)
0a07db0
elasticsearchmachine pushed a commit that referenced this pull request Nov 6, 2025
@afoucret
[LTR] Fix feature display order when using explain. (#137671) (#137686)
76ae142
Kubik42 pushed a commit to Kubik42/elasticsearch that referenced this pull request Nov 10, 2025
@afoucret @Kubik42
[LTR] Fix feature display order when using explain. (elastic#137671)
b4f374c
elasticsearchmachine pushed a commit that referenced this pull request Nov 13, 2025
@afoucret
[LTR] Fix feature display order when using explain. (#137671) (#137684)
dfe61f9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markjhoy markjhoy markjhoy approved these changes

@mridula-s109 mridula-s109 mridula-s109 approved these changes

Assignees

No one assigned

Labels

auto-backport Automatically create backport pull requests when merged >bug :Search Relevance/Search Catch all for Search Relevance Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v8.19.8 v9.0.9 v9.1.8 v9.2.2 v9.3.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@afoucret @elasticsearchmachine @markjhoy @mridula-s109