perf(dflash): add opt-in exact prefill bands

[cheese-cakee]

Summary

Adds an opt-in exact-prefill band scheduler for DeepSeek4 while keeping q1 exact prefill as the safe default.

• enables exact q2-q4 prefill bands behind DFLASH_DS4_EXACT_PREFILL_BANDS=1;
• preserves q1 numerical order for attention, MoE routing/FFN work, cache updates, logits, snapshots, and repeated-request lifecycle;
• bounds graph/scratch behavior and keeps an explicit q1 escape hatch;
• reconciles the implementation with current main, including DSpark AR-fallback and fused-verifier diagnostics;
• suppresses the fused-verifier warning during intentional no-hook exact-prefill calls.

Current head

• PR head: 42d5c3e7f6672a840e2e1c4cbaf036a653f12804
• reconciled base: 99ab4cebd331310adcc37c5ab89e30323b3ddf27
• scope: 9 product/test files; no local evidence, state, handoff, or research files
• git diff --check: passed
• independent final source review: clean

HIP and unit validation

Lucebox4 matched Release builds used HIP architectures gfx1151;gfx1201, HIP graphs enabled, and wave32.

• final server SHA-256: 7092537d30f279977089dbdbaa1afb259bcd233e56667303a9412a5f59a0f728
• final DeepSeek unit binary SHA-256: 9be55c0737faba4cd14cb0cfd1cf9bdda5b7ab8591f2f076fcfe47eb45752a14
• exact-head full test_deepseek4_unit run on Lucebox4 gfx1201: passed
• full DeepSeek unit suite on Lucebox4 for the tested compute delta: passed

Sealed evidence roots:

• build: /home/cheese/pr590/evidence/pr590-publish-build-42d5c3e7-20260818
• build seal: 66808f5ba6b045e94ba326b70c44e49a2747162fa80688f6241584bc43e1a0d8
• exact-head unit: /home/cheese/pr590/evidence/pr590-publish-unit-r9700-42d5c3e7-20260818
• unit seal: b88eb169d7ab15ca081c77ac6352b96476e9a1a88c5dd22c74797a0622ea0e9d

Measured prefill result

On Lucebox4, the cache-cold candidate placement screen measured:

Topology	Median prefill	MAD
R9700 + Strix heterogeneous	15.1159 tok/s	0.1421
Strix-only	4.5689 tok/s	0.0252

Candidate heterogeneous / candidate Strix-only: 3.3084x, exceeding the 1.5x placement goal.

The screen used five measured requests per topology, exact prefill with chunk 4, caches disabled, and all retained responses reported cache_hit=false with zero cached-prefix tokens. Outputs were stable within each topology. This is a raw placement goal screen, not a claim that exact bands alone provide 3.3084x over q1.

Aggregate evidence: /home/cheese/pr590/evidence/t07-goal-combined-3605716a-20260818-r4
Seal SHA-256: 11bbd6a66f7d549c0a80f886234d047920938d442906c41438210d107100e3b5

Review status

The branch is reconciled with current main and ready for repository CI and maintainer review. No merge-ready claim is made until required live checks complete.

[cubic-dev-ai]

40 issues found across 170 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/server/http_server.cpp">

<violation number="1">
P1: When a local request uses `tools` with a non-null `tool_choice`, this branch routes it to the concurrent scheduler, which never passes the tool-choice hint to the backend. Preserve the classic path for these requests or add equivalent tool-choice inputs to the sequence-engine contract.</violation>
</file>

<file name="server/src/server/scheduler.cpp">

<violation number="1">
P1: When concurrent serving is enabled, this admits the raw rendered prompt and bypasses `prepare_prompt`, so configured FlowKV/PFlash prompt processing is silently ignored. Prepare the effective prompt through the same request pipeline before admission, or reject unsupported prompt-processing modes explicitly.</violation>
</file>

<file name="server/src/qwen35/qwen35_backend.h">

<violation number="1">
P1: When a hybrid `Qwen35MoeBackend` is configured with `max_concurrency > 1`, this inherited accessor exposes `Qwen35SeqEngine` even though that engine bypasses the MoE hybrid execution path. Return `nullptr` or reject concurrent serving for MoE backends until the engine supports `hybrid_forward_*`.</violation>
</file>

<file name="harness/benchmarks/concurrency/canonical_concurrent_benchmark.py">

<violation number="1">
P2: When any request in a wave fails, this condition skips retirement waiting and immediately starts the next wave. Abort before advancing or wait for every known response ID so a failed stream cannot leave scheduler work overlapping the next measurement.</violation>

<violation number="2">
P2: The canonical DDTree launcher uses `--ddtree-budget 22`, but this proof computes acceptance with a fixed `16 * steps` denominator. Derive the denominator from the actual candidates offered per step, including shortened or adaptive steps, and record that telemetry.

(Based on your team's feedback about offered-candidate acceptance denominators.)</violation>

<violation number="3">
P2: When one successful request has no output timing, `statistics.median` receives `None` alongside numeric timings and the runner exits with an exception. Filter absent timings and only publish the median/max when every successful request has a numeric value.</violation>
</file>

<file name="server/src/qwen35/graph_builders.cpp">

<violation number="1">
P2: When a caller supplies a positive `n_prefill_segments` count with a null `prefill_segments` array, this loop crashes instead of returning the builder's expected `false` failure. Reject a null segment array before iterating, and validate the segment count as non-negative.</violation>
</file>

<file name="harness/benchmarks/concurrency/run_qwen36_concurrency.sh">

<violation number="1">
P2: When `PREFILL_FIRST_BURST_STEPS` is positive, the Luce cases claim to measure a TTFT policy that the server never consumes. Add the server-side policy implementation or remove/reject this unsupported benchmark option before publishing its results.</violation>

<violation number="2">
P2: When `IDLE_PREFILL_TOKENS` exceeds 4096 with the default zero burst steps, this runner accepts a benchmark combination the protocol says is invalid. Reject that combination or require a positive `PREFILL_FIRST_BURST_STEPS` before launching cases.</violation>

<violation number="3">
P2: When a screen selects only one engine, the unconditional binary checks still require the unused engine and prevent valid Luce-only or llama-only runs. Validate each server binary only when its variant is selected.</violation>

<violation number="4">
P2: On a host without `awk` or `ldd`, the runner fails after creating the output rather than reporting a missing dependency up front. Include both commands in the preflight dependency check.</violation>
</file>

<file name="harness/benchmarks/concurrency/summarize_concurrency.py">

<violation number="1">
P2: A DDTree report with positive aggregate proof but no per-response telemetry passes this check and publishes AL/acceptance. Validate every measured request's `ddtree_metrics` and positive step count before using the aggregate proof, so missing concurrency records fail closed.</violation>

<violation number="2">
P2: When every ragged report omits `selected_output_set_sha256`, the summary claims `Stable output: yes` and still publishes comparison deltas. Treat missing or non-string output hashes as unknown (`n/a`) or reject the reports before calculating stability and deltas.</violation>
</file>

<file name="harness/benchmarks/concurrency/run_qwen36_canonical_concurrency.sh">

<violation number="1">
P2: Every case rereads the full target and optional draft just to write metadata, making repeated runs unnecessarily I/O-heavy and potentially perturbing file-cache state between cases. Hash each artifact once before the case loops and reuse the resulting digests.</violation>

<violation number="2">
P2: When a DDTree run receives a non-Q8_0 drafter, the metadata still claims `draft_quant: Q8_0` because the runner validates readability only. Validate the GGUF quantization or record the detected value so benchmark reports cannot mislabel the measured recipe.</violation>
</file>

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

<violation number="1">
P2: When `DFLASH_DS4_Q5_VERIFY=1` uses fused verification, this enables q=5 without providing the output storage required by the wide graph. The verifier silently splits q=5 into q=4 plus q=1, adding work instead of using the opt-in wide path; enable `keep_logits_` for q=5 or pass an output buffer.</violation>
</file>

<file name="harness/benchmarks/deepseek4/ds4_publication_decode_client.py">

<violation number="1">
P2: When an SSE `data:` line is malformed, this branch drops its content and can still mark the run successful. Return a failed record on JSON decode errors so dropped chunks cannot enter the throughput summary.</violation>

<violation number="2">
P2: The client includes the first output token in the numerator even though timing starts when that token arrives, overstating every reported decode rate by one token. Use `(completion_tokens - 1) / decode_s`, matching the repository's established decode metric.</violation>
</file>

<file name="harness/benchmarks/concurrency/concurrent_benchmark.py">

<violation number="1">
P2: When the server returns a boolean or negative usage count, these checks mark it complete and the benchmark aggregates invalid accounting; negative prompt counts can pass the fixed-token gate and produce misleading rates. Validate both usage counts as non-boolean, non-negative integers before storing them, matching `native_prefill_values()`.</violation>
</file>

<file name="server/src/qwen35/qwen35_target_graph.cpp">

<violation number="1">
P2: When `in.q_capture` is enabled for a ragged paged step, this branch leaves `q_fa_out` null and the per-layer `cache.q_cap` remains stale. Assign `*q_fa_out = Qfa` before calling `paged_read`.</violation>
</file>

<file name="server/src/common/moe_hybrid_placement.cpp">

<violation number="1">
P2: When critical-path placement runs with many layers and unequal expert sizes, this exact frontier can grow combinatorially and make model initialization consume excessive memory and time. Bound or quantize the frontier, or use a bounded knapsack/greedy algorithm so placement startup remains predictable.</violation>
</file>

<file name="harness/qualification/deepseek4/analyze_rocprof_overlap.py">

<violation number="1">
P2: When `--timeline-max` is enabled, the timeline header emits four comma-separated fields, but every burst row emits three. Keep the header at three columns and print `merge_gap_us` separately so CSV consumers can parse the timeline.</violation>

<violation number="2">
P2: With a finite but enormous option such as `--window-start-s 1e308`, this conversion raises `OverflowError` after the finite check passes. Validate unit-scaled values before converting, or report the range error through `argparse`.</violation>
</file>

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

<violation number="1">
P2: This test does not actually force the partitioned path. ggml-cuda reads GGML_CUDA_PAGED_ATTN_FORCE_PARTITIONS once into a function-local static (`static const int forced_partitions = [](){ ... }();` in paged-attn.cu:855-859) on the first paged-attn compute. PartitionedPathMatchesReference is registered first and runs several full computes with the env unset, so the static is captured as 0 before DirectPathMatchesReference sets the ScopedEnvVar. Setting the env at runtime here has no effect on the already-captured value, so the "direct" case runs the default partition path and no longer pins the partitioned path that the old CTest invocation (env set before process start) exercised.</violation>

<violation number="2">
P3: The reference mirror does not actually treat a negative causal position as a padding row the way the kernel does. When query_positions[seq] < 0 on a row whose physical slot is in range, the code sets kv_seq_len = position + 1; for position < -1 that is negative and `std::vector<float> scores(kv_seq_len)` becomes undefined behavior instead of "leave zero output" as the comment claims. The kernel treats any negative query position as a dead row (valid_query = false) regardless of slot validity. The current test only pairs the -1 position with the out-of-range padding slot (which is skipped by the earlier `continue`), so the divergence is untested and latent; clamp the position to >= 0 to match the kernel's dead-row semantics.</violation>
</file>

<file name="server/tests/test_server_parallel.py">

<violation number="1">
P2: When --max-concurrency is in 33..64, test_parallel_more_than_slots crashes the whole suite instead of testing. It sets count = 2 * n and passes it to make_math_prompts, which raises ValueError for count > 64. The suite advertises support for up to 64 streams (the main() range check and the other tests accept 64), so this is inconsistent. Cap count at 64 (use min(2*n, 64)) or add a skip for n > 32 so the suite doesn't abort with a traceback.</violation>
</file>

<file name="harness/qualification/deepseek4/qualify_ds4_q5_amd.sh">

<violation number="1">
P2: When this launcher runs from cron, a container, or another environment without `HOME`, `set -u` aborts at server-env construction before the qualification starts. Use a default for `HOME` (or validate it explicitly) before expanding it.</violation>
</file>

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

<violation number="1">
P2: The new tensor validation checks are placed at the end of the test, but `w.fc`, `w.hidden_norm`, and `w.layers[0]` are all dereferenced earlier (the `w.fc->ne[0]`, `w.hidden_norm->ne[0]`, `w.layers[0].wq->ne[0]`, and `ggml_backend_tensor_get(w.hidden_norm, ...)` calls above). If any of these were null/empty, the code would already crash with undefined behavior before `CHECK_NOT_NULL(w.fc)` is reached, so these checks can never catch the exact condition they guard and offer no real protection. Move them immediately after the successful load (before any tensor iteration/printing) so a null tensor yields a clean assertion failure instead of a segfault.</violation>
</file>

<file name="server/src/server/server_status.h">

<violation number="1">
P3: When concurrent slots are simultaneously prefilling and decoding, the status page emits `badge-mixed` without a matching CSS rule, so the new phase loses its phase styling. Add a `.badge-mixed` style to the status page.</violation>
</file>

<file name="harness/benchmarks/concurrency/generate_prompts.py">

<violation number="1">
P3: When `MAX_TOKENS` is overridden, the generated `he-raw` manifest still records `max_tokens: 128` while the benchmark sends the override. Remove this unused field or populate it from the same runtime setting so manifests describe the measured workload.</violation>
</file>

<file name="server/src/qwen35/graph_builders.h">

<violation number="1">
P3: When `n_prefill_tokens > 0`, `seq_slot` is ignored; each `QwenPrefillSegment::seq_slot` selects the recurrent-state slab. Document that `seq_slot` applies only to non-segment steps.</violation>
</file>

<file name="server/test/CppUnitTestFramework.hpp">

<violation number="1">
P3: The message passed to SKIP (e.g. "CUDA device unavailable") is silently discarded: the catch sets only test_skipped and ConsoleLogger::SkipTest prints just the test name, so TestSkippedException::m_message is never surfaced and callers can't see why a test was skipped. Surface the reason (e.g. pass the exception message to SkipTest or log it before clearing).</violation>

<violation number="2">
P3: For a test skipped via SKIP, ExitTest (which resets m_indent_level to 0) is never called and SkipTest does not reset it, while EnterTest already incremented it. Each skipped test leaks one indent level, so indentation of all subsequent verbose/section/assert logging for the rest of the run drifts. Reset m_indent_level in the skipped branch, e.g. in SkipTest.</violation>
</file>

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

<violation number="1">
P3: When the CPU backend is unavailable, the preceding `CHECK(backend != nullptr)` records a failure but the immediately following `SKIP(...)` throws `TestSkippedException`. In `CppUnitTestFramework::TestRegistry::Run`, a caught `TestSkippedException` marks the test skipped and takes precedence over `HaveChecksFailed()`, so the recorded failure is discarded and the test is reported as a skip (exit 77, not a CI failure). The old `if (!backend) return 1;` failed hard instead. Same applies to the `ctx` and `buffer` guards. Use `REQUIRE` for these environment preconditions so a genuinely broken setup fails the test rather than silently skipping it.</violation>
</file>

<file name="optimizations/paged_attention/CONCURRENCY_OPT_QWEN36_EXPERIMENTS.md">

<violation number="1">
P3: This newly added file is a Qwen3.6 concurrency/experiments ledger, but the PR is a DeepSeek4 opt-in exact-prefill-bands change. Nothing in the ledger covers DeepSeek4 exact prefill, and none of the PR's 9 product/test files or its DS4 feature is referenced here. Unrelated documentation mixed into a focused PR makes review and history harder to reason about; move this ledger to a separate PR (or a Qwen-concurrency doc update).</violation>
</file>

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

<violation number="1">
P3: The added null checks run after the code has already dereferenced the same pointers. The spot-check `printf` block above uses `w.tok_embd->ne[0]`, `w.output->type`, and `w.out_norm->type`, and the `print_layer` loop reads `w.layers[il]` — so if any of these were null, the test would crash or read invalid memory before `CHECK_NOT_NULL` ever executes. The checks therefore cannot actually catch the null case they appear to validate. Move these guards before the dereferences and use `REQUIRE` so they abort the test with a clean message instead of crashing.</violation>
</file>

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

<violation number="1">
P3: The comment "Invalid asks are hard errors, not busy" is not actually verified. The two checks here only assert `!is_admitted(...)`; a `busy` result also fails `is_admitted`, so a regression where `admit` returns `busy` for an empty or oversize prompt would still pass this block. Assert the concrete status (`SeqEngine::AdmitResult::Status::failed`) and `!is_busy(...)` so the claimed classification is enforced, as the later "never-fits" block does.</violation>
</file>

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

<violation number="1">
P2: This conversion registers the kernel validation as an always-run CppUnit/CTest case, but the numerical checks only print PASS/FAIL and never assert. A kernel regression (mean diff ≥ 1e-2, attention diff ≥ 5e-2, or a slow/broken e2e path) will print FAIL while the test still returns success, so CI stays green on a broken flashprefill kernel. Since the whole purpose of this test is numerical validation, gate each check on a REQUIRE (e.g. REQUIRE(max_diff_mean < 1e-2f) and REQUIRE(max_diff_attn < 5e-2f)).</violation>
</file>

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

<violation number="1">
P3: All ~20 gate tests are now folded into a single TEST_CASE (feature_gate_suite), so the framework can no longer report or filter each rule cluster independently. The old RUN_TEST harness reported each function as its own named test. Create one TEST_CASE per test function (or at least per cluster) so a failure is attributed to a specific gate rule and a subset can be run via a keyword.</violation>
</file>

<file name="README.md">

<violation number="1">
P3: The new "Recommended Setups" DeepSeek rows recommend `--ds4-prefill exact` and the DSpark/fused-verify flags but never enable `DFLASH_DS4_EXACT_PREFILL_BANDS`, the headline feature this PR adds. Without it, `--ds4-prefill exact` runs the q1 single-token default, so users following the documented recommended DeepSeek config never get the q2–q4 exact prefill bands the PR exists to provide (per the PR description the bands activate only with `DFLASH_DS4_EXACT_PREFILL_BANDS=1`). Add the flag to the recommended DeepSeek rows (and note it in the README), or explicitly document that the recommended setup intentionally stays on the q1 default.</violation>
</file>

<sub>Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.

Re-trigger cubic</sub>