Skip to content

perf(ds4): accelerate long context and add PFlash - #664

Merged
davide221 merged 8 commits into
mainfrom
codex/ds4-pflash-long-context
Sep 8, 2026
Merged

davide221 merged 8 commits into
mainfrom
codex/ds4-pflash-long-context

Conversation

@davide221

@davide221 davide221 commented Aug 26, 2026 •

Copy link
Copy Markdown
Contributor

Merge-ready update — 2026-09-08

Current head: 13648dba4528ad592f85a766cde4cfc0ff6fb581. Main through 561a7e9d0 (#716 DS4 disk-prefix-cache integration) is merged without dropping its snapshot or disk-cache changes. The subsequent main change, #719, touches only README.md; GitHub still reports no merge conflicts. Cubic review is complete, with all discussions resolved after inspection; this is not a formal Cubic approval. Code review is complete, with the hardware-CI exceptions below explicitly retained. No merge has been performed.

  • Fixed cached verifier raw-row capture: runtime read indices now track ring-write indices on every replay. Sparse attention, native F16 KV, packed small-width indexing, adaptive verification, and existing opt-in defaults remain intact.
  • Added a regression that fails 48/72 cases before the fix and passes 72/72 after, covering widths 2–5, F16/F32, ring wraps, and positions through 128K. Existing saved-row attention 80/80 and RoPE replay 20/20 also pass.
  • Fixed per-token indexer mask alignment when the scorer skips a query prefix: 1/5 before → 5/5 after exact offset/layout cases. PFlash now accepts finite [0,1] ratios and rejects NaN/infinities; zero and NaN regressions are reproduced and fixed, with eight admission cases passing.
  • Clean lb8/gfx1151 build with ROCm 7.2.4 and HIP graphs enabled. DS4 suite (including the new disk-snapshot tests), server 453/453, feature gate, sequence-engine 15, batch-plan 62, and paged-KV 25/25 checks pass. The two-device HC test skips with only Strix exposed.
  • New-head live lb8 smoke: strict 7,680-token retrieval 5/5, 59 output tokens, matching known-good response SHA256 df780faf7676ccc768737cb23d0534555c6e4785382a7f225d4842e82dcffb87. Adaptive DSpark (cap 4), all six experts, sparse F16 fused verification active, prefix/PFlash caches off. One cold request with chunk 512 is correctness evidence, not a matched throughput comparison.
  • Corrected the sparse-verifier documentation to distinguish single-lane/ratio-4 selection from other batched layouts. Its example export is now commented out and visibly experimental. PFlash has its own section; legacy daemon GPU/residency limits are explicit and covered by a parser contract test.
  • Reviewed and resolved discussions with evidence. Intentional lazy PFlash reload and legacy wire limits are documented, and the Qwen35-only memory heuristic / expanded negative-test / two performance suggestions are non-blocking follow-ups, not claimed implemented. Cubic's final two comments repeat the saved-row rule despite the ratio-4 qualification and misattribute a neighboring helper comment; code-backed dispositions are posted. The RTX3090 runner is confirmed offline.
  • CI passes on this head: Linux, Windows, Strix Halo, DGX GB10, workspace. R9700 passed the DS4 suite and 34 other selected suites, but its global free-VRAM leak check failed under an unrelated 24.5 GB / 100%-busy model workload. The unchanged registry test passes 5/5 on lb8's idle physical R9700, with zero-byte delta every run. This host-side HIP allocation test does not launch gfx1151 numerical kernels on R9700. The CI job remains red, not relabeled; GitHub refuses a scoped rerun while the parent workflow is active. No check or threshold was weakened.

Performance boundaries: previous-head 8K off/on medians were 30.05 → 33.00 tok/s decode; these are not measurements of this new head. The 123K off-profile baseline and subsequent guarded correctness attempts produced no valid result because of swapping. The final isolated request stopped at ~1.066 GiB cumulative swap-out with ~10.53 GiB still available, so no OOM diagnosis or long-context correctness/throughput result is claimed. No fresh long-context gain, universal byte-identity guarantee, or Vulkan-parity claim is made. All temporary test servers are stopped.

New CI run · Final review-fix evidence

Previous cached-RoPE fix — 2026-09-08

Previous head: cbeea1f78925578be37e3d6b8d00e37378294786. The tested sparse-verification fix is now pushed; fresh CI and Cubic review are required. This is not a merge or performance-parity declaration.

  • Fused inverse RoPE now reads runtime token positions during graph replay. Cached forward-Q RoPE remains standalone to retain the validated verifier output; sparse flash attention, native F16 KV, adaptive DSpark, and uncached prefill fusion remain enabled.
  • Exact executable sources were validated on lb8 (gfx1151, ROCm 7.2.4, HIP graphs on): 20/20 new replay cases, 80/80 saved-row cases, 451/451 server tests, feature gate, and two unchanged 7,680-token adaptive sparse F16 retrieval requests. Both responses match the known-good output byte for byte. This does not imply universal byte identity.
  • No profiling experiments or diagnostic switches are included. Long-context speed retention and matched normal/ROCprof validation remain pending; no fresh Vulkan-win claim is made.

Prior-head review — 2026-09-07

Previously reviewed head: 059086abbd7a80ce84ab4e048aeabcc97f089eb4. Main was integrated at that revision. This PR does not include changes unique to #667. The CI and review results in this section predate the new fix.

  • All four original correctness findings are fixed and have regression coverage: saved raw-row visibility, synchronization before parking, failed drafter-load cleanup, and rejection of unsupported DS4 split/PFlash configurations. Current main's paged/PFlash guard is preserved.
  • CI passed on that head: Strix Halo, R9700, DGX GB10, Linux build, Windows build, and workspace import smoke.
  • RTX 3090 GPU CI and speed profile fail before building because the NVIDIA driver is unavailable. These are acknowledged infrastructure exceptions, not test passes; no CI checks were disabled or bypassed.
  • Cubic completed its full prior-head review. Its two remaining P3 comments are performance follow-ups, with code-backed dispositions in their threads: heterogeneous direct-API compression batching and a smaller near-512 sort network. Neither is claimed implemented.

Rollout boundary

Keep DFLASH_DS4_SPARSE_DECODE_FLASH=0 (the default is off) until broader workload and performance qualification. The sparse example in server/docs/DS4.md remains an experimental opt-in. The prior head failed strict five-line formatting with sparse F16/F32 verification. The new cached-RoPE fix resolves that reproduced failure in two repeated F16 adaptive runs; it does not establish universal output identity or qualify every context/checkpoint.

AR and adaptive DSpark q<=4 with fused explicit attention both pass the same 7,680-token strict retrieval task and produce identical response hashes. This single-task result is not a universal byte-identity guarantee. PFlash is lossy, optional, and supported for monolithic serving only; do not enable it for true-context or exact-retrieval performance comparisons.

Previous full-suite verification (059086abb)

Clean ROCm 7.2.4 / gfx1151 build with HIP graphs enabled; 451/451 server tests; 80/80 saved-row GPU cases (q=1..5, 2K/30K compressed rows, F16/F32, mask/direct selector); feature-gate, ROCm-format, packed-indexer, dispatch and fused-combine checks; 8/8 paged-attention checks. The two-device HC subtest skips with only Strix exposed. No warning-free claim.

Final real-model PFlash/FlowKV lifecycle smoke: two successful HTTP requests with correct short answers, 2,578 target tokens → 1,288 effective tokens, one target park/restore cycle, and one compression-content cache hit on the second turn. This used exact prefill and request-scoped drafter residency. The initial harness assumed the whole-prompt log format and two fresh compressions; its post-run assertion was corrected by separately validating the actual multi-turn FlowKV route against both saved responses and lifecycle logs. No kernel/server failure occurred, and the temporary server was stopped afterward.

CI run · Matched runtime controls

Changes

  • Add opt-in sparse fused verifier attention with direct F16 KV at long context.
  • Add packed small-CM rocWMMA indexer dispatch for widths 2–5 on RDNA3.5/RDNA4; qualified bit-identical to the generic indexer.
  • Add DS4 PFlash with Qwen3-0.6B, cross-tokenizer conversion and drafter residency lifecycle.
  • Correct batched verifier visibility masks and preserve overwritten raw rows.

Historical measurements — before the current main integration

The original branch reported 122,879 prompt tokens, 256 output tokens and six experts: client decode 17.513 → 22.513 tok/s (+28.6%), unchanged output SHA, zero swap. The cited Vulkan reference was 32.60 tok/s. These are historical results, not remeasured current-head performance and not a parity claim.

The original PFlash smoke reported 8,192 target tokens → 1,739 effective tokens with 2.18 s Qwen scoring. Compression changes effective context and must not be used to claim matched-context speed.

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 10 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread server/src/deepseek4/deepseek4_backend.cpp Outdated
Comment thread server/src/deepseek4/deepseek4_fused_verify.inc
Comment thread server/src/deepseek4/deepseek4_backend.cpp
Comment thread server/src/common/model_capabilities.h Outdated
@davide221

Copy link
Copy Markdown
Contributor Author

@cubic-dev-ai Please re-review the full current PR diff against main, especially the four findings from your previous review. Review only; do not generate or push fixes. Your check on the merge commit explicitly skipped AI review.

The updated branch preserves current main and adds these corrections:

  • Saved raw KV rows participate in exact indexed verification; the existing causal mask controls visibility. Unsupported coarse batched selection retains explicit attention.
  • Target, speculative, and expert backends synchronize before parking releases their buffers.
  • Failed PFlash loads release partially initialized drafter state before target restoration.
  • DS4 layer-split PFlash is rejected; current main's paged-serving/PFlash guard is retained and tested.

Fresh lucebox8 verification of the current code (ROCm 7.2.4, gfx1151, HIP graphs enabled): clean build; 451/451 server tests; 80/80 saved-row cases; feature-gate, dispatch, fused-combine, and ROCm format tests pass; 8/8 paged-attention tests pass. The latest commit only documents the supported PFlash placements.

The PR remains draft. Fresh GitHub CI is running. A real-model adaptive retrieval smoke recovered the expected values but failed the strict five-line formatting check; that result is not counted as a pass, and an AR control is being checked. No new Vulkan speed or universal byte-identity claim is being made.

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai Please re-review the full current PR diff against main, especially the four findings from your previous review. Review only; do not generate or push fixes. Your check on the merge commit explicitly skipped AI review.

The updated branch preserves current main and adds these corrections:

  • Saved raw KV rows participate in exact indexed verification; the existing causal mask controls visibility. Unsupported coarse batched selection retains explicit attention.
    ...

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

@davide221

Copy link
Copy Markdown
Contributor Author

Current-head runtime qualification update (lucebox8, gfx1151, ROCm 7.2.4,
HIP graphs on; exact executable sources of 059086abb, built before its
docs-only final commit):

Same 7,680-token strict retrieval task Result
AR, sparse decode off PASS, 5/5 and correct five-line format
Adaptive DSpark q<=4, fused explicit verification, native F16 KV PASS, same response SHA256 as AR
Adaptive DSpark, fused sparse verification, native F16 KV FAIL format: all five values present on one line
Adaptive DSpark, fused sparse verification, F32 KV Same failure and response SHA256 as sparse F16

The strict validator was not weakened. This isolates the difference to the
sparse path on this prompt, not specifically native F16. It does not establish
the difference's root cause or general model quality. Sparse verification is
already opt-in/default-off; keep it disabled for production qualification.
These smokes are not a Vulkan benchmark or a universal byte-identity claim.

Fresh local unit/kernel suites still pass, including 80 saved-row cases and
451 server tests. DGX GB10 CI passed. RTX GPU CI and speed profile stop at
nvidia-smi before building: unavailable NVIDIA driver, exit 9. Other CI and
the requested Cubic full-diff review are still pending/running.

The fixes are pushed and current main is integrated; the PR remains draft and
has not been merged or deployed.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 13 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread server/src/deepseek4/deepseek4_backend.cpp
Comment thread server/deps/llama.cpp/ggml/src/ggml-cuda/fattn.cu
@davide221
davide221 marked this pull request as ready for review September 7, 2026 16:46

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 13 files

Requires human review: Auto-approval blocked by 2 unresolved issues from a previous review of this commit.

Re-trigger cubic

Bind fused attention rotations to runtime positions during graph replay. Retain standalone forward-Q RoPE in cached decode and verifier graphs while preserving sparse flash attention, F16 KV, fused inverse RoPE, and uncached prefill fusion.

Add width 1-5 F16/F32 replay and standalone-RoPE regression coverage, plus invalid-position contract checks. Document the narrow correctness qualification without claiming universal identity or revalidated long-context performance.
@davide221

Copy link
Copy Markdown
Contributor Author

@cubic please review the new bug-fix commit cbeea1f78925578be37e3d6b8d00e37378294786, especially the runtime RoPE input contract, graph-replay position binding, and cached forward-Q fusion guard. Profiling and diagnostic code are excluded.

The fix:

  • Bind fused inverse RoPE to the existing runtime I32 position tensor, rather than a build-time kv_start reused at later positions.
  • Keep forward-Q RoPE standalone in cached decode/verification. Matched-row explicit attention and then a forward-Q-only fallback both restored the failing strict retrieval output. Sparse flash attention, native F16 KV, fused inverse RoPE, adaptive DSpark, and uncached prefill fusion remain enabled.

Validation on lucebox8, Strix Halo gfx1151, ROCm 7.2.4, HIP graphs enabled:

  • GPU replay regression: 0/20 before, 20/20 after; widths 1–5, F16/F32 KV, positions 7680/7688/131072/7680 on one graph. Correctly positioned static graphs match exactly; standalone RoPE matches within 2e-5. Malformed runtime-position inputs are rejected.
  • DS4 suite passes, including 80/80 saved-row cases; server suite 451/451; feature gate passes. The two-device HC subtest skips with only Strix exposed.
  • Two repeated, unchanged 7,680-token strict five-key retrieval requests pass with adaptive DSpark, all six experts, sparse F16 verification, and caches off. Both outputs exactly match the good control SHA256 df780faf7676ccc768737cb23d0534555c6e4785382a7f225d4842e82dcffb87.
  • All executable-source hashes match the lb8 tested build. Binary SHA256: 68de1ac35307a472945197d5068e692c16ca2542c33812c1b5677326f6c31656.

This fixes the reproduced failure, not a proof of universal byte identity. The first internal numerical divergence in fused forward-Q RoPE has not been localized; the guarded path is the tested fix. Sparse verification remains opt-in. Full 128K performance retention and a matched normal/ROCprof comparison remain unverified. The previous green CI/Cubic results belong to 059086abb, not this new commit; fresh checks/review are required. No merge or deployment is requested by this comment.

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

@cubic please review the new bug-fix commit cbeea1f78925578be37e3d6b8d00e37378294786, especially the runtime RoPE input contract, graph-replay position binding, and cached forward-Q fusion guard. Profiling and diagnostic code are excluded.

The fix:

  • Bind fused inverse RoPE to the existing runtime I32 position tensor, rather than a build-time kv_start reused at later positions.
    ...

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

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 15 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/docs/DS4.md">

<violation number="1" location="server/docs/DS4.md:565">
P3: The sparse-flash paragraph implies the switch to sparse top-k attention applies generically per layer, but the code only enables it for lane_q==1 or ratio==4 (deepseek4_fused_verify.inc). For batched verification lanes with other ratios, the verifier stays on explicit attention regardless of row savings. Restrict the doc to say the switch applies only in single-lane or ratio-4 layouts, since saved raw rows force explicit attention for other batched lanes.</violation>
</file>

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

<violation number="1" location="server/src/deepseek4/deepseek4_graph.cpp:2359">
P1: When cached verification wraps and preserved raw suffix rows are appended, this condition still enables the sparse flash path. Keep that batched layout on explicit attention, and enable sparse attention only when no preserved-row suffix is present.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread server/src/deepseek4/deepseek4_graph.cpp
Comment thread server/docs/DS4.md Outdated
Keep raw-row snapshots aligned with the current ring indices on cached graph replay, without disabling sparse F16 attention. Add 72 exact replay checks over widths 2-5 and F16/F32 storage, including ring wraps and 128K positions. Clarify the sparse verifier layout qualification.
@davide221

Copy link
Copy Markdown
Contributor Author

Updated #664 with current main's #716 disk-prefix-cache integration and a targeted cached-verifier replay fix. No sparse-attention default was changed.

@cubic-dev-ai please review the new head, especially the preserved-row report and the new regression.

The saved suffix was already included by deepseek4_indexed_attention_rows() and respected by both attention selector paths (80/80 existing GPU cases pass). I reproduced a separate problem in how that suffix was captured: its physical row views used graph-build kv_start, while cached verification replays at later ring positions. Changing only F16 dispatch or disabling sparse attention would not repair that snapshot in every cached verifier path.

The fix gathers the few overwritten rows using runtime I32 indices corresponding to the I64 ring-write indices, before the writes execute. It preserves the cache dtype and retains sparse attention, native F16 KV, packed indexing, and adaptive verification. It does not add per-ring-position graph-cache entries or convert the entire KV cache.

Fresh lucebox8 / gfx1151 / ROCm 7.2.4 / HIP-graphs-ON verification:

  • New exact saved-row replay regression, widths 2–5, F16/F32, wrap positions and 128K positions: 24/72 before → 72/72 after. The old implementation was extracted without changing its static row-selection behavior for the failing baseline.
  • Existing saved-row attention: 80/80, F16/F32, 2K/30K compressed rows, mask/direct selector paths.
  • RoPE replay: 20/20.
  • Full DS4 suite: pass, including main's new snapshot/disk round-trip checks.
  • Server tests: 453/453; feature gate: pass; sequence engine: 15 checks; batch plan: 62 checks; paged KV: 25/25.
  • Local/remote executable-source hashes match; clean scoped diff.

The sparse-verifier docs now explicitly limit batched sparse selection to ratio-4 layouts. Coarse-selection batched layouts remain explicit. The two earlier P3 performance follow-ups retain their previously documented non-blocking disposition; neither is claimed implemented here.

This is correctness qualification, not a new throughput claim. The earlier 8K off/on measurements belong to the previous head. The 123K baseline was stopped after memory pressure and swapping; no long-context result is claimed. The PR is not merged, and fresh CI/review must be assessed on this head.

Artifacts: /data/pr664-merge-ready-20260908.tr05W6/ on lucebox8, with the original red source archive, fail-before/pass-after logs, and matching build.

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Updated #664 with current main's #716 disk-prefix-cache integration and a targeted cached-verifier replay fix. No sparse-attention default was changed.

@cubic-dev-ai please review the new head, especially the preserved-row report and the new regression.

The saved suffix was already included by deepseek4_indexed_attention_rows() and respected by both attention selector paths (80/80 existing GPU cases pass). I reproduced a separate problem in how that suffix was captured: its physical row views used graph-build kv_start, while cached verification replays at later ring positions. Changing only F16 dispatch or disabling sparse attention would not repair that snapshot in every cached verifier path.
...

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

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 15 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_backend.cpp">

<violation number="1" location="server/src/deepseek4/deepseek4_backend.cpp:2986">
P2: When a `compress` command arrives through the daemon/string protocol, `handle_compress` builds `CompressRequest` without setting `drafter_gpu` or `residency_action`, so the ~1.5 GB PFlash drafter always loads on GPU 0 and stays resident (`KeepLoaded`) until the next park/free_drafter. The new typed `compress_batch` path honors `ReleaseAfterUse` and `drafter_gpu`; the string path cannot express either, so request-scoped residency and multi-GPU drafter placement are silently unavailable over the daemon protocol.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread server/src/deepseek4/deepseek4_graph.cpp
Comment thread server/docs/DS4.md Outdated
Comment thread server/src/deepseek4/deepseek4_backend.cpp Outdated
Comment thread server/src/deepseek4/deepseek4_backend.cpp
Comment thread server/docs/DS4.md Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 15 files

Requires human review: Auto-approval blocked by 5 unresolved issues from previous reviews.
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.

Re-trigger cubic

Comment thread server/src/deepseek4/deepseek4_backend.cpp
Comment thread server/src/deepseek4/deepseek4_backend.cpp
Comment thread server/tests/test_deepseek4_unit.cpp
@davide221

Copy link
Copy Markdown
Contributor Author

Pushed 13648dba4528ad592f85a766cde4cfc0ff6fb581 to address the new review findings. @cubic please re-review this revision, especially the mask-suffix binding and PFlash input validation.

  • Indexer visibility: the production scorer now slices the per-token mask alongside the scored query/position suffix. The unchanged pass-through behavior fails four of five exact mask-offset cases; the fix passes 5/5, including the no-slice case. Null masks stay null, and first_scored == 0 adds no graph operation. The test checks values, shape, and contiguous layout.
  • PFlash ratio validation: one admission predicate is used both before loading and before scoring. It accepts finite [0,1] (zero keeps the scorer's minimum chunk), and rejects NaN/infinities. The new admission test reproduces both zero rejection and NaN acceptance before the fix, then passes all eight boundary cases.
  • Docs: the sparse export is now visibly commented-out/experimental in the example. PFlash has its own top-level section after the complete DSpark documentation.
  • Legacy daemon finding: documented rather than silently changing the shared wire protocol. compress <file> <keep-x1000> <drafter> [nopark] has no device/residency fields in the existing common daemon contract. DS4's HTTP route directly calls the typed API, which already honors both controls; it does not serialize that request through this legacy line parser. The new parser contract test checks tokens, zero ratio, paths with spaces, nopark, backend-local GPU 0, KeepLoaded, and emitted output. GPU placement/request-scoped residency should use HTTP or the typed API; a versioned cross-backend wire extension is separate work.

New lb8/gfx1151 validation: full DS4 suite passes, including the new cases, 72/72 saved-row replay, 80/80 saved-row attention, and 20/20 RoPE replay. Server tests 453/453, feature gate 1/1. Clean scoped diff and matching local/remote executable-source hashes. Binary SHA256: 08d17bebc4c1f4ee4c5771cadf8351bb2544a598426d4661976a2ab5206e1b51.

The previous revision's 8K live adaptive sparse F16 retrieval passed 5/5 with the known-good response hash; the final revision is being checked separately. The attempted 123K request stopped at the conservative cumulative swap-out guard (35,949 pages, about 140 MiB; about 11 GiB still available), while a limited CPU test build was also running. It is not a valid long-context correctness or performance result and is not an OOM finding. No new Vulkan or long-context speed claim is made.

Fresh CI/review must be assessed on this new head. The RTX 3090 runner is confirmed offline; no checks were disabled. No merge or deployment performed.

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Pushed 13648dba4528ad592f85a766cde4cfc0ff6fb581 to address the new review findings. @cubic please re-review this revision, especially the mask-suffix binding and PFlash input validation.

  • Indexer visibility: the production scorer now slices the per-token mask alongside the scored query/position suffix. The unchanged pass-through behavior fails four of five exact mask-offset cases; the fix passes 5/5, including the no-slice case. Null masks stay null, and first_scored == 0 adds no graph operation. The test checks values, shape, and contiguous layout.
  • PFlash ratio validation: one admission predicate is used both before loading and before scoring. It accepts finite [0,1] (zero keeps the scorer's minimum chunk), and rejects NaN/infinities. The new admission test reproduces both zero rejection and NaN acceptance before the fix, then passes all eight boundary cases.
  • Docs: the sparse export is now visibly commented-out/experimental in the example. PFlash has its own top-level section after the complete DSpark documentation.
    ...

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

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 15 files

You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.

Re-trigger cubic

Comment thread server/src/deepseek4/deepseek4_graph.cpp
Comment thread server/src/deepseek4/deepseek4_internal.h
@davide221

Copy link
Copy Markdown
Contributor Author

Final revision: 13648dba4528ad592f85a766cde4cfc0ff6fb581, conflict-free. All review discussions have been inspected and resolved with fixes or explicit dispositions; no merge performed.

Validation summary:

  • Full lb8 DS4 suite passes, including mask suffix 5/5, saved-row replay 72/72, saved-row attention 80/80, RoPE replay 20/20, and the new PFlash boundary/parser tests. Server 453/453, feature gate 1/1.
  • Final-head adaptive DSpark + sparse F16 8K retrieval: 5/5, 59 output tokens, the known-good response hash. Native F16, packed indexing, and sparse verification remain available; their rollout defaults are unchanged.
  • Linux, Windows, Strix, DGX, workspace CI pass on the final head.
  • R9700 CI passes DS4 and 34 other selected suites, but the global free-VRAM check fails on the occupied runner. Read-only inspection found an unrelated Qwen server using ~24.5 GB and 100% GPU. The unchanged host-side registry allocation test passes five times on lb8's idle R9700 with zero-byte delta each time. No test threshold or check was disabled. GitHub refuses the scoped rerun while this workflow remains active; RTX3090 is offline.
  • Cubic completed the final-head review (COMMENTED, not formal approval). Its last two comments were resolved with code evidence: the sparse layout is qualified for ratio 4 and preserves/masks the saved suffix; the cited header comment belongs to indexed attention, not the adjacent raw-row gather helper. Previous legitimate correctness findings have fail-before/pass-after tests.

The 123K attempts stopped at their swap guards, including the final isolated attempt; none produced a valid long-context result. This is not a fresh long-context performance or Vulkan-parity qualification. Sparse verification remains experimental/opt-in. All temporary test servers are stopped; other people's workloads were not interrupted.

Artifacts remain in /data/pr664-merge-ready-20260908.tr05W6/ on lb8 and /private/tmp/pr664-merge-ready-20260908.Fr1rLT/ locally.

@davide221
davide221 merged commit 93bd925 into main Sep 8, 2026
6 of 8 checks passed
@davide221
davide221 deleted the codex/ds4-pflash-long-context branch September 18, 2026 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant