Skip to content

fix: include recurrent/hybrid models in checkpoint-creation gate (hydra_vortex #316)#5

Merged
ddvnguyen merged 1 commit into
hydra-forkfrom
fork/hydra-316-checkpoint-recurrent-gate
Jun 21, 2026
Merged

fix: include recurrent/hybrid models in checkpoint-creation gate (hydra_vortex #316)#5
ddvnguyen merged 1 commit into
hydra-forkfrom
fork/hydra-316-checkpoint-recurrent-gate

Conversation

@ddvnguyen

Copy link
Copy Markdown
Owner

Implements the C++ side of ddvnguyen/hydra_vortex#316.

What & why

do_checkpoint in tools/server/server-context.cpp was gated on
ctx_tgt_seq_rm_type == FULL || == RS || n_swa > 0 || rpc_port > 0. For the
hybrid qwen35moe arch, common_context_can_seq_rm()'s generic probe reports
PART (llama_n_rs_seq() == 0, the 2-token smoke-test removal succeeds), and
there's no SWA, so checkpoint creation depended entirely on rpc_port > 0 — a
flag that's only set for Hydra's cross-node migration RPC. Any standalone
server with no RPC peer never created a checkpoint.

The checkpoint search path (a few hundred lines below, the ik_llama.cpp#1762
port) already special-cases is_rec = llama_model_is_recurrent() || llama_model_is_hybrid() for matching/restoring checkpoints — but with none
ever created, the search always came up empty and do_reset was always true,
forcing a full prompt re-process on every request, including exact repeats.

Fix: add the same is_rec condition to the creation gate, so recurrent/hybrid
models get checkpoints on their own merits rather than as a side effect of
rpc_port.

Verification (RTX 5060 Ti, Qwopus3.6-35B-A3B-v1-APEX-I-Balanced.gguf)

Repro: 3 consecutive identical chat-completion requests, same content, max_tokens=5,
temperature=0.

Build Flags Call 1 Call 2 Call 3
before (c357ad2) minimal (no --rpc-port) 0/35 cached 0/35 cached 0/35 cached
before (c357ad2) + --rpc-port only 0/35 cached 31/35 cached 31/35 cached
after (this branch) minimal (no --rpc-port) 0/35 cached 31/35 cached 31/35 cached

Also confirmed against the full production hydra-head flag set (kv-unified,
cache-reuse, cache-type-k/v q8_0, flash-attn, etc., minus --rpc-port) — same
0% before / fixed after pattern, isolating rpc_port/is_rec as the only
variable that matters here.

Test plan

  • cmake --build build_sm120 --target llama-server — OK
  • cmake --build build_sm60 --target llama-server — OK
  • Live repro on RTX 5060 Ti against Qwopus3.6-35B-A3B (qwen35moe, hybrid arch) — cache hit restored without --rpc-port

do_checkpoint only allowed FULL/RS seq_rm types, n_swa>0, or rpc_port>0.
For hybrid models (e.g. qwen35moe) the generic seq_rm probe reports PART
(llama_n_rs_seq == 0, smoke-test removal succeeds), so checkpoint
creation was gated entirely on rpc_port - a standalone server with no
RPC peer never created one. The checkpoint search path a few hundred
lines down already special-cases is_rec (ik_llama.cpp#1762 port) but
had nothing to find, so every request forced a full re-prefill (0%
cache hit on identical repeats). Add the same is_rec condition to the
creation gate.

Implements the C++ side of ddvnguyen/hydra_vortex#316.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@hydra-z hydra-z Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review — approved, with one P2 polish finding (filed as ggml-org#321)

Correctness — ✅ fix is correct.

The patch adds the same is_rec = llama_model_is_recurrent(model_tgt) || llama_model_is_hybrid(model_tgt) condition already used by the checkpoint search path (line 4006-4007) to the creation gate. This restores the invariant the search path was relying on: a checkpoint can only be found if it can be created. Without this, the ik_llama.cpp#1762 port below silently did nothing on standalone invocations because the slot's prompt.checkpoints was perpetually empty.

Risk — ✅ low / additive.

  • Monotonic-additive OR clause: non-hybrid models are completely unaffected. Models already producing checkpoints via FULL/RS/n_swa>0/rpc_port>0 keep the same behaviour.
  • Verified model_tgt is the right handle (line 680: llama_model * model_tgt = nullptr;, set at line 949: model_tgt = llama_init->model();). The llama_model_is_recurrent / llama_model_is_hybrid pair is already used twice elsewhere in this same function, so the public API is exercised.

Test evidence — ✅ convincing.

The A/B matrix in the PR body is the right shape: 3×3 (build × rpc-port × call), isolating rpc_port/is_rec as the only variable that matters. Bisection against the full production flag set (kv-unified, cache-reuse, cache-type-k/v q8_0, flash-attn) minus --rpc-port and observing the same 0%-then-fixed pattern is a strong null-hypothesis control. The "production hydra-head is unaffected because it always sets --rpc-port" disclosure in the parent PR is honest and worth keeping — it prevents the wrong urgency assignment on the merge.

Cross-repo coordination — ✅ on track.

  • mergeStateStatus: CLEAN here, no CI checks pending.
  • The Hydra parent PR (ggml-org#320) explicitly gates its own merge on this PR being merged first. That's the correct workflow per docs/workflow/08-llama-fork.md.

One P2 finding — filed as hydra_vortex#321

The is_rec expression is now duplicated 3x in the same function (update_slots):

Line Purpose
4006–4007 Checkpoint search path (pre-existing)
4162–4163 Checkpoint creation gate (this PR)
4330 eff_checkpoint_min_step (pre-existing)

A small refactor (a const bool is_rec = ...; at the top of the function, or an is_rec() member on server_context_impl) would make "what counts as recurrent/hybrid" a single source of truth. Risk of 3-way drift is low per change but high blast radius if a new arch lands that's recurrent-like but not exactly recurrent. Not blocking this PR — the fix is correct as-is — but worth a quick follow-up.

Verdict: ship it once ggml-org#320's merge-gate (fork PR #5 reaches hydra-fork) is satisfied.

@ddvnguyen ddvnguyen merged commit f518cff into hydra-fork Jun 21, 2026
ddvnguyen pushed a commit to ddvnguyen/hydra_vortex that referenced this pull request Jun 21, 2026
PR ddvnguyen/llama.cpp#5 was merged via merge commit (not squash),
advancing hydra-fork to f518cff1. The original submodule bump in
0b30575 pointed at the PR head (dc8b56962), which is reachable via
ancestry but no longer the branch tip - the strict
'git ls-remote | grep' check in 04-commit-pr.md / 05-deploy.md would
fail. Bump to the merge commit to satisfy the check.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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