Skip to content

fix(embedding): honour query_param as a query instruction for Volcengine dense embedders - #6088

Open
OnePerson2020 wants to merge 1 commit into
volcengine:mainfrom
OnePerson2020:feat/volcengine-query-instruction
Open

OnePerson2020 wants to merge 1 commit into
volcengine:mainfrom
OnePerson2020:feat/volcengine-query-instruction

Conversation

@OnePerson2020

Copy link
Copy Markdown

Description

embedding.dense.query_param had no effect for provider: volcengine: the factory never passed it to VolcengineDenseEmbedder, and the config lowercased it for every provider. Volcengine's multimodal/vision embedding models take the query side as an instruction text prefixed to the query, where case matters. The dense embedder now prefixes query_param to text queries (is_query=True); documents and image queries are embedded unchanged, so turning it on needs no reindex. Lowercasing is kept for the other providers, whose values are parameter names like query.

On our memory store (Volcengine vision embedding, 90 vague queries over a 300-document pool) the instruction raised MRR from 0.831 to 0.906.

Human Involvement

  • A human participated in the implementation or review loop
  • This PR was generated entirely by AI agents without human participation in the loop

Related Issue

Fixes #6087

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

  • VolcengineDenseEmbedder: query_param argument; _query_input() prefixes it to str queries in embed / embed_async.
  • EmbeddingConfig: pass query_param in the ("volcengine", "dense") factory entry; sync_provider_backend no longer lowercases query_param/document_param for Volcengine; field description mentions the Volcengine meaning.
  • Tests: test_dense_embedder_prefixes_text_queries_only, test_embedding_config_passes_volcengine_query_param_unchanged.

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows
echo '{}' > /tmp/min.conf
OPENVIKING_CONFIG_FILE=/tmp/min.conf uv run --extra test pytest tests/unit/test_*embed*.py tests/unit/embedder tests/unit/test_jina_embedder.py tests/unit/test_gemini_embedder.py tests/unit/test_voyage_embedder.py tests/unit/test_ollama_embedding_factory.py -q --no-cov
uv run --extra dev ruff check openviking_cli/utils/config/embedding_config.py tests/unit/test_volcengine_embedder_input.py
git diff --check
  • Without the code change both new tests fail; with it they pass.
  • Embedding test files above: the same pre-existing failures before and after (test_dense_embedder_passes_local_provider_duration_to_metrics, test_openai_factory_max_tokens_also_forwarded also fail on main in my environment); no new failures.
  • Ruff: the two touched files above are clean; volcengine_embedders.py already has an import-order (I001) and formatting difference on main, which I left untouched to keep the diff minimal. git diff --check clean.
  • Running in our deployment since 2026-10-12.

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Additional Notes

No API or data-format change; no new dependency.

🤖 Generated with Claude Code

@ZaynJarvis ZaynJarvis added the kernel Core server, runtime, storage, retrieval, SDK, and CLI. label Oct 15, 2026
OnePerson2020 pushed a commit to OnePerson2020/OpenViking that referenced this pull request Oct 15, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kernel Core server, runtime, storage, retrieval, SDK, and CLI.

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

[Bug]: embedding.dense.query_param is ignored (and lowercased) for Volcengine dense embedders

2 participants