DSA top-k review findings: a guard comment overclaims, the window clamps and non-positive topk are ungated, and the launcher swallows its launch error

[localai-bot]

A fresh review of #505 (PR #542, merged d861819bb) returned PASS with 6 non-blocking findings. It could not find a single input where the two-pass threshold selection diverges from DsaTopkSelect — 3,000,081 fuzzed shapes across three independent implementations, zero divergence — and it reproduced both the defect and the fix on real sm_121a hardware. This issue tracks the six findings it did raise.

1. The w < topk guard defends a failure mode that cannot occur (prose defect)

src/vt/cuda/cuda_deepseek_v4.cu:685-689, and the same claim in the spec, the PR body and the commit body. The code says pass 2's w < topk bound and pass 1's if (best < 0) break exist so "a NaN row … cannot write past its own row into the next one."

That is not provable, because pass 2 can never emit more than topk for any input, NaN included: the predicate is satisfied by exactly rank(th) elements, and better(th, NaN) == false means a NaN is always skipped in pass 1 and never satisfies the pass-2 predicate, so w == rank(th) <= topk always. The reviewer proved it by removing the bound and by weakening it to w <= topk — both left the device suite 4/4 SUCCESS and a 3M-shape fuzz clean.

Keep the guard, it is cheap and harmless. Fix the sentence: it is belt-and-braces, and the emit count is bounded by construction. Overclaiming what a guard defends is worse than having no comment, because the next reader trusts it.

2. No test exercises win_start < 0 or win_end > num_keys

cuda_deepseek_v4.cu:628-629. Mutating either clamp away (s0 = ws[t], s1 = we[t]) leaves all four device cases green, while the reviewer's fuzz detects both immediately. On device those become out-of-bounds logits reads. The lines predate #505 so that PR owed nothing here, but the new cases are the natural home and it is two lines of setup.

3. topk <= 0 diverges between the two arms

DsaTopkSelect asserts VT_CHECK(topk > 0, "topk must be positive") (deepseek_v4_dsa.cpp:76). DsaTopkLaunch / DsaTopkKernel have no equivalent and silently return an empty vector. The "two independent implementations agree" gate cannot see this because no case passes a non-positive topk.

4. DsaTopkLaunch has no post-launch cudaGetLastError()

cuda_deepseek_v4.cu:1187-1192. Sibling launchers do — e.g. DecodeAttnGLaunch with Check(cudaGetLastError(), "decode_attn_g launch"). This is precisely why the #505 fault surfaced as the misleading cudaStreamDestroy: an illegal memory access — the error text the entire evidence chain ended up quoting — instead of at the launch site. Adding the check makes the next fault in this kernel attributable.

5. "Strictly cheaper" should not be read as real-geometry-ready

Pass 1 is O(topk*n) with one global load per iteration and one thread per token row. The real caller passes nk == T with we[t] = t+1 (deepseek_v4.cpp:806-812), so at V4-Pro's index_topk=1024 the last row of a 4k prompt is ~4.2M serial dependent loads in a single thread. Asymptotically unchanged from the pre-fix kernel and explicitly out of scope, but the spec's "strictly cheaper overall" is true only relative to the old code. Qualify it.

6. Record trivia

• The spec cites the pre-fix kernel span as :624-665; the actual span is 624-669.
• The code comment at :650-653 says the literals "overflowed … on any window wider than topk". n > topk is the condition to enter the branch; the overflow itself needs n > 512 for chosen[512] and topk > 64 for picked[64]. Confirmed: at the old gate shape topk=3, nk=5 the pre-fix body is ASan-clean.
• Process, not fixable retroactively: b649a1ea2 introduced the spec in the same commit as the code, where CLAUDE.md requires the spec to be committed before implementation.

Scope

Items 1-5 plus the two trivia in 6. Out of scope: the real-geometry DSA sparse path and the compressed-key-space candidate window, which remain named residuals on the row.

Also worth recording from the review's own methodology, because it is a trap for anyone gating this suite: a first scoped attempt used -ts='*DSA top-k*' (suite filter) instead of -tc= (case filter) and printed test cases: 0 | 0 passed | 0 failed | 23 skipped next to Status: SUCCESS! — a live false green.