Refresh llama.cpp upstream patch queue - #1085
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThis change refreshes the embedded llama.cpp patch queue with Skippy staged-runtime APIs, GLM-DSA graph execution, native MTP support, runtime loading controls, Metal GLM-MoE kernels, and expanded backend tests. ChangesSkippy staged runtime and GLM-DSA execution
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
This pull request is currently a draft. Reviews will not take place until the PR is marked as ready for review. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (15)
third_party/llama.cpp/patches/0021-ggml-add-GLM-DSA-sparse-execution-primitives.patch (2)
6595-6607: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftThread-count clamp is a partial workaround for a documented correctness bug, not a fix.
The comment states the 256-thread sparse-attn kernel "can leave rows unwritten" on Apple Metal for large-top-k prefill shapes, and this function only clamps threads for
n_batch > 1 && n_top_k >= 64orn_top_k > 512. If the underlying kernel defect also manifests for shapes outside these two conditions (e.g. decode with very specific top_k boundaries not yet observed), rows would silently go unwritten with no detection. Given this is a data-correctness hazard silently masked by empirical shape-based clamping rather than fixed at the root (buffer/thread indexing bug in the 256-threaddsa_sparse_attnkernel), please confirm the coverage is exhaustive or file a tracked issue to fix the root cause.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0021-ggml-add-GLM-DSA-sparse-execution-primitives.patch` around lines 6595 - 6607, Replace the shape-based workaround in ggml_metal_glm_dsa_sparse_attn_threads_for_shape with a root-cause fix in the 256-thread dsa_sparse_attn kernel’s buffer/thread indexing so no rows can remain unwritten for any shape. If the kernel fix cannot be made here, document the exhaustive coverage assumptions and create a tracked issue for the underlying correctness defect rather than relying on these partial clamps.
32-37: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
RPC_PROTO_PATCH_VERSIONnot bumped despite the assert's own instruction.
GGML_OP_COUNTis bumped 101→107, butRPC_PROTO_PATCH_VERSION(line 32) stays at3. The verystatic_assertbeing edited says to "updateRPC_PROTO_PATCH_VERSION" wheneverGGML_OP_COUNTchanges — six new ops (DSA_SPARSE_MASK/ATTN/TOP1_ATTN,MOE_ROUTE_WEIGHTS/WEIGHTED_SUM/MUL_MAT_ID) were added without bumping the protocol version, so RPC clients/servers built against different op sets would report as protocol-compatible.🔧 Proposed fix
-#define RPC_PROTO_PATCH_VERSION 3 +#define RPC_PROTO_PATCH_VERSION 4🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0021-ggml-add-GLM-DSA-sparse-execution-primitives.patch` around lines 32 - 37, Update RPC_PROTO_PATCH_VERSION alongside the GGML_OP_COUNT change in the static assertion block, bumping it from 3 to the next protocol version required for the six newly added operations. Keep the GGML_OP_COUNT assertion at 107 and ensure the client/server compatibility marker reflects the changed operation set.third_party/llama.cpp/patches/0022-skippy-expose-GLM-DSA-staged-runtime-controls.patch (2)
2599-2602: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winUnguarded
get_gf_res_prev()dereference can crash KV page import.Every other call site in this patch treats
get_gf_res_prev()as nullable (res != nullptr ? res->get_skippy_glm_dsa_top_k() : nullptrat lines 2068-2069 and 2441-2442), but here it is dereferenced directly.skippy_import_kv_pagecan legitimately be called before any decode has built a graph result.🛡️ Proposed guard
- session->ctx->get_gf_res_prev()->reset(); + if (llm_graph_result * res_prev = session->ctx->get_gf_res_prev()) { + res_prev->reset(); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0022-skippy-expose-GLM-DSA-staged-runtime-controls.patch` around lines 2599 - 2602, Guard the get_gf_res_prev() result in skippy_import_kv_page before calling reset(), since it may be null before the first decode builds a graph result. Preserve the reset behavior when a previous graph result exists.
2437-2439: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAlign GLM-DSA sideband storage with
request_countin the batched frame path.
glm_dsa_top_k_storage/offsets/bytes_per_requestare sized byrequest_count, the top-k tensor is validated as request-majorrequest_count, and the per-request output copy usesrequest_idx == iin this path. Userequest_countfor the input sideband storage/scan or document/assert thatrequest_count == n_tokenshere.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0022-skippy-expose-GLM-DSA-staged-runtime-controls.patch` around lines 2437 - 2439, Align the GLM-DSA sideband input storage and scan in the batched frame path with request_count, matching glm_dsa_top_k_storage, glm_dsa_top_k_offsets, glm_dsa_top_k_bytes_per_request, and the request-major tensor validation. Ensure per-request processing uses the same request_count indexing as the output copy, or add an explicit assertion/documented invariant that request_count equals n_tokens if the input path must remain token-sized.third_party/llama.cpp/patches/0026-Bump-Skippy-ABI-for-GLM-DSA-runtime-config.patch (1)
16-19: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPatch-level bump for a struct-layout change.
0022-skippy-expose-GLM-DSA-staged-runtime-controls.patchappends six new fields tostruct skippy_runtime_config(glm_dsa_policy_profile…glm_dsa_compact_flash_min_kv) plususe_mmap_prefetch/use_mmap_buffer, and adds a new activation flag. Any consumer compiled against patch 31 that passes a smaller struct by value/pointer will now be read out of bounds. Confirm that a minor bump (or an explicit size/versioned-struct handshake) isn't required here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0026-Bump-Skippy-ABI-for-GLM-DSA-runtime-config.patch` around lines 16 - 19, Update the ABI versioning around SKIPPY_ABI_VERSION_MAJOR, SKIPPY_ABI_VERSION_MINOR, and SKIPPY_ABI_VERSION_PATCH to use the required minor bump or an explicit struct-size/version handshake for the expanded skippy_runtime_config and new activation flag; do not leave this struct-layout change represented only by a patch-level increment.third_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patch (1)
6382-6384: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUnscoped
set_test_envcalls inbuild_graphleak global state into later test cases.These fixtures set process-wide env vars from
build_graph/initialize_tensorsand never restore them, so every subsequent test case in the same run inherits e.g.SKIPPY_GLM_DSA_EXPERIMENTAL_SELECTED_ROW_FLASH=1,LLAMA_GLM_DSA_EXPERIMENTAL_SELECTED_ROW_FLASH_TILED=1and the multihead/split tensor-name overrides. Results then depend on test ordering and filters. The patch already introducesscoped_test_envand uses it correctly ineval/eval_perfoverrides elsewhere — the same pattern should apply here (e.g. hold the scoped guards as fixture members, or set them ineval/eval_perfrather than in graph construction).Also applies to: 6483-6487, 6614-6617, 6703-6703, 6779-6780
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patch` around lines 6382 - 6384, Scope every environment override created by build_graph/initialize_tensors, including the listed GLM DSA selected-row, tiled, tensor-name, multihead, and split overrides, using the existing scoped_test_env pattern. Store guards as fixture members or move setup into eval/eval_perf, ensuring variables are restored before subsequent test cases while preserving each test’s intended configuration.third_party/llama.cpp/patches/0024-Support-GLM-DSA-fused-KV_B-tensors.patch (1)
77-83: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftFused-KV_B GLM-DSA checkpoints load successfully but can never execute. The loader now accepts models without split
wk_b/wv_band falls back to a fusedwkv_b, while the GLM-DSA graph constructor unconditionally rejects exactly that layout — so the failure surfaces as a runtime exception on the first decode instead of a clear load-time error.
third_party/llama.cpp/patches/0024-Support-GLM-DSA-fused-KV_B-tensors.patch#L77-L83: either keepwk_b/wv_brequired for GLM-DSA, or splitwkv_bintowk_b/wv_bat load so the graph precondition holds.third_party/llama.cpp/patches/0025-Run-GLM-DSA-through-IndexShare-graph.patch#L29-L33: if the fused layout is meant to be supported, replace thethrowwith a fused-wkv_bcode path; otherwise drop this check once the loader enforces the requirement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0024-Support-GLM-DSA-fused-KV_B-tensors.patch` around lines 77 - 83, Make fused KV_B handling consistent across both patches: in third_party/llama.cpp/patches/0024-Support-GLM-DSA-fused-KV_B-tensors.patch lines 77-83, either require split wk_b/wv_b for GLM-DSA or split wkv_b during loading; in third_party/llama.cpp/patches/0025-Run-GLM-DSA-through-IndexShare-graph.patch lines 29-33, if fused tensors remain supported, replace the graph constructor’s rejection with fused-wkv_b handling, otherwise remove the check after enforcing split tensors at load time.third_party/llama.cpp/patches/0040-Fix-GLM-DSA-native-MTP-execution.patch (1)
176-181: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the rope width for MTP nope-view byte offsets.
In
third_party/llama.cpp/patches/0040-Fix-GLM-DSA-native-MTP-execution.patch:176-181and:203-208,ggml_view_3d(..., ..., ..., 0)already selects thepeslab. The offset argument should therefore beggml_row_size(*->type, n_embd_indexer_head_rope), notn_embd_indexer_head_nope; otherwise Q/K nope slices start past the intended boundary when the rope size differs from the nope size.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0040-Fix-GLM-DSA-native-MTP-execution.patch` around lines 176 - 181, The MTP nope views use the nope width for their byte offset instead of the rope width. Update the offset argument in the ggml_view_3d calls for indexer_q_nope and the corresponding K-nope view to use ggml_row_size of the tensor type with n_embd_indexer_head_rope, while preserving the existing pe-slab selection and other dimensions.third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch (2)
184-195: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRoute every active-count branch through the matching active-specific factory.
Both sites introduce reduced active counts but still select the ordinary eight-slot pipeline, so active counts 2/4/6 have no runtime effect.
third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch#L184-L195: call the_active6,_active4, and_active2factories for the corresponding branches.third_party/llama.cpp/patches/0034-ggml-use-GLM-max-active-policy-for-Q3-down-kernels.patch#L75-L83: make the q3_active_count branches select those same active-specific factories.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch` around lines 184 - 195, Route each reduced active-count branch through its matching active-specific pipeline factory. In third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch lines 184-195, update the active6, active4, and active2 branches to call the corresponding _active6, _active4, and _active2 factory symbols; apply the same q3_active_count selection in third_party/llama.cpp/patches/0034-ggml-use-GLM-max-active-policy-for-Q3-down-kernels.patch lines 75-83. Leave the ordinary eight-slot factory only for the non-reduced active-count path.
22-49: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake the active paths compile the exact Metal kernels.
The active factory builds pipelines from
kernel_mul_mv_id_q3_K_weighted_reduce_slots_sg_r8_nb8_w0..., while the active Metal entry points arekernel_mul_mv_id_q3_K_wr_slots_r8_a6,...a4, and...a2. No aliases are added for these names, soactive6/active4/active2dispatches resolve the wrong kernel; use matching names, add[[host_name]]aliases, or rewrite the active dispatch to call the new entry points. Also applies to lines 252-298.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch` around lines 22 - 49, Update ggml_metal_library_get_pipeline_mul_mv_id_q3_weighted_reduce_slots_sg_r8_nb8_w0 and the corresponding active4/active2 factories so active6/active4/active2 dispatch resolves the exact Metal entry points kernel_mul_mv_id_q3_K_wr_slots_r8_a6, kernel_mul_mv_id_q3_K_wr_slots_r8_a4, and kernel_mul_mv_id_q3_K_wr_slots_r8_a2. Align the generated pipeline names, add matching host-name aliases, or route dispatch directly to those entry points; apply the same correction to the related factories in the later diff section.third_party/llama.cpp/patches/0003-Add-staged-sampling-checkpoints-and-part-loading.patch (2)
1178-1226: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTrim committed token history with the session.
skippy_trim_sessionremoves the KV suffix and lowersn_past, but leavestoken_historyintact. Subsequent sampled decoding applies repetition penalties to tokens that were trimmed away. Resize history when it exceedstoken_count.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0003-Add-staged-sampling-checkpoints-and-part-loading.patch` around lines 1178 - 1226, Update skippy_trim_session to resize session->token_history when it contains more than token_count entries, truncating it to the retained token count alongside the KV cache and n_past updates. Preserve existing validation and memory-trimming behavior.
491-530: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep sampler history synchronized after token verification.
Both verification paths commit tokens to the session but skip
skippy_record_tokens, so later sampled decoding uses incomplete repetition-penalty history.
third_party/llama.cpp/patches/0003-Add-staged-sampling-checkpoints-and-part-loading.patch#L491-L530: recordtoken_idsafter successful token-batch verification.third_party/llama.cpp/patches/0003-Add-staged-sampling-checkpoints-and-part-loading.patch#L607-L638: accept token IDs in the activation-frame helper and record them after successful verification.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0003-Add-staged-sampling-checkpoints-and-part-loading.patch` around lines 491 - 530, Keep sampler history synchronized in both verification paths: update skippy_verify_token_batch to call skippy_record_tokens(session, token_ids, token_count) after skippy_decode_batch succeeds, and update the activation-frame helper at third_party/llama.cpp/patches/0003-Add-staged-sampling-checkpoints-and-part-loading.patch lines 607-638 to accept token IDs and record them after successful verification.third_party/llama.cpp/patches/0002-Add-early-staged-model-family-and-chat-support.patch (2)
1258-1267: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not rescale activation input in non-first Gemma4 stages.
A preceding stage already emits the scaled hidden state. When
il_start > 0,build_inp_embd(nullptr)consumes that activation, then Line 1267 appliessqrt(n_embd)again. Gate this scaling to the first stage, as the Gemma/Gemma2/Gemma3 implementations do.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0002-Add-early-staged-model-family-and-chat-support.patch` around lines 1258 - 1267, Update the activation scaling in the staged input path near build_inp_embd so sqrtf(n_embd) is applied only when processing the first stage (il_start == 0). Preserve the existing token-dependent scaling for non-staged and first-stage inputs, while leaving preceding-stage hidden states unscaled.
1857-1866: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdvertise the implemented chat-template capability.
skippy_apply_chat_templateis public in this patch, butskippy_abi_features()omitsSKIPPY_FEATURE_CHAT_TEMPLATE. Clients following the ABI’s feature-probe contract will incorrectly disable chat templating.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0002-Add-early-staged-model-family-and-chat-support.patch` around lines 1857 - 1866, The feature mask returned by skippy_abi_features must advertise the implemented chat-template capability. Add SKIPPY_FEATURE_CHAT_TEMPLATE to the returned feature flags alongside the existing capabilities, preserving all currently advertised features.third_party/llama.cpp/patches/0001-Add-Skippy-ABI-and-package-writer-foundation.patch (1)
1854-1860: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPopulate
element_countinstead of always returning zero.The public tensor-info API reports
0for every tensor, despite parsed dimensions being available ininfo->tensors. Compute the product ofmeta.newith overflow checking.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0001-Add-Skippy-ABI-and-package-writer-foundation.patch` around lines 1854 - 1860, Update the tensor-info construction block that assigns out_tensor->element_count to compute the product of the parsed dimensions in info->tensors instead of always returning zero. Multiply meta.ne values with overflow checking, and preserve the existing success/error flow by reporting overflow through the established out_error mechanism.
🧹 Nitpick comments (2)
third_party/llama.cpp/patches/0022-skippy-expose-GLM-DSA-staged-runtime-controls.patch (1)
617-639: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBoth branches of
skippy_glm_dsa_sideband_top_k_count_is_validreturn the same expression.The
token_count > 1branch and the fall-through decode branch are identical (actual_top_k <= expected_top_k), so the split only adds dead code. Either collapse it to a single check or make the decode branch actually stricter if that was the intent.♻️ Collapse to one check
- // Prefill sidebands can be chunk-local: llama.cpp emits the actual - // per-chunk top-k width, while the receiving staged session may have a - // larger accumulated KV prefix. - if (token_count > 1) { - return actual_top_k <= expected_top_k; - } - - // Decode producers export the actual top-k tensor width from their graph. - // The receiving session can have a larger accumulated KV prefix than the - // producer's current sideband width, especially when an OpenAI generation - // loop advances through cached or fused decode steps. The sideband is still - // valid as long as it is non-empty, token-major, and does not claim more - // rows than the receiver can make visible. - return actual_top_k <= expected_top_k; + // Prefill and decode sidebands are both chunk-local: llama.cpp emits the + // actual per-chunk/per-step top-k width, while the receiving staged session + // may have a larger accumulated KV prefix. The sideband is valid as long as + // it does not claim more rows than the receiver can make visible. + GGML_UNUSED(token_count); + return actual_top_k <= expected_top_k;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0022-skippy-expose-GLM-DSA-staged-runtime-controls.patch` around lines 617 - 639, simplify skippy_glm_dsa_sideband_top_k_count_is_valid by retaining the nonzero checks and replacing the redundant token_count branches with a single actual_top_k <= expected_top_k validation; remove the now-unnecessary comments and conditional split.third_party/llama.cpp/patches/0038-ggml-shrink-GLM-Q2-gate-up-active-dispatch.patch (1)
19-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
_pad0is being repurposed as the active-expert count.Host and kernel independently duplicate
args._pad0 > 0 && args._pad0 < neX ? args._pad0 : neX. Reusing a field literally named_pad0for semantic data makes the host/device contract easy to break the next time the args struct is extended. Consider renaming the field (e.g.n_active_slots) and factoring the clamp into one shared helper so the two sides cannot drift.Also applies to: 37-38
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0038-ggml-shrink-GLM-Q2-gate-up-active-dispatch.patch` around lines 19 - 25, The active-expert count currently reuses the opaque _pad0 field and duplicates its clamping logic across host and kernel code. Rename the field to a semantic name such as n_active_slots in the shared argument contract and all affected patches (including 37–38), then centralize the >0 and <neX fallback calculation in a shared helper used by both sides.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@third_party/llama.cpp/patches/0025-Run-GLM-DSA-through-IndexShare-graph.patch`:
- Around line 79-82: Move the h_nextn capture in the main graph before the final
build_norm call: assign and label res->t_h_nextn from the raw post-FFN cur, then
apply model.output_norm to cur. Keep graph_mtp’s ordering consistent so
nextn.hnorm receives the pre-output_norm hidden state.
---
Outside diff comments:
In
`@third_party/llama.cpp/patches/0001-Add-Skippy-ABI-and-package-writer-foundation.patch`:
- Around line 1854-1860: Update the tensor-info construction block that assigns
out_tensor->element_count to compute the product of the parsed dimensions in
info->tensors instead of always returning zero. Multiply meta.ne values with
overflow checking, and preserve the existing success/error flow by reporting
overflow through the established out_error mechanism.
In
`@third_party/llama.cpp/patches/0002-Add-early-staged-model-family-and-chat-support.patch`:
- Around line 1258-1267: Update the activation scaling in the staged input path
near build_inp_embd so sqrtf(n_embd) is applied only when processing the first
stage (il_start == 0). Preserve the existing token-dependent scaling for
non-staged and first-stage inputs, while leaving preceding-stage hidden states
unscaled.
- Around line 1857-1866: The feature mask returned by skippy_abi_features must
advertise the implemented chat-template capability. Add
SKIPPY_FEATURE_CHAT_TEMPLATE to the returned feature flags alongside the
existing capabilities, preserving all currently advertised features.
In
`@third_party/llama.cpp/patches/0003-Add-staged-sampling-checkpoints-and-part-loading.patch`:
- Around line 1178-1226: Update skippy_trim_session to resize
session->token_history when it contains more than token_count entries,
truncating it to the retained token count alongside the KV cache and n_past
updates. Preserve existing validation and memory-trimming behavior.
- Around line 491-530: Keep sampler history synchronized in both verification
paths: update skippy_verify_token_batch to call skippy_record_tokens(session,
token_ids, token_count) after skippy_decode_batch succeeds, and update the
activation-frame helper at
third_party/llama.cpp/patches/0003-Add-staged-sampling-checkpoints-and-part-loading.patch
lines 607-638 to accept token IDs and record them after successful verification.
In
`@third_party/llama.cpp/patches/0021-ggml-add-GLM-DSA-sparse-execution-primitives.patch`:
- Around line 6595-6607: Replace the shape-based workaround in
ggml_metal_glm_dsa_sparse_attn_threads_for_shape with a root-cause fix in the
256-thread dsa_sparse_attn kernel’s buffer/thread indexing so no rows can remain
unwritten for any shape. If the kernel fix cannot be made here, document the
exhaustive coverage assumptions and create a tracked issue for the underlying
correctness defect rather than relying on these partial clamps.
- Around line 32-37: Update RPC_PROTO_PATCH_VERSION alongside the GGML_OP_COUNT
change in the static assertion block, bumping it from 3 to the next protocol
version required for the six newly added operations. Keep the GGML_OP_COUNT
assertion at 107 and ensure the client/server compatibility marker reflects the
changed operation set.
In
`@third_party/llama.cpp/patches/0022-skippy-expose-GLM-DSA-staged-runtime-controls.patch`:
- Around line 2599-2602: Guard the get_gf_res_prev() result in
skippy_import_kv_page before calling reset(), since it may be null before the
first decode builds a graph result. Preserve the reset behavior when a previous
graph result exists.
- Around line 2437-2439: Align the GLM-DSA sideband input storage and scan in
the batched frame path with request_count, matching glm_dsa_top_k_storage,
glm_dsa_top_k_offsets, glm_dsa_top_k_bytes_per_request, and the request-major
tensor validation. Ensure per-request processing uses the same request_count
indexing as the output copy, or add an explicit assertion/documented invariant
that request_count equals n_tokens if the input path must remain token-sized.
In
`@third_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patch`:
- Around line 6382-6384: Scope every environment override created by
build_graph/initialize_tensors, including the listed GLM DSA selected-row,
tiled, tensor-name, multihead, and split overrides, using the existing
scoped_test_env pattern. Store guards as fixture members or move setup into
eval/eval_perf, ensuring variables are restored before subsequent test cases
while preserving each test’s intended configuration.
In `@third_party/llama.cpp/patches/0024-Support-GLM-DSA-fused-KV_B-tensors.patch`:
- Around line 77-83: Make fused KV_B handling consistent across both patches: in
third_party/llama.cpp/patches/0024-Support-GLM-DSA-fused-KV_B-tensors.patch
lines 77-83, either require split wk_b/wv_b for GLM-DSA or split wkv_b during
loading; in
third_party/llama.cpp/patches/0025-Run-GLM-DSA-through-IndexShare-graph.patch
lines 29-33, if fused tensors remain supported, replace the graph constructor’s
rejection with fused-wkv_b handling, otherwise remove the check after enforcing
split tensors at load time.
In
`@third_party/llama.cpp/patches/0026-Bump-Skippy-ABI-for-GLM-DSA-runtime-config.patch`:
- Around line 16-19: Update the ABI versioning around SKIPPY_ABI_VERSION_MAJOR,
SKIPPY_ABI_VERSION_MINOR, and SKIPPY_ABI_VERSION_PATCH to use the required minor
bump or an explicit struct-size/version handshake for the expanded
skippy_runtime_config and new activation flag; do not leave this struct-layout
change represented only by a patch-level increment.
In
`@third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch`:
- Around line 184-195: Route each reduced active-count branch through its
matching active-specific pipeline factory. In
third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch
lines 184-195, update the active6, active4, and active2 branches to call the
corresponding _active6, _active4, and _active2 factory symbols; apply the same
q3_active_count selection in
third_party/llama.cpp/patches/0034-ggml-use-GLM-max-active-policy-for-Q3-down-kernels.patch
lines 75-83. Leave the ordinary eight-slot factory only for the non-reduced
active-count path.
- Around line 22-49: Update
ggml_metal_library_get_pipeline_mul_mv_id_q3_weighted_reduce_slots_sg_r8_nb8_w0
and the corresponding active4/active2 factories so active6/active4/active2
dispatch resolves the exact Metal entry points
kernel_mul_mv_id_q3_K_wr_slots_r8_a6, kernel_mul_mv_id_q3_K_wr_slots_r8_a4, and
kernel_mul_mv_id_q3_K_wr_slots_r8_a2. Align the generated pipeline names, add
matching host-name aliases, or route dispatch directly to those entry points;
apply the same correction to the related factories in the later diff section.
In `@third_party/llama.cpp/patches/0040-Fix-GLM-DSA-native-MTP-execution.patch`:
- Around line 176-181: The MTP nope views use the nope width for their byte
offset instead of the rope width. Update the offset argument in the ggml_view_3d
calls for indexer_q_nope and the corresponding K-nope view to use ggml_row_size
of the tensor type with n_embd_indexer_head_rope, while preserving the existing
pe-slab selection and other dimensions.
---
Nitpick comments:
In
`@third_party/llama.cpp/patches/0022-skippy-expose-GLM-DSA-staged-runtime-controls.patch`:
- Around line 617-639: simplify skippy_glm_dsa_sideband_top_k_count_is_valid by
retaining the nonzero checks and replacing the redundant token_count branches
with a single actual_top_k <= expected_top_k validation; remove the
now-unnecessary comments and conditional split.
In
`@third_party/llama.cpp/patches/0038-ggml-shrink-GLM-Q2-gate-up-active-dispatch.patch`:
- Around line 19-25: The active-expert count currently reuses the opaque _pad0
field and duplicates its clamping logic across host and kernel code. Rename the
field to a semantic name such as n_active_slots in the shared argument contract
and all affected patches (including 37–38), then centralize the >0 and <neX
fallback calculation in a shared helper used by both sides.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d8ab8897-98ba-4e66-a12e-649575374934
📒 Files selected for processing (44)
third_party/llama.cpp/patches/0001-Add-Skippy-ABI-and-package-writer-foundation.patchthird_party/llama.cpp/patches/0002-Add-early-staged-model-family-and-chat-support.patchthird_party/llama.cpp/patches/0003-Add-staged-sampling-checkpoints-and-part-loading.patchthird_party/llama.cpp/patches/0004-Add-lanes-external-media-and-chat-grammar-support.patchthird_party/llama.cpp/patches/0005-Add-resident-prefix-cache-and-session-refinements.patchthird_party/llama.cpp/patches/0006-Expand-staged-execution-across-dense-and-recurrent-f.patchthird_party/llama.cpp/patches/0007-Expand-staged-execution-across-VL-and-broad-model-fa.patchthird_party/llama.cpp/patches/0008-Add-external-decode-media-prefill-and-newer-family-s.patchthird_party/llama.cpp/patches/0009-Add-chat-grammar-device-enumeration-and-runtime-even.patchthird_party/llama.cpp/patches/0010-Add-MTP-execution-support-and-sampling-cleanup.patchthird_party/llama.cpp/patches/0011-Pass-reasoning-format-through-stage-chat-templates.patchthird_party/llama.cpp/patches/0012-Wire-mmap-and-mlock-runtime-load-options.patchthird_party/llama.cpp/patches/0013-Add-external-MTP-draft-sidecar-attachment.patchthird_party/llama.cpp/patches/0014-Add-non-frame-native-MTP-decode-ABI.patchthird_party/llama.cpp/patches/0015-Fix-stage-activation-graph-input-allocation.patchthird_party/llama.cpp/patches/0016-Recognize-thinking-field-in-chat-auto-parser.patchthird_party/llama.cpp/patches/0017-Expose-stateful-N-gram-cache-ABI.patchthird_party/llama.cpp/patches/0018-Remove-legacy-session-checkpoint-ABI.patchthird_party/llama.cpp/patches/0019-Re-prime-native-MTP-after-state-restoration.patchthird_party/llama.cpp/patches/0020-Fix-N-gram-confidence-threshold-indexing.patchthird_party/llama.cpp/patches/0021-ggml-add-GLM-DSA-sparse-execution-primitives.patchthird_party/llama.cpp/patches/0022-skippy-expose-GLM-DSA-staged-runtime-controls.patchthird_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patchthird_party/llama.cpp/patches/0024-Support-GLM-DSA-fused-KV_B-tensors.patchthird_party/llama.cpp/patches/0025-Run-GLM-DSA-through-IndexShare-graph.patchthird_party/llama.cpp/patches/0026-Bump-Skippy-ABI-for-GLM-DSA-runtime-config.patchthird_party/llama.cpp/patches/0026-Run-GLM-DSA-through-IndexShare-graph.patchthird_party/llama.cpp/patches/0027-Fix-GLM-DSA-Metal-get_rows-placement.patchthird_party/llama.cpp/patches/0027-Use-DSA-KV-cache-for-GLM-DSA.patchthird_party/llama.cpp/patches/0028-ggml-default-GLM-MoE-two-phase-Metal-path.patchthird_party/llama.cpp/patches/0029-ggml-add-GLM-MoE-Metal-selector-diagnostics.patchthird_party/llama.cpp/patches/0030-ggml-skip-zero-weight-GLM-MoE-gate-up-slots.patchthird_party/llama.cpp/patches/0031-tests-cover-full-GLM-MoE-selected-chain.patchthird_party/llama.cpp/patches/0032-tests-add-GLM-Q2Q3-selected-weight-roofline.patchthird_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patchthird_party/llama.cpp/patches/0034-ggml-use-GLM-max-active-policy-for-Q3-down-kernels.patchthird_party/llama.cpp/patches/0035-ggml-honor-explicit-Q2-gate-up-Metal-variant-flags.patchthird_party/llama.cpp/patches/0036-ggml-make-GLM-MoE-roofline-honor-active-experts.patchthird_party/llama.cpp/patches/0037-ggml-avoid-GLM-Q3-fused-tail-under-active-policy.patchthird_party/llama.cpp/patches/0038-ggml-shrink-GLM-Q2-gate-up-active-dispatch.patchthird_party/llama.cpp/patches/0039-ggml-skip-inactive-GLM-Q3-down-slots.patchthird_party/llama.cpp/patches/0040-Fix-GLM-DSA-native-MTP-execution.patchthird_party/llama.cpp/patches/0041-Adapt-Skippy-model-loading-to-load-modes.patchthird_party/llama.cpp/upstream.txt
💤 Files with no reviewable changes (2)
- third_party/llama.cpp/patches/0027-Use-DSA-KV-cache-for-GLM-DSA.patch
- third_party/llama.cpp/patches/0026-Run-GLM-DSA-through-IndexShare-graph.patch
👮 Files not reviewed due to content moderation or server errors (11)
- third_party/llama.cpp/patches/0004-Add-lanes-external-media-and-chat-grammar-support.patch
- third_party/llama.cpp/patches/0005-Add-resident-prefix-cache-and-session-refinements.patch
- third_party/llama.cpp/patches/0006-Expand-staged-execution-across-dense-and-recurrent-f.patch
- third_party/llama.cpp/patches/0007-Expand-staged-execution-across-VL-and-broad-model-fa.patch
- third_party/llama.cpp/patches/0008-Add-external-decode-media-prefill-and-newer-family-s.patch
- third_party/llama.cpp/patches/0009-Add-chat-grammar-device-enumeration-and-runtime-even.patch
- third_party/llama.cpp/patches/0010-Add-MTP-execution-support-and-sampling-cleanup.patch
- third_party/llama.cpp/patches/0011-Pass-reasoning-format-through-stage-chat-templates.patch
- third_party/llama.cpp/patches/0013-Add-external-MTP-draft-sidecar-attachment.patch
- third_party/llama.cpp/patches/0014-Add-non-frame-native-MTP-decode-ABI.patch
- third_party/llama.cpp/patches/0015-Fix-stage-activation-graph-input-allocation.patch
Collaborator
Author
|
Addressed review feedback in b71dd13. Fixed in follow-up patches 0042–0045:
No additional change was needed for the element-count overflow check, first-stage Gemma scaling, chat-template feature bit, or history trimming: later patches in the queue already provide those final-tree behaviors. I also kept Skippy ABI at 0.1.32 because that is already the contract on Validation: clean 45-patch replay with matching source tree, |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
third_party/llama.cpp/patches/0005-Add-resident-prefix-cache-and-session-refinements.patch (1)
252-253: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftBound KV-page token counts before allocating.
Both public ABI paths allocate from caller-controlled
token_countbefore checking cache capacity. A malformed request can exhaust memory or throw through the C ABI. Reject counts abovecells.size()before allocation; also guardn_tokens * n_pos.Proposed fix
+ if (token_count > cells.size()) { + error = "token count exceeds KV cache capacity"; + return false; + } std::vector<uint32_t> cell_idxs(static_cast<size_t>(token_count), std::numeric_limits<uint32_t>::max());+ if (desc.token_count > v_cells[strm].size() || + n_pos != 0 && n_tokens > std::numeric_limits<size_t>::max()/n_pos) { + error = "native KV page exceeds KV cache capacity"; + return false; + } udata->token.resize(n_tokens);Also applies to: 483-488
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0005-Add-resident-prefix-cache-and-session-refinements.patch` around lines 252 - 253, Validate caller-controlled token counts against cache capacity before allocating cell_idxs in both public ABI paths, including the corresponding path near the second allocation. Reject token_count values greater than cells.size() and guard any n_tokens * n_pos multiplication against overflow or capacity before allocation, returning the existing failure result without throwing through the C ABI.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@third_party/llama.cpp/patches/0005-Add-resident-prefix-cache-and-session-refinements.patch`:
- Around line 252-253: Validate caller-controlled token counts against cache
capacity before allocating cell_idxs in both public ABI paths, including the
corresponding path near the second allocation. Reject token_count values greater
than cells.size() and guard any n_tokens * n_pos multiplication against overflow
or capacity before allocation, returning the existing failure result without
throwing through the C ABI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: decd822a-8c20-4caa-a295-a008831d0218
📒 Files selected for processing (45)
third_party/llama.cpp/patches/0001-Add-Skippy-ABI-and-package-writer-foundation.patchthird_party/llama.cpp/patches/0002-Add-early-staged-model-family-and-chat-support.patchthird_party/llama.cpp/patches/0003-Add-staged-sampling-checkpoints-and-part-loading.patchthird_party/llama.cpp/patches/0004-Add-lanes-external-media-and-chat-grammar-support.patchthird_party/llama.cpp/patches/0005-Add-resident-prefix-cache-and-session-refinements.patchthird_party/llama.cpp/patches/0006-Expand-staged-execution-across-dense-and-recurrent-f.patchthird_party/llama.cpp/patches/0007-Expand-staged-execution-across-VL-and-broad-model-fa.patchthird_party/llama.cpp/patches/0008-Add-external-decode-media-prefill-and-newer-family-s.patchthird_party/llama.cpp/patches/0009-Add-chat-grammar-device-enumeration-and-runtime-even.patchthird_party/llama.cpp/patches/0010-Add-MTP-execution-support-and-sampling-cleanup.patchthird_party/llama.cpp/patches/0011-Pass-reasoning-format-through-stage-chat-templates.patchthird_party/llama.cpp/patches/0012-Wire-mmap-and-mlock-runtime-load-options.patchthird_party/llama.cpp/patches/0013-Add-external-MTP-draft-sidecar-attachment.patchthird_party/llama.cpp/patches/0014-Add-non-frame-native-MTP-decode-ABI.patchthird_party/llama.cpp/patches/0015-Fix-stage-activation-graph-input-allocation.patchthird_party/llama.cpp/patches/0016-Recognize-thinking-field-in-chat-auto-parser.patchthird_party/llama.cpp/patches/0017-Expose-stateful-N-gram-cache-ABI.patchthird_party/llama.cpp/patches/0018-Remove-legacy-session-checkpoint-ABI.patchthird_party/llama.cpp/patches/0019-Re-prime-native-MTP-after-state-restoration.patchthird_party/llama.cpp/patches/0020-Fix-N-gram-confidence-threshold-indexing.patchthird_party/llama.cpp/patches/0021-ggml-add-GLM-DSA-sparse-execution-primitives.patchthird_party/llama.cpp/patches/0022-skippy-expose-GLM-DSA-staged-runtime-controls.patchthird_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patchthird_party/llama.cpp/patches/0024-Support-GLM-DSA-fused-KV_B-tensors.patchthird_party/llama.cpp/patches/0025-Run-GLM-DSA-through-IndexShare-graph.patchthird_party/llama.cpp/patches/0026-Bump-Skippy-ABI-for-GLM-DSA-runtime-config.patchthird_party/llama.cpp/patches/0027-Fix-GLM-DSA-Metal-get_rows-placement.patchthird_party/llama.cpp/patches/0028-ggml-default-GLM-MoE-two-phase-Metal-path.patchthird_party/llama.cpp/patches/0029-ggml-add-GLM-MoE-Metal-selector-diagnostics.patchthird_party/llama.cpp/patches/0030-ggml-skip-zero-weight-GLM-MoE-gate-up-slots.patchthird_party/llama.cpp/patches/0031-tests-cover-full-GLM-MoE-selected-chain.patchthird_party/llama.cpp/patches/0032-tests-add-GLM-Q2Q3-selected-weight-roofline.patchthird_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patchthird_party/llama.cpp/patches/0034-ggml-use-GLM-max-active-policy-for-Q3-down-kernels.patchthird_party/llama.cpp/patches/0035-ggml-honor-explicit-Q2-gate-up-Metal-variant-flags.patchthird_party/llama.cpp/patches/0036-ggml-make-GLM-MoE-roofline-honor-active-experts.patchthird_party/llama.cpp/patches/0037-ggml-avoid-GLM-Q3-fused-tail-under-active-policy.patchthird_party/llama.cpp/patches/0038-ggml-shrink-GLM-Q2-gate-up-active-dispatch.patchthird_party/llama.cpp/patches/0039-ggml-skip-inactive-GLM-Q3-down-slots.patchthird_party/llama.cpp/patches/0040-Fix-GLM-DSA-native-MTP-execution.patchthird_party/llama.cpp/patches/0041-Adapt-Skippy-model-loading-to-load-modes.patchthird_party/llama.cpp/patches/0042-Harden-staged-session-and-sideband-bookkeeping.patchthird_party/llama.cpp/patches/0043-Correct-GLM-DSA-staged-graph-contracts.patchthird_party/llama.cpp/patches/0044-Fix-GLM-DSA-Metal-dispatch-safety.patchthird_party/llama.cpp/patches/0045-Scope-GLM-DSA-backend-test-environment.patch
🚧 Files skipped from review as they are similar to previous changes (32)
- third_party/llama.cpp/patches/0017-Expose-stateful-N-gram-cache-ABI.patch
- third_party/llama.cpp/patches/0014-Add-non-frame-native-MTP-decode-ABI.patch
- third_party/llama.cpp/patches/0031-tests-cover-full-GLM-MoE-selected-chain.patch
- third_party/llama.cpp/patches/0012-Wire-mmap-and-mlock-runtime-load-options.patch
- third_party/llama.cpp/patches/0036-ggml-make-GLM-MoE-roofline-honor-active-experts.patch
- third_party/llama.cpp/patches/0013-Add-external-MTP-draft-sidecar-attachment.patch
- third_party/llama.cpp/patches/0030-ggml-skip-zero-weight-GLM-MoE-gate-up-slots.patch
- third_party/llama.cpp/patches/0006-Expand-staged-execution-across-dense-and-recurrent-f.patch
- third_party/llama.cpp/patches/0020-Fix-N-gram-confidence-threshold-indexing.patch
- third_party/llama.cpp/patches/0018-Remove-legacy-session-checkpoint-ABI.patch
- third_party/llama.cpp/patches/0035-ggml-honor-explicit-Q2-gate-up-Metal-variant-flags.patch
- third_party/llama.cpp/patches/0039-ggml-skip-inactive-GLM-Q3-down-slots.patch
- third_party/llama.cpp/patches/0025-Run-GLM-DSA-through-IndexShare-graph.patch
- third_party/llama.cpp/patches/0019-Re-prime-native-MTP-after-state-restoration.patch
- third_party/llama.cpp/patches/0040-Fix-GLM-DSA-native-MTP-execution.patch
- third_party/llama.cpp/patches/0032-tests-add-GLM-Q2Q3-selected-weight-roofline.patch
- third_party/llama.cpp/patches/0027-Fix-GLM-DSA-Metal-get_rows-placement.patch
- third_party/llama.cpp/patches/0016-Recognize-thinking-field-in-chat-auto-parser.patch
- third_party/llama.cpp/patches/0021-ggml-add-GLM-DSA-sparse-execution-primitives.patch
- third_party/llama.cpp/patches/0011-Pass-reasoning-format-through-stage-chat-templates.patch
- third_party/llama.cpp/patches/0034-ggml-use-GLM-max-active-policy-for-Q3-down-kernels.patch
- third_party/llama.cpp/patches/0029-ggml-add-GLM-MoE-Metal-selector-diagnostics.patch
- third_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patch
- third_party/llama.cpp/patches/0009-Add-chat-grammar-device-enumeration-and-runtime-even.patch
- third_party/llama.cpp/patches/0008-Add-external-decode-media-prefill-and-newer-family-s.patch
- third_party/llama.cpp/patches/0007-Expand-staged-execution-across-VL-and-broad-model-fa.patch
- third_party/llama.cpp/patches/0004-Add-lanes-external-media-and-chat-grammar-support.patch
- third_party/llama.cpp/patches/0041-Adapt-Skippy-model-loading-to-load-modes.patch
- third_party/llama.cpp/patches/0022-skippy-expose-GLM-DSA-staged-runtime-controls.patch
- third_party/llama.cpp/patches/0003-Add-staged-sampling-checkpoints-and-part-loading.patch
- third_party/llama.cpp/patches/0010-Add-MTP-execution-support-and-sampling-cleanup.patch
- third_party/llama.cpp/patches/0002-Add-early-staged-model-family-and-chat-support.patch
ndizazzo
approved these changes
Jul 26, 2026
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
third_party/llama.cpp/patches/0005-Add-resident-prefix-cache-and-session-refinements.patch (2)
896-910: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftKeep Skippy session metadata consistent with imported KV state.
Both state/VK-page import APIs advance
n_past, but they do not restore, invalidate, or block access totoken_historyandsignal_history. Imported sessions can report a past position N while those histories are absent or too short, causing prefix save/restore, checkpoint, and signal APIs to fail or behave differently from the imported cache. Include the token/signal history in the transfer or require/initialize metadata before allowing imports to advance session state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0005-Add-resident-prefix-cache-and-session-refinements.patch` around lines 896 - 910, Update skippy_update_session_state_after_import and both state/VK-page import paths so imported KV state cannot advance n_past without corresponding token_history and signal_history metadata. Transfer those histories with the import when available, or explicitly require and initialize missing entries to the imported length before updating session state. Preserve existing entries, truncate excess data, and keep checkpoint_valid invalidated after import.
450-477: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject imported KV pages with no selected layers.
expected_bytesstays 0 whenselectedis empty, so a descriptor with a non-overlapping layer range andlayer_count == 0can pass validation, allocate KV cells viafind_slot()/apply_ubatch(), and then copy no K/V payload into those cells. Add the empty-selectedguard before allocation.Proposed fix
if (selected.size() != desc.layer_count) { error = "native KV page layer count does not match runtime"; return false; } + if (selected.empty()) { + error = "native KV page must contain at least one KV layer"; + return false; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@third_party/llama.cpp/patches/0005-Add-resident-prefix-cache-and-session-refinements.patch` around lines 450 - 477, Reject descriptors that produce no selected KV layers before any allocation or cell application occurs. In the validation flow that builds selected in the native KV page import path, add an empty-selected check after layer-count validation and return false with an appropriate error; ensure find_slot() and apply_ubatch() are not reached for this case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@third_party/llama.cpp/patches/0005-Add-resident-prefix-cache-and-session-refinements.patch`:
- Around line 896-910: Update skippy_update_session_state_after_import and both
state/VK-page import paths so imported KV state cannot advance n_past without
corresponding token_history and signal_history metadata. Transfer those
histories with the import when available, or explicitly require and initialize
missing entries to the imported length before updating session state. Preserve
existing entries, truncate excess data, and keep checkpoint_valid invalidated
after import.
- Around line 450-477: Reject descriptors that produce no selected KV layers
before any allocation or cell application occurs. In the validation flow that
builds selected in the native KV page import path, add an empty-selected check
after layer-count validation and return false with an appropriate error; ensure
find_slot() and apply_ubatch() are not reached for this case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 81cb25c3-da1d-42b4-882b-bfe7a9ca3768
📒 Files selected for processing (1)
third_party/llama.cpp/patches/0005-Add-resident-prefix-cache-and-session-refinements.patch
michaelneale
added a commit
that referenced
this pull request
Jul 27, 2026
The refreshed llama.cpp (#1085) enables mtmd video by default, which pulls in tools/mtmd/mtmd-helper.cpp's ffmpeg subprocess path via sheredom/subprocess.h. That header calls posix_spawn_file_actions_addchdir_np, which is unavailable on iOS, so the Swift XCFramework's iOS slice fails to compile mtmd-helper.cpp. mesh-llm does not use mtmd video. Set MTMD_VIDEO=OFF in build-llama.sh (covers Linux, macOS, and the Apple XCFramework targets) and in build-windows.ps1 for parity. Verified locally: the mtmd target now builds without subprocess.h. Assisted-by: Claude Sonnet Co-authored-by: Michael Neale <14976+michaelneale@users.noreply.github.com>
michaelneale
added a commit
that referenced
this pull request
Jul 27, 2026
* origin/main: Fix Metal small-batch matmul parity for GLM verification (#1078) Handle K-only transposed KV page import and export (#1084) Refresh llama.cpp upstream patch queue (#1085) chore: improve embedded native-runtime compatibility guidance (#1043) fix(console-ui): chat transcript snapping during live status updates (#1083) ci: bump Linux CUDA slim container to gha-convention base runner image fix: record activation cache prefix identities (#1041) fix: read-only model download caches (#1042) ci: disable sccache for Windows ROCm native runtime build (#1087) ci: fix v0.74 release GPU builds (sccache disk-only + force_hosted_runners) (#1086) Make release sccache failures non-fatal (#1079) Keep client-only nodes out of model election (#1074) # Conflicts: # crates/mesh-llm-host-runtime/src/runtime/auto_join.rs # crates/mesh-llm-host-runtime/src/runtime/tests/auto_join.rs # third_party/llama.cpp/patches/0004-Add-lanes-external-media-and-chat-grammar-support.patch
This was referenced Jul 28, 2026
This was referenced Aug 8, 2026
This was referenced Aug 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
76f46ad2toff067f76llama_load_modeAPIWhy
The llama.cpp Upstream Canary stopped while applying the existing queue. Upstream changed the model-loader API and incorporated overlapping GLM-DSA / chat-parser work, so the old patches no longer applied cleanly.
The refreshed queue preserves the staged-runtime behavior while incorporating those upstream changes. GLM-DSA stage boundaries now reject a stage that starts on a shared-indexer layer, because the required top-k indices are produced by the preceding full-indexer layer and are not part of the activation sideband.
Fixes the failure in https://github.com/Mesh-LLM/mesh-llm/actions/runs/30146939161.
Validation
scripts/prepare-llama.sh pinnedreplay; resulting tree matched the built checkoutMESH_LLM_AUTO_GENERATE_CODESIGN=0 just buildcargo check -p skippy-ffi -p skippy-runtime -p skippy-server -p skippy-model-package -p skippy-correctness -p llama-spec-benchscripts/skippy-ci-smoke.sh(dense two-stage binary execution, recurrent state handoff, OpenAI surface, exact-prefix reuse)Summary by CodeRabbit