fix(deepseek4): make batched image serving safe for many users

[davide221]

Batched DeepSeek V4 Vision serving (#760) had four problems that Qwen3.8 does not. They all come from the staged prefill that DS4V image blocks need (whole-block bidirectional attention, which the 16-row batched step cannot run).

Problem	Before	After
Eviction	An evicted image slot was rebuilt from token ids, so its image rows came back as text (silent wrong answer)	SeqEngine::kv_recomputable(); the scheduler parks another decoder, and DeepSeek4 refuses to evict an image slot or suspend one mid-staging
Freeze during image prefill	The whole staged prefill ran inside one step: every stream froze (33 s for an 8-image request)	A shared pass of about 256 rows, spread over steps by layer (6 of 43 layers per step while others decode; whole when nothing decodes). A staging slot sits out (reports advanced) until its prefix is copied in
Encode on the scheduler thread	About 0.8 s per image, plus the single-request memory transitions, on every admission	With --mmproj-device, a persistent encoder worker takes admitted requests in order and publishes each image as it lands; a pass only takes rows whose images are ready
Staging memory	One full cache allocated lazily per concurrent request, mid-request	One per slot, allocated and logged at startup (286 MB for 4 slots at 8K ctx)

vision::staged_prefill_chunk picks each chunk (whole image blocks, no chunk or tail below the layer-major minimum) and has exhaustive layout tests. It also fixes a hang found while testing: 2 text rows before an image left no valid chunk under a small budget.

Measured on lucebox6 (Strix Halo decoder, R9700 encoder, 4 slots)

• Mixed traffic. Two streaming text requests while three one-image requests and one eight-image request arrive: the longest pause in the text streams is 0.67 s (no gap over 1 s), where it used to be the whole prefill. All image answers correct, both essays complete.
• Layer slicing is exact. With the decode batch held fixed, a whole pass and the same pass in 8 slices give byte-identical answers on two charts (2,239 and 2,432 chars); slicing costs nothing (3,517 vs 3,617 ms).
• Forced eviction (1,280-token pool, 1 MB offload budget). Text slots were parked for recompute and resumed; the image slot was never evicted, and its answer is identical to the solo run (2,227 of 2,227 chars).
• Throughput. Four concurrent image answers finish in 37.5 s, against 35.1 s before: passes no longer wait for every request's images.
• Sanity 2/2; test_server_unit 598/598; DS4V image tests pass (944,190 checks).

Review (cubic, 5 findings) fixed in 3d6afe5. Layer slicing in dad736d.

🤖 Generated with Claude Code

[cubic-dev-ai]

2 issues found across 10 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="server/src/deepseek4/deepseek4_seq_engine.cpp">

<violation number="1" location="server/src/deepseek4/deepseek4_seq_engine.cpp:306">
P2: Failed staging removes the request before `retire()` can cancel its encoder, so a queued `--mmproj-device` encode continues after the request fails and can delay later users. Cancel the staged image payload before erasing it, and apply the same cleanup when admission fails before `pending_images_.push_back()`.</violation>
</file>

<file name="server/src/deepseek4/deepseek4_backend.cpp">

<violation number="1" location="server/src/deepseek4/deepseek4_backend.cpp:1343">
P2: `encode_stop_` is written under `encode_mutex_` in `release_vision()` but read without that mutex by the worker callback, creating a shutdown data race. Read it under the same mutex or make it atomic.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Failed staging removes the request before retire() can cancel its encoder, so a queued --mmproj-device encode continues after the request fails and can delay later users. Cancel the staged image payload before erasing it, and apply the same cleanup when admission fails before pending_images_.push_back().

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/src/deepseek4/deepseek4_seq_engine.cpp, line 306:

<comment>Failed staging removes the request before `retire()` can cancel its encoder, so a queued `--mmproj-device` encode continues after the request fails and can delay later users. Cancel the staged image payload before erasing it, and apply the same cleanup when admission fails before `pending_images_.push_back()`.</comment>

<file context>
@@ -296,7 +295,26 @@ SeqEngine::StepResult DeepSeek4SeqEngine::step(const StepPlan & plan) {
+            const bool copied = !failed && staged.done >= staged.prefix && !staged.staging;
+            if (failed) fail_prefill(slice.slot, result.prefills, staged.error);
+            if (failed || copied) {
+                pending_images_.erase(pending_images_.begin() + (pending - pending_images_.data()));
+            }
+            if (!copied) {
</file context>

[davide221]

Fixed in 3d6afe5: a staged request that fails now cancels its queued encode before it is dropped.

[cubic-dev-ai]

P2: encode_stop_ is written under encode_mutex_ in release_vision() but read without that mutex by the worker callback, creating a shutdown data race. Read it under the same mutex or make it atomic.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/src/deepseek4/deepseek4_backend.cpp, line 1343:

<comment>`encode_stop_` is written under `encode_mutex_` in `release_vision()` but read without that mutex by the worker callback, creating a shutdown data race. Read it under the same mutex or make it atomic.</comment>

<file context>
@@ -1148,99 +1182,212 @@ ImagePrepareStatus DeepSeek4Backend::prepare_images(
-            for (Seq * m : members) m->item->error = error.empty() ? "staged prefill failed" : error;
-            continue;
+        try {
+            const auto cancelled = [&] { return images->stream_cancelled() || encode_stop_; };
+            for (size_t i = 0; ok && i < images->prepared_.images.size(); ++i) {
+                vision::ImageRows one;
</file context>

Suggested change

	const auto cancelled = [&] { return images->stream_cancelled() || encode_stop_; };
	const auto cancelled = [&] {
	std::lock_guard<std::mutex> lock(encode_mutex_);
	return images->stream_cancelled() || encode_stop_;
	};

[davide221]

Fixed in 3d6afe5: encode_stop_ is now std::atomic.

[davide221]

@cubic-dev-ai review

[cubic-dev-ai]

@cubic-dev-ai review

@davide221 I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

1 existing issue remains and 2 new issues found across 11 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="server/test/test_ds4v_image_integration.cpp">

<violation number="1" location="server/test/test_ds4v_image_integration.cpp:93">
P3: The walk verifies progress, tail size, and whole-image blocks, but never asserts that a chunk respects the budget. A regression that always returns the entire remaining prefix passes every walk assertion (n >= 5, leftover 0, chunk covers whole blocks), so only the three hard-coded requires pin budget behavior — and all three use budget 256. The budget 5/8/16 loops, which are the small-budget cases the hang fix targets, would not catch such a regression.

Add a size assertion with the legitimate growth cases excepted: `n` may exceed `budget` only when the chunk starts in a block it must finish (block_begin <= done < block_end) or when the no-stub-tail rule forces growth to the end (n == prefix - done). An oracle comparison like check_oracle would be stronger still.</violation>

<violation number="2" location="server/test/test_ds4v_image_integration.cpp:94">
P3: The `walk` assertions throw with a fixed string, so a regression anywhere among the ~1700 walks / ~600K chunk calls cannot be localized. Sibling assertions in this same function embed context (check_oracle reports position/preferred/capacity); these should too.

Include `done`, `n`, `budget`, `prefix`, and `layout.size()` in each message so a failing iteration identifies the exact configuration.</violation>
</file>

<sub>Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.

Re-trigger cubic</sub>

[cubic-dev-ai]

P3: The walk assertions throw with a fixed string, so a regression anywhere among the ~1700 walks / ~600K chunk calls cannot be localized. Sibling assertions in this same function embed context (check_oracle reports position/preferred/capacity); these should too.

Include done, n, budget, prefix, and layout.size() in each message so a failing iteration identifies the exact configuration.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/test/test_ds4v_image_integration.cpp, line 94:

<comment>The `walk` assertions throw with a fixed string, so a regression anywhere among the ~1700 walks / ~600K chunk calls cannot be localized. Sibling assertions in this same function embed context (check_oracle reports position/preferred/capacity); these should too.

Include `done`, `n`, `budget`, `prefix`, and `layout.size()` in each message so a failing iteration identifies the exact configuration.</comment>

<file context>
@@ -77,6 +77,44 @@ void validation_and_lookup() {
+            int done = 0;
+            while (done < prefix) {
+                const int n = staged_prefill_chunk(view(layout), uint64_t(done), prefix - done, budget, 5);
+                require(n >= 5, "staged chunk makes progress");
+                require(prefix - done - n == 0 || prefix - done - n >= 5, "no stub tail");
+                for (const TokenSpan & span : layout) {
</file context>

Suggested change

	require(n >= 5, "staged chunk makes progress");
	require(prefix - done - n == 0 || prefix - done - n >= 5, "no stub tail");
	require(n >= 5, "staged chunk makes progress at position=" + std::to_string(done) +
	" prefix=" + std::to_string(prefix) + " budget=" + std::to_string(budget));
	require(prefix - done - n == 0 || prefix - done - n >= 5,
	"no stub tail at position=" + std::to_string(done) + " n=" + std::to_string(n) +
	" budget=" + std::to_string(budget));

[cubic-dev-ai]

P3: The walk verifies progress, tail size, and whole-image blocks, but never asserts that a chunk respects the budget. A regression that always returns the entire remaining prefix passes every walk assertion (n >= 5, leftover 0, chunk covers whole blocks), so only the three hard-coded requires pin budget behavior — and all three use budget 256. The budget 5/8/16 loops, which are the small-budget cases the hang fix targets, would not catch such a regression.

Add a size assertion with the legitimate growth cases excepted: n may exceed budget only when the chunk starts in a block it must finish (block_begin <= done < block_end) or when the no-stub-tail rule forces growth to the end (n == prefix - done). An oracle comparison like check_oracle would be stronger still.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/test/test_ds4v_image_integration.cpp, line 93:

<comment>The walk verifies progress, tail size, and whole-image blocks, but never asserts that a chunk respects the budget. A regression that always returns the entire remaining prefix passes every walk assertion (n >= 5, leftover 0, chunk covers whole blocks), so only the three hard-coded requires pin budget behavior — and all three use budget 256. The budget 5/8/16 loops, which are the small-budget cases the hang fix targets, would not catch such a regression.

Add a size assertion with the legitimate growth cases excepted: `n` may exceed `budget` only when the chunk starts in a block it must finish (block_begin <= done < block_end) or when the no-stub-tail rule forces growth to the end (n == prefix - done). An oracle comparison like check_oracle would be stronger still.</comment>

<file context>
@@ -77,6 +77,44 @@ void validation_and_lookup() {
+        const auto walk = [&](const std::vector<TokenSpan> & layout, int prefix, int budget) {
+            int done = 0;
+            while (done < prefix) {
+                const int n = staged_prefill_chunk(view(layout), uint64_t(done), prefix - done, budget, 5);
+                require(n >= 5, "staged chunk makes progress");
+                require(prefix - done - n == 0 || prefix - done - n >= 5, "no stub tail");
</file context>