[aiqubits]

Overview

Fix /v1/systemone returning HTTP 500 when multiple questions share option tokens and the server uses multiple slots.

Limit the shared prompt prefix to each task's first required output position, so option-marker embeddings remain available to send_decision() in the final batch.

Fixes #30482

Additional information

None

Requirements

• I have read and agree with the contributing guidelines
• AI usage disclosure: No

[ngxson]

/bot review

[ggml-gh-bot]

Automated code review

I've traced the change through the shared-prefix machinery in server-context.cpp (prompt fill loop, wait_shared/copy_prompt_to, send_decision) and the decision::pos_first() definition. The fix is correct: a child continues from n_tokens_shared, and send_decision() only reads embeddings for positions present in the slot's final batch, so any marker/order position below n_tokens_shared would yield the "failed to get embeddings" 500 from #30482. Capping n_shared at each task's pos_first() keeps every read position at or above the split point, and the existing n_decision_first break (server-context.cpp:4053) then guarantees the whole read region lands in one batch for both parent and children. The -1 return of pos_first() (labels-only tasks) safely wraps to SIZE_MAX and is a no-op in the min, which is the right behavior.

Review of tools/server/server-decision.cpp:1275:

Will slow the review

(point 1) The new line is ~150 chars, well past the 120-column limit in .clang-format, and it merges two logically distinct caps ("each task keeps a token of its own" and "no read position may be shared") into one expression. Splitting it would both fix the width and make the two invariants readable, e.g.:

n_shared = std::min(n_shared, tasks[j].tokens.size() - 1);
// the outputs are read from one batch per task, so the shared prefix must stop before the first read position
n_shared = std::min({n_shared, (size_t) parent.decision.pos_first(), (size_t) tasks[j].decision.pos_first()});

(point 2) No regression test. server_decision_group_tasks() is a pure function over server_task vectors, so a small case in tests/test-server-component.cpp (two decision tasks with markers whose common prefix extends past pos_first(), asserting the resulting n_tokens_shared) would pin down exactly the bug from #30482 and is cheap to run in CI.

Nits

(point 3) The correctness of (size_t) parent.decision.pos_first() when it is -1 relies on unsigned wraparound producing SIZE_MAX (which std::min then ignores). That is guaranteed by the language, but it is subtle enough that the one-line comment from point 1 (or an explicit pos_first >= 0 guard) would spare the next reader a double-take.

Notes confirming correctness, no action needed

(point 4) Applying parent.decision.pos_first() only inside the j loop is fine: when the group has a single task the loop is skipped, but so is grouping entirely (end - i < 2 fallback). The pos_first() == 0 case correctly degrades to no sharing (n_shared == 0 fallback), and pointer/order-based decision types are covered since pos_first() folds in pointer and non-zero order entries.

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

[aiqubits]

@taronaeo @allozaur