[timothywang21]

Overview

Currently, the RERANK endpoint does not support multi-modality. This PR adds that functionality. Targets Qwen 3-VL Rerank model.

NOTE: Rerank endpoint is not OpenAI API but rather JINA/TEI format.

Builds on #28876, and #29556

Code Changes

1. Extracted the inline post_rerank lambda into a proper handle_rerank_impl() method
2. Updated Readme

Additional information

This is the final PR that allows for full multimodal rerank and embedding functionality which means Qwen 3VL Embedding and Rerank models can be fully utilized.

From #18665 (comment)

Requirements

• I have read and agree with the contributing guidelines
• AI usage disclosure: Assisted-by: Opencode/Qwen3.8 27B but code has been verified by myself

[timothywang21]

Pinging @ngxson

[ngxson]

/bot review

[ngxson]

/bot review

[ggml-gh-bot]

Automated code review

Note: the previous automated review run failed with a 503 (diff fetch error), so no findings were recorded - this is the first substantive review of the full diff.

Review of PR #29625 (commit eb30d6a)

Overall the approach is sound: extracting handle_rerank_impl, reusing handle_media/process_mtmd_prompt, and the marker-splicing in format_prompt_rerank all follow existing infrastructure. Findings below, grouped by severity.

Blocking (correctness)

(point 1) tools/server/server-common.cpp ~line 1885 (format_prompt_rerank, the rerank_prompt == nullptr else-branch): media files are silently dropped when mctx != nullptr but the model has no "rerank" chat template. The upfront guard only catches mctx == nullptr, so a user running a text-only reranker with an mmproj loaded who sends image objects gets scores computed without the images, with no error. Add a guard in this branch too - if !query_files.empty() || !doc_files.empty(), throw std::invalid_argument (e.g. "model does not provide a rerank chat template; multimodal input is not supported") instead of tokenizing plain text.

Will slow the review

(point 2) tools/server/server-context.cpp ~5485 and ~5508: the parse_item call sites catch only std::invalid_argument, but handle_media throws std::runtime_error for a failed download ("Failed to download image") and for undecodable raw base64 ("Invalid base64 value"). Those escape to ex_wrapper and come back as a bare 500 instead of the endpoint's normal 400 JSON error. Either catch std::exception at both call sites, or normalize the errors in add_image to std::invalid_argument.

(point 3) Comment/whitespace style in handle_rerank_impl, per AGENTS.md:

• Line 5434: "LAMDA FUNCTION DECLARATION" is a typo (lambda), and the comment is verbose; also "compatabile" is misspelled. The inline comments "// Parsing the "text" field, ..." / "// Parsing the "image" field, ..." restate what the code says - drop them.
• Line 5501 is a whitespace-only line (trailing spaces), and the if (!documents_json.is_array() ...) block at ~5500-5505 is mis-indented (8 spaces instead of 4). "// Parse query " at 5479 has a trailing space.
• The // marker is appended by format_prompt_rerank, one per decoded file comment inside add_image is fine, but "Resolve JINA/TEI format ambiguity" should be spelled "Jina/TEI" like the rest of the file.

(point 4) Design consistency worth being ready to defend: the embeddings endpoint (your own #29556) accepts multimodal input via the OAI content array ({"content": [{"type": "text"|"image_url", ...}]}), while this PR introduces a second, different schema ({"text", "image"}) for rerank. Two divergent multimodal shapes across adjacent endpoints is a maintenance cost. I understand the rationale (rerank is Jina/TEI format, not OpenAI), but reviewers may ask whether oaicompat_content_load_media could be reused - have an answer ready, or consider accepting both shapes.

Nits

(point 5) server-common.cpp ~1849: if ((query_files.empty() && doc_files.empty()) == false && ...) - write if ((!query_files.empty() || !doc_files.empty()) && ...).

(point 6) server-common.cpp ~1881 vs 1873: the text-only path tokenizes the template with add_special = false, but process_mtmd_prompt hardcodes add_special = true. For a model that adds BOS, the text-only and multimodal rerank prompts would differ by a BOS token. Harmless for Qwen (no BOS), but worth a comment or an explicit choice.

(point 7) The mmproj check only fires inside format_prompt_rerank, i.e. after handle_media has already downloaded/decoded all images for query and every document. Checking ctx_server.mctx == nullptr up-front in the handler when any image is present would fail fast.

(point 8) handle_rerank_impl: top_n is unvalidated. format_response_rerank does elements.resize(std::min(top_n, (int)elements.size())), so a negative top_n throws std::length_error and surfaces as a 500. This predates the PR, but since the handler was extracted/rewritten, clamping top_n to >= 0 here would be a trivially cheap fix.

(point 9) README says the TEI format is "text-only", but the code happily accepts image objects with texts and processes them. Either reject images when is_tei_format or soften the doc wording.

(point 10) The slot.prompt_clear() gate in server-context.cpp ~3271 is functionally redundant with the existing non-cached flow (keep_first(0) plus slot.mem.seq_rm(slot.id, 0, -1) at ~3431 already wipes both the token list and the KV). It is harmless and arguably documents intent, but the comment is three lines where one would do; consider trimming to e.g. // stateless tasks must not inherit any state from a previous request on this slot.

(point 11) The README diff adds a stray extra blank line in the /v1/responses/input_tokens section (~line 1689) - unrelated whitespace, drop it.

Checked and OK

• slot.prompt_clear() runs before pos_next() is computed, but with cleared tokens and n_past = 0 all downstream reads (keep_first, checkpoint search) are safe no-ops.
• File ordering (query files then doc files) matches marker emission order for the {query}-before-document} template; mtmd assigns bitmaps in marker order, so images splice correctly.
• Exceptions thrown while building tasks all occur before rd.post_tasks, so no orphaned tasks on parse failure.
• --reranking alias does exist (common/arg.cpp:3481), so the updated README requirement is accurate.
• Response shape, usage.prompt_tokens accumulation, return_text behavior, and top_n truncation all match the documented README examples.

This review was generated automatically by pi coding agent using zai-org/GLM-5.3. It may contain mistakes. Maintainers make the final call.

[timothywang21]

@ngxson I updated the PR with the bot's proposed fixes.

[ngxson]

reduce the docs, just enough to mention that it's the same input format as /embeddings endpoint

[timothywang21]

I trimmed the README but I want to clarify that the rerank endpoint does not have the same input format as the /embeddings endpoint. Embeddings is OpenAI-format while Rerank is Jina/TEI. There is no OpenAI rerank format.

[ngxson]

/bot review

[ggml-gh-bot]

Automated code review

Follow-up review of PR #29625 (commit 5993b5b, delta since eb30d6a)

I reviewed the fix commit against the findings from the previous review. All blocking and "will slow the review" points were addressed correctly. Findings on the delta:

Fixes verified

(point 1) The new guard in format_prompt_rerank (server-common.cpp ~1884) is correct: the branch is reachable (mctx loaded, no "rerank" template, images present), and the std::invalid_argument surfaces as a proper 400 via ex_wrapper (server.cpp:60-64), which runs before any task is posted. Good.

(point 2) The try/catch in add_image (server-context.cpp ~5459-5466) is correct: I checked every throw path in handle_media - the only non-std::invalid_argument throws are runtime_error from common_remote_get_content failures and undecodable raw base64, both now converted and caught at the two parse_item call sites as 400s. e.what() lifetime is fine.

(point 3, 5, 8, 9, 11) Comment rewrites, the condition rewrite, top_n < 0 validation, TEI README wording, and the stray blank line are all addressed. The trimmed prompt_clear comment is fine.

Remaining nits (carried over, non-blocking)

(point 3) The add_special asymmetry from the previous review (previous point 6) is still present: in the rerank-template branch, the text-only path tokenizes with add_special = false (server-common.cpp ~1879) while the multimodal path goes through process_mtmd_prompt, which hardcodes add_special = true (server-common.cpp ~955). Harmless for Qwen3-VL (no BOS), but a one-line comment stating this is deliberate would close it out.

(point 4) The mmproj check still fires only inside format_prompt_rerank, after all query/document images have been downloaded and base64-decoded. Checking ctx_server.mctx == nullptr up-front in handle_rerank_impl when any image is present would fail fast and skip wasted downloads. Same for the new "no rerank chat template" guard - it could be checked once before the per-document loop instead of inside format_prompt_rerank per iteration. Minor.

(point 5) Note for completeness, not a demand: a corrupt image that passes handle_media (valid base64 bytes) but fails mtmd bitmap init throws std::runtime_error from process_mtmd_prompt during task creation, which ex_wrapper maps to a 500 rather than a 400. This is identical to the existing /v1/chat/completions multimodal path, so it is consistent pre-existing behavior - just be aware the new guard does not cover it.

Checked and OK

• No orphaned tasks: all new throws (mmproj guard, template guard) happen before rd.post_tasks.
• top_n clamp interplays correctly with format_response_rerank's elements.resize(std::min(top_n, ...)) now that negatives are rejected.
• The rewritten parse_item schema comment documents the accepted input shape without restating the code.

No blocking findings remain on the delta.

This review was generated automatically by pi coding agent using zai-org/GLM-5.3. It may contain mistakes. Maintainers make the final call.

[timothywang21]

The build failed because of a rebase semantic break. The is_stateless_task check in my code was superseded by #30498. I removed the redundant prompt clear check. The CI should be successful now.