Add SemanticHighlighterQueryEnricherProcessor

[nomoa]

Description

A new search request processor to enrich the semantic highlighter model id.
Works the same way as the neural_query_enricher allowing to define a global or a per field model id.
The processor inspects the top level highlight config but also any highlight definitions added to nested query inner hits.

Limitations: it does not inspect aggregations (which could possibly be useful for top hits aggregation) and this is left as a possible followup. Main reason for not doing it now is that the batch inference does not support aggregations yet.

Related Issues

Resolves #1916

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.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 584cbea)

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 Map.of key/value ordering In the updated getRequestProcessors, the arguments to Map.of(...) no longer alternate correctly as key/value pairs. AgenticQueryTranslatorProcessor.TYPE is followed by new AgenticQueryTranslatorProcessor.Factory(...) and then SemanticHighlighterQueryEnricherProcessor.TYPE and its factory — this is only correct if the previous pair was fixed. Verify the arguments strictly alternate as (TYPE, Factory instance) pairs; from the diff hunk it looks like the Agentic factory line was moved/kept while a stray comma structure could produce a compile error or a wrong mapping (e.g., factory instance used as a key). Double-check the final method compiles and each TYPE maps to its own Factory. return Map.of( NeuralQueryEnricherProcessor.TYPE, new NeuralQueryEnricherProcessor.Factory(), NeuralSparseTwoPhaseProcessor.TYPE, new NeuralSparseTwoPhaseProcessor.Factory(), AgenticQueryTranslatorProcessor.TYPE, new AgenticQueryTranslatorProcessor.Factory(clientAccessor, xContentRegistry, settingsAccessor), SemanticHighlighterQueryEnricherProcessor.TYPE, new SemanticHighlighterQueryEnricherProcessor.Factory() ); Immutability contract The factory wraps fieldMap with Collections.unmodifiableMap(fieldMap) and the processor stores it in fieldDefaultIdMap. However, enrichHighlight calls Optional.ofNullable(this.fieldDefaultIdMap).orElseGet(Collections::emptyMap).getOrDefault(...), which is fine, but the returned unmodifiable map still wraps the mutable underlying fieldMap reference passed by pipeline config. If the pipeline framework retains and mutates the config map, the processor could observe changes. Consider defensively copying (Map.copyOf(fieldMap)) instead of only wrapping, to guarantee immutability. return new SemanticHighlighterQueryEnricherProcessor( tag, description, ignoreFailure, modelId, fieldMap != null ? Collections.unmodifiableMap(fieldMap) : null );

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 584cbea

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Ensure nested visitor covers all wrappers The visitor only inspects NestedQueryBuilder directly, but nested queries can also appear inside HasChildQueryBuilder, HasParentQueryBuilder, or ConstantScoreQueryBuilder wrappers whose getChildVisitor may not traverse them. Verify the visitor traversal reaches all nested clauses (e.g., via InnerHitContextBuilder.extractInnerHits) so inner_hits highlighters aren't silently skipped. src/main/java/org/opensearch/neuralsearch/processor/SemanticHighlighterQueryEnricherProcessor.java [58-62] +@Override +public SearchRequest processRequest(SearchRequest searchRequest) { + EventStatsManager.increment(EventStatName.SEMANTIC_HIGHLIGHTING_QUERY_ENRICHER_EXECUTIONS); + Optional<SearchSourceBuilder> source = Optional.ofNullable(searchRequest.source()); + source.map(SearchSourceBuilder::highlighter).ifPresent(this::enrichHighlight); + source.map(SearchSourceBuilder::query).ifPresent(qb -> qb.visit(new NestedQueryHighlightVisitor())); - Suggestion importance[1-10]: 4 __ Why: Raises a potentially valid concern about the visitor traversal missing nested clauses in certain query wrappers, but only asks for verification and provides no concrete code change (improved_code is identical to existing_code).	Low
	Clarify enrichment when global options lack model_id When global options carry a non-model_id entry (e.g. random_option) and the global type is semantic, the field will inherit those options at merge time. Enriching the field with a different resolved model_id here can conflict with the intent when the user later adds a global model_id. Consider only enriching per-field options if the resolved fieldModelId differs from the (absent) global one, and ensure per-field enrichment is consistent when global options exist without a model_id. src/main/java/org/opensearch/neuralsearch/processor/SemanticHighlighterQueryEnricherProcessor.java [85-99] for (HighlightBuilder.Field field : Optional.ofNullable(hlBuilder.fields()).orElseGet(Collections::emptyList)) { - // Enrich if either: - // - the global type is semantic and the field specific type is unset - // - the field specific type is set to semantic if ((globalIsSemantic && field.highlighterType() == null) || SemanticHighlightingConstants.HIGHLIGHTER_TYPE.equals(field.highlighterType())) { String fieldModelId = (String) Optional.ofNullable(this.fieldDefaultIdMap) .orElseGet(Collections::emptyMap) .getOrDefault(field.name(), modelId); Suggestion importance[1-10]: 2 __ Why: The suggestion is vague, does not identify a concrete bug, and the improved_code is essentially identical to the existing_code with only a comment removed. Low impact.	Low

---

Previous suggestions

Suggestions up to commit f1569c1

Category	Suggestion	Impact
General	Recurse into nested subqueries for inner_hits The visitor only traverses NestedQueryBuilder inner_hits but ignores inner_hits declared on HasChildQueryBuilder/HasParentQueryBuilder (join queries also support inner_hits and highlight). If these are meant to be supported, extend the visitor; otherwise document the limitation. Also, deeply nested queries (nested-within-nested) are handled via getChildVisitor, but ensure the child visitor recursion actually visits sub-queries of NestedQueryBuilder (OpenSearch's default visit implementation may not descend into nested.query()). src/main/java/org/opensearch/neuralsearch/processor/SemanticHighlighterQueryEnricherProcessor.java [114-128] private class NestedQueryHighlightVisitor implements QueryBuilderVisitor { @Override public void accept(QueryBuilder qb) { if (qb instanceof NestedQueryBuilder nested) { Optional.ofNullable(nested.innerHit()) .map(InnerHitBuilder::getHighlightBuilder) .ifPresent(SemanticHighlighterQueryEnricherProcessor.this::enrichHighlight); + // Also recurse into the inner query to catch nested-within-nested inner_hits + if (nested.query() != null) { + nested.query().visit(this); + } } } @Override public QueryBuilderVisitor getChildVisitor(BooleanClause.Occur occur) { return this; } } Suggestion importance[1-10]: 5 __ Why: Highlights a potential gap in handling nested-within-nested inner_hits and join queries. However, QueryBuilderVisitor framework may already traverse subqueries via getChildVisitor, so the recursion addition could be redundant; the concern is legitimate but not verified.	Low
	Reconsider global-semantic fallback without default When only per-field defaults are configured (modelId is null) but the global highlighter type is semantic, globalIsSemantic is set to true. This causes fields without an explicit type to be treated as semantic and enriched from fieldDefaultIdMap, which may not be intended when there is no global default. Consider whether this fallback behavior is correct, or gate globalIsSemantic on the presence of a resolvable default. src/main/java/org/opensearch/neuralsearch/processor/SemanticHighlighterQueryEnricherProcessor.java [78-84] +boolean globalIsSemantic = false; +if (SemanticHighlightingConstants.HIGHLIGHTER_TYPE.equals(hlBuilder.highlighterType())) { + globalIsSemantic = true; + if (modelId != null) { + hlBuilder.options(enrichWithModelId(globalOptions, modelId)); + } +} - Suggestion importance[1-10]: 3 __ Why: The suggestion raises a valid design question but provides an improved_code identical to the existing_code, offering no concrete change. The behavior described may actually be intentional (fall back to per-field map for semantic global type).	Low

Suggestions up to commit 6cc8350

Category	Suggestion	Impact
Possible issue	Ensure root nested query is visited QueryBuilderVisitor.accept is only invoked for children of a compound query; the top-level query itself may not be visited depending on OpenSearch's visitor contract. If the root query is a NestedQueryBuilder (not wrapped in a bool), its innerHit highlight may be skipped. Explicitly handle the root query before calling visit to guarantee coverage. src/main/java/org/opensearch/neuralsearch/processor/SemanticHighlighterQueryEnricherProcessor.java [58-62] @Override public SearchRequest processRequest(SearchRequest searchRequest) { EventStatsManager.increment(EventStatName.SEMANTIC_HIGHLIGHTING_QUERY_ENRICHER_EXECUTIONS); Optional<SearchSourceBuilder> source = Optional.ofNullable(searchRequest.source()); source.map(SearchSourceBuilder::highlighter).ifPresent(this::enrichHighlight); - source.map(SearchSourceBuilder::query).ifPresent(qb -> qb.visit(new NestedQueryHighlightVisitor())); + source.map(SearchSourceBuilder::query).ifPresent(qb -> { + NestedQueryHighlightVisitor visitor = new NestedQueryHighlightVisitor(); + visitor.accept(qb); + qb.visit(visitor); + }); Suggestion importance[1-10]: 6 __ Why: This is a valid concern — QueryBuilder.visit behavior for the root query varies, and the test testProcessRequest_whenNestedInnerHitsHighlight_thenEnriched may rely on implementation-specific behavior. Explicitly handling the root query adds robustness for a top-level NestedQueryBuilder.	Low
General	Avoid skipping field enrichment when global model_id exists Returning early when a global model_id is present skips enrichment of per-field highlighters even when they are semantic and lack their own model_id. Since field-level options are typically merged over global ones only when both exist, and the current logic explicitly documents that field-level options win, this early return may leave semantic fields without a resolved model. Consider only skipping global enrichment while still processing field-level enrichment. src/main/java/org/opensearch/neuralsearch/processor/SemanticHighlighterQueryEnricherProcessor.java [70-76] private void enrichHighlight(HighlightBuilder hlBuilder) { Map<String, Object> globalOptions = hlBuilder.options(); boolean userSuppliedGlobalModelId = globalOptions != null && globalOptions.containsKey(SemanticHighlightingConstants.MODEL_ID); - if (userSuppliedGlobalModelId) { - // if the user provided a global model_id there's no need to enrich anything. - return; - } Suggestion importance[1-10]: 3 __ Why: The suggestion questions the intended behavior, but the PR test testProcessRequest_whenGlobalSemanticAndGlobalModelIdSet_thenUntouched explicitly documents that this early return is intentional (user's global model_id covers every field). The improved_code is also incomplete/truncated.	Low

[nomoa]

unsure about this, if 3.8.0 has been released, this should probably be 3.9.0 but current dependencies still target 3.8

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.50%. Comparing base (5f9fba8) to head (584cbea).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1917      +/-   ##
============================================
+ Coverage     83.45%   83.50%   +0.05%     
- Complexity     3898     3916      +18     
============================================
  Files           291      292       +1     
  Lines         13844    13902      +58     
  Branches       2304     2319      +15     
============================================
+ Hits          11553    11609      +56     
- Misses         1455     1457       +2     
  Partials        836      836              

☔ 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.

[github-actions]

Persistent review updated to latest commit f1569c1

[github-actions]

Persistent review updated to latest commit 584cbea