nick-boss-tech commented on PR #5126: URL: https://github.com/apache/solr/pull/5126#issuecomment-6401869746
🤖 *AI text below* 🤖 *(reviewed by Nick Shanin)* @dsmiley, I now have a [small production fix and regression tests on top of your PR](https://github.com/nick-boss-tech/solr/compare/74b09fb443f060caac3f8ec9701909cec276cfb4...8a685825b6202f45eb981d3b8e7f05b01a5c1829). On Linux, the new assertion fails on your current code because a boost-only term appears in the snippet, and passes with the fix. My earlier explanation was too compressed, and I missed an important qualification: this example requires `hl.weightMatches=false`. It did not reproduce with the default `true`. You are right that `getHighlightQuery()` excludes eDisMax boost clauses. The distinction is that [`parseHighlightQuery` reads `hl.q`, rather than ordinary `q`](https://github.com/apache/solr/blob/74b09fb443f060caac3f8ec9701909cec276cfb4/solr/core/src/java/org/apache/solr/response/transform/HighlightAugmenterFactory.java#L125-L135). Without `hl.q` or transformer-local `q`, that helper returns null before reaching `getHighlightQuery()`. The transformer then uses `context.getQuery()`, which contains the scoring boosts. Using your existing document 2, the title is "Fox cameo" and the body is "A story about cats. Cats sleep a lot.". The important request parameters are: ```text q=fox defType=edismax qf=body_t title_t bq=body_t:cats fl=id,body_hl:[hl f=body_t] hl.weightMatches=false ``` The document matches fox through its title. The cats clause is a ranking boost. Before the fix, its inline `body_hl` is `A story about <em>cats</em>. <em>Cats</em> sleep a lot.`. Adding `hl.q=fox` removes that body snippet. The existing highlight component also leaves the body without a snippet. With the fix, the implicit path agrees with those two paths. The patch uses the search request's existing parser and its highlighting query for the implicit path. It preserves explicit `hl.q` and local `q` overrides, and keeps the context-query fallback when no matching search parser is available. It changes only `HighlightAugmenterFactory.java` and `TestHighlightAugmenterFactory.java`. At final commit `8a685825b6`, the transformer test class passes 11 of 11 tests and `TestUnifiedSolrHighlighter` passes 33 of 33. Tidy, compilation with Error Prone, and `:solr:core:check -x test` all pass. The tests cover both weightMatches settings, the component on and off, explicit/local highlight overrides, `q.alt`, and a locally selected main-query parser. By "body" I meant the PR description's Solution bullet saying it works "in real-time get". I was comparing that wording with your JIRA note that RTG is unsupported. That was a description clarification. The proposed code change is limited to query selection; RTG support and the alias issue in #5188 stay separate. Would this evidence and the two-file fix be sufficient to address the query-selection concern in this PR? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
