Refresh llama.cpp upstream patch queue - #1099
Conversation
📝 WalkthroughWalkthroughThe llama.cpp patch queue adds and evolves the Skippy C ABI, staged execution, sampling, resident-prefix and MTP support, GLM-DSA graph and Metal execution, KV-page handling, chat parsing, and extensive backend test coverage. ChangesSkippy staged runtime
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
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 |
|
This pull request is currently a draft. Reviews will not take place until the PR is marked as ready for review. |
Rebase-conflict rationaleThis is a queue rebase, not a feature change. The canary failed because patch 0002 carried an obsolete common/CMakeLists.txt preimage; upstream had reordered the common source list, so git am --3way could no longer construct a useful fake ancestor. Rebasing the full 47-commit series onto upstream revision 9a3bf2b8 was safer than hand-editing that first mail patch and deferring the later conflicts. Is ours in conflict with theirs?At the patch-text level, yes: both upstream and the carried queue changed the same files and, in several cases, the same control-flow or declaration anchors. At the behavior level, no mutually exclusive intent was identified. Each resolution keeps the upstream behavior and the staged-runtime behavior, selecting the appropriate path by architecture or execution mode. The highest residual risk is the Metal encoder refactor, which is why this remains a draft pending CI canary coverage. 1. Staged OpenAI-MoE filteringThe carried patch makes a stage run layers il_start through il_end, starts from an activation input for a nonzero il_start, and returns an activation boundary when include_output is false. New upstream behavior added two independent output paths:
The rebase makes stage-local selection require all three conditions: this is the final layer of the selected stage, output IDs exist, and embeddings_nextn_masked is enabled. The stage-boundary return remains before t_h_nextn and the output head, so an activation-only stage neither constructs classifier output nor accidentally fills a full-model next-token result. With filtering disabled, control flow remains upstream behavior. 2. GLM-DSA Metal integrationCurrent upstream substantially refactored the Metal encoder path: a formerly monolithic encode implementation is now split into individual operation helpers and updated fusion/concurrency handling. The carried GLM-DSA patch adds sparse-attention, selected-row, routing, weighted-reduction, and GLM-MoE Metal machinery across the operator registry, pipeline lookup, argument structs, C++ dispatch, and Metal shaders. Replacing upstream sections wholesale would have discarded current upstream behavior. Two overlaps were resolved deliberately:
This preserves upstream's new Metal support while keeping all carried GLM-DSA kernels and their registrations connected to the current encoder architecture. 3. Architecture-test metadataThe architecture test helper builds synthetic GGUF metadata. Upstream added MiniMax M3 / MSA requirements: its indexer uses one head per GQA/KV head, plus block-size and local-block metadata. The carried GLM-DSA tests require different metadata: one indexer head, a 128-wide key, fixture-controlled top-k, and IndexShare metadata for top-k frequency, skip offset, and full/shared indexer-type sequences. The resolved helper selects the M3 head count only for MiniMax M3, preserves its block/local fields, and selects the GLM-DSA key/top-k and IndexShare fixtures only for GLM-DSA. Neither synthetic model consumes the other architecture's metadata. Validation and remaining coverage
The scheduled canary remains the Linux/Rust and smoke-test validation for this exact upstream revision. No mesh wire-protocol change or intentional staged-runtime ABI change is included here. |
![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 (16)
third_party/llama.cpp/patches/0021-ggml-add-GLM-DSA-sparse-execution-primitives.patch (5)
2951-2956: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
GGML_TYPE_Q2_0was replaced byGGML_TYPE_TQ2_0in the mul_mv/mul_mv_id type switches, removing Q2_0 dispatch.Both switches previously had a
case GGML_TYPE_Q2_0:arm settingN_SG_Q2_0/N_R0_Q2_0. This patch substitutesGGML_TYPE_TQ2_0in place of it rather than adding it, soGGML_TYPE_Q2_0now falls through to:default: GGML_LOG_ERROR("Asserting on type %d\n", (int) tsrc0); GGML_ABORT("not implemented");The
kernel_mul_mv_q2_0_f32kernel and theN_R0_Q2_0/N_SG_Q2_0constants are still present in the patch, andggml_metal_device_supports_opstill reportsGGML_OP_MUL_MATas supported — so a Q2_0 weight tensor aborts the process at encode time instead of falling back. If the intent was to add TQ2_0, both cases need to be present.🐛 Proposed fix — keep both quant types
+ case GGML_TYPE_Q2_0: + { + nsg = N_SG_Q2_0; + nr0 = N_R0_Q2_0; + } break; case GGML_TYPE_TQ2_0: { nsg = N_SG_TQ2_0; nr0 = N_R0_TQ2_0; } break;Also applies to: 3199-3204
🤖 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 2951 - 2956, Restore the GGML_TYPE_Q2_0 dispatch arms in both mul_mv and mul_mv_id type switches, preserving their N_SG_Q2_0 and N_R0_Q2_0 assignments. Keep the newly added GGML_TYPE_TQ2_0 cases alongside them so both quantization types are supported.
3255-3282: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
suffixaliases thenamebuffer that is overwritten a few lines later.
snprintf(name, sizeof(name), "_r%d", nr0); suffix = name;makessuffixpoint intoname, which is then reused bysnprintf(name, 256, "%s_nsg=%d", base, nsg);. It happens to work only becausebaseis formatted fromsuffixfirst. Use a dedicated buffer so a future reordering cannot silently corrupt the kernel name.♻️ Proposed fix
+ char suffix_buf[32]; ... - } else if (nr0 != N_R0_Q2_K) { - snprintf(name, sizeof(name), "_r%d", nr0); - suffix = name; - } + } else if (nr0 != N_R0_Q2_K) { + snprintf(suffix_buf, sizeof(suffix_buf), "_r%d", nr0); + suffix = suffix_buf; + }(and the equivalent change in the
GGML_TYPE_Q3_Karm)🤖 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 3255 - 3282, In the kernel-name construction logic, update the suffix handling in the GGML_TYPE_Q2_K and GGML_TYPE_Q3_K cases so the generated "_r%d" suffix is stored in a dedicated buffer rather than aliasing the reusable name buffer. Keep suffix pointing to the dedicated storage before the later "%s_nsg=%d" formatting step, while preserving the existing GLM-down suffix behavior.
31371-31375: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDefault-initialize
indexer_types.
std::array<int32_t, LLAMA_MAX_LAYERS> indexer_types;is left indeterminate while the adjacent fields all have initializers. The comment documents-1as "unknown", so make that the default rather than relying on every construction path to populate it beforeindexer_types_presentis consulted.🛡️ Proposed fix
- std::array<int32_t, LLAMA_MAX_LAYERS> indexer_types; // -1 unknown, 0 shared, 1 full + std::array<int32_t, LLAMA_MAX_LAYERS> indexer_types = { }; // -1 unknown, 0 shared, 1 fullNote
{ }value-initializes to0; if-1is the intended sentinel, prefer an explicit fill in the constructor orggml-style reset helper.🤖 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 31371 - 31375, Default-initialize the indexer_types member used by the surrounding indexer state, ensuring every element starts at -1 to represent “unknown.” Add this initialization in the relevant constructor or ggml-style reset helper rather than using empty-brace array initialization, which would produce zeros; preserve indexer_types_present and the existing adjacent field defaults.
1101-1125: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winUnconditional 9 MiB of fusion scratch is allocated for every Metal context.
GGML_METAL_FUSION_SCRATCH_SIZE(1 MiB) is allocated for allGGML_METAL_MAX_COMMAND_BUFFERS + 1lanes atggml_metal_inittime, even though the private-resource paths that consume it are gated behind opt-in env flags (GGML_METAL_EXPERIMENTAL_GLM_MOE_PRIVATE_SCRATCH,..._TWO_PHASE,..._DUAL_LANE). Every process that touches the Metal backend pays ~9 MiB of wired device memory it will never use.Consider allocating lazily on first use, or gating the allocation on the same predicate the consumers use.
🤖 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 1101 - 1125, Remove the unconditional fusion-scratch allocation from ggml_metal_init and avoid freeing uninitialized lanes in ggml_metal_free. Allocate fusion_scratch lazily on first use, or gate initialization with the same opt-in predicate used by the private-resource consumers (GGML_METAL_EXPERIMENTAL_GLM_MOE_PRIVATE_SCRATCH, TWO_PHASE, and DUAL_LANE), while preserving allocation-failure handling for enabled paths.
1129-1137: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
n_cb_activenarrows the command-buffer status sweep across graphs.
ggml_metal_synchronizenow iteratescb_idx <= ctx->n_cb_active. When a graph setsn_cb_active = 0(becausen_nodes_1 == 0), command buffers retained from a previous, wider graph at indices1..n_cbare no longer status-checked or reported on failure. Please confirm those buffers are already released/reset elsewhere before the nextsynchronize.#!/bin/bash # Check how cmd_bufs entries are reset/released between graphs. rg -nP -C8 'cmd_bufs\[' -g '**/ggml-metal-context.m'Also applies to: 1307-1312
🤖 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 1129 - 1137, Update ggml_metal_synchronize to status-check every allocated command buffer through ctx->n_cb, not only the graph’s ctx->n_cb_active count, so buffers retained from wider graphs are still reported. Verify the cmd_bufs reset/release lifecycle before synchronization and preserve the same full-range behavior at the other affected occurrence.third_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patch (1)
220-221: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFail closed when determining phase-gate success.
The summary treats any occurrence of
"OK"as a passed gate. A successful command can still emit a failure summary alongside anotherOKline, producing a false-positive report. Parse an exact success marker or propagate an explicit gate result instead.🤖 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 220 - 221, Update the phase-gate result handling around phase_text and the appended summary line to fail closed: do not treat arbitrary occurrences of “OK” as success. Parse the command’s exact success marker or propagate an explicit exit/result status, and report the gate as passed only when that definitive success condition is met; otherwise retain the failure/inspection message.third_party/llama.cpp/patches/0025-Run-GLM-DSA-through-IndexShare-graph.patch (2)
18-33: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winValidate the complete staged layer range before indexing.
il_startis used without bounds validation, while onlyil_endis clamped. A negative or oversizedlayer_startcan indexmodel.layers[il]; an empty range later emitsstage_boundarywithil_end - 1. Reject or normalize0 <= il_start < il_end <= n_layerbefore the validation loop.Also applies to: 68-76
🤖 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/0025-Run-GLM-DSA-through-IndexShare-graph.patch` around lines 18 - 33, Validate the staged range immediately after computing il_start and il_end, requiring 0 <= il_start < il_end <= effective_n_layers before any layer indexing or stage-boundary use. Reject invalid or empty ranges consistently, then keep the existing full-indexer and split K_B/V_B validation for the validated range.
29-33: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not reject the fused KV_B format introduced by patch 0024.
third_party/llama.cpp/patches/0024-Support-GLM-DSA-fused-KV_B-tensors.patchmakeswk_b/wv_boptional and populateswkv_bfor fused files, but this loop throws whenever either split tensor is absent. Fused GLM-DSA GGUFs therefore fail before graph construction. Select the split or fused graph path based on the tensors actually loaded.🤖 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/0025-Run-GLM-DSA-through-IndexShare-graph.patch` around lines 29 - 33, Update the GLM-DSA validation and graph construction around the loop checking model.layers[il].wk_b and wv_b to accept either split K_B/V_B tensors or the fused wkv_b tensor. Select the split or fused graph path based on which tensors are loaded, and only reject a layer when neither valid representation is available.third_party/llama.cpp/patches/0046-Fix-empty-transposed-KV-page-import-and-export.patch (1)
144-176: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winMake the KV-page import test non-vacuous.
The payload is imported back into the same cache that produced it, and the cached K data is never cleared or changed. An import implementation that ignores the payload could therefore still pass the final byte comparison. Import into a fresh cache, or clear/overwrite the destination page before importing, then verify the restored payload.
🤖 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/0046-Fix-empty-transposed-KV-page-import-and-export.patch` around lines 144 - 176, Make the K-only roundtrip test around stage_import_kv_page non-vacuous by importing the exported payload into a fresh cache or after clearing/overwriting the destination page. Ensure the subsequent stage_export_kv_page comparison verifies that import restored the data from payload rather than returning unchanged cached contents.third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch (2)
22-48: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winRoute active-count Q3 kernels through the specialized pipelines.
The active-count branches currently return the generic
...r8_nb8_w0pipeline, whosensgremains 8. Consequently, the newa6/a4/a2kernels are not dispatched and the max-active policy has no runtime effect.
third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch#L22-L48: compile the active-specifickernel_mul_mv_id_q3_K_wr_slots_r8_a{6,4,2}entry point instead of the generic kernel.third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch#L184-L195: call the active6/active4/active2 pipeline getters.third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch#L252-L297: ensure the newly added active entry points are the symbols used by the pipeline builder.third_party/llama.cpp/patches/0034-ggml-use-GLM-max-active-policy-for-Q3-down-kernels.patch#L75-L83: select the corresponding active getter for eachq3_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 22 - 48, Route active-count Q3 kernels through the specialized entry points instead of the generic nsg=8 pipeline. In third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch lines 22-48, update ggml_metal_library_get_pipeline_mul_mv_id_q3_weighted_reduce_slots_sg_r8_nb8_w0_active to compile kernel_mul_mv_id_q3_K_wr_slots_r8_a6, a4, or a2 based on active_slots; lines 184-195 must call the corresponding active6/active4/active2 pipeline getters, and lines 252-297 must register those active entry-point symbols with the pipeline builder. In third_party/llama.cpp/patches/0034-ggml-use-GLM-max-active-policy-for-Q3-down-kernels.patch lines 75-83, select the matching active getter for each q3_active_count value.
397-408: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winActivate the active-count paths in these test cases.
The static-motif and
glb_moe_down_reduceentries only encodeactive_expertsvalues; withoutGGML_METAL_ENABLE_Q3_DOWN_SLOT_PARALLEL_REDUCE_ACTIVE6/4/2, those cases hit the fallback Q3 reduce path. Scope those env vars around the added cases in both eval and perf registrations, and avoid suppressing the max-active policy cases when coverage is meant to exercise 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/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch` around lines 397 - 408, Scope GGML_METAL_ENABLE_Q3_DOWN_SLOT_PARALLEL_REDUCE_ACTIVE6/4/2 around the added active-count test registrations for test_glm_moe_static_motif and test_glm_moe_down_reduce in third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch ranges 397-408 and 416-426, covering both eval and perf cases. In third_party/llama.cpp/patches/0034-ggml-use-GLM-max-active-policy-for-Q3-down-kernels.patch range 91-103, preserve the max-active policy cases without disabling or overriding the environment settings needed to exercise that coverage.third_party/llama.cpp/patches/0032-tests-add-GLM-Q2Q3-selected-weight-roofline.patch (1)
32-38: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply the roofline environment settings during evaluation too.
eval_perf()sets the reference, chunk, and bypass variables, but these variants are also registered inmake_test_cases_eval()via a base class that does not provide an inheritedeval()with the same setup. Add an overriddeneval()with the samescoped_test_envsetup or remove these cases from the evaluation list.🤖 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/0032-tests-add-GLM-Q2Q3-selected-weight-roofline.patch` around lines 32 - 38, Ensure the GLM roofline variants registered by make_test_cases_eval() apply the same GGML_METAL_ENABLE_GLM_MOE_DECODE_MOTIF_REFERENCE, GGML_METAL_EXPERIMENTAL_GLM_MOE_Q2_WEIGHT_ROOFLINE_CHUNKS, and bypass-route scoped_test_env settings during evaluation. Add an eval() override alongside eval_perf() that preserves the bypass_route value and delegates to the base test evaluation, or remove these variants from make_test_cases_eval().third_party/llama.cpp/patches/0018-Remove-legacy-session-checkpoint-ABI.patch (1)
20-28: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winBump the Skippy ABI version for the removed exports.
This patch removes public functions and a public feature flag, but does not update
SKIPPY_ABI_VERSION_PATCH. Clients compiled against the previous ABI can still observe the old version while those symbols are unavailable. Increment the ABI version, or document and enforce a deliberate compatibility policy.Also applies to: 37-42
🤖 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/0018-Remove-legacy-session-checkpoint-ABI.patch` around lines 20 - 28, Update SKIPPY_ABI_VERSION_PATCH in the same patch to reflect removal of the skippy_checkpoint_session and skippy_restore_session_checkpoint exports and the associated public feature flag. Ensure clients can distinguish this ABI from the previous version; if compatibility is intentionally preserved instead, document and enforce that policy consistently.third_party/llama.cpp/patches/0042-Harden-staged-session-and-sideband-bookkeeping.patch (1)
93-95: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winValidate
request_countbefore narrowing it toint32_t.
GGML_ASSERTis not a release-build guard. A value aboveINT32_MAXcan narrow to an invalid or negativen_tokens, causing incorrect batch allocation and iteration. ReturnSKIPPY_STATUS_INVALID_ARGUMENTbefore the cast.Proposed fix
- const int32_t n_tokens = static_cast<int32_t>(request_count); - GGML_ASSERT(static_cast<size_t>(n_tokens) == request_count); + if (request_count > static_cast<size_t>(std::numeric_limits<int32_t>::max())) { + skippy_set_error(out_error, SKIPPY_STATUS_INVALID_ARGUMENT, "request_count is too large"); + return SKIPPY_STATUS_INVALID_ARGUMENT; + } + const int32_t n_tokens = static_cast<int32_t>(request_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/0042-Harden-staged-session-and-sideband-bookkeeping.patch` around lines 93 - 95, Validate request_count against INT32_MAX and return SKIPPY_STATUS_INVALID_ARGUMENT before assigning it to the int32_t n_tokens variable; remove the GGML_ASSERT-based narrowing check and preserve the existing llama_batch initialization for valid counts.third_party/llama.cpp/patches/0017-Expose-stateful-N-gram-cache-ABI.patch (1)
52-66: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClear the retained N-gram cache layers on reset.
skippy_ngram_cache_reset()only clearshistoryandcontext, whileskippy_ngram_cache_draft()also usesdynamicandstatic_cache. Since this API is request-local and draft state can leak from a prior conversation, reset those layers too, or document that they are intentionally sticky/shared.🤖 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 skippy_ngram_cache_reset() to clear the dynamic and static_cache layers in addition to history and context whenever reset is requested, ensuring all request-local draft state is discarded together.third_party/llama.cpp/patches/0010-Add-MTP-execution-support-and-sampling-cleanup.patch (1)
1981-1983: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMissing
skippy_set_errorbefore UNSUPPORTED return leavesout_errorunset.In
skippy_decode_step_frame_batch_sampled, the Gemma3n AltUp branch returnsSKIPPY_STATUS_UNSUPPORTEDdirectly without callingskippy_set_error, unlike the sibling check a few lines earlier in the same function (skippy_output_activation_flags(...) != 0) which does call it. Callers relying onout_errorfor a message when status != OK will get a stale/null error pointer for this specific failure path.🐛 Proposed fix
if ((input_desc->flags & SKIPPY_ACTIVATION_FLAG_GEMMA3N_ALTUP) != 0) { - return SKIPPY_STATUS_UNSUPPORTED; + skippy_set_error(out_error, SKIPPY_STATUS_UNSUPPORTED, "batched activation decode does not support Gemma3n AltUp sidebands yet"); + return SKIPPY_STATUS_UNSUPPORTED; } else if (n_embd_inp == n_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/0010-Add-MTP-execution-support-and-sampling-cleanup.patch` around lines 1981 - 1983, Update the Gemma3n AltUp branch in skippy_decode_step_frame_batch_sampled to call skippy_set_error with an appropriate unsupported-operation message before returning SKIPPY_STATUS_UNSUPPORTED, matching the existing sibling failure path and ensuring out_error is populated.
🧹 Nitpick comments (6)
third_party/llama.cpp/patches/0021-ggml-add-GLM-DSA-sparse-execution-primitives.patch (5)
6902-6965: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the private-scratch ownership contract for
fused_range_outputs.The
set_fused_range_outputs/tracks_fused_range_outputmechanism deliberately withholds fused intermediates fromggml_mem_ranges, which is correct only as long as those intermediates never escape the fused span. That invariant is enforced today bycan_fuse_filtered_subgraph's use-count andview_srcchecks, but the connection between the two is not stated anywhere. A short comment onset_fused_range_outputsnaming the invariant ("callers must have validated viacan_fuse*that untracked nodes have no consumers outside the span") would make a future regression much harder to introduce.Also applies to: 11505-11517
🤖 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 6902 - 6965, Document the ownership invariant at set_fused_range_outputs: callers must first validate through can_fuse_filtered_subgraph or the relevant can_fuse* checks that untracked fused intermediates have no consumers or view sources outside the fused span. Keep the implementation unchanged and place the concise contract comment directly with the fused-range output mechanism.
5766-6320: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffThe env-flag surface for GLM-DSA Metal paths has grown past the point of being auditable.
There are now ~70
GGML_METAL_EXPERIMENTAL_*/SKIPPY_GLM_DSA_*predicates selecting among a dozen near-identical Q2/Q3 gate-up and down-projection kernels, several with overlapping enable/disable pairs and shape-conditional defaults. Two concrete consequences:
- The effective kernel for a given build is not determinable without tracing the predicate chain, which makes performance regressions hard to bisect.
- Several flags are mutually exclusive by construction but nothing enforces or reports that.
Since the dispatch logger already exists, consider emitting a one-time summary of the resolved configuration at first encode, and pruning the variants that lost their bake-off.
🤖 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 5766 - 6320, The GLM-DSA Metal dispatch configuration is too difficult to audit because numerous overlapping environment predicates select mutually exclusive kernel variants without visibility. Consolidate or remove obsolete Q2/Q3 gate-up and down-projection flags and variants that lost their bake-off, enforce mutually exclusive selections, and use the existing dispatch logger to emit a one-time summary of the resolved configuration at first encode, centered on the predicate helpers such as ggml_metal_glm_dsa_q2_gate_up_swiglu_pair_sg_any_variant_enabled and the relevant encode/dispatch path.
1298-1305: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEnv override silently discards the caller's
n_cb.
SKIPPY_GLM_DSA_METAL_PROFILE_N_CBreplaces the value the backend requested with no validation and no log line. A stray or malformed value (e.g.0, negative, non-numeric) becomes the scheduling width for the whole session with no diagnostic. At minimum clamp to[1, GGML_METAL_MAX_COMMAND_BUFFERS]and log when the override fires.🤖 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 1298 - 1305, Validate the SKIPPY_GLM_DSA_METAL_PROFILE_N_CB override in ggml_metal_set_n_cb before replacing the caller’s n_cb: parse it strictly, clamp valid values to [1, GGML_METAL_MAX_COMMAND_BUFFERS], and retain the caller-provided value for malformed or non-positive input. Log whenever the environment override is applied, including the effective value.
137-218: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCache the diagnostic env lookup and harden the
snprintfoffset arithmetic.
ggml_backend_sched_glm_route_diag_enabled()callsgetenvtwice on everyggml_backend_sched_split_graphinvocation, and on truncationsnprintfreturns the would-be length, sooffsetcan exceedsizeof(ops). The loop guard prevents an out-of-bounds write today, but the invariant is fragile.♻️ Proposed hardening
-static bool ggml_backend_sched_glm_route_diag_enabled() { - const char * log = getenv("GGML_METAL_MOE_DISPATCH_LOG"); - if (log && atoi(log) != 0) { - return true; - } - log = getenv("SKIPPY_GLM_DSA_LOG_METAL_DISPATCH"); - return log && atoi(log) != 0; -} +static bool ggml_backend_sched_glm_route_diag_enabled() { + static const bool enabled = [] { + const char * log = getenv("GGML_METAL_MOE_DISPATCH_LOG"); + if (log && atoi(log) != 0) { + return true; + } + log = getenv("SKIPPY_GLM_DSA_LOG_METAL_DISPATCH"); + return log && atoi(log) != 0; + }(); + return enabled; +}- if (written < 0) { + if (written < 0 || (size_t) written >= sizeof(ops) - offset) { break; }🤖 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 137 - 218, Update ggml_backend_sched_glm_route_diag_enabled to cache the diagnostic environment lookup result instead of calling getenv on every invocation, while preserving both supported environment variables and their precedence. In ggml_backend_sched_log_glm_route_splits, guard the snprintf offset update against truncation by only advancing offset when the written length fits the remaining buffer; otherwise terminate the operation-string construction safely without allowing offset to exceed sizeof(ops).
1341-1540: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winEnvironment-variable configuration is re-read on hot paths in both the Metal pipeline selector and the scheduler diagnostics. The shared root cause is that every
*_enabled()/*_requested()helper callsgetenv(often two or three times) on each invocation, even though the values cannot change during the process lifetime. Cache each helper's result in a function-localstatic const.
third_party/llama.cpp/patches/0021-ggml-add-GLM-DSA-sparse-execution-primitives.patch#L1341-L1540: convert theggml_metal_glm_dsa_*_requested()/*_enabled()helpers inggml-metal-device.cppto cache their parsed value, so pipeline-cache hits no longer paygetenv/atoi.third_party/llama.cpp/patches/0021-ggml-add-GLM-DSA-sparse-execution-primitives.patch#L137-L218: cacheggml_backend_sched_glm_route_diag_enabled()inggml-backend.cppsoggml_backend_sched_split_graphdoes not callgetenvtwice per split.🤖 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 1341 - 1540, Cache the parsed environment-variable results in function-local static const values for every ggml_metal_glm_dsa_*_requested() and *_enabled() helper in the 1341-1540 range, preserving each helper’s current fallback and validation behavior. Also update ggml_backend_sched_glm_route_diag_enabled() in third_party/llama.cpp/patches/0021-ggml-add-GLM-DSA-sparse-execution-primitives.patch lines 137-218 to cache its result, eliminating repeated getenv/atoi calls while retaining existing configuration semantics.third_party/llama.cpp/patches/0010-Add-MTP-execution-support-and-sampling-cleanup.patch (1)
4029-4046: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffMTP draft metadata smuggled into the
llama_tokenoutput buffer.
skippy_verify_tokens_frame_sampledpacksmtp_draft.token_count,mtp_draft.token_ids[0], and a truncatedproposal_compute_us(cast into allama_token/int32) into extra slots ofoutput_tokens, relying on the caller reservingtoken_count + 3capacity and knowing this undocumented convention. Every other MTP-producing entry point (skippy_decode_step_frame_sampled_mtp) returns the draft via the dedicatedskippy_native_mtp_draftstruct instead. This inconsistent, type-unsafe encoding is easy to misuse from FFI callers and loses precision onproposal_compute_usfor larger values.Please confirm intentional design vs. oversight — if intentional, consider adding an
out_mtp_draftout-parameter (as used elsewhere) instead of overloading the token buffer.🤖 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 4029 - 4046, Remove the MTP metadata packing from skippy_verify_tokens_frame_sampled’s output_tokens buffer and expose the draft through a dedicated skippy_native_mtp_draft out-parameter, matching skippy_decode_step_frame_sampled_mtp. Update the function signature and all callers to pass and consume this structured result, while keeping output_tokens limited to actual verified tokens and preserving proposal_compute_us precision.
🤖 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/0010-Add-MTP-execution-support-and-sampling-cleanup.patch`:
- Around line 1981-1983: Update the Gemma3n AltUp branch in
skippy_decode_step_frame_batch_sampled to call skippy_set_error with an
appropriate unsupported-operation message before returning
SKIPPY_STATUS_UNSUPPORTED, matching the existing sibling failure path and
ensuring out_error is populated.
In `@third_party/llama.cpp/patches/0017-Expose-stateful-N-gram-cache-ABI.patch`:
- Around line 52-66: Update skippy_ngram_cache_reset() to clear the dynamic and
static_cache layers in addition to history and context whenever reset is
requested, ensuring all request-local draft state is discarded together.
In
`@third_party/llama.cpp/patches/0018-Remove-legacy-session-checkpoint-ABI.patch`:
- Around line 20-28: Update SKIPPY_ABI_VERSION_PATCH in the same patch to
reflect removal of the skippy_checkpoint_session and
skippy_restore_session_checkpoint exports and the associated public feature
flag. Ensure clients can distinguish this ABI from the previous version; if
compatibility is intentionally preserved instead, document and enforce that
policy consistently.
In
`@third_party/llama.cpp/patches/0021-ggml-add-GLM-DSA-sparse-execution-primitives.patch`:
- Around line 2951-2956: Restore the GGML_TYPE_Q2_0 dispatch arms in both mul_mv
and mul_mv_id type switches, preserving their N_SG_Q2_0 and N_R0_Q2_0
assignments. Keep the newly added GGML_TYPE_TQ2_0 cases alongside them so both
quantization types are supported.
- Around line 3255-3282: In the kernel-name construction logic, update the
suffix handling in the GGML_TYPE_Q2_K and GGML_TYPE_Q3_K cases so the generated
"_r%d" suffix is stored in a dedicated buffer rather than aliasing the reusable
name buffer. Keep suffix pointing to the dedicated storage before the later
"%s_nsg=%d" formatting step, while preserving the existing GLM-down suffix
behavior.
- Around line 31371-31375: Default-initialize the indexer_types member used by
the surrounding indexer state, ensuring every element starts at -1 to represent
“unknown.” Add this initialization in the relevant constructor or ggml-style
reset helper rather than using empty-brace array initialization, which would
produce zeros; preserve indexer_types_present and the existing adjacent field
defaults.
- Around line 1101-1125: Remove the unconditional fusion-scratch allocation from
ggml_metal_init and avoid freeing uninitialized lanes in ggml_metal_free.
Allocate fusion_scratch lazily on first use, or gate initialization with the
same opt-in predicate used by the private-resource consumers
(GGML_METAL_EXPERIMENTAL_GLM_MOE_PRIVATE_SCRATCH, TWO_PHASE, and DUAL_LANE),
while preserving allocation-failure handling for enabled paths.
- Around line 1129-1137: Update ggml_metal_synchronize to status-check every
allocated command buffer through ctx->n_cb, not only the graph’s
ctx->n_cb_active count, so buffers retained from wider graphs are still
reported. Verify the cmd_bufs reset/release lifecycle before synchronization and
preserve the same full-range behavior at the other affected occurrence.
In
`@third_party/llama.cpp/patches/0023-tests-cover-native-GLM-DSA-execution-paths.patch`:
- Around line 220-221: Update the phase-gate result handling around phase_text
and the appended summary line to fail closed: do not treat arbitrary occurrences
of “OK” as success. Parse the command’s exact success marker or propagate an
explicit exit/result status, and report the gate as passed only when that
definitive success condition is met; otherwise retain the failure/inspection
message.
In
`@third_party/llama.cpp/patches/0025-Run-GLM-DSA-through-IndexShare-graph.patch`:
- Around line 18-33: Validate the staged range immediately after computing
il_start and il_end, requiring 0 <= il_start < il_end <= effective_n_layers
before any layer indexing or stage-boundary use. Reject invalid or empty ranges
consistently, then keep the existing full-indexer and split K_B/V_B validation
for the validated range.
- Around line 29-33: Update the GLM-DSA validation and graph construction around
the loop checking model.layers[il].wk_b and wv_b to accept either split K_B/V_B
tensors or the fused wkv_b tensor. Select the split or fused graph path based on
which tensors are loaded, and only reject a layer when neither valid
representation is available.
In
`@third_party/llama.cpp/patches/0032-tests-add-GLM-Q2Q3-selected-weight-roofline.patch`:
- Around line 32-38: Ensure the GLM roofline variants registered by
make_test_cases_eval() apply the same
GGML_METAL_ENABLE_GLM_MOE_DECODE_MOTIF_REFERENCE,
GGML_METAL_EXPERIMENTAL_GLM_MOE_Q2_WEIGHT_ROOFLINE_CHUNKS, and bypass-route
scoped_test_env settings during evaluation. Add an eval() override alongside
eval_perf() that preserves the bypass_route value and delegates to the base test
evaluation, or remove these variants from make_test_cases_eval().
In
`@third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch`:
- Around line 22-48: Route active-count Q3 kernels through the specialized entry
points instead of the generic nsg=8 pipeline. In
third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch
lines 22-48, update
ggml_metal_library_get_pipeline_mul_mv_id_q3_weighted_reduce_slots_sg_r8_nb8_w0_active
to compile kernel_mul_mv_id_q3_K_wr_slots_r8_a6, a4, or a2 based on
active_slots; lines 184-195 must call the corresponding active6/active4/active2
pipeline getters, and lines 252-297 must register those active entry-point
symbols with the pipeline builder. In
third_party/llama.cpp/patches/0034-ggml-use-GLM-max-active-policy-for-Q3-down-kernels.patch
lines 75-83, select the matching active getter for each q3_active_count value.
- Around line 397-408: Scope
GGML_METAL_ENABLE_Q3_DOWN_SLOT_PARALLEL_REDUCE_ACTIVE6/4/2 around the added
active-count test registrations for test_glm_moe_static_motif and
test_glm_moe_down_reduce in
third_party/llama.cpp/patches/0033-ggml-add-active-count-Q3-GLM-MoE-down-kernels.patch
ranges 397-408 and 416-426, covering both eval and perf cases. In
third_party/llama.cpp/patches/0034-ggml-use-GLM-max-active-policy-for-Q3-down-kernels.patch
range 91-103, preserve the max-active policy cases without disabling or
overriding the environment settings needed to exercise that coverage.
In
`@third_party/llama.cpp/patches/0042-Harden-staged-session-and-sideband-bookkeeping.patch`:
- Around line 93-95: Validate request_count against INT32_MAX and return
SKIPPY_STATUS_INVALID_ARGUMENT before assigning it to the int32_t n_tokens
variable; remove the GGML_ASSERT-based narrowing check and preserve the existing
llama_batch initialization for valid counts.
In
`@third_party/llama.cpp/patches/0046-Fix-empty-transposed-KV-page-import-and-export.patch`:
- Around line 144-176: Make the K-only roundtrip test around
stage_import_kv_page non-vacuous by importing the exported payload into a fresh
cache or after clearing/overwriting the destination page. Ensure the subsequent
stage_export_kv_page comparison verifies that import restored the data from
payload rather than returning unchanged cached contents.
---
Nitpick comments:
In
`@third_party/llama.cpp/patches/0010-Add-MTP-execution-support-and-sampling-cleanup.patch`:
- Around line 4029-4046: Remove the MTP metadata packing from
skippy_verify_tokens_frame_sampled’s output_tokens buffer and expose the draft
through a dedicated skippy_native_mtp_draft out-parameter, matching
skippy_decode_step_frame_sampled_mtp. Update the function signature and all
callers to pass and consume this structured result, while keeping output_tokens
limited to actual verified tokens and preserving proposal_compute_us precision.
In
`@third_party/llama.cpp/patches/0021-ggml-add-GLM-DSA-sparse-execution-primitives.patch`:
- Around line 6902-6965: Document the ownership invariant at
set_fused_range_outputs: callers must first validate through
can_fuse_filtered_subgraph or the relevant can_fuse* checks that untracked fused
intermediates have no consumers or view sources outside the fused span. Keep the
implementation unchanged and place the concise contract comment directly with
the fused-range output mechanism.
- Around line 5766-6320: The GLM-DSA Metal dispatch configuration is too
difficult to audit because numerous overlapping environment predicates select
mutually exclusive kernel variants without visibility. Consolidate or remove
obsolete Q2/Q3 gate-up and down-projection flags and variants that lost their
bake-off, enforce mutually exclusive selections, and use the existing dispatch
logger to emit a one-time summary of the resolved configuration at first encode,
centered on the predicate helpers such as
ggml_metal_glm_dsa_q2_gate_up_swiglu_pair_sg_any_variant_enabled and the
relevant encode/dispatch path.
- Around line 1298-1305: Validate the SKIPPY_GLM_DSA_METAL_PROFILE_N_CB override
in ggml_metal_set_n_cb before replacing the caller’s n_cb: parse it strictly,
clamp valid values to [1, GGML_METAL_MAX_COMMAND_BUFFERS], and retain the
caller-provided value for malformed or non-positive input. Log whenever the
environment override is applied, including the effective value.
- Around line 137-218: Update ggml_backend_sched_glm_route_diag_enabled to cache
the diagnostic environment lookup result instead of calling getenv on every
invocation, while preserving both supported environment variables and their
precedence. In ggml_backend_sched_log_glm_route_splits, guard the snprintf
offset update against truncation by only advancing offset when the written
length fits the remaining buffer; otherwise terminate the operation-string
construction safely without allowing offset to exceed sizeof(ops).
- Around line 1341-1540: Cache the parsed environment-variable results in
function-local static const values for every ggml_metal_glm_dsa_*_requested()
and *_enabled() helper in the 1341-1540 range, preserving each helper’s current
fallback and validation behavior. Also update
ggml_backend_sched_glm_route_diag_enabled() in
third_party/llama.cpp/patches/0021-ggml-add-GLM-DSA-sparse-execution-primitives.patch
lines 137-218 to cache its result, eliminating repeated getenv/atoi calls while
retaining existing configuration semantics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a609078d-a921-4209-a4a2-c4090aa28ef3
📒 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
There was a problem hiding this comment.
I'm good with this, but we should consider implementing the changes suggested from @CodeRabbit before merging. I have an agent working on this currently... please stand by
…-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)
2 participants
Summary
Root cause
The upstream canary failed before compilation because patch 0002 referenced an obsolete common/CMakeLists.txt preimage. Later upstream Metal refactors required a full queue refresh rather than a one-hunk patch fix.
Validation
Summary by CodeRabbit
New Features
Bug Fixes
Tests