Repository navigation
server : auto-insert media marker in embedding / multimodal prompts - #25093
TheOneWhoWill wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes multimodal /embedding requests in the server by ensuring the mtmd media marker is present in the text prompt before tokenization, aligning server behavior with the multimodal CLI and preventing marker/bitmap count mismatches.
Changes:
- Query the active marker from the mtmd context (
mtmd_get_marker()). - Auto-prepend media markers to the prompt before calling
mtmd_tokenize()when markers are missing.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
e50014f to
46cb6ea
Compare
The /embedding (and /embeddings, /v1/embeddings) endpoints failed with "number of media markers in text (0) does not match number of bitmaps (1)" when passing multimodal data via the "content" object format. The server initializes the mtmd context with a randomized media marker (via get_media_marker()), but process_mtmd_prompt() passed the raw prompt string to mtmd_tokenize() without ensuring it contained the required markers. The CLI (mtmd-cli.cpp) already handles this by auto-prepending markers, but the server did not. Fix: query the actual marker from the mtmd context via mtmd_get_marker() and auto-insert one per file if the prompt lacks them. server: auto-insert missing media markers in process_mtmd_prompt Fixes the /embedding endpoint when multimodal data is provided without corresponding media markers in the prompt string. Counts existing markers and prepends only the missing number so the count matches files.size(). Assisted-by: GitHub Copilot Potential fix for pull request finding This just makes the wording more accurate Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> server: auto-insert missing media markers in process_mtmd_prompt Fixes the /embedding endpoint when multimodal data is provided without corresponding media markers in the prompt string. Counts existing markers and prepends only the missing number so the count matches files.size(). Assisted-by: GitHub Copilot Update tools/server/server-common.cpp Forgot to remove merge conflict headers Co-authored-by: AGawas <94751172+aln730@users.noreply.github.com>
|
Hi @angt, would you mind reviewing this bug fix? All tests related to embedding and vision pass, 24 lines |
ngxson
left a comment
There was a problem hiding this comment.
if the problem is related to /embeddings endpoint, the fix should be isolated to that endpoint only
a patch to process_mtmd_prompt is not a valid solution, the function is shared among different code paths
|
That's a valid point, I'm going to move the code to the handler |
|
hmm ok so on second look, seems like /embeddings use the same schema as raw /completions (not chat completions), that is a bit messy. so, this error is indeed by design, user need to manually add the media marker according to what the model expects prepending it automatically to the beginning of the prompt is quite risky as it will degrade the performance if users don't know about that (and model may expect placement differently) I think it will be less messy if it simply reuse the same logic in chat completion everywhere, will have a look later this week closing it now because it works but not the optimum solution (explained above) |
|
Alrighty then, if inserting the markers is something more model specific and not standardized we wouldn't want to risk making an implementation choice like this. If you ever need the code just ping me |
The /embedding (and /embeddings, /v1/embeddings) endpoints failed with "number of media markers in text (0) does not match number of bitmaps (1)" when passing multimodal data via the "content" object format.
The server initializes the mtmd context with a randomized media marker (via get_media_marker()), but process_mtmd_prompt() passed the raw prompt string to mtmd_tokenize() without ensuring it contained the required markers. The CLI (mtmd-cli.cpp) already handles this by auto-prepending markers, but the server did not.
Fix: query the actual marker from the mtmd context via mtmd_get_marker() and auto-insert one per file if the prompt lacks them.
Overview
Fixes #25088
Essentially calls to the /embedding endpoint were failing because the process_mtmd_prompt function in tools/server/server-common.cpp passes the raw text from a user's prompt without including the placeholder marker from mtmd_default_marker() and one is required for each attatched image. I added a simple check for existence and inserted 1 per image.
Requirements