Skip to content

feat(server): carry drafter feature ring in concurrent prefix checkpoints - #730

Open
Graffioh wants to merge 1 commit into
Luce-Org:mainfrom
Graffioh:devin/prefix-cache-qwen-spec
Open

Graffioh wants to merge 1 commit into
Luce-Org:mainfrom
Graffioh:devin/prefix-cache-qwen-spec

Conversation

@Graffioh

@Graffioh Graffioh commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Speculative continuous batching rebuilds the DFlash draft K/V window from each sequence slot's target feature ring. Restoring only attention K/V and recurrent state leaves that ring stale after slot reuse, reducing draft acceptance.

This change adds an optional feature-ring slab to paged prefix snapshots and includes it in the resident-byte estimate. Qwen35SeqEngine owns feature eligibility and resets draft state on admission, stale-restore fallback, recompute, and retirement. Featureless restores mask out unavailable history until the current sequence rebuilds it; captures can include features again after the entire ring has been overwritten. The scheduler and model-neutral PrefixStore contract remain unchanged.

Snapshot validation checks the feature tensor's dimensions, layout, capacity, and width before transferring data. DFLASH_PREFIX_NO_FEAT=1 remains a documented debug control for exercising featureless restores; =0 leaves feature payloads enabled.

Validation on the rebased branch:

  • HIP build: dflash_server and all four targeted test binaries succeeded.
  • test_recurrent_snapshot: 6/6 passed, including short-prefix copying, byte accounting, slot/dead-row isolation, malformed feature dimensions, and atomic replacement failure.
  • test_seq_engine_contract: 33 checks; test_seq_slot_manager: 306 checks; test_parallel_prefix_txn: 29 checks.
  • Qwen3.8-27B + GGUF DFlash2 smoke test on gfx1151, with --max-concurrency 2: cold request, unrelated request reusing the slot, then prefix restore. Repeated with DFLASH_PREFIX_NO_FEAT=0 and =1; logs confirmed a 982-token restore with feat=1 and feat=0, respectively. Greedy responses matched the cold request and each other.

Rebased onto origin/main at f6c54171. The new recompute path uses the same draft-state reset as admission and retirement. No new performance benchmark is claimed.

@Graffioh
Graffioh force-pushed the devin/prefix-cache-qwen-spec branch from bb1ab3b to 34271ff Compare September 14, 2026 08:27
@Graffioh
Graffioh marked this pull request as ready for review September 14, 2026 08:31

@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 6 files

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

Re-trigger cubic

Comment thread server/src/qwen35/concurrency/qwen35_seq_engine.cpp
@Graffioh

Graffioh commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the featureless-restore finding (squashed into 1fd73fa0).

Reachability: today a spec engine can never produce a featureless checkpoint — fixed_chain_ready_ requires cache.target_feat non-null, so every capture_prefix under speculation carries the slab. The path only exists for restore-side compatibility, but the defensive fix is now explicit rather than implied.

Fix (instead of full AR fallback): a per-slot slot_ring_valid_from_ floor records the restore cut when a featureless checkpoint lands. prepare_chain_drafts seeds next_pos past it, so draft_kv_begin_step bulk-appends only rows the occupying sequence wrote itself — foreign rows keep slot_pos = -1 and stay masked out of the draft context. Speculation keeps running on an honest (if short) context rather than burning draft compute on poisoned proposals, which is strictly better than AR fallback for prompts where recent context suffices.

Two extra pieces:

  • capture_prefix on such a slot now omits the feature slab too — foreign rows must not be blessed into a feat=1 checkpoint that would propagate staleness to downstream restores.
  • DFLASH_PREFIX_NO_FEAT=1 is kept as a permanent bench/debug knob (drops the payload from captures). It exercises this cold-ring path and measures the slab's contribution to acceptance.

E2E verified on Qwen3.8-27B + DFlash2: with the flag set, restores log feat=0, output stays byte-identical to the feat run, and decode runs on the self-written suffix (~17 tok/s on the prefix-dependent echo workload vs 61 tok/s warm — expected since the needed context is deliberately in the untrusted region).

@Graffioh
Graffioh force-pushed the devin/prefix-cache-qwen-spec branch from c508ec7 to 1fd73fa Compare September 14, 2026 11:01

@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 3 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread server/src/qwen35/concurrency/qwen35_seq_engine.cpp Outdated
@Graffioh
Graffioh force-pushed the devin/prefix-cache-qwen-spec branch from 1fd73fa to 7648998 Compare September 14, 2026 11:33
@Graffioh

Graffioh commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the capture-suppression expiry in 7648998e.

Valid point — the floor is now self-expiring rather than lifetime. Ring rows are keyed by position % target_feat_cap, so the last foreign row (floor - 1) is overwritten once the sequence writes cap positions past the restore cut. capture_prefix now carries the slab when checkpoint.tokens >= ring_floor + target_feat_cap.

The floor itself stays in place after expiry — next_pos has already advanced past it, so the prepare_chain_drafts clamp is a no-op, and the record remains accurate as "this slot once restored without features".

Verified: rebuild clean, all tests pass, normal path e2e unchanged (feat=1 captures at 218.7MB, byte-identical output at ~61 tok/s). The expiry boundary itself can't be reached e2e without a 4k+ token generation on a floored slot — and floored slots only arise under DFLASH_PREFIX_NO_FEAT, which suppresses captures unconditionally — so it's verified by inspection against the ring indexing (position % cap overwrite semantics).

@Graffioh

Copy link
Copy Markdown
Contributor Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@Graffioh 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 6 files

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

Re-trigger cubic

Comment thread server/docs/PREFIX_CACHE.md Outdated
Comment thread server/src/qwen35/concurrency/qwen35_seq_engine.cpp
Comment thread server/src/qwen35/concurrency/qwen35_seq_engine.cpp Outdated
@Graffioh
Graffioh force-pushed the devin/prefix-cache-qwen-spec branch from 7648998 to 44bc61c Compare September 15, 2026 08:21
…ints

The merged concurrent prefix cache restores paged attention K/V and
recurrent slabs but drops the slot's target_feat ring — the features a
fixed-chain DFlash drafter rebuilds its K/V window from. A restored
prefix left the drafter proposing from stale context: output stayed
correct under greedy verification but acceptance collapsed (measured
2.3 vs 8.0 accepted/round, 18 vs 61 tok/s on Qwen3.8-27B + DFlash2
after a poisoned slot).

- Paged snapshots gain an optional verbatim feature-slab payload:
  speculating engines (fixed_chain_ready_) carry it, AR engines stay
  lean, and the resident-byte estimate charges for it.
- paged_snapshot_matches validates payload compatibility and rejects
  feature-bearing checkpoints into ring-less caches.
- Per-slot draft K/V state resets on every slot-reuse path so stale
  next_pos/slot_pos cannot leak into a new sequence's drafts.
- A featureless restore (defensive path) floors the ring: the drafter
  only bulk-appends rows the occupying sequence wrote itself, and later
  captures on that slot omit the slab rather than bless foreign rows.
- DFLASH_PREFIX_NO_FEAT=1 drops the payload (bench/debug knob).

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@Graffioh
Graffioh force-pushed the devin/prefix-cache-qwen-spec branch from 44bc61c to a72ac37 Compare September 18, 2026 07:20
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