Repository navigation
fix(server): apply default_instruction to qwen2_flash queries - #621
Merged
Merged
Conversation
Qwen2FlashAdapter.encode never read the profile's `default_instruction`
runtime option. A query without an explicit instruction was formatted as
"Instruct: \nQuery:{text}" for Qwen3-Embedding-0.6B/8B, stella_en_1.5B_v5
and R3-embedding-0.6b on the CUDA flash path, while the SentenceTransformer
CPU fallback and the SGLang adapter applied the instruction.
Fall back to `default_instruction` for queries when the request gives no
instruction, matching XLMRobertaFlashAdapter and the SentenceTransformer
fallback: an explicit instruction (including "") wins, and documents get
no instruction.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthrough
ChangesQuery instruction handling
Suggested reviewers: Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to No actionable issue is identified that would prevent merging after normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
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.
Problem
Qwen2FlashAdapter.encodenever reads the profile'sdefault_instructionruntime option. It only uses the request'sinstruction(params.instruction, oroptions.instructionon the HTTP path). A query sent withis_query=trueand no explicit instruction is therefore formatted with an empty instruction:instead of
User impact
Every query to a model served by the
qwen2_flashadapter on CUDA without an explicit instruction is embedded with an empty instruction. That is the default way most clients call these models. Instruction-tuned embedders expect the instruction on the query side, so these query embeddings are degraded. Documents are not affected. The shipped configs that use this adapter, all with aquery_templateand adefault_instruction, are:Qwen/Qwen3-Embedding-8BQwen/Qwen3-Embedding-0.6BNovaSearch/stella_en_1.5B_v5tencent/R3-embedding-0.6bThe behaviour also differed across adapters for the same models. The SGLang embedding adapter (e.g.
Qwen/Qwen3-Embedding-4B) appliesdefault_instruction, and so doesqwen2_flash's own CPU fallback,SentenceTransformerDenseAdapter. The same model could therefore embed queries differently on CPU and on CUDA.Fix
When
is_queryis true and the request gives no instruction (instruction is None), useoptions["default_instruction"]. The options have already been merged from the profile'sadapter_options.runtimeand the request, so a request-leveldefault_instructionstill overrides the profile. The precedence matchesXLMRobertaFlashAdapterandSentenceTransformerDenseAdapter:""and the sibling adapters keep it;Note that the SGLang and PyTorch embedding adapters use
instruction or default_instruction, so for them""falls back to the default. This PR mirrors the flash and SentenceTransformer semantics instead, because SentenceTransformer is this adapter's CPU fallback and the two should format text the same way.Tests
The new file
packages/sie_server/tests/adapters/test_qwen2_flash.pyruns on CPU and loads no models. It stubs the flash transformer stack, captures the exact text handed to the tokenizer, and builds options from the shipped model YAMLs throughmerge_runtime_options, the same merge the HTTP and queue paths use.qwen2_flashmodel config applies its owndefault_instruction(parametrized over the YAMLs).""instruction is kept.default_instructionoverrides the profile.input_token_countsmatch the formatted text, including the instruction tokens.Against
mainbefore the fix, 6 of these tests fail with'Instruct: \nQuery:…'. All pass with the fix.Validation
mise run lint: passedmise run typecheck: passedmise run test -- packages/sie_server/tests/adapters/: 4542 passed, 57 skippedmise run test: 12627 passed, 564 skipped, 5 failed. The 5 failures are macOS-localPermissionErrors from renaming read-only directories inconfig/test_serving_artifacts.pyandcore/test_model_loader_cache.py. They fail the same way with this change reverted, so they are unrelated to it.Follow-up (not in this PR)
RoPEFlashAdapterhas the same gap. It is used byNovaSearch/stella_en_400M_v5, which also configures adefault_instruction. This PR keeps toqwen2_flash, and the same change can follow separately.Compatibility
No public API, wire or config changes. This is a runtime behaviour fix for queries to the models above. Query embeddings for these models change after the fix and now match the published recipe. Document embeddings are unchanged, so existing indexes do not need to be re-embedded.
Summary by CodeRabbit