Refresh llama.cpp upstream patch queue - #1105
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR refreshes a 47-patch llama.cpp queue with Skippy staged inference, expanded ABI and session APIs, ordered model-part loading, native MTP draft propagation, GLM-DSA execution, Metal kernels, KV-page handling, and backend and regression tests. ChangesSkippy staged runtime and verification
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (28)
third_party/llama.cpp/patches/0029-ggml-add-GLM-MoE-Metal-selector-diagnostics.patch (1)
82-91: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInclude the explicit slot-1 request in default suppression.
pair_sg_slot1_dualis omitted from the “no requested variant” check. Whensrc1_override != nullptr, an explicit slot-1 request can therefore enable bothpair_sg_slot1_dualand the slot-4 default, causing ambiguous or incorrect kernel selection.Proposed fix
+ const bool pair_sg_slot1_dual_requested = + ggml_metal_glm_dsa_q2_gate_up_swiglu_pair_sg_slot1_dual_enabled(); const bool pair_sg_slot1_dual_default = pair_sg_default_shape && + !pair_sg_slot1_dual_requested && !pair_sg_slot8_requested && ... const bool pair_sg_slot1_dual = pair_sg && dst->type == GGML_TYPE_F32 && - (ggml_metal_glm_dsa_q2_gate_up_swiglu_pair_sg_slot1_dual_enabled() || + (pair_sg_slot1_dual_requested || (pair_sg_slot1_dual_default && !pair_sg_slot4_dual_default));Also applies to: 96-115
🤖 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/0029-ggml-add-GLM-MoE-Metal-selector-diagnostics.patch` around lines 82 - 91, Update the default-suppression checks defining pair_sg_slot1_dual_default and the corresponding logic around pair_sg_slot1_dual to include the explicit slot-1 request predicate. Ensure src1_override requests suppress the slot-1 default so slot-1 and slot-4 variants cannot both be enabled, including the additional affected block.third_party/llama.cpp/patches/0045-Fix-GLM-DSA-Metal-dispatch-safety.patch (1)
21-35: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep unsupported active-slot values on the generic pipeline.
ggml_metal_library_get_pipeline_mul_mv_id_q3_weighted_reduce_slots_sg_r8_nb8_w0_activeonly supports{2, 4, 6}, but this caller also createsn_active_slotsfrom requested counts 1–8. The current fall-through maps 1, 3, 5, and 7 to the..._a2pipeline, whose device specialization is fixed at 2 active slots, so these values can use the wrong dispatch geometry. Add validation/conversion or fall back tokernel_mul_mv_id_q3_K_weighted_reduce_slots_sg_r8_nb8_w0for unsupported values.🤖 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/0045-Fix-GLM-DSA-Metal-dispatch-safety.patch` around lines 21 - 35, Update the active-slot pipeline selection around active_slots so only values 2, 4, and 6 use their specialized kernels; route 1, 3, 5, and 7 to the generic kernel_mul_mv_id_q3_K_weighted_reduce_slots_sg_r8_nb8_w0 pipeline instead of falling through to the a2 specialization. Preserve the existing pipeline lookup and compilation behavior for each selected base name.third_party/llama.cpp/patches/0036-ggml-make-GLM-MoE-roofline-honor-active-experts.patch (1)
18-23: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse the same supported active-count domain across stages.
This code applies any positive max-active value below
contract.top_k, while the Q3 down policy accepts only 2, 4, or 6. For a value such as 3, the selected-weight scan changes its dispatch but Q3 down does not enter the active-count path, leaving the stages inconsistent. Reuse a shared normalized helper or enforce the same whitelist 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/0036-ggml-make-GLM-MoE-roofline-honor-active-experts.patch` around lines 18 - 23, The active expert count normalization in the GLM-MoE roofline path must use the same supported domain as Q3 down. Update the logic around max_active_experts, active_top_k, and active_top_k_u64 to accept only the supported active counts (2, 4, or 6), while preserving contract.top_k when no supported override applies.third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch (1)
184-195: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winWire the active-count pipelines through both dispatch layers.
Both branches select the generic eight-slot getter instead of the corresponding active6/active4/active2 getter, so the new active-count kernels are not used.
third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch#L184-L195: select the matching active getter for each environment flag.third_party/llama.cpp/patches/0034-ggml-use-GLM-max-active-policy-for-Q3-down-kernels.patch#L75-L83: select the matching active getter for each normalized max-active 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/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch` around lines 184 - 195, Update the active-count pipeline selection in third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch lines 184-195 so the active6, active4, and active2 environment flags call their corresponding active-count pipeline getters instead of the generic eight-slot getter. Apply the same matching active-count getter selection for each normalized max-active count in third_party/llama.cpp/patches/0034-ggml-use-GLM-max-active-policy-for-Q3-down-kernels.patch lines 75-83.third_party/llama.cpp/patches/0038-ggml-shrink-GLM-Q2-gate-up-active-dispatch.patch (1)
19-25: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not shrink non-pair dispatch without updating its kernel.
The host now uses
active_slots * ne21even whenpair_sgis false, but this patch only changes thepair_sgkernel’s grid-Z decoding. The non-pair kernel therefore still appears to expectne20 * ne21; reducing its grid can omit experts beyondactive_slots. Keep the old non-pair dispatch or update its corresponding kernel mapping.🤖 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, Update the ne123 dispatch calculation so active_slots is used only when pair_sg is true; preserve the existing ne20*ne21 calculation for non-pair dispatch unless the corresponding non-pair kernel’s grid-Z mapping is also updated to honor active_slots.third_party/llama.cpp/patches/0001-Add-Skippy-ABI-and-package-writer-foundation.patch (2)
1834-1860: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPopulate
element_countin tensor introspection.
skippy_model_info_tensor_atalways returnselement_count = 0, so the public metadata API reports incorrect tensor information. Compute it from the stored dimensions before returning.🤖 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 1834 - 1860, Update skippy_model_info_tensor_at to derive out_tensor->element_count from the tensor’s stored dimensions in info->ctx instead of always assigning zero. Use the tensor identified by tensor_id, compute the product of its dimensions, and preserve the existing metadata population and success return behavior.
1524-1543: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not silently ignore activation buffers.
skippy_prefill_chunkaccepts input/output activation buffers but discards them and still returns the token-decode result. Callers can receive success without their output buffer being populated. Implement activation handling here, or reject non-null activation arguments and require the frame API.🤖 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 1524 - 1543, Update skippy_prefill_chunk to stop silently ignoring activation buffers: either implement the input/output activation handling and populate out_output_activation_bytes, or explicitly reject non-null input_activations/output_activations (and invalid capacities) with an error status before calling skippy_decode_tokens, directing callers to the frame API. Preserve token-only behavior when activation arguments are absent.third_party/llama.cpp/patches/0004-Add-lanes-external-media-and-chat-grammar-support.patch (5)
1696-1706: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRequire valid logits before sampling the current session.
This API can be called before any decode has produced logits, but
skippy_sample_tokencan fall through toskippy_greedy_sample, which dereferences the logits pointer without checking it. Return a runtime/invalid-state error when no current logits row exists.🤖 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/0004-Add-lanes-external-media-and-chat-grammar-support.patch` around lines 1696 - 1706, Update skippy_session_sample_current to validate that the session has a current logits row before calling skippy_sample_token; when logits are unavailable, set an appropriate runtime/invalid-state error through out_error and return that status, while preserving existing argument validation and normal sampling behavior.
2205-2244: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not silently “trim” recurrent state by changing only
n_past.For recurrent-only memory, and for the recurrent component of hybrid memory, this code updates
session->n_pastwithout removing/replaying the corresponding recurrent state. Subsequent tokens therefore depend on tokens that were supposedly trimmed. Reject unsupported recurrent trims or implement state-aware rollback.🤖 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/0004-Add-lanes-external-media-and-chat-grammar-support.patch` around lines 2205 - 2244, Update skippy_trim_session to handle recurrent-only and recurrent hybrid memory explicitly: do not merely assign session->n_past after trimming attention KV state. Reject trims when recurrent state cannot be rolled back, or implement state-aware rollback/replay so recurrent state matches token_count; preserve existing attention-only trimming behavior.
1771-1787: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not free a lane after a failed reset.
skippy_session_freeignores the result ofskippy_session_resetand marks the lane available regardless. If reset fails, a later session can inherit stale KV/recurrent state. Keep the lane occupied and return the reset failure, or perform a guaranteed cleanup before releasing it.🤖 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/0004-Add-lanes-external-media-and-chat-grammar-support.patch` around lines 1771 - 1787, The skippy_session_free cleanup must not release a lane when skippy_session_reset fails. Capture and propagate the reset result, only mark stage_model->lane_in_use false after successful cleanup, and ensure the failure is returned through out_error while preserving the session’s occupied state.
1709-1747: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not report success after chat-sampling initialization fails.
The exception handlers clear the sampling state and return
SKIPPY_STATUS_OK. Malformed metadata or grammar setup therefore silently disables the requested sampler. Return an error status and preserve the failure details.🤖 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/0004-Add-lanes-external-media-and-chat-grammar-support.patch` around lines 1709 - 1747, Update the exception handlers in skippy_session_configure_chat_sampling so failures during metadata parsing or chat grammar initialization return an error status instead of skippy_success. Preserve the exception details by setting out_error with an appropriate runtime/error status and message, then clear sampling state before returning.
1673-1693: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSynchronize runtime state when changing session position.
skippy_session_set_positiononly changessession->n_pastand truncates bookkeeping vectors. It does not trim or reset the lane’s KV/recurrent state, so the next decode uses stale cache contents with a new logical position. Route backward moves through the runtime trim path and reject unsupported forward moves.🤖 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/0004-Add-lanes-external-media-and-chat-grammar-support.patch` around lines 1673 - 1693, Update skippy_session_set_position to synchronize runtime state: reject forward positions beyond the current session position, and route backward moves through the lane/runtime trim path so KV or recurrent state is truncated or reset consistently. Preserve the existing argument validation and bookkeeping updates, but do not only modify n_past and history vectors.third_party/llama.cpp/patches/0042-Harden-staged-session-and-sideband-bookkeeping.patch (1)
90-97: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValidate
request_countbefore narrowing it.The
size_tvalue is cast toint32_tbeforeGGML_ASSERT; in release builds the assertion may be absent, allowing truncation and an incorrect batch size. Add an explicit runtime check againstINT32_MAXbefore the cast.🤖 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/0042-Harden-staged-session-and-sideband-bookkeeping.patch` around lines 90 - 97, In skippy_decode_batch_sampled, validate request_count against INT32_MAX before converting it to int32_t, using the existing error-handling path for oversized requests. Keep the GGML_ASSERT only as a post-cast consistency check if needed, and ensure llama_batch_init receives a validated, non-truncated token count.third_party/llama.cpp/patches/0002-Add-early-staged-model-family-and-chat-support.patch (1)
1258-1267: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAvoid re-scaling non-first Gemma4 activations.
For
stage_filtered && il_start > 0,build_inp_embd(nullptr)supplies the incoming activation, but this unconditionalggml_scale(..., sqrtf(n_embd))applies the embedding scale again. Other Gemma variants guard this path. Apply the scale only for the first stage or unfiltered execution.🤖 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 `inpL` scaling immediately after `build_inp_embd` so `sqrtf(n_embd)` is applied only when execution is unfiltered or `il_start == 0`; preserve the existing scale of 1.0 for non-token inputs and avoid re-scaling incoming activations when `stage_filtered && il_start > 0`.third_party/llama.cpp/patches/0003-Add-staged-sampling-checkpoints-and-part-loading.patch (2)
395-412: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winValidate the
pathsarray before dereferencing it.
llama_model_load_from_partschecksn_pathsbut notpaths. Passingpaths == nullptrwith a nonzero count dereferences a null pointer in the loop.🤖 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 395 - 412, Update llama_model_load_from_parts to validate paths is non-null before reserving or iterating when n_paths is nonzero; log the invalid input and return nullptr, while preserving the existing empty-list and per-entry validation behavior.
264-295: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winKeep the ordered GGUF contexts alive for their weights.
ctx_gguf.get()is passed intollama_tensor_weight, butctx_ggufis only loop-local. Sincecontextsis separate fromctx_gguf, those tensor weights can hold metadata pointers once the part context is destroyed. Addctx_ggufto the retained contexts and pass that retained context into the weights.🤖 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 264 - 295, Retain each ordered part’s GGUF context beyond the loop iteration by adding ctx_gguf to the existing contexts collection, and pass that retained context—not the loop-local ctx_gguf.get()—to llama_tensor_weight. Keep the retained context associated with the corresponding file and preserve the current tensor deduplication logic.third_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patch (3)
7161-7164: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGate the 1.9 GiB allocation behind an explicit perf opt-in.
test_glm_decode_block_byte_ceilingis registered in every default perf run, despite its declared 1,908,557,824-byte operation. A normal fulltest-backend-ops perfrun can therefore OOM supported backends.Proposed fix
- test_cases.emplace_back(new test_glm_decode_block_byte_ceiling()); + if (getenv("GGML_TEST_ENABLE_GLM_DECODE_BLOCK_BYTE_CEILING") != nullptr) { + test_cases.emplace_back(new test_glm_decode_block_byte_ceiling()); + }🤖 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 7161 - 7164, Gate registration of test_glm_decode_block_byte_ceiling behind the repository’s explicit performance opt-in mechanism, so default test-backend-ops perf runs do not allocate its 1.9 GiB workload. Preserve registration when the perf option is enabled and leave the other GLM test cases unchanged.
6382-6384: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winScope graph-selection environment overrides to each test.
These
set_test_env()calls persist after graph construction, making later test behavior dependent on execution order. Move each override into ascoped_test_envheld across the correspondingeval()/eval_perf()call.
third_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patch#L6382-L6384: scope selected-row tiled-path overrides.third_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patch#L6483-L6487: scope the selected-row flash enablement.third_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patch#L6614-L6617: scope compact multihead selection.third_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patch#L6703-L6703: scope compact NWG8 selection.third_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patch#L6779-L6780: scope compact split-exact selection.🤖 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, Graph-selection environment overrides persist across tests; replace each set_test_env call with a scoped_test_env whose lifetime covers the corresponding eval() or eval_perf() call. Apply this in third_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patch at lines 6382-6384 for selected-row tiled overrides, 6483-6487 for selected-row flash enablement, 6614-6617 for compact multihead selection, 6703 for compact NWG8 selection, and 6779-6780 for compact split-exact selection.
6630-6633: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake compact-fixture tensor names match the loader.
The fixture loaders look for
real_qandreal_compact_k, but the multihead test renames those tensors before initialization and the split test never assigns those names. When a fixture directory is supplied, both tests return without loading fixture data.
third_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patch#L6630-L6633: update the loader to match final names, or retain fixture-loader names.third_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patch#L6782-L6786: assign the names expected by the fixture loader before initialization.🤖 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 6630 - 6633, Align the compact-fixture tensor names with the loader in both test paths: at the multihead candidate setup around lines 6630-6633, either preserve the loader names real_q and real_compact_k or update the loader to use the final names; at the split test setup around lines 6782-6786, assign real_q and real_compact_k before initialization. Ensure fixture data loads whenever a fixture directory is provided.third_party/llama.cpp/patches/0005-Add-resident-prefix-cache-and-session-refinements.patch (1)
532-538: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake KV-page import atomic on allocation failure.
seq_rmdeletes the target range beforefind_slotconfirms that replacement cells are available. If allocation fails, the API returns an error after destroying the existing sequence state. Preflight capacity or add rollback so failed imports do not corrupt the session.🤖 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 532 - 538, Make the KV-page import flow around find_slot and apply_ubatch atomic: verify replacement KV-cache capacity before calling seq_rm, or preserve and restore the existing sequence state if allocation fails. Ensure a failed import returns the existing error without deleting or otherwise corrupting the target sequence range.third_party/llama.cpp/patches/0008-Add-external-decode-media-prefill-and-newer-family-s.patch (2)
377-385: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winGuard transposed-V handling when no V stream exists.
The new zero-byte checks cover only the non-transposed path. With
v_transenabled and no V tensor, the transposed branches still dereferencelayer->v_stream[strm]/v. Skip absent V storage in both export and import before calling backend tensor accessors.Also applies to: 411-419
🤖 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/0008-Add-external-decode-media-prefill-and-newer-family-s.patch` around lines 377 - 385, Extend the zero-byte V-storage guard to the transposed-V branches in both export and import, before dereferencing layer->v_stream[strm] or calling backend tensor accessors. Update the relevant logic in the export path and llama_kv_cache::stage_import_kv_page so absent V streams are skipped consistently with the existing non-transposed handling.
922-936: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftHonor and validate explicit positions on every prefill path.
The new
*_with_positionsAPIs ignore supplied positions on the raw-token path because it still callsskippy_decode_tokens. Additionally, a non-null position array with the wrongposition_countsilently falls back to synthesized positions. Reject mismatches and route first-stage decoding through a position-aware batch path.Also applies to: 1065-1071
🤖 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/0008-Add-external-decode-media-prefill-and-newer-family-s.patch` around lines 922 - 936, Update both position-handling blocks to reject any non-null positions whose position_count differs from expected_position_count instead of synthesizing fallback positions. Ensure explicit positions are preserved on every prefill path, including raw-token decoding, by routing first-stage decoding through the position-aware batch path rather than skippy_decode_tokens; retain synthesized positions only when no explicit array is supplied.third_party/llama.cpp/patches/0007-Expand-staged-execution-across-VL-and-broad-model-fa.patch (2)
329-339: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not apply raw-embedding scaling to mid-stage activations.
When
il_start > 0,build_inp_embd(nullptr)supplies a post-layer activation, but the existing AFMoE MuP scaling and MiniCPM3scale_embdpath still rescale it as if it were a raw embedding. Guard these scales for the first stage only.Also applies to: 1685-1694
🤖 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/0007-Expand-staged-execution-across-VL-and-broad-model-fa.patch` around lines 329 - 339, Guard the AFMoE MuP embedding scale and MiniCPM3 scale_embd path in the graph constructor so they run only when processing the first stage (when il_start is 0). Preserve the existing scaling behavior for raw embeddings while leaving mid-stage activations from build_inp_embd(nullptr) unscaled; apply the same fix to the corresponding logic around the additional referenced location.
2925-2927: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGate activation-sideband requirements on slice position.
Both validators reject the first Gemma3n and RWKV7 slices unless their downstream-only sidebands are present. Add
config.layer_start > 0to both conditions solayer_start == 0slices are not required to carrySKIPPY_ACTIVATION_FLAG_GEMMA3N_ALTUPorSKIPPY_ACTIVATION_FLAG_RWKV7_V_FIRST.
third_party/llama.cpp/patches/0007-Expand-staged-execution-across-VL-and-broad-model-fa.patch#L2925-L2927third_party/llama.cpp/patches/0008-Add-external-decode-media-prefill-and-newer-family-s.patch#L819-L821🤖 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/0007-Expand-staged-execution-across-VL-and-broad-model-fa.patch` around lines 2925 - 2927, The Gemma3n and RWKV7 activation-sideband validators must only require downstream-only sidebands for non-first slices. Update the conditions in third_party/llama.cpp/patches/0007-Expand-staged-execution-across-VL-and-broad-model-fa.patch lines 2925-2927 and third_party/llama.cpp/patches/0008-Add-external-decode-media-prefill-and-newer-family-s.patch lines 819-821 to also require config.layer_start > 0, preserving validation for later slices while allowing layer_start == 0 without SKIPPY_ACTIVATION_FLAG_GEMMA3N_ALTUP or SKIPPY_ACTIVATION_FLAG_RWKV7_V_FIRST.third_party/llama.cpp/patches/0017-Expose-stateful-N-gram-cache-ABI.patch (1)
52-66: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClear every mutable cache on reset.
common_ngram_cache_draft()usesdynamicandstatic_cache, but both reset branches only clearhistoryandcontext. After a request reset, stale n-gram candidates can therefore leak into the next request’s drafts. Clear all three cache objects wheneverresetis true.Proposed fix
if (token_count == 0) { if (reset) { cache->history.clear(); cache->context.clear(); + cache->dynamic.clear(); + cache->static_cache.clear(); } return SKIPPY_STATUS_OK; } ... if (reset) { cache->history.assign(token_ids, token_ids + token_count); cache->context.clear(); + cache->dynamic.clear(); + cache->static_cache.clear(); common_ngram_cache_update(🤖 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/0017-Expose-stateful-N-gram-cache-ABI.patch` around lines 52 - 66, Update both reset branches in common_ngram_cache_draft() to clear dynamic and static_cache in addition to history and context, ensuring every mutable cache is emptied whenever reset is true, including the token_count == 0 path.third_party/llama.cpp/patches/0013-Add-external-MTP-draft-sidecar-attachment.patch (1)
94-106: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject MTP attachments on non-output stages.
skippy_mtp_available()only reports MTP whenstage_model->config.include_outputis true, but this API accepts any target with a context and returns success. Attaching to a non-output stage therefore creates an MTP context that the runtime silently ignores. Requiretarget_model->config.include_outputhere, or make the availability contract consistent.Proposed validation
if (target_model->mtp_ctx != nullptr) { skippy_set_error(out_error, SKIPPY_STATUS_INVALID_ARGUMENT, "target model already has an MTP draft context"); return SKIPPY_STATUS_INVALID_ARGUMENT; } + if (!target_model->config.include_output) { + skippy_set_error(out_error, SKIPPY_STATUS_UNSUPPORTED, + "MTP draft attachment requires an output-enabled target model"); + return SKIPPY_STATUS_UNSUPPORTED; + }🤖 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/0013-Add-external-MTP-draft-sidecar-attachment.patch` around lines 94 - 106, Update skippy_model_attach_mtp_draft_model to reject targets whose config.include_output is false, alongside the existing target/context/path validation, returning SKIPPY_STATUS_INVALID_ARGUMENT through skippy_set_error. Preserve the existing already-attached validation and successful attachment behavior for output stages.third_party/llama.cpp/patches/0010-Add-MTP-execution-support-and-sampling-cleanup.patch (1)
1788-1803: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftKeep native MTP state coherent across both batch APIs.
third_party/llama.cpp/patches/0010-Add-MTP-execution-support-and-sampling-cleanup.patch#L1788-L1803: synchronize each session after batched token decode, using the token’s pre-decode position.third_party/llama.cpp/patches/0010-Add-MTP-execution-support-and-sampling-cleanup.patch#L2025-L2051: perform the equivalent synchronization after batched frame decode.🤖 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/0010-Add-MTP-execution-support-and-sampling-cleanup.patch` around lines 1788 - 1803, Keep native MTP state coherent after both batched decode paths. At the token batch site (third_party/llama.cpp/patches/0010-Add-MTP-execution-support-and-sampling-cleanup.patch lines 1788-1803), synchronize every session after llama_decode using that token’s pre-decode n_past position; apply the equivalent synchronization at the frame batch site (same file lines 2025-2051). Preserve existing error handling and sampling behavior.third_party/llama.cpp/patches/0012-Wire-mmap-and-mlock-runtime-load-options.patch (1)
19-21: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftAlign Skippy ABI versioning with all breaking runtime changes. The patches remove/rename public symbols and change exported signatures during the same
0.1.xABI line, with only later patch-specific patch-level bumps. Retain compatibility shims forskippy_decode_step, frame/native decode APIs,skippy_session_native_seq_id, andskippy_status_string, and preserve the previousskippy_runtime_configlayout/version gate for the new mmap/mlock fields.🤖 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/0012-Wire-mmap-and-mlock-runtime-load-options.patch` around lines 19 - 21, Align the ABI versioning across the runtime changes by retaining compatibility shims for skippy_decode_step, frame/native decode APIs, skippy_session_native_seq_id, and skippy_status_string, while preserving the prior skippy_runtime_config layout and version gate when adding has_mmap_override, use_mmap, and use_mlock. Apply the corresponding compatibility and versioning updates in third_party/llama.cpp/patches/0012-Wire-mmap-and-mlock-runtime-load-options.patch:19-21, third_party/llama.cpp/patches/0011-Pass-reasoning-format-through-stage-chat-templates.patch:16-30 and :38-45, and third_party/llama.cpp/patches/0010-Add-MTP-execution-support-and-sampling-cleanup.patch:171-257 and :289-295; keep the existing public symbols, signatures, and layout accessible through the 0.1.x ABI line.
🧹 Nitpick comments (1)
third_party/llama.cpp/patches/0045-Fix-GLM-DSA-Metal-dispatch-safety.patch (1)
84-89: 🚀 Performance & Scalability | 🔵 TrivialBenchmark the global sparse-attention thread cap.
This changes the previous shape-dependent cap to 32 threads for every shape. It improves dispatch safety, but may regress decode or prefill throughput; add representative performance coverage before relying on this default.
🤖 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/0045-Fix-GLM-DSA-Metal-dispatch-safety.patch` around lines 84 - 89, Benchmark the global cap applied by ggml_metal_glm_dsa_sparse_attn_threads, covering representative decode and prefill shapes and comparing against the previous shape-dependent behavior. Add performance coverage that detects throughput regressions before retaining the 32-thread default.
🤖 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/0001-Add-Skippy-ABI-and-package-writer-foundation.patch`:
- Around line 1834-1860: Update skippy_model_info_tensor_at to derive
out_tensor->element_count from the tensor’s stored dimensions in info->ctx
instead of always assigning zero. Use the tensor identified by tensor_id,
compute the product of its dimensions, and preserve the existing metadata
population and success return behavior.
- Around line 1524-1543: Update skippy_prefill_chunk to stop silently ignoring
activation buffers: either implement the input/output activation handling and
populate out_output_activation_bytes, or explicitly reject non-null
input_activations/output_activations (and invalid capacities) with an error
status before calling skippy_decode_tokens, directing callers to the frame API.
Preserve token-only behavior when activation arguments are absent.
In
`@third_party/llama.cpp/patches/0002-Add-early-staged-model-family-and-chat-support.patch`:
- Around line 1258-1267: Update the `inpL` scaling immediately after
`build_inp_embd` so `sqrtf(n_embd)` is applied only when execution is unfiltered
or `il_start == 0`; preserve the existing scale of 1.0 for non-token inputs and
avoid re-scaling incoming activations when `stage_filtered && il_start > 0`.
In
`@third_party/llama.cpp/patches/0003-Add-staged-sampling-checkpoints-and-part-loading.patch`:
- Around line 395-412: Update llama_model_load_from_parts to validate paths is
non-null before reserving or iterating when n_paths is nonzero; log the invalid
input and return nullptr, while preserving the existing empty-list and per-entry
validation behavior.
- Around line 264-295: Retain each ordered part’s GGUF context beyond the loop
iteration by adding ctx_gguf to the existing contexts collection, and pass that
retained context—not the loop-local ctx_gguf.get()—to llama_tensor_weight. Keep
the retained context associated with the corresponding file and preserve the
current tensor deduplication logic.
In
`@third_party/llama.cpp/patches/0004-Add-lanes-external-media-and-chat-grammar-support.patch`:
- Around line 1696-1706: Update skippy_session_sample_current to validate that
the session has a current logits row before calling skippy_sample_token; when
logits are unavailable, set an appropriate runtime/invalid-state error through
out_error and return that status, while preserving existing argument validation
and normal sampling behavior.
- Around line 2205-2244: Update skippy_trim_session to handle recurrent-only and
recurrent hybrid memory explicitly: do not merely assign session->n_past after
trimming attention KV state. Reject trims when recurrent state cannot be rolled
back, or implement state-aware rollback/replay so recurrent state matches
token_count; preserve existing attention-only trimming behavior.
- Around line 1771-1787: The skippy_session_free cleanup must not release a lane
when skippy_session_reset fails. Capture and propagate the reset result, only
mark stage_model->lane_in_use false after successful cleanup, and ensure the
failure is returned through out_error while preserving the session’s occupied
state.
- Around line 1709-1747: Update the exception handlers in
skippy_session_configure_chat_sampling so failures during metadata parsing or
chat grammar initialization return an error status instead of skippy_success.
Preserve the exception details by setting out_error with an appropriate
runtime/error status and message, then clear sampling state before returning.
- Around line 1673-1693: Update skippy_session_set_position to synchronize
runtime state: reject forward positions beyond the current session position, and
route backward moves through the lane/runtime trim path so KV or recurrent state
is truncated or reset consistently. Preserve the existing argument validation
and bookkeeping updates, but do not only modify n_past and history vectors.
In
`@third_party/llama.cpp/patches/0005-Add-resident-prefix-cache-and-session-refinements.patch`:
- Around line 532-538: Make the KV-page import flow around find_slot and
apply_ubatch atomic: verify replacement KV-cache capacity before calling seq_rm,
or preserve and restore the existing sequence state if allocation fails. Ensure
a failed import returns the existing error without deleting or otherwise
corrupting the target sequence range.
In
`@third_party/llama.cpp/patches/0007-Expand-staged-execution-across-VL-and-broad-model-fa.patch`:
- Around line 329-339: Guard the AFMoE MuP embedding scale and MiniCPM3
scale_embd path in the graph constructor so they run only when processing the
first stage (when il_start is 0). Preserve the existing scaling behavior for raw
embeddings while leaving mid-stage activations from build_inp_embd(nullptr)
unscaled; apply the same fix to the corresponding logic around the additional
referenced location.
- Around line 2925-2927: The Gemma3n and RWKV7 activation-sideband validators
must only require downstream-only sidebands for non-first slices. Update the
conditions in
third_party/llama.cpp/patches/0007-Expand-staged-execution-across-VL-and-broad-model-fa.patch
lines 2925-2927 and
third_party/llama.cpp/patches/0008-Add-external-decode-media-prefill-and-newer-family-s.patch
lines 819-821 to also require config.layer_start > 0, preserving validation for
later slices while allowing layer_start == 0 without
SKIPPY_ACTIVATION_FLAG_GEMMA3N_ALTUP or SKIPPY_ACTIVATION_FLAG_RWKV7_V_FIRST.
In
`@third_party/llama.cpp/patches/0008-Add-external-decode-media-prefill-and-newer-family-s.patch`:
- Around line 377-385: Extend the zero-byte V-storage guard to the transposed-V
branches in both export and import, before dereferencing layer->v_stream[strm]
or calling backend tensor accessors. Update the relevant logic in the export
path and llama_kv_cache::stage_import_kv_page so absent V streams are skipped
consistently with the existing non-transposed handling.
- Around line 922-936: Update both position-handling blocks to reject any
non-null positions whose position_count differs from expected_position_count
instead of synthesizing fallback positions. Ensure explicit positions are
preserved on every prefill path, including raw-token decoding, by routing
first-stage decoding through the position-aware batch path rather than
skippy_decode_tokens; retain synthesized positions only when no explicit array
is supplied.
In
`@third_party/llama.cpp/patches/0010-Add-MTP-execution-support-and-sampling-cleanup.patch`:
- Around line 1788-1803: Keep native MTP state coherent after both batched
decode paths. At the token batch site
(third_party/llama.cpp/patches/0010-Add-MTP-execution-support-and-sampling-cleanup.patch
lines 1788-1803), synchronize every session after llama_decode using that
token’s pre-decode n_past position; apply the equivalent synchronization at the
frame batch site (same file lines 2025-2051). Preserve existing error handling
and sampling behavior.
In
`@third_party/llama.cpp/patches/0012-Wire-mmap-and-mlock-runtime-load-options.patch`:
- Around line 19-21: Align the ABI versioning across the runtime changes by
retaining compatibility shims for skippy_decode_step, frame/native decode APIs,
skippy_session_native_seq_id, and skippy_status_string, while preserving the
prior skippy_runtime_config layout and version gate when adding
has_mmap_override, use_mmap, and use_mlock. Apply the corresponding
compatibility and versioning updates in
third_party/llama.cpp/patches/0012-Wire-mmap-and-mlock-runtime-load-options.patch:19-21,
third_party/llama.cpp/patches/0011-Pass-reasoning-format-through-stage-chat-templates.patch:16-30
and :38-45, and
third_party/llama.cpp/patches/0010-Add-MTP-execution-support-and-sampling-cleanup.patch:171-257
and :289-295; keep the existing public symbols, signatures, and layout
accessible through the 0.1.x ABI line.
In
`@third_party/llama.cpp/patches/0013-Add-external-MTP-draft-sidecar-attachment.patch`:
- Around line 94-106: Update skippy_model_attach_mtp_draft_model to reject
targets whose config.include_output is false, alongside the existing
target/context/path validation, returning SKIPPY_STATUS_INVALID_ARGUMENT through
skippy_set_error. Preserve the existing already-attached validation and
successful attachment behavior for output stages.
In `@third_party/llama.cpp/patches/0017-Expose-stateful-N-gram-cache-ABI.patch`:
- Around line 52-66: Update both reset branches in common_ngram_cache_draft() to
clear dynamic and static_cache in addition to history and context, ensuring
every mutable cache is emptied whenever reset is true, including the token_count
== 0 path.
In
`@third_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patch`:
- Around line 7161-7164: Gate registration of test_glm_decode_block_byte_ceiling
behind the repository’s explicit performance opt-in mechanism, so default
test-backend-ops perf runs do not allocate its 1.9 GiB workload. Preserve
registration when the perf option is enabled and leave the other GLM test cases
unchanged.
- Around line 6382-6384: Graph-selection environment overrides persist across
tests; replace each set_test_env call with a scoped_test_env whose lifetime
covers the corresponding eval() or eval_perf() call. Apply this in
third_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patch
at lines 6382-6384 for selected-row tiled overrides, 6483-6487 for selected-row
flash enablement, 6614-6617 for compact multihead selection, 6703 for compact
NWG8 selection, and 6779-6780 for compact split-exact selection.
- Around line 6630-6633: Align the compact-fixture tensor names with the loader
in both test paths: at the multihead candidate setup around lines 6630-6633,
either preserve the loader names real_q and real_compact_k or update the loader
to use the final names; at the split test setup around lines 6782-6786, assign
real_q and real_compact_k before initialization. Ensure fixture data loads
whenever a fixture directory is provided.
In
`@third_party/llama.cpp/patches/0029-ggml-add-GLM-MoE-Metal-selector-diagnostics.patch`:
- Around line 82-91: Update the default-suppression checks defining
pair_sg_slot1_dual_default and the corresponding logic around pair_sg_slot1_dual
to include the explicit slot-1 request predicate. Ensure src1_override requests
suppress the slot-1 default so slot-1 and slot-4 variants cannot both be
enabled, including the additional affected block.
In
`@third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch`:
- Around line 184-195: Update the active-count pipeline selection in
third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch
lines 184-195 so the active6, active4, and active2 environment flags call their
corresponding active-count pipeline getters instead of the generic eight-slot
getter. Apply the same matching active-count getter selection for each
normalized max-active count in
third_party/llama.cpp/patches/0034-ggml-use-GLM-max-active-policy-for-Q3-down-kernels.patch
lines 75-83.
In
`@third_party/llama.cpp/patches/0036-ggml-make-GLM-MoE-roofline-honor-active-experts.patch`:
- Around line 18-23: The active expert count normalization in the GLM-MoE
roofline path must use the same supported domain as Q3 down. Update the logic
around max_active_experts, active_top_k, and active_top_k_u64 to accept only the
supported active counts (2, 4, or 6), while preserving contract.top_k when no
supported override applies.
In
`@third_party/llama.cpp/patches/0038-ggml-shrink-GLM-Q2-gate-up-active-dispatch.patch`:
- Around line 19-25: Update the ne123 dispatch calculation so active_slots is
used only when pair_sg is true; preserve the existing ne20*ne21 calculation for
non-pair dispatch unless the corresponding non-pair kernel’s grid-Z mapping is
also updated to honor active_slots.
In
`@third_party/llama.cpp/patches/0042-Harden-staged-session-and-sideband-bookkeeping.patch`:
- Around line 90-97: In skippy_decode_batch_sampled, validate request_count
against INT32_MAX before converting it to int32_t, using the existing
error-handling path for oversized requests. Keep the GGML_ASSERT only as a
post-cast consistency check if needed, and ensure llama_batch_init receives a
validated, non-truncated token count.
In `@third_party/llama.cpp/patches/0045-Fix-GLM-DSA-Metal-dispatch-safety.patch`:
- Around line 21-35: Update the active-slot pipeline selection around
active_slots so only values 2, 4, and 6 use their specialized kernels; route 1,
3, 5, and 7 to the generic
kernel_mul_mv_id_q3_K_weighted_reduce_slots_sg_r8_nb8_w0 pipeline instead of
falling through to the a2 specialization. Preserve the existing pipeline lookup
and compilation behavior for each selected base name.
---
Nitpick comments:
In `@third_party/llama.cpp/patches/0045-Fix-GLM-DSA-Metal-dispatch-safety.patch`:
- Around line 84-89: Benchmark the global cap applied by
ggml_metal_glm_dsa_sparse_attn_threads, covering representative decode and
prefill shapes and comparing against the previous shape-dependent behavior. Add
performance coverage that detects throughput regressions before retaining the
32-thread default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 85403ea9-da54-4a42-806d-5993796848ad
📒 Files selected for processing (48)
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-ggml-metal-make-small-batch-matmul-batch-invariant.patchthird_party/llama.cpp/patches/0045-Fix-GLM-DSA-Metal-dispatch-safety.patchthird_party/llama.cpp/patches/0046-Fix-empty-transposed-KV-page-import-and-export.patchthird_party/llama.cpp/patches/0047-Scope-GLM-DSA-backend-test-environment.patchthird_party/llama.cpp/upstream.txt
i386
reviewed
Jul 28, 2026
i386
left a comment
i386
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed current head 7b89aa76.
The refreshed llama.cpp queue applies cleanly against the new pinned upstream (9a3bf2b8); I found no blocking issue in this refresh. The PR checks are still in progress.
i386
approved these changes
Jul 28, 2026
i386
approved these changes
Jul 28, 2026
Return VerifyWindow MTP drafts through the typed ABI, advance the Skippy ABI mirror, and update Rust callers and tests. Harden ngram/KV state handling, GLM phase and range gates, and Metal dispatch configuration and active-count routing. Keep the later split wk_b/wv_b graph contract, retain dynamic environment reads required by scoped tests, and avoid a duplicate Metal builder registry because those findings are superseded by later queue policy and existing symbol compilation. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
ndizazzo
force-pushed
the
agent/refresh-llama-patch-queue
branch
from
July 28, 2026 22:03
7b89aa7 to
61e67fe
Compare
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 (4)
third_party/llama.cpp/patches/0008-Add-external-decode-media-prefill-and-newer-family-s.patch (1)
922-936: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject malformed position sidebands instead of silently replacing them.
A non-null
positionsbuffer with the wrongposition_countfalls through to synthesized sequential positions. The new*_with_positionsAPIs then return success with incorrect model positions. ReturnSKIPPY_STATUS_INVALID_ARGUMENTwhenpositions != nullptrand the count is not exactlyn_tokens * n_pos_per_embd.🤖 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/0008-Add-external-decode-media-prefill-and-newer-family-s.patch` around lines 922 - 936, Update the position-selection logic to reject malformed sidebands: when positions is non-null and position_count differs from the expected n_tokens * n_pos_per_embd value, return SKIPPY_STATUS_INVALID_ARGUMENT instead of synthesizing positions. Preserve the existing memcpy path for valid buffers and fallback generation only when positions is null.third_party/llama.cpp/patches/0009-Add-chat-grammar-device-enumeration-and-runtime-even.patch (1)
777-909: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winThread-protect runtime event progress state.
emit_progress()usessequenceandlast_progress_stepfrom the progress callback, so both fields must be protected across all event-emission paths. Use aspinlock/mutexaround state mutation and call the user reporter only after releasing the lock.🤖 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/0009-Add-chat-grammar-device-enumeration-and-runtime-even.patch` around lines 777 - 909, Protect runtime event state in skippy_runtime_event_scope, especially sequence and last_progress_step, with a mutex or spinlock across all emission paths, including emit and emit_progress. Serialize state mutation and event construction under the lock, then release it before invoking reporter.callback; update clear and related state access as needed to avoid races.third_party/llama.cpp/patches/0005-Add-resident-prefix-cache-and-session-refinements.patch (2)
450-477: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject KV-page descriptors that select zero layers.
When the requested range matches no runtime layers,
selectedremains empty anddesc.layer_count == 0passes validation. The function then allocates/removes token slots and returns success for a zero-byte import. Require a non-empty selected-layer set and validate the range against the model’s actual layers before mutating the cache.🤖 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, Require the KV-page import validation around selected and desc.layer_count to reject an empty selected-layer set, and validate desc.layer_start/end against the runtime layers before any cache mutation. Return an error for ranges selecting zero layers, while preserving existing layer-count and layout checks for valid non-empty ranges.
532-538: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftAvoid destructive mutation before allocation succeeds.
seq_rmclears the requested KV positions beforefind_slotconfirms that replacement cells are available. If allocation fails, the existing page has already been deleted and cannot be restored. Preflight capacity or add rollback so failed imports are atomic.🤖 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 532 - 538, Update the imported-page flow around find_slot and apply_ubatch so seq_rm is not performed until replacement KV cache cells are confirmed available, or restore the original page if allocation fails. Ensure a failed import leaves the existing page unchanged while successful imports retain the current replacement behavior.
🤖 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 450-477: Require the KV-page import validation around selected and
desc.layer_count to reject an empty selected-layer set, and validate
desc.layer_start/end against the runtime layers before any cache mutation.
Return an error for ranges selecting zero layers, while preserving existing
layer-count and layout checks for valid non-empty ranges.
- Around line 532-538: Update the imported-page flow around find_slot and
apply_ubatch so seq_rm is not performed until replacement KV cache cells are
confirmed available, or restore the original page if allocation fails. Ensure a
failed import leaves the existing page unchanged while successful imports retain
the current replacement behavior.
In
`@third_party/llama.cpp/patches/0008-Add-external-decode-media-prefill-and-newer-family-s.patch`:
- Around line 922-936: Update the position-selection logic to reject malformed
sidebands: when positions is non-null and position_count differs from the
expected n_tokens * n_pos_per_embd value, return SKIPPY_STATUS_INVALID_ARGUMENT
instead of synthesizing positions. Preserve the existing memcpy path for valid
buffers and fallback generation only when positions is null.
In
`@third_party/llama.cpp/patches/0009-Add-chat-grammar-device-enumeration-and-runtime-even.patch`:
- Around line 777-909: Protect runtime event state in
skippy_runtime_event_scope, especially sequence and last_progress_step, with a
mutex or spinlock across all emission paths, including emit and emit_progress.
Serialize state mutation and event construction under the lock, then release it
before invoking reporter.callback; update clear and related state access as
needed to avoid races.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 860cd88a-b06f-41fa-be68-59490474b67e
📒 Files selected for processing (57)
crates/skippy-bench/src/verify_window_local.rscrates/skippy-correctness/src/runner/state_handoff.rscrates/skippy-correctness/tests/parity_models/mod.rscrates/skippy-ffi/src/lib.rscrates/skippy-runtime/src/activation.rscrates/skippy-server/src/binary_transport/binary_messaging/connection.rscrates/skippy-server/src/binary_transport/restore_prefill_decode.rscrates/skippy-server/src/binary_transport/stage_execution.rscrates/skippy-server/src/runtime_state.rsthird_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-ggml-metal-make-small-batch-matmul-batch-invariant.patchthird_party/llama.cpp/patches/0045-Fix-GLM-DSA-Metal-dispatch-safety.patchthird_party/llama.cpp/patches/0046-Fix-empty-transposed-KV-page-import-and-export.patchthird_party/llama.cpp/patches/0047-Scope-GLM-DSA-backend-test-environment.patchthird_party/llama.cpp/upstream.txt
🚧 Files skipped from review as they are similar to previous changes (41)
- third_party/llama.cpp/upstream.txt
- third_party/llama.cpp/patches/0012-Wire-mmap-and-mlock-runtime-load-options.patch
- third_party/llama.cpp/patches/0013-Add-external-MTP-draft-sidecar-attachment.patch
- third_party/llama.cpp/patches/0024-Support-GLM-DSA-fused-KV_B-tensors.patch
- third_party/llama.cpp/patches/0036-ggml-make-GLM-MoE-roofline-honor-active-experts.patch
- third_party/llama.cpp/patches/0039-ggml-skip-inactive-GLM-Q3-down-slots.patch
- third_party/llama.cpp/patches/0041-Adapt-Skippy-model-loading-to-load-modes.patch
- third_party/llama.cpp/patches/0011-Pass-reasoning-format-through-stage-chat-templates.patch
- third_party/llama.cpp/patches/0030-ggml-skip-zero-weight-GLM-MoE-gate-up-slots.patch
- third_party/llama.cpp/patches/0035-ggml-honor-explicit-Q2-gate-up-Metal-variant-flags.patch
- third_party/llama.cpp/patches/0040-Fix-GLM-DSA-native-MTP-execution.patch
- third_party/llama.cpp/patches/0037-ggml-avoid-GLM-Q3-fused-tail-under-active-policy.patch
- third_party/llama.cpp/patches/0014-Add-non-frame-native-MTP-decode-ABI.patch
- third_party/llama.cpp/patches/0006-Expand-staged-execution-across-dense-and-recurrent-f.patch
- third_party/llama.cpp/patches/0015-Fix-stage-activation-graph-input-allocation.patch
- third_party/llama.cpp/patches/0020-Fix-N-gram-confidence-threshold-indexing.patch
- third_party/llama.cpp/patches/0043-Correct-GLM-DSA-staged-graph-contracts.patch
- third_party/llama.cpp/patches/0038-ggml-shrink-GLM-Q2-gate-up-active-dispatch.patch
- third_party/llama.cpp/patches/0025-Run-GLM-DSA-through-IndexShare-graph.patch
- third_party/llama.cpp/patches/0028-ggml-default-GLM-MoE-two-phase-Metal-path.patch
- third_party/llama.cpp/patches/0019-Re-prime-native-MTP-after-state-restoration.patch
- third_party/llama.cpp/patches/0027-Fix-GLM-DSA-Metal-get_rows-placement.patch
- third_party/llama.cpp/patches/0022-skippy-expose-GLM-DSA-staged-runtime-controls.patch
- third_party/llama.cpp/patches/0032-tests-add-GLM-Q2Q3-selected-weight-roofline.patch
- third_party/llama.cpp/patches/0018-Remove-legacy-session-checkpoint-ABI.patch
- third_party/llama.cpp/patches/0017-Expose-stateful-N-gram-cache-ABI.patch
- third_party/llama.cpp/patches/0034-ggml-use-GLM-max-active-policy-for-Q3-down-kernels.patch
- third_party/llama.cpp/patches/0021-ggml-add-GLM-DSA-sparse-execution-primitives.patch
- third_party/llama.cpp/patches/0044-ggml-metal-make-small-batch-matmul-batch-invariant.patch
- third_party/llama.cpp/patches/0046-Fix-empty-transposed-KV-page-import-and-export.patch
- third_party/llama.cpp/patches/0004-Add-lanes-external-media-and-chat-grammar-support.patch
- third_party/llama.cpp/patches/0016-Recognize-thinking-field-in-chat-auto-parser.patch
- third_party/llama.cpp/patches/0007-Expand-staged-execution-across-VL-and-broad-model-fa.patch
- third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch
- third_party/llama.cpp/patches/0002-Add-early-staged-model-family-and-chat-support.patch
- third_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patch
- third_party/llama.cpp/patches/0031-tests-cover-full-GLM-MoE-selected-chain.patch
- third_party/llama.cpp/patches/0029-ggml-add-GLM-MoE-Metal-selector-diagnostics.patch
- third_party/llama.cpp/patches/0042-Harden-staged-session-and-sideband-bookkeeping.patch
- third_party/llama.cpp/patches/0045-Fix-GLM-DSA-Metal-dispatch-safety.patch
- third_party/llama.cpp/patches/0010-Add-MTP-execution-support-and-sampling-cleanup.patch
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
michaelneale
added a commit
that referenced
this pull request
Jul 29, 2026
…-retention * origin/main: Refresh llama.cpp upstream patch queue (#1105) Revert "Refresh llama.cpp upstream patch queue (#1099)" Refresh llama.cpp upstream patch queue (#1099) chore(code-quality): generalized code-quality refactoring (#1098) Document canonical Homebrew tap (#1102) Rename the Node SDK npm package scope (#1101)
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.
Reapplies the llama.cpp patch-queue refresh from #1099 after its merge was reverted.
Summary
9a3bf2b84923a85583b4ee8177b0cca13824bb03.Root cause
The upstream canary failed before compilation because patch 0002 referenced an obsolete
common/CMakeLists.txtpreimage. Later upstream Metal refactors required a full queue refresh rather than a one-hunk patch fix.Validation
LLAMA_WORKDIR=<temporary directory> scripts/prepare-llama.sh pinnedjust build(patched Metal ABI built successfully; mesh-llm binary produced)./target/debug/mesh-llm --versionSupersedes #1099 following its revert.
Summary by CodeRabbit