Skip to content

Add applyInRetrieval parameter for selective relevance cutoff - #1412

Merged
wanliAlex merged 52 commits into
releases/2.26from
li/add-relevance-cutoff-control
Apr 8, 2026
Merged

Add applyInRetrieval parameter for selective relevance cutoff#1412
wanliAlex merged 52 commits into
releases/2.26from
li/add-relevance-cutoff-control

Conversation

@wanliAlex

@wanliAlex wanliAlex commented Apr 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Adds applyInRetrieval parameter (tensor | both) to relevanceCutoff for hybrid disjunction search — lets you apply relevance cutoff to only the tensor leg instead of both; defaults to both (preserves existing behaviour)
  • Adds overrideLimitPlusOffset and postProcessCandidates / overrideTotalHitsWithPostProcessCandidates controls for fine-grained candidate management
  • Fixes a pagination edge case in relevance cutoff

Details

  • applyInRetrieval defaults to both (explicit, matches previous implicit behaviour); lexical is blocked pending tensor targetHits scaling fix
  • applyInRetrieval is validated to only be accepted when hybridParameters.retrievalMethod is disjunction
  • The selective cutoff implementation clones the Vespa query so the main query is never mutated; only the target leg's sub-query is reduced
  • probeDepth is applied only to the non-target leg when applyInRetrieval=tensor is set (and only when sort_by is active)
  • overrideLimitPlusOffset=true sets tensor targetHits to exactly relevantCandidates (expanding beyond limit+offset when needed)
  • All new changes are in semi_structured_vespa_index only (structured_vespa_index is being deprecated)

Test plan

  • Unit tests for model validation (applyInRetrieval enum values, default=both, invalid values)
  • Unit tests for API validation (disjunction-only constraint)
  • Unit tests for Vespa query param passing (applyInRetrieval always present with default both)
  • Java unit tests for overrideLimitPlusOffset behaviour in updateQueryHitsOffsetsAndTargetHits
  • Java unit tests for postProcessCandidates field in MarqoMetadataFields JSON serialisation
  • Integration tests for applyInRetrieval=tensor with AND/OR search, sort_by, and facets
  • Integration tests asserting anchor-doc relevance ordering for selective cutoff
  • Integration test for applyInRetrieval=lexical (blocked pending tensor targetHits scaling fix)

🤖 Generated with Claude Code

wanliAlex and others added 25 commits April 7, 2026 11:09
…leg cutoff

When using hybrid disjunction search with relevance cutoff, this new parameter
allows targeting the cutoff to only the lexical or tensor retrieval leg instead
of applying it uniformly to both. Only accepted when retrievalMethod is disjunction.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ly, restore non-target

Instead of bypassing updateQueryHitsOffsetsAndTargetHits with
isRelevanceCutoffEnabled=false (which skips offset=0, rerank count, tensor
YQL targetHits, and facets adjustments), let it run normally and then restore
only the non-target sub-query's hits back to limit+offset.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tests cover: both/lexical/tensor modes, selective cutoff behavior,
pagination, sort_by integration, facets+trackTotalHits, and validation
that non-disjunction retrieval methods are rejected.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The non-target retrieval leg should have a stable pool size independent
of pagination. Using probeDepth (a fixed, query-independent value) instead
of limit+offset prevents the retrieval pool from growing with offset.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…OR probe

Crafted test with 8 documents where 3 match quoted AND terms ("ocean" "species").
With applyInRetrieval='tensor' + strict cutoff + lexicalOperand='or' for probe:
- Lexical leg is unrestricted → all 3 AND matches appear with _lexical_score
- Tensor leg is limited by cutoff
- Contrasted with 'both' mode where strict cutoff may lose AND matches

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
9 docs all contain "ocean" and "species". 1 short anchor doc ("ocean species")
scores highest in lexical, making the 0.9 cutoff threshold very strict. With
applyInRetrieval='tensor', lexical is unrestricted so all 9 AND matches
appear. With 'both', fewer results since both legs are capped.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…-query

Instead of modifying both sub-queries after creation (restore non-target,
keep target), now:
1. Let updateQueryHitsOffsetsAndTargetHits run fully
2. Restore main query hits to limit+offset (prevents downstream truncation)
3. Restore original tensor YQL on main query
4. Sub-queries inherit restored values automatically
5. Only reduce the target sub-query (hits, rerankCount, tensor YQL)

Also sets QUERY_RERANK_COUNT on the target sub-query so Vespa's
second-phase ranking matches the reduced hits.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Instead of modifying the main query then restoring it, clone the query
before updateQueryHitsOffsetsAndTargetHits. The clone receives all cutoff
modifications (hits, offset, tensor YQL, facets). The original stays
untouched with hits=limit+offset.

- Target sub-query: created from clone (cutoff-reduced)
- Non-target sub-query: created from original (unreduced)
- Final Result: uses original query (no truncation)

No save/restore of tensor YQL needed — each sub-query inherits the right
values from its source query.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds price field to test docs and a test that verifies all 9 AND-matched
docs are returned sorted by price ascending when applyInRetrieval='tensor'
is combined with sortBy. Also adds sort_by/facets/trackTotalHits support
to the _search helper.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…idates

When targeting a specific retrieval leg, relevantCandidates only reflects
that leg's cutoff and would incorrectly trim the combined sort pool.
Allowed when applyInRetrieval is 'both' or None (existing behavior).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two tests showing facets behavior with selective cutoff:
- affectFacets=False: facets count all 9 matching docs (5 deep, 4 surface)
- affectFacets=True: facets count only the 1 doc passing cutoff (conservative,
  fewer facet counts than actual results since cutoff only controls tensor leg)

Also adds category field to test docs for faceting.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Corrected docstring to describe the actual behavior (cutoff limits lexical,
tensor unrestricted) and fixed copied comment that incorrectly said
"should not have tensor score" instead of "should not have lexical score".

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The non-target leg should use probeDepth for a stable retrieval pool,
not limit+offset which grows with pagination. Uses max(probeDepth,
limit+offset) to ensure the final Result isn't truncated for large offsets.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
With sort_by: max(probeDepth, limit+offset) for a stable sort candidate pool.
Without sort_by: limit+offset is sufficient, consistent with normal RRF
behavior and avoids fetching unnecessary results from Vespa.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…olved

With applyInRetrieval='lexical', the tensor leg becomes unrestricted but
its YQL targetHits stays at limit+offset — scaling it up for a large
retrieval pool is not yet implemented. Only 'tensor' and 'both' are
allowed for now.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Always clone the query for cutoff/sortBy manipulation. The original query
is never modified — its hits=limit ensures the final Result is correctly
bounded. For selective cutoff (tensor target), the non-target lexical leg
is created from the original and manually set to offset=0, hits=probeDepth
for a stable retrieval pool.

Also adds pagination integration test for applyInRetrieval=tensor + sortBy
verifying consistent sort order and _sortCandidates across pages.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add post_process_size to MarqoFields model (query_result.py)
- Expose as _postProcessCandidates in hybrid search response
- Fix Java tests: update MarqoMetadataFields constructors from 3 to 4 args
  and add postProcessSize assertion in the complete serialization test

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…sWithPostProcessCandidates

- Rename field to postProcessCandidates across Python model and Java test
- Add overrideTotalHitsWithPostProcessCandidates field to RelevanceCutoffModel
- When enabled, totalHits is overridden with postProcessCandidates from Vespa

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix hybrid_search.py: post_process_size → post_process_candidates
- Add _postProcessCandidates assertions to tensor and both cutoff tests
- Add overrideTotalHitsWithPostProcessCandidates + totalHits assertion
  to pagination test
- Update HybridSearcher.java MarqoMetadataFields to 4 fields with
  postProcessCandidates tracked in both sort and non-sort paths
- Remove test_apply_in_lexical_is_not_supported (covered by unit tests)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Lightweight script to trigger Vespa bootstrap from within Marqo container.
Reads the jar from vespa/target/, copies it into the Vespa application
package, and deploys via the config API.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When true, uses max(relevantCandidates, limit+offset) instead of min,
expanding the retrieval pool to fetch all relevant documents even when
they exceed limit+offset. Default false preserves existing behavior.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
wanliAlex and others added 5 commits April 7, 2026 15:47
…set mode

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
'both' makes the default explicit and matches the previous implicit
behaviour. Simplify the override_sort_candidates validator to drop the
now-redundant is-not-None guard, and update all tests accordingly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
applyInRetrieval now always has a value (defaults to 'both'), so the
conditional is no longer needed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Since applyInRetrieval now defaults to Both instead of None, the old
is not None guard was always true — incorrectly blocking the Both default
when retrievalMethod is not disjunction. Changed to != ApplyInRetrieval.Both
so only explicit non-Both values require disjunction retrieval.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@wanliAlex
wanliAlex changed the base branch from mainline to releases/2.26 April 7, 2026 06:19
@wanliAlex
wanliAlex requested a review from papa99do April 7, 2026 06:28
if (isSelectiveCutoff) {
// Target from cutoffQuery (reduced), non-target from original (unreduced)
if (applyInRetrieval == ApplyInRetrieval.LEXICAL) {
// This is not happening as this is blocked by the Python API

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

maybe throw an exception when this branch is reached. So it matches the python impl and we don't need to test it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Added an exception

MARQO_SEARCH_METHOD_LEXICAL,
verbose);
queryLexical.setOffset(0);
queryLexical.setHits(relevanceCutoffProbeDepth);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

add some comment to explain why we increase the limit to relevanceCutoffProbeDepth, and the implications.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Added

if (STANDARD_SEARCH_TYPES.contains(rankingMethod)) {
Query combinedQuery =
createSubQuery(query, retrievalMethod, rankingMethod, verbose);
createSubQuery(cutoffSortByQuery, retrievalMethod, rankingMethod, verbose);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why do we need to apply this to non-rrf queries?

@wanliAlex wanliAlex Apr 7, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

RelevanceCutoff was supported for these queries before this change, so we need to use cutoffSortByQuery here. In the new logic:

  1. If there is a relevance cutoff, the behaviour is the same;
  2. If there is no relevance cutoff, the cutoffSortByQuery is identical to query, so results remain the same

Comment on lines 525 to +526
sortCandidates = hitsForPostProcessing.size();
postProcessCandidates = hitsForPostProcessing.size();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

so sortCandidates always equals to postProcessCandidates?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes

// overrideLimitPlusOffset=true: newHits=200, newTensorTargetHits=relevantCandidates=200
assertThat(result.getHits()).isEqualTo(200);
String updatedYql = result.properties().getString("marqo__yql.tensor");
assertThat(updatedYql).contains("targetHits: 200");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit. maybe very the whole marqo__yql.tensor to check hnsw.exploreAdditionalHits is also changed. same for other tests cases verifying marqo__yql.tensor

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done

default_ids = [hit["_id"] for hit in result_default["hits"]]
both_ids = [hit["_id"] for hit in result_both["hits"]]
self.assertEqual(default_ids, both_ids)
self.assertEqual(result_default["_relevantCandidates"], result_both["_relevantCandidates"])

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit. add assertion to the new _postProcessCandidates

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

removed this test class and combine it with the other test class


@pytest.mark.skip_for_multinode(
"Multi-nodes will return different lexical results so we can not assert on the results.")
class TestRelevanceCutoffApplyInRetrieval(MarqoTestCase):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why do we need a different test class for this single test case? can we merge it in the test class below?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done


# All 9 docs returned
self.assertEqual(9, len(returned_ids))
self.assertEqual(set(expected_order), set(returned_ids))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit. no need to test this since the next assertion assumes the set match

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

search_query_dict=search_query_dict
).body.decode('utf-8'))

def test_apply_in_tensor_preserves_all_lexical_and_matches(self):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

add a test case to verify probeDepth applied to lexical leg.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

added a test

)
self.assertEqual(sq.relevance_cutoff.apply_in_retrieval, 'tensor')

def test_apply_in_retrieval_both_default_accepted_with_any_retrieval_method(self):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this only verify disjunction method

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Revised some unittests

wanliAlex and others added 15 commits April 7, 2026 23:40
…g tests

The LEXICAL branch in selective cutoff was dead code (blocked at the Python
API). Replaced with a RuntimeException to make the invariant explicit.
Made ApplyInRetrieval package-private and added ApplyInRetrievalFromStringTest
to cover parsing of all enum values including the unsupported lexical value.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tests

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…assertions

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…utoff logic

Moved the disjunction sub-query building out of search() into a package-private
method so it can be unit tested. Added BuildDisjunctionSubQueriesTest covering:
- selective cutoff (tensor): lexical leg uses probeDepth from original query
- non-selective: both legs use cutoffQuery
- selective cutoff (lexical): throws RuntimeException

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…arch()

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
postProcessCandidates is a primitive int and is always set in production,
so passing null for it is not a real scenario. Removed the all-null test
and updated the partial-null test to reflect realistic field nullability.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove unused LexicalOperand import
- Merge tensor/lexical search rejection tests into one with subtests
- Fix test_apply_in_retrieval_both_default_accepted_with_any_retrieval_method
  to actually test all three retrieval methods (was only testing disjunction)
- Consolidate both_allowed and both_default_allowed override_sort_candidates
  tests into a single parameterised test

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…validator

- apply_in_retrieval field defaults to None on RelevanceCutoffModel
- SearchQuery validator sets it to Both when None, so downstream code always
  sees a resolved value; also blocks any explicit value when retrievalMethod
  is not disjunction
- Moved ApplyInRetrieval import to top of api_models.py
- Updated unit tests accordingly

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Restore all 13 pre-existing tests in test_relevance_cutoff_model.py unchanged
- Consolidate the 7 new tests added in this PR (lexicalOperand, applyInRetrieval)
  to use RelativeMaxScore with parameters instead of always GapDetection
- Simplify applyInRetrieval validator in SearchQuery (no functional change)
- Remove TestRelevanceCutoffApplyInRetrieval integ test class (superseded)
- Add test_apply_in_tensor_preserves_update_to_probe_depth_lexical_matches
- Remove redundant length assertion from sort test
- Remove device="cpu" from search calls in sort feature integ tests

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@papa99do papa99do left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@wanliAlex
wanliAlex merged commit 41ac1ec into releases/2.26 Apr 8, 2026
40 checks passed
@wanliAlex
wanliAlex deleted the li/add-relevance-cutoff-control branch April 8, 2026 01:53
This was referenced Apr 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants