feat(embedding): split query and document embedders#636
Closed
kfiramar wants to merge 2 commits intovolcengine:mainfrom
Closed
feat(embedding): split query and document embedders#636kfiramar wants to merge 2 commits intovolcengine:mainfrom
kfiramar wants to merge 2 commits intovolcengine:mainfrom
Conversation
Add first-class query/document dense embedding configuration and wire it through retrieval and indexing. - add dense_query and dense_document partial overrides on top of the base dense config - encapsulate provider-specific query/document defaults in embedder classes via contextual init hooks - use query embedders for retrieval paths and document embedders for schema/indexing paths - document the new config shape and add focused coverage for config merging and Jina defaults Follow-up to volcengine#608.
Add a service-level regression test for the new query/document embedding split. The test verifies OpenVikingService initializes both embedders and passes the query embedder into VikingFS during runtime initialization.
This was referenced Mar 15, 2026
Collaborator
|
referencing #608 review, two things needs confirmation
|
1 task
Collaborator
|
#608 is merged, this PR might be redundant. if no further update will be closed in future. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces first-class query-time and document-time dense embedding configuration in OpenViking.
The goal is to solve the architectural problem raised during review of #608: some embedding providers need different request semantics for retrieval queries versus indexed documents, but that behavior should not be modeled as provider-specific branching in the service layer or as provider-only fields in the shared base config.
This change makes the query/document split explicit in configuration and wiring while keeping provider-specific contextual behavior inside embedder implementations.
Why
Modern retrieval models often distinguish between short search queries and document or passage embeddings. That distinction matters for retrieval quality, but the current single dense-embedder configuration does not model it cleanly.
This PR provides a long-term structure for that split by:
What Changed
embedding.dense_queryandembedding.dense_documentas optional partial overrides on top of the existingembedding.denseconfigembedding.denseas the shared base so common settings do not need to be duplicatedEmbeddingConfig.get_query_embedder()EmbeddingConfig.get_document_embedder()EmbeddingConfig.get_embedder()as a backward-compatible alias for the query embedderJinaDenseEmbedderto map query embeddings toretrieval.queryand document embeddings toretrieval.passageDesign Notes
This PR intentionally addresses the architecture, not a single provider.
Important design constraints:
dense_queryanddense_documentare optionalThis keeps the feature lean while giving providers room to support contextual embedding behavior cleanly.
Backward Compatibility
Existing configurations continue to work unchanged.
If
embedding.dense_queryandembedding.dense_documentare omitted:embedding.denseget_embedder()continues to behave as beforeThe new config keys are additive overrides, not a breaking replacement.
Example Configuration
Providers that do not need different query/document behavior can continue using only
embedding.dense.Validation
Automated validation performed for this PR:
ruff checkon touched source and test filesVikingFSManual checks:
Scope
This PR is intentionally limited to query/document dense embedder architecture.
It does not:
Related