[Fusion] Report matched_queries for named sub-queries in fused mode

[martin-gaievski]

Description

In fused mode (hybrid with the fusion parameter), matched_queries silently disappears from the response
for a sub-query that carries _name. Today hybrid reports it in every configuration. This PR closes both
halves of that gap.

matched_queries is a fetch-phase field built from the names registered while the query is converted on the
shard (QueryShardContext#addNamedQuery → ParsedQuery#namedFilters()); MatchedQueriesPhase then builds its own Weight per registered name and evaluates it per hit. So a named sub-query has to be registered,
never executed, for its name to be reported.

Fused mode converted legs only via the non-scoring Tail, and the coordinator passed an empty leg list whenever
the Tail was not built (needsTail false). The result is an empty namedFilters(), so
MatchedQueriesPhase.getProcessor returns null and the field is absent — at HTTP 200, with no warning. The
boundary is a request-shape detail rather than anything the user said about names: track_total_hits one above
the fused window reports names, exactly at the window does not. Same for the other three Tail triggers
(aggregations, highlight, non-_score sort, collapse expansion) — turning any of them on brings the field back.

A second, independent loss existed in every configuration, Tail or not: a materializable kNN/neural leg is
replaced by an ids-address substitute, and the substitute was a fresh builder carrying no name.

Examples

1. Create index and ingest documents

PUT /products
{
  "settings": { "index": { "knn": true, "number_of_shards": 1, "number_of_replicas": 0 } },
  "mappings": { "properties": {
    "title": { "type": "text" },
    "brand": { "type": "keyword" },
    "vec":   { "type": "knn_vector", "dimension": 2,
               "method": { "name": "hnsw", "space_type": "l2", "engine": "lucene" } }
  }}
}

PUT /products/_doc/1?refresh=true
{ "title": "wireless noise cancelling headphones", "brand": "acme",   "vec": [1.0, 1.0] }

PUT /products/_doc/2?refresh=true
{ "title": "wireless mechanical keyboard",         "brand": "globex", "vec": [1.2, 1.0] }

2. Search query 1

POST /products/_search?size=2
{
  "track_total_hits": false,
  "query": {
    "hybrid": {
      "fusion": {
        "window_size": 10,
        "normalization": { "technique": "min_max" },
        "combination":   { "technique": "arithmetic_mean" }
      },
      "queries": [
        { "match": { "title": { "query": "wireless", "_name": "text_leg" } } },
        { "term":  { "brand": { "value": "acme",     "_name": "brand_leg" } } }
      ]
    }
  }
}

Before fix no matched_queries anywhere:

"hits": [
  { "_id": "1", "_score": 0.5005 },
  { "_id": "2", "_score": 0.5 }
]

After fix, and byte-for-byte what classic hybrid returns for the same legs:

"hits": [
  { "_id": "1", "_score": 0.5005, "matched_queries": ["text_leg", "brand_leg"] },
  { "_id": "2", "_score": 0.5,    "matched_queries": ["text_leg"] }
]

Flipping track_total_hits to true — or adding an aggregation, a highlight, a non-_score sort, or collapse
expansion — brings the field back on the old code, which is what makes this hard to notice: nothing in the
request mentions named queries, and the ranking is unaffected either way.

Related Issues

#1930

Check List

• New functionality includes testing.
• New functionality has been documented.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR created.

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.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit dfee492)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Unguarded wire-format change HybridFusionQueryBuilder adds a third readNamedWriteableList/writeNamedWriteableList for namedOnlyQueries with no version guard. The inline comment argues this is safe because the query is only built when every node supports fused mode and it has never shipped, but the previous list layout ships with the fused-mode support version already committed as MINIMAL_SUPPORTED_VERSION_FUSED_MODE_IN_HYBRID_QUERY. If any node in a mixed cluster has an older fused-mode capable version (i.e., the two-list format from the initial fused-mode release), a new writer sending three lists to an old reader will corrupt deserialization (old reader will consume the third list's bytes as unrelated stream data), and old writer sending two lists to a new reader will fail at EOF. This should be gated with in.getVersion().onOrAfter(V) / out.getVersion().onOrAfter(V) on the new field on both sides, using the first unreleased version that contains this change. Uncertain: whether any prior fused-mode-supporting version has actually shipped in a release — if not, the risk is confined to development/rolling upgrades between snapshots, but the guard is essentially free and future-proof. public HybridFusionQueryBuilder(StreamInput in) throws IOException { super(in); this.ids = in.readStringArray(); this.indices = in.readStringArray(); this.scores = in.readFloatArray(); this.tailQueries = in.readNamedWriteableList(QueryBuilder.class); this.innerHitsQueries = in.readNamedWriteableList(QueryBuilder.class); // No wire-version gate: this query is built only for a cluster whose every node supports fused mode (see // HybridQueryBuilder#requireClusterSupportsFusedMode), and it has never shipped in a released version. this.namedOnlyQueries = in.readNamedWriteableList(QueryBuilder.class); } @Override protected void doWriteTo(StreamOutput out) throws IOException { out.writeStringArray(ids); out.writeStringArray(indices); out.writeFloatArray(scores); out.writeNamedWriteableList(tailQueries); out.writeNamedWriteableList(innerHitsQueries); out.writeNamedWriteableList(namedOnlyQueries); }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to c72bf75
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Verify wire compatibility of new field Reading an extra NamedWriteableList unconditionally will break wire compatibility if a mixed-version node (that serialized with the older writer, without namedOnlyQueries) sends this builder. Since the constructor comment states this is safe because of a cluster version gate, ensure that gate is actually enforced before this builder can be received from any node; otherwise gate this read with a version check to prevent stream corruption. src/main/java/org/opensearch/neuralsearch/query/HybridFusionQueryBuilder.java [169] +// Verified safe by HybridQueryBuilder#requireClusterSupportsFusedMode gate. this.namedOnlyQueries = in.readNamedWriteableList(QueryBuilder.class); Suggestion importance[1-10]: 4 __ Why: The suggestion asks to verify an existing wire-compatibility gate that the PR already documents (HybridQueryBuilder#requireClusterSupportsFusedMode). It's a verification-only request with the improved_code merely adding a comment, providing marginal value.	Low
General	Reduce false positives in name detection The contains check on the rendered JSON can produce false positives if any field or value in the query body contains the substring "_name": (e.g., a term query matching that literal string). Consider a more robust rendered-form check, or fall back to always carrying legs when any leg is present (the current fail-open path), to avoid the risk of an incorrect but plausible-looking match. src/main/java/org/opensearch/neuralsearch/query/HybridFusionOrchestrator.java [63] -private static final String QUERY_NAME_KEY = String.format(Locale.ROOT, "\"%s\":", AbstractQueryBuilder.NAME_FIELD.getPreferredName()); +private static final String QUERY_NAME_KEY = String.format(Locale.ROOT, ",\"%s\":\"", AbstractQueryBuilder.NAME_FIELD.getPreferredName()); Suggestion importance[1-10]: 3 __ Why: The suggestion raises a valid concern about false positives, but the PR already explicitly documents that the check is deliberately over-inclusive and that false positives cost only one harmless registration. The proposed improved_code is only a marginal refinement and doesn't fully eliminate false positives.	Low

---

Previous suggestions

Suggestions up to commit c6bdb4e

Category	Suggestion	Impact
General	Tighten name detection to avoid false positives anyLegNamed currently returns true for any leg whose rendered form contains the substring "_name":, which can produce false positives when a leg is a match/term query against a field literally called _name (the rendered form includes "_name": as the field key). Anchor the detection to the shape actually emitted by printBoostAndQueryName (e.g., check for "_name":" under the top-level query object, or use a JSON parser) to avoid unnecessary payload inflation on unrelated queries. src/main/java/org/opensearch/neuralsearch/query/HybridFusionOrchestrator.java [119-124] boolean tailNeeded = needsTail(source, ranked.ids().length); -// A leg's _name only reaches matched_queries if its builder is converted on the shard, and the Tail is the only -// thing that converts legs. When the Tail is not built, carry the same leg forms for registration alone: the fetch -// phase re-evaluates every named query from its own weights, so nothing has to execute for one to be reported. boolean namesOnly = tailNeeded == false && anyLegNamed(legs); List<QueryBuilder> legsInTailForm = tailNeeded || namesOnly ? legQueriesForTail(legs, legHits) : List.of(); Suggestion importance[1-10]: 4 __ Why: The concern about false positives from a field literally named _name has some merit, but the PR author explicitly documents the detection as "deliberately over-inclusive" with acceptable cost (one extra registration). The improved_code is essentially identical to the existing_code, offering no concrete fix.	Low
	Harden name-only leg conversion side effects Calling toQuery on the raw builder skips the standard AbstractQueryBuilder.toQuery wrapping that registers the builder's own _name and applies boost handling. Prefer invoking AbstractQueryBuilder#toQuery (i.e., call namedOnlyQuery.toQuery(context) on the abstract-typed reference, which is already the case) but ensure the builder has been rewritten first — as noted in the class, an un-rewritten builder may refuse to compile. Guard against toQuery returning a Query the shard treats as expensive by wrapping in a try that logs/skips a leg whose conversion throws, so a single bad leg does not fail the whole fused query. src/main/java/org/opensearch/neuralsearch/query/HybridFusionQueryBuilder.java [247-251] private void registerNamedOnlyQueries(QueryShardContext context) throws IOException { for (QueryBuilder namedOnlyQuery : namedOnlyQueries) { + // Conversion is for the side effect of addNamedQuery only; the resulting Query is discarded. namedOnlyQuery.toQuery(context); } } Suggestion importance[1-10]: 2 __ Why: The suggestion is vague and the improved_code is essentially identical to the existing_code aside from an added comment. The rewrite concern is already addressed by doRewrite walking namedOnlyQueries, and silently swallowing exceptions could mask real errors.	Low

[github-actions]

Persistent review updated to latest commit c72bf75

[codecov]

Codecov Report

❌ Patch coverage is 87.75510% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.83%. Comparing base (329672c) to head (dfee492).

Files with missing lines	Patch %	Lines
...h/neuralsearch/query/HybridFusionQueryBuilder.java	83.33%	1 Missing and 3 partials ⚠️
...h/neuralsearch/query/HybridFusionOrchestrator.java	92.00%	2 Missing ⚠️

Additional details and impacted files

@@                        Coverage Diff                        @@
##             feature/fusion-hybrid-query    #1967      +/-   ##
=================================================================
- Coverage                          83.83%   83.83%   -0.01%     
- Complexity                          4126     4138      +12     
=================================================================
  Files                                306      306              
  Lines                              14572    14614      +42     
  Branches                            2437     2443       +6     
=================================================================
+ Hits                               12216    12251      +35     
- Misses                              1482     1485       +3     
- Partials                             874      878       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[bzhangam]

For neural sparse we need to trigger inference call again during the rewrite. Same thing can happen for the tail rewrite. Seems like there is no elegant way to avoid this unless we rewrite the leg in the coordinator and dispatch the rewritten leg query?

Or prioritize #1558 can help on duplicated inference call.

[martin-gaievski]

that's mostly pre-existing rather than new here. Tail is rewritten in this same method and is on by default, so a neural_sparse leg with a model_id already pays 2 inferences in fused mode vs classic's 1. Measured: classic 1, fused+Tail 2, Top-only+named 2. It's one extra coordinator call per model leg, never per shard.

This PR adds that cost in one shape only: Top-only and a named leg. knn/neural/neural_knn are unaffected, they arrive materialized to ids.

Pre-rewriting leg on the coordinator doesn't work cleanly: it registers an async action mid-drain, and a nested fused-hybrid leg can't share one rewritten form between the round-1 fan-out and Tail. #1558 is the right approqch, same coordinator node, one request, and it also fixes the pre-existing Tail case. I'll file the follow-up.

[vibrantvarun]

Read the fused-mode matched_queries fix end-to-end (Top-only registration + materialized-leg name inheritance). The design mirrors the existing innerHitsQueries register-don't-execute pattern cleanly, and the oracle-anchored ITs are strong. A few things worth considering inline — mostly the materialized-leg deviation being under-tested/undocumented, plus a couple of nits. Nothing that looks like a correctness defect in the released contract.

[vibrantvarun]

For a materialized kNN/neural leg, the substitute registered under the leg's _name here is a filter-only bool{ids, _index}. Two behavioral gaps vs classic hybrid seem worth pinning:

• Under include_named_queries_score, a filter-only bool contributes 0 to score, so the named-query score is reported as 0.0 rather than the ANN similarity classic reports.
• The substitute only addresses the ids the leg returned (≤ window_size), so a doc matched by this leg but outside its own window won't get the name; classic re-evaluates the weight and would.

Both are inherent to not re-running the vector query and are documented in the code comment, but neither is covered by an IT against the classic oracle (the scoring IT uses function_score, i.e. non-materializable, legs). Could we add an IT pinning the materialized-leg score (expected 0.0) and the recall bound, so the asymmetry is anchored to behavior rather than only a comment?

[martin-gaievski]

ack, will add tests

[vibrantvarun]

Confirmed on dfee4926 — the two new ITs (testFusedKnnTail_whenIncludeNamedQueriesScore_thenMaterializedLegReportsZeroAndRealLegReportsItsScore and testFusedKnnTail_whenAnnLegMatchedBeyondItsWindow_thenTheTruncatedDocLosesTheAnnName) pin both the score-0.0 and the recall bound against the real behavior. Thanks for adding them.

[vibrantvarun]

Two small things:

1. This reads "matching classic hybrid", but for materialized kNN/neural legs the include_named_queries_score value and recall differ (see the orchestrator comment) — a short caveat here would keep it from being read as full parity.
2. The sibling fusion entries (Implement base classes and enable fusion for hybrid query (limited to min_max and arithmetic mean) #1933/In-query fusion advanced query features #1943/[Fusion] Wire z_score and l2 into fused mode #1962) each end with a ([#1967](...))-style PR link; this line is missing it.

[martin-gaievski]

ack, will correct this entry

[vibrantvarun]

The updated entry reads well — dropping "matching classic hybrid" for the caveat about the returned-docs bound and the substitute score under include_named_queries_score, plus the #1967 link, covers both points. Thanks.

[vibrantvarun]

In the names-only path legsInTailForm carries all legs, and registerNamedOnlyQueries then calls toQuery() on each — including unnamed, non-materializable legs that a Top-only query would otherwise never compile on the shard. For a request with one named leg + one unnamed heavy leg (e.g. script_score), that's an extra shard-side compile per unnamed leg for no reporting benefit. Bounded by max_leg_searches so minor — but would filtering the name-only carry to legs that actually render a _name be worth it, or is avoiding a re-decision of named-ness at conversion time the deliberate tradeoff?

[martin-gaievski]

It's not a deliberate tradeoff. you and @bzhangam landed on the same line from opposite ends. You're counting the wasted shard-side toQuery compile per unnamed leg; his concern was that a carried model-resolving leg re-runs its rewrite and pays a second ML inference on the coordinator. One filter fixes both, so it's going in this PR.

anyLegNamed already computes the per-leg predicate, so this is roughly ten lines: filter legsInTailForm on queryName() != null || rendersQueryName(leg) in the names-only branch. It has to be that predicate rather than the shallow queryName() check, or a _name nested under nested/function_score/a knn filter gets dropped. Semantics-preserving: AbstractQueryBuilder.toQuery registers only when a name is set, and namedOnlyQueries never reaches buildSelfErasedQuery(). It also removes the mutually-exclusive dual ternary the bot flagged, since the two branches stop sharing one list.

[vibrantvarun]

The namedLegsForRegistration + per-leg carriesQueryName refactor is exactly it: unnamed legs are no longer carried or compiled on the shard, and as a bonus it removes the extra neural_sparse coordinator inference in the unnamed Top-only case @bzhangam raised. Nice touch pinning the later-named-leg hit alignment (testBuildFusedQuery_whenOnlyALaterLegIsNamed_thenTheSubstituteAddressesThatLegsOwnHits) — that's the exact off-by-one the refactor could have introduced.

[vibrantvarun]

namedOnlyQueries is (de)serialized unconditionally with no TransportVersion gate. This looks fine for the released contract (type is new-in-3.8.0, version-gated via requireClusterSupportsFusedMode, never shipped). The one residual case is a mixed cluster of two different 3.8.0 pre-release snapshots — both pass the min-version gate but disagree on wire format, so the old reader stops after innerHitsQueries and the trailing list corrupts the stream. Worth a one-line note in the PR description so the unconditional read is a recorded decision.

[martin-gaievski]

makes sense, will do the change

[vibrantvarun]

The min-supported-version round-trip test (testSerializationRoundTrip_whenStreamPinnedToMinimumSupportedVersion_thenAllThreeListsSurvive) is a good way to record the wire contract. The one case it doesn't cover is old-reader/new-writer across two different pre-release 3.8.0 snapshots — but that's unreleased and low-risk, so fine to leave (or a one-line note in the description if you want it on record). No blocker either way.

[vibrantvarun]

nit: builder.toString() works here (the json builder is byte-backed), but Strings.toString(builder) is the idiomatic way to render an XContentBuilder. Also — already acknowledged in the javadoc, just flagging — the "_name": substring check false-positives on a leg that queries a field literally named _name (e.g. {"term":{"_name":...}}); harmless (one no-op registration), since the shallow queryName() check covers a leg's own name.

[martin-gaievski]

there's no Strings.toString(XContentBuilder) overload in core OpenSearch, form that would compile, Strings.toString(XContentType.JSON, leg), but imo it's worse here: it catches IOException internally and returns an error-JSON string, which contains no "_name":. I incline for keeping existing code as is

[vibrantvarun]

Agreed — withdrawing the suggestion. I checked org.opensearch.core.common.Strings: there's no toString(XContentBuilder) overload, and the toString(MediaType, ToXContent) form catches IOException and returns error-JSON with no "_name":, which would turn the current fail-open into fail-closed and silently drop names. Keeping the existing code is the correct call. Thanks for folding the _name false-positive note into the carriesQueryName javadoc.

[github-actions]

Persistent review updated to latest commit dfee492

[vibrantvarun]

Thanks for the quick turnaround — all five points are addressed. The materialized-leg score/recall deviation is now pinned by ITs, the CHANGELOG caveat + link are in, the name-only carry is filtered to named legs (which also removes the extra neural_sparse coordinator inference in the unnamed Top-only case), and the serialization round-trip is version-pinned. Confirmed my Strings.toString nit was wrong — that form would flip the fail-open detection to fail-closed. LGTM.