[codex] Update pinned llama.cpp revision - #906
Merged
Merged
Conversation
📝 WalkthroughWalkthroughSkippy’s public C ABI, session/runtime controls, and runtime-slice graph plumbing are expanded across the patch series. The changes add and revise session, sampling, chat, backend, telemetry, sideband, and model-family execution entry points, along with patch-series metadata updates. ChangesSkippy ABI and session APIs
Runtime-slice loaders and graph plumbing
Patch metadata and upstream refresh
Sequence Diagram(s)sequenceDiagram
participant skippy_prefill_chunk_frame_sampled
participant skippy_decode_activation_frame
participant llama_decode
participant skippy_copy_output_activation_frame
skippy_prefill_chunk_frame_sampled->>skippy_decode_activation_frame: filtered token_ids and activation frame
skippy_decode_activation_frame->>llama_decode: decode batch
llama_decode-->>skippy_decode_activation_frame: logits and activations
skippy_decode_activation_frame->>skippy_copy_output_activation_frame: copy output frame
skippy_copy_output_activation_frame-->>skippy_prefill_chunk_frame_sampled: output payload
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
third_party/llama.cpp/patches/0083-skippy-add-model-open-runtime-events-ABI.patch (1)
118-148: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDocument
detail_ptrlifetime and callback threading in the ABI.This is a public C callback contract. Add a short comment that
detail_ptris only valid for the duration of the callback and whether callbacks may arrive from loader/worker threads, so consumers don’t retain borrowed pointers or assume single-threaded delivery.Suggested ABI comment
struct skippy_runtime_event_v1 { + // Any pointer fields are borrowed and valid only for the duration of the callback. + // Callbacks may be emitted from the model-open thread or loader worker callbacks + // as indicated by `emitter`; consumers that retain data must copy it. uint32_t abi_version;🤖 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/0083-skippy-add-model-open-runtime-events-ABI.patch` around lines 118 - 148, The public ABI in skippy_runtime_event_v1 and skippy_runtime_event_callback needs a short contract note for detail_ptr and delivery context. Add comments near skippy_runtime_event_v1 and skippy_runtime_event_reporter_v1 stating that detail_ptr is a borrowed pointer valid only during the callback, and clarify whether the callback may be invoked from loader/worker threads or only on a single thread. Keep the guidance close to these symbols so consumers of the ABI know not to retain the pointer or assume serialized delivery.third_party/llama.cpp/patches/0092-Add-Skippy-greedy-sampling-fast-path.patch (1)
29-30: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache the env gate outside the sampling hot path.
skippy_sample_token_ithcan hit this on every token, so re-reading the process environment here adds avoidable overhead to the fast path.Proposed refactor
static bool skippy_mtp_greedy_sampling_fastpath_enabled() { - return skippy_env_enabled("SKIPPY_NATIVE_MTP_GREEDY_SAMPLING_FASTPATH"); + static const bool enabled = + skippy_env_enabled("SKIPPY_NATIVE_MTP_GREEDY_SAMPLING_FASTPATH"); + return enabled; }🤖 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/0092-Add-Skippy-greedy-sampling-fast-path.patch` around lines 29 - 30, The fast-path env check in skippy_mtp_greedy_sampling_fastpath_enabled() is being evaluated from skippy_sample_token_ith on every token, so move that gate out of the sampling hot path and cache the result once at initialization or first use. Update the sampling path to read the cached boolean instead of calling skippy_env_enabled("SKIPPY_NATIVE_MTP_GREEDY_SAMPLING_FASTPATH") repeatedly, keeping the logic centralized in skippy_mtp_greedy_sampling_fastpath_enabled or its caller.third_party/llama.cpp/patches/0019-Add-stage-sampling-config-ABI.patch (1)
176-179: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftAvoid replaying the full token history for every sampled token.
Each sampled step rebuilds the sampler and re-accepts all prior tokens, making generation cost grow with accumulated context. Keep sampler state on the session or replay only the configured penalty window.
🤖 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/0019-Add-stage-sampling-config-ABI.patch` around lines 176 - 179, The sampling path currently replays the entire session->token_history every time a token is generated, which makes llama_sampler_sample in this patch scale poorly as context grows. Update the code around session/token_history and llama_sampler_accept so sampler state is preserved across steps on the session, or limit replay to the configured penalty window instead of re-accepting all prior tokens on every call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@third_party/llama.cpp/patches/0094-Support-GLM-DSA-IndexShare-layers.patch`:
- Around line 58-62: The IndexShare handling in the graph build path assumes
`shared_top_k` always exists, but staged slices can start at an `IndexShare`
layer and hit the assert. Update the `top_k`/`shared_top_k` logic in the
affected DSA `IndexShare` branch so stage boundaries are handled explicitly:
either validate and reject unsupported `layer_start` values before execution, or
preserve and pass the predecessor `top_k` into the slice so `shared_top_k` is
initialized before it is reused.
---
Outside diff comments:
In `@third_party/llama.cpp/patches/0002-Implement-skippy-model-info-ABI.patch`:
- Around line 248-257: Populate the tensor metadata in
skippy_model_info_tensor_at() instead of leaving element_count as zero; this ABI
already reads the tensor name, type, and byte size from GGUF, so compute
element_count from the same GGUF tensor info (using the tensor’s
shape/dimensions and type) and assign it to out_tensor->element_count alongside
the other fields.
In
`@third_party/llama.cpp/patches/0003-Implement-single-stage-runtime-ABI-baseline.patch`:
- Around line 108-112: The feature mask returned by skippy_abi_features() is
missing the runtime capability introduced by this patch, so callers probing
features will not discover the new runtime path. Update skippy_abi_features() to
advertise SKIPPY_FEATURE_RUNTIME_SLICE alongside the existing flags, keeping the
returned bitmask in sync with the new skippy_model_open() and staged session
entry points.
In `@third_party/llama.cpp/patches/0007-Implement-GGUF-slice-writer-ABI.patch`:
- Around line 156-164: The GGUF header reparse in skippy_model_info_open is too
strict because it hard-rejects any header whose version is not exactly
GGUF_VERSION. Update the version check so it accepts older GGUF revisions that
llama.cpp still supports, matching the loader behavior, and ensure the manual
parsing path used by slice writing continues only to validate truly incompatible
headers rather than all non-current versions.
In `@third_party/llama.cpp/patches/0008-Add-GGUF-package-part-composer-ABI.patch`:
- Around line 375-389: The full-package recomposition path in
skippy_write_slice_gguf is copying slice metadata wholesale via
gguf_set_kv(out_ctx, sources.front()->ctx), which leaves per-slice keys like
skippy.slice.stage_index, skippy.slice.layer_start/layer_end, and
skippy.slice.include_* in the final GGUF. Update the materialization logic to
strip or overwrite these slice-specific KV entries after copying the first
source context, while preserving the package-level keys already being set on
out_ctx.
In `@third_party/llama.cpp/patches/0011-Add-native-stage-KV-page-ABI.patch`:
- Around line 396-400: Move the KV removal in the import flow so that `seq_rm()`
in the import path is only called after `find_slot(ubatch, false)` succeeds;
right now the code mutates the live cache before allocation can fail. Update the
import logic around the `desc.token_start`/`desc.token_count` handling to first
reserve slots with `find_slot`, return the existing error if it fails, and only
then delete the old token range and proceed with the import.
- Around line 601-611: The import path in stage_import_kv_page handling
currently mutates backend KV tensors before waiting for pending context work,
which should be aligned with the export path. Update the KV import flow around
stage_import_kv_page() and the surrounding session->ctx usage so the context is
synchronized before any imported KV tensors are written, then keep the existing
post-write synchronize if still needed. Locate the fix in the import code that
updates session->n_past and calls session->ctx->synchronize().
- Around line 304-307: The KV page descriptor validation currently only checks
the SKIPPY_KV_PAGE_FLAG_V_TRANSPOSED bit and can let unknown future/garbage
flags pass. Update the import-time validation around desc.flags so it first
rejects any bits outside the supported flag set, then keep the existing
V-transposed compatibility check. Use the same validation path that sets the
error message in the native KV page import logic.
In `@third_party/llama.cpp/patches/0013-Expose-stage-token-EOG-ABI.patch`:
- Around line 19-23: The public C ABI now exports skippy_token_is_eog, so the
staged-runtime ABI version must be updated in the same patch. Locate the ABI
version definition used by the skippy/llama.cpp patch queue and bump
SKIPPY_ABI_VERSION_PATCH (or MINOR/MAJOR if appropriate) so downstream callers
can detect the new contract alongside the new symbol.
In `@third_party/llama.cpp/patches/0017-Expose-stage-chat-template-ABI.patch`:
- Around line 18-22: The Skippy ABI target is missing the implementation of
skippy_apply_chat_template even though it is declared in the public API, so
consumers can’t resolve the symbol unless common is linked separately. Update
the build setup in the target definition to include the stage-chat.cpp
implementation in the exported Skippy/llama ABI library, and verify
skippy_apply_chat_template is available from that library without requiring
common.
- Around line 158-162: The new SKIPPY_FEATURE_CHAT_TEMPLATE bit is defined in
the public skippy_feature enum but is not being reported by
skippy_abi_features(), so clients cannot detect the ABI support. Update the
feature mask returned by skippy_abi_features() to include
SKIPPY_FEATURE_CHAT_TEMPLATE alongside the other advertised bits, and verify any
related feature aggregation logic in the same ABI exposure path stays
consistent.
In
`@third_party/llama.cpp/patches/0018-Support-more-runtime-slice-model-families.patch`:
- Around line 497-500: The Gemma4 staged input path in build_inp_embd is still
being rescaled unconditionally, which corrupts non-first-stage activation frames
when il_start > 0. Update the inpL setup so the ggml_scale call in this block is
skipped for staged hidden-state inputs (the same way the other Gemma staged
paths guard it), and only apply the sqrtf(n_embd) scaling when the source is
actual token embeddings rather than build_inp_embd(nullptr).
In `@third_party/llama.cpp/patches/0019-Add-stage-sampling-config-ABI.patch`:
- Around line 24-28: The new SKIPPY_FEATURE_SAMPLING_CONFIG bit is declared but
not exposed through feature probing, so clients cannot detect the sampled decode
APIs. Update skippy_abi_features() to include SKIPPY_FEATURE_SAMPLING_CONFIG in
the returned ABI feature mask, alongside the other SKIPPY_FEATURE_* flags, so
capability checks report sampling support correctly.
- Around line 133-138: The gating in skippy_sampling_enabled() is too strict
because it requires sampling->flags to be nonzero even though no sampling flag
constants are defined in the public config ABI. Update the sampling enable check
to rely on the versioned fields that are actually exposed (for example, version
and temperature) or add a real public flag definition and use it consistently in
skippy_sampling_enabled() and the related skippy_sampling_config header so
callers with valid populated configs are not forced into greedy decoding.
- Around line 107-125: The sampling history is only updated in
skippy_decode_tokens, so activation-frame decoding can apply penalties against
stale or empty token_history. Update skippy_decode_activation_frame to record
tokens on successful decode as well, ideally by reusing skippy_record_tokens
after the batch succeeds, so both decode paths keep the same sampling history
state.
In `@third_party/llama.cpp/patches/0022-Add-stage-logit-bias-sampling-ABI.patch`:
- Around line 75-84: The sampling chain setup in the stage-logit-bias path is
still adding llama_sampler_init_temp() even when the greedy branch is selected,
which breaks deterministic behavior. Update the logic around the sampler chain
construction so the temperature sampler is only added for positive temperatures,
and is completely skipped when temperature <= 0.0f before calling
llama_sampler_init_greedy(); use the existing sampling, llama_sampler_chain_add,
and llama_sampler_init_greedy symbols to locate the fix.
In `@third_party/llama.cpp/patches/0023-Add-stage-session-trim-ABI.patch`:
- Around line 87-116: The trim path only updates runtime memory and
session->n_past, but leaves session->token_history unchanged, so later sampling
in skippy_sample_token can still apply penalties to removed tokens. Update the
trim implementation to also shrink session->token_history to token_count
whenever the trim succeeds, alongside the existing memory seq_rm logic, and keep
the unsupported-memory handling in the same stage-session trim flow.
In
`@third_party/llama.cpp/patches/0024-Add-native-stage-session-checkpoint-ABI.patch`:
- Around line 144-147: The checkpoint handling in skippy_checkpoint_session and
skippy_restore_session_checkpoint only records checkpoint_n_past and later trims
state, so it can silently restore a mutated prefix instead of the original
checkpointed one. Update the native/hybrid KV session flow to either snapshot
the KV/cached prefix when creating the checkpoint or invalidate checkpoint_valid
whenever any operation can rewrite tokens before checkpoint_n_past, and make the
restore path fail if the checkpoint is no longer guaranteed to match the saved
state.
In
`@third_party/llama.cpp/patches/0028-Load-stage-models-from-ordered-GGUF-parts.patch`:
- Around line 217-234: `llama_model_load_from_parts` currently assumes the
`paths` array is non-null and dereferences `paths[i]`, which can crash for
direct API callers. Add an early null check at the start of
`llama_model_load_from_parts` alongside the existing `n_paths == 0` validation,
log an error with `__func__`, and return `nullptr` before any indexing; keep the
rest of the validation in place for `paths[i]` entries and the call to
`llama_model_load_from_file_impl`.
In `@third_party/llama.cpp/patches/0030-Expose-stage-KV-cache-type-config.patch`:
- Around line 37-42: The cache type initialization in skippy_session_create is
treating 0 as “unset,” which blocks callers from selecting GGML_TYPE_F32. Update
the sentinel handling for params.type_k and params.type_v so the enum value 0 is
accepted as a valid setting, and reserve a different invalid value such as -1
for the unset/default path; keep the fallback to GGML_TYPE_F16 only when the
config field is truly unset.
In `@third_party/llama.cpp/patches/0031-Expose-stage-generation-signal-ABI.patch`:
- Around line 338-344: The checkpoint restore logic in the session rollback path
currently truncates `token_history` and `signal_history` if they are longer than
the saved checkpoint sizes, but it still restores `n_past`, which can leave
history shorter than the checkpoint expects. In the restore code that uses
`checkpoint_n_past`, `checkpoint_token_history_size`, and
`checkpoint_signal_history_size`, detect when the current histories are shorter
than the recorded checkpoint sizes and fail the restore instead of proceeding.
Either invalidate the checkpoint during
`skippy_trim_session`/`skippy_session_set_position` when truncation would drop
below the saved sizes, or return an error in the restore branch before resetting
`n_past`.
In `@third_party/llama.cpp/patches/0032-Expose-selected-backend-device-ABI.patch`:
- Around line 19-23: The addition of selected_backend_device changes the
skippy_runtime_config ABI, so update the ABI version constant as part of this
change. Bump SKIPPY_ABI_VERSION_PATCH (or the appropriate version component) in
the runtime config/ABI definitions, and keep the Rust mirror constants in
skippy-ffi/src/lib.rs in sync in the same patch. If this field is meant to land
with a later ABI bump, move it there instead so the staged layout and version
stay aligned.
In
`@third_party/llama.cpp/patches/0034-Add-shared-execution-lanes-to-skippy-ABI.patch`:
- Around line 637-645: Validate config->lane_count in the stage model setup
before assigning stage_model->lane_count or using it to size params.n_seq_max
and lane-related state. Add an upper-bound check against the project’s supported
lane/sequence limit in the code that initializes llama_context_params and the
stage_model fields, and reject invalid values early rather than letting a large
caller-supplied lane_count drive oversized allocations or context settings.
In
`@third_party/llama.cpp/patches/0035-Expose-stage-batch-and-flash-attention-config.patch`:
- Line 52: The flash attention assignment in the llama.cpp parameter setup is
treating zero-initialized config values as an explicit Disabled setting instead
of preserving the intended Auto default. Update the logic around the
params.flash_attn_type assignment so that the skippy_runtime_config
flash_attn_type field is only mapped directly when it contains a real
user-selected value, and treat the zero/default case as
LLAMA_FLASH_ATTN_TYPE_AUTO; use the existing symbols skippy_runtime_config,
flash_attn_type, and params.flash_attn_type to locate the fix.
In
`@third_party/llama.cpp/patches/0036-Expose-tool-aware-chat-template-ABI.patch`:
- Around line 281-285: Update the feature advertisement path so the new ABI is
actually reported: the public enum addition in skippy_feature is not enough
unless skippy_abi_features() also includes SKIPPY_FEATURE_CHAT_TEMPLATE_TOOLS in
its returned mask. Locate the feature-bit assembly in skippy_abi_features() and
add the new flag alongside the existing feature constants so consumers gating on
features can detect the chat-template tools ABI.
In
`@third_party/llama.cpp/patches/0037-Apply-chat-grammar-during-stage-sampling.patch`:
- Around line 24-28: The new SKIPPY_FEATURE_CHAT_SAMPLING_GRAMMAR flag is
declared in the skippy feature enum, but the ABI feature advertisement path is
still missing it. Update skippy_abi_features() to include this flag alongside
the existing feature bits so callers can detect grammar sampling support through
the ABI. Use the existing skippy_feature enum and skippy_abi_features() as the
symbols to locate and wire in the new feature.
- Around line 358-364: The catch blocks in the chat sampling setup are
swallowing invalid metadata errors by clearing sampling and returning success,
so update the error handling in the affected chat-grammar initialization path to
propagate a failure instead. In the exception handlers around the metadata_json
parsing/configuration logic, keep the cleanup but return an error status such as
SKIPPY_STATUS_INVALID_ARGUMENT or SKIPPY_STATUS_UNSUPPORTED via out_error
instead of skippy_success, so callers can detect malformed chat sampling
metadata.
- Around line 96-99: The new skippy_regex_escape helper uses std::regex and
std::regex_replace, so add the missing <regex> include in the patch’s include
section to remove the dependency on transitive headers. Update the same
translation unit where skippy_regex_escape is introduced so the compiler can
resolve std::regex symbols portably.
In `@third_party/llama.cpp/patches/0038-Add-resident-prefix-cache-ABI.patch`:
- Around line 174-185: The prefix restore logic in skippy_session_restore_prefix
updates n_past and token_history but leaves the chat sampling state from the
previous conversation intact. After replacing the restored prefix history, make
sure to reset the sampler state by calling skippy_clear_chat_sampling(session)
before synchronizing, so sampling_chain, grammar_sampler, and
sampling_accepted_token_count cannot be reused across different prefixes. Keep
the fix localized to the skippy_session_restore_prefix flow and preserve the
existing checkpoint and sync behavior.
In
`@third_party/llama.cpp/patches/0039-Restore-exact-prefix-state-cache-ABI.patch`:
- Around line 428-438: Add checked arithmetic before comparing against
desc.payload_bytes in the native KV page descriptor validation logic. In the
loop over selected layers, compute expected_bytes with overflow-safe
multiplication/addition using desc.token_count, desc.k_row_bytes,
desc.v_row_bytes, desc.v_element_bytes, and hparams.n_embd_v_gqa(layer->il), and
fail early with an error if any intermediate overflows instead of letting
wraparound occur. Keep the validation in the same descriptor-checking path so
the later copy loops only run after a trustworthy size check.
In `@third_party/llama.cpp/patches/0045-Preserve-resident-prefix-lanes.patch`:
- Around line 65-75: The preserve-on-free path in the session cleanup logic is
ignoring failures from llama_memory_seq_rm, which can leave stale KV state while
still treating the lane as a resident-prefix hit. Update the free-path in the
preserve_prefix_on_free block to check the result of both llama_memory_seq_rm
calls and either fail/abort the cleanup or clear the resident-prefix bookkeeping
when trimming does not succeed. Use the session fields preserve_prefix_on_free,
borrowed_sequence, stage_model, seq_id, checkpoint_seq_id, and
preserve_prefix_tokens to locate the logic.
In
`@third_party/llama.cpp/patches/0046-Remap-recurrent-state-imports-to-active-session.patch`:
- Around line 18-26: The recurrent state import in the patch is using a
hard-coded payload offset for seq_id, which makes the remapping fragile if the
serialized header layout changes. Update the state import logic to derive the
seq_id position from an explicit local header structure or an upstream helper
instead of using the sizeof(uint32_t) magic offset, and keep the
validation/memcpy in the same remapping block so the sequence ID is written to
the correct field.
In
`@third_party/llama.cpp/patches/0050-Support-more-decoder-runtime-slice-families.patch`:
- Around line 340-344: The deepstack input path in the graph builder is being
skipped for filtered stages that start after layer 0, which breaks staged
execution for Qwen3VL. Update the logic around the deepstack addition in the
relevant graph-building code so that `stage_filtered` does not suppress
`ggml_add` when `il` is within `n_deepstack_layers`; either propagate the
deepstack sideband into mid-stage inputs and keep the per-layer `ds` addition,
or explicitly prevent split points before the deepstack range in the same
stage-selection path.
In
`@third_party/llama.cpp/patches/0055-Support-Qwen2MoE-runtime-slice-execution.patch`:
- Around line 19-23: The tied-output fallback in the tensor loading path is
still being treated as an embedding and can be filtered out when only output is
requested. Update the logic around the `output` tensor creation in the llama.cpp
patch so that the `LLM_TENSOR_TOKEN_EMBD` fallback is loaded through an
output-preserving path, or otherwise bypass embedding filtering whenever
`include_output` is true. Make sure the `create_tensor` call for
`LLM_TENSOR_OUTPUT` and the fallback lookup for `LLM_TENSOR_TOKEN_EMBD` both
keep `model.output` populated on slices that need logits.
In `@third_party/llama.cpp/patches/0057-Support-RWKV7-activation-sideband.patch`:
- Around line 617-627: The session-level copy path in
skippy_session_copy_output_activation_frame is dropping the RWKV7 v_first
sideband because it passes null input descriptors/payloads into
skippy_copy_output_activation_frame, causing skippy_output_activation_flags to
lose the flag on later slices. Update this flow so the active sideband state is
preserved and forwarded across session copies, using the existing
skippy_session_copy_output_activation_frame and
skippy_copy_output_activation_frame entry points to thread the RWKV7 V_FIRST
flag and payload through instead of hard-coding null.
In
`@third_party/llama.cpp/patches/0063-Allow-tied-output-embeddings-in-final-runtime-slices.patch`:
- Around line 18-21: The validation in the runtime slice setup now permits
non-first slices with embeddings when include_output is enabled, but the error
message in the same check still says only the first slice may include token
embeddings. Update the skippy_set_error message in the validation block that
checks config->include_embeddings, config->layer_start, and
!config->include_output so it clearly states the new rule and matches the actual
guard.
In
`@third_party/llama.cpp/patches/0065-Support-broad-llama-family-runtime-slices.patch`:
- Around line 1179-1188: The staged-activation path in the input embedding setup
applies ggml_scale unconditionally, which incorrectly rescales imported hidden
states on non-first slices. Update the logic around skippy_graph_get_filter(),
build_inp_embd(), and the subsequent ggml_scale call so scale_embd is only
applied when processing the first slice (or when stage filtering is disabled),
and bypass scaling when il_start > 0 with a nullptr embedding input.
In
`@third_party/llama.cpp/patches/0068-Support-DeepSeek-OCR-and-Qwen3-VL-MoE-runtime-slices.patch`:
- Around line 54-58: The DeepStack residual path in the sliced runtime can
become semantically incomplete when a stage starts after layer 0, so add an
upfront validation in skippy_finish_model_open to reject nonzero
config.layer_start for architectures that use n_deepstack_layers. Locate the
model setup/validation logic alongside skippy_finish_model_open and enforce that
Qwen3VL-MoE and DeepSeek-OCR require layer_start=0, returning an
invalid-argument error with a clear message before any stage slicing proceeds.
In
`@third_party/llama.cpp/patches/0071-Expose-external-decode-stage-filter-ABI.patch`:
- Around line 46-55: The execution-time stage filter in
skippy_session_begin_external_decode is incorrectly disabled by checking
config.filter_tensors_on_load, which makes external decode skip the configured
layer slice when full tensors are kept loaded. Update
skippy_session_begin_external_decode to always build and apply the
skippy_graph_filter from session->stage_model->config, using the existing fields
like layer_start, layer_end, include_embeddings, and include_output, and remove
the conditional gate on filter_tensors_on_load.
In
`@third_party/llama.cpp/patches/0072-Support-external-media-prefill-in-staged-runtime-ABI.patch`:
- Around line 19-31: This patch adds the new public ABI symbol
skippy_prefill_chunk_frame_sampled but does not bump the staged-runtime ABI
version. Update the ABI version patch in skippy/common.h as part of this same
patch so the new export is reflected in the public versioning, and keep the
change aligned with the existing SKIPPY_ABI_VERSION_PATCH/SKIPPY_API symbols
used by the ABI headers.
In
`@third_party/llama.cpp/patches/0075-Carry-activation-frame-position-sideband.patch`:
- Around line 149-154: The token-decode branch in the activation frame path is
ignoring supplied positions, so update the `skippy_decode_tokens` flow to either
accept and forward `positions`/`position_count` like
`skippy_decode_activation_frame`, or explicitly reject non-empty positions
before taking that branch. Use the `skippy_is_filtered(...)` conditional and the
`skippy_decode_tokens` call site to ensure the position-aware ABI is never
silently dropped.
- Around line 104-117: The position-sideband handling in the decode path
currently falls back to synthesized positions whenever positions is non-null but
position_count is wrong, which masks malformed caller input. Update the logic
around positions, position_count, and expected_position_count so that a non-null
positions buffer with a mismatched count is treated as an error and stops
processing instead of entering the n_pos_per_embd fallback branches. Keep the
existing memcpy and synthesized-position paths only for valid inputs or when
positions is null, and ensure the code returns a failure for malformed
sidebands.
In `@third_party/llama.cpp/patches/0076-Support-Gemma3n-runtime-slices.patch`:
- Around line 257-263: The AltUp decoder copy path is using the start of
embd_storage as if the active slice were always zero, but the graph in
ggml_view_2d_slice and the stage_boundary logic clearly use i_altup_act to
select the runtime slice. Update the decoder loop that fills embd_storage to
apply an offset based on i_altup_act * n_tokens before copying into the active
slice, and keep the slice selection consistent with the cur/t_embd setup used in
the AltUp graph path.
- Around line 23-25: Add the ABI version bump for the new staged-runtime flag:
introducing SKIPPY_ACTIVATION_FLAG_GEMMA3N_ALTUP changes the skippy runtime ABI,
so update SKIPPY_ABI_VERSION_PATCH in skippy/common.h in the same change. Also
mirror the new flag in crates/skippy-ffi/src/lib.rs so the Rust FFI constants
stay in sync with the C header and the patch remains internally consistent.
In
`@third_party/llama.cpp/patches/0078-Contain-chat-grammar-sampler-exceptions.patch`:
- Around line 41-50: The prefill failure path in the chat grammar sampler
initialization leaves session->grammar_sampler attached, so extend the existing
try/catch around the token prefill in the chat session setup to clean up the
sampler before returning false. Update the failure handling in the prefill block
that calls skippy_tokenize_text and llama_sampler_accept so it releases or nulls
grammar_sampler the same way the post-init accept/apply failure path does, and
keep the existing skippy_set_error reporting intact.
In
`@third_party/llama.cpp/patches/0082-Compact-KV-cache-cells-during-optimized-memory-updates.patch`:
- Around line 139-147: The transposed V compaction in
llama_kv_cache_copy_tensor_bytes currently computes byte offsets from
ggml_type_size(v->type) and cells_per_stream, which is unsafe for
block-quantized layouts. Update the transposed-copy logic in the V tensor move
loop to use the tensor’s actual stride information from v->nb (matching the
non-transposed path’s use of ggml_row_size()), and verify the correct nb index
based on the transposed shape before copying each embedding slice.
In
`@third_party/llama.cpp/patches/0085-Add-Skippy-batched-stage-execution-ABI.patch`:
- Around line 352-356: The padding logic in the activation-input copy path can
overflow when n_embd_inp is smaller than n_embd. Update the branch around the
memcpy/memset in the Skippy batched stage execution patch so the code explicitly
handles n_embd_inp < n_embd before writing into embd_storage, and only
copies/pads after validating the destination size using the n_embd_inp and
n_embd values. Use the surrounding input_payload/dst handling in this block to
add a safe fallback or assertion that prevents copying more floats than the
allocated storage can hold.
- Around line 214-230: The batched decode path in
skippy_decode_step_frame_batch_sampled currently allows the same skippy_session
pointer to appear more than once, which reuses the same seq_id and n_past and
then advances that session twice. Add a duplicate-session guard before
populating the batch (alongside the existing ctx/stage_model checks), or
otherwise assign unique per-session positions, so each session appears only once
in this one-token-per-session ABI. Apply the same validation in the pos_storage
filling logic for the other affected block as well.
In `@third_party/llama.cpp/patches/0086-Add-Skippy-native-MTP-n1-sidecar.patch`:
- Around line 342-350: The MTP sidecar state is being advanced inside
skippy_mtp_propose_next even when no draft output is requested, which can leave
mtp_ctx and the session bookkeeping out of sync. Update the logic around the
out_mtp_draft handling so that any llama_decode/mtp_ctx advancement is either
skipped when the caller passes null or is immediately rolled back/kept
consistent with mtp_has_pending_draft, mtp_pending_draft_pos, and
mtp_pending_draft_token. Make sure the behavior in skippy_mtp_propose_next is
consistent across all related proposal paths referenced by the patch so a null
draft request never leaves stale pending-draft state.
In
`@third_party/llama.cpp/patches/0088-Align-Skippy-sampling-and-detokenization-parity.patch`:
- Around line 197-235: The manual detokenization in the token-to-text loop is
currently rendering special tokens because `llama_token_to_piece` is called with
the special-token flag enabled. Update the detokenization path inside the
`llama_detokenize` replacement logic to skip special tokens like the previous
behavior, using the same token iteration and error handling in this block while
changing the `llama_token_to_piece` call to preserve parity.
In `@third_party/llama.cpp/patches/0089-Remove-unused-Skippy-ABI-exports.patch`:
- Around line 20-93: This patch removes public Skippy C ABI exports, so the ABI
version must be updated to reflect the breaking change. Bump
SKIPPY_ABI_VERSION_PATCH (or MINOR/MAJOR if appropriate) in skippy/common.h, and
make the matching SKIPPY_ABI_VERSION_* constants in skippy-ffi/src/lib.rs
consistent in the same change. Use the existing ABI version definitions around
skippy_abi_version and SKIPPY_ABI_VERSION_* to locate the updates.
In
`@third_party/llama.cpp/patches/0092-Add-Skippy-greedy-sampling-fast-path.patch`:
- Around line 59-66: The fast-path in skippy greedy sampling is bypassing lazy
chat-sampling state by checking only session->grammar_sampler, which can wrongly
skip grammar setup and clear active metadata. Update the early-return guard in
the greedy sampling branch to also respect session->chat_sampling_metadata (and
any equivalent lazy sampler state used by the normal path) before calling
skippy_clear_chat_sampling and skippy_greedy_sample_ith, so the fast path only
runs when no chat-sampling state is still pending.
---
Nitpick comments:
In `@third_party/llama.cpp/patches/0019-Add-stage-sampling-config-ABI.patch`:
- Around line 176-179: The sampling path currently replays the entire
session->token_history every time a token is generated, which makes
llama_sampler_sample in this patch scale poorly as context grows. Update the
code around session/token_history and llama_sampler_accept so sampler state is
preserved across steps on the session, or limit replay to the configured penalty
window instead of re-accepting all prior tokens on every call.
In
`@third_party/llama.cpp/patches/0083-skippy-add-model-open-runtime-events-ABI.patch`:
- Around line 118-148: The public ABI in skippy_runtime_event_v1 and
skippy_runtime_event_callback needs a short contract note for detail_ptr and
delivery context. Add comments near skippy_runtime_event_v1 and
skippy_runtime_event_reporter_v1 stating that detail_ptr is a borrowed pointer
valid only during the callback, and clarify whether the callback may be invoked
from loader/worker threads or only on a single thread. Keep the guidance close
to these symbols so consumers of the ABI know not to retain the pointer or
assume serialized delivery.
In
`@third_party/llama.cpp/patches/0092-Add-Skippy-greedy-sampling-fast-path.patch`:
- Around line 29-30: The fast-path env check in
skippy_mtp_greedy_sampling_fastpath_enabled() is being evaluated from
skippy_sample_token_ith on every token, so move that gate out of the sampling
hot path and cache the result once at initialization or first use. Update the
sampling path to read the cached boolean instead of calling
skippy_env_enabled("SKIPPY_NATIVE_MTP_GREEDY_SAMPLING_FASTPATH") repeatedly,
keeping the logic centralized in skippy_mtp_greedy_sampling_fastpath_enabled or
its caller.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 09436e77-79b7-4825-a5a8-de8ae7f075b7
📒 Files selected for processing (97)
third_party/llama.cpp/patches/0001-Draft-skippy-ABI-header.patchthird_party/llama.cpp/patches/0002-Implement-skippy-model-info-ABI.patchthird_party/llama.cpp/patches/0003-Implement-single-stage-runtime-ABI-baseline.patchthird_party/llama.cpp/patches/0004-Implement-runtime-slice-tensor-filtering.patchthird_party/llama.cpp/patches/0005-Add-activation-frame-runtime-ABI.patchthird_party/llama.cpp/patches/0006-Execute-runtime-slice-activation-frames.patchthird_party/llama.cpp/patches/0007-Implement-GGUF-slice-writer-ABI.patchthird_party/llama.cpp/patches/0008-Add-GGUF-package-part-composer-ABI.patchthird_party/llama.cpp/patches/0009-Support-Qwen35MoE-runtime-slice-packages.patchthird_party/llama.cpp/patches/0010-Handle-Qwen35MoE-recurrent-only-runtime-slices.patchthird_party/llama.cpp/patches/0011-Add-native-stage-KV-page-ABI.patchthird_party/llama.cpp/patches/0012-Add-stage-session-reset-ABI.patchthird_party/llama.cpp/patches/0013-Expose-stage-token-EOG-ABI.patchthird_party/llama.cpp/patches/0014-Position-stage-token-batches-from-session-offset.patchthird_party/llama.cpp/patches/0015-Optimize-native-KV-page-cell-lookup.patchthird_party/llama.cpp/patches/0016-Add-batched-token-verification-ABI.patchthird_party/llama.cpp/patches/0017-Expose-stage-chat-template-ABI.patchthird_party/llama.cpp/patches/0018-Support-more-runtime-slice-model-families.patchthird_party/llama.cpp/patches/0019-Add-stage-sampling-config-ABI.patchthird_party/llama.cpp/patches/0020-Add-staged-batched-verification-frame-ABI.patchthird_party/llama.cpp/patches/0021-Add-recurrent-state-checkpoint-ABI.patchthird_party/llama.cpp/patches/0022-Add-stage-logit-bias-sampling-ABI.patchthird_party/llama.cpp/patches/0023-Add-stage-session-trim-ABI.patchthird_party/llama.cpp/patches/0024-Add-native-stage-session-checkpoint-ABI.patchthird_party/llama.cpp/patches/0025-Fix-staged-verification-activation-handoff.patchthird_party/llama.cpp/patches/0026-Strip-split-metadata-from-stage-GGUF-artifacts.patchthird_party/llama.cpp/patches/0027-Expose-stage-chat-template-thinking-toggle.patchthird_party/llama.cpp/patches/0028-Load-stage-models-from-ordered-GGUF-parts.patchthird_party/llama.cpp/patches/0029-Support-Qwen3MoE-runtime-slice-execution.patchthird_party/llama.cpp/patches/0030-Expose-stage-KV-cache-type-config.patchthird_party/llama.cpp/patches/0031-Expose-stage-generation-signal-ABI.patchthird_party/llama.cpp/patches/0032-Expose-selected-backend-device-ABI.patchthird_party/llama.cpp/patches/0033-Expose-external-media-prefill-ABI.patchthird_party/llama.cpp/patches/0034-Add-shared-execution-lanes-to-skippy-ABI.patchthird_party/llama.cpp/patches/0035-Expose-stage-batch-and-flash-attention-config.patchthird_party/llama.cpp/patches/0036-Expose-tool-aware-chat-template-ABI.patchthird_party/llama.cpp/patches/0037-Apply-chat-grammar-during-stage-sampling.patchthird_party/llama.cpp/patches/0038-Add-resident-prefix-cache-ABI.patchthird_party/llama.cpp/patches/0039-Restore-exact-prefix-state-cache-ABI.patchthird_party/llama.cpp/patches/0040-Add-borrowed-resident-prefix-session-ABI.patchthird_party/llama.cpp/patches/0041-Compute-stage-generation-signals-lazily.patchthird_party/llama.cpp/patches/0042-Avoid-heap-batch-allocation-for-single-token-decode.patchthird_party/llama.cpp/patches/0043-Default-stage-threads-like-llama-server.patchthird_party/llama.cpp/patches/0044-Allow-borrowing-resident-prefix-cache-sequences.patchthird_party/llama.cpp/patches/0045-Preserve-resident-prefix-lanes.patchthird_party/llama.cpp/patches/0046-Remap-recurrent-state-imports-to-active-session.patchthird_party/llama.cpp/patches/0047-Support-initial-dense-runtime-slice-families.patchthird_party/llama.cpp/patches/0048-Support-additional-dense-runtime-slice-families.patchthird_party/llama.cpp/patches/0049-Avoid-rescaling-staged-activation-inputs.patchthird_party/llama.cpp/patches/0050-Support-more-decoder-runtime-slice-families.patchthird_party/llama.cpp/patches/0051-Support-LFM2-runtime-slice-execution.patchthird_party/llama.cpp/patches/0052-Support-Mamba-runtime-slice-execution.patchthird_party/llama.cpp/patches/0053-Support-Jamba-runtime-slice-execution.patchthird_party/llama.cpp/patches/0054-Support-RWKV6-runtime-slice-execution.patchthird_party/llama.cpp/patches/0055-Support-Qwen2MoE-runtime-slice-execution.patchthird_party/llama.cpp/patches/0056-Expose-skippy-session-native-sequence-id.patchthird_party/llama.cpp/patches/0057-Support-RWKV7-activation-sideband.patchthird_party/llama.cpp/patches/0058-Support-Phi2-runtime-slice-execution.patchthird_party/llama.cpp/patches/0059-Support-Granite-runtime-slice-variants.patchthird_party/llama.cpp/patches/0060-Support-Hunyuan-Dense-runtime-slice-execution.patchthird_party/llama.cpp/patches/0061-Support-Hunyuan-MoE-runtime-slice-execution.patchthird_party/llama.cpp/patches/0062-Support-PhiMoE-runtime-slice-execution.patchthird_party/llama.cpp/patches/0063-Allow-tied-output-embeddings-in-final-runtime-slices.patchthird_party/llama.cpp/patches/0064-Support-Qwen35-and-Hunyuan-VL-runtime-slices.patchthird_party/llama.cpp/patches/0065-Support-broad-llama-family-runtime-slices.patchthird_party/llama.cpp/patches/0066-Fix-BitNet-tied-output-runtime-slices.patchthird_party/llama.cpp/patches/0067-Fix-StarCoder-positional-embeddings-in-runtime-slices.patchthird_party/llama.cpp/patches/0068-Support-DeepSeek-OCR-and-Qwen3-VL-MoE-runtime-slices.patchthird_party/llama.cpp/patches/0069-Avoid-double-counting-optional-filtered-tensors.patchthird_party/llama.cpp/patches/0070-Skip-recurrent-layers-in-native-KV-page-export.patchthird_party/llama.cpp/patches/0071-Expose-external-decode-stage-filter-ABI.patchthird_party/llama.cpp/patches/0072-Support-external-media-prefill-in-staged-runtime-ABI.patchthird_party/llama.cpp/patches/0073-Expose-model-info-tensor-element-counts.patchthird_party/llama.cpp/patches/0074-Pad-activation-frame-decode-batches-to-native-input-width.patchthird_party/llama.cpp/patches/0075-Carry-activation-frame-position-sideband.patchthird_party/llama.cpp/patches/0076-Support-Gemma3n-runtime-slices.patchthird_party/llama.cpp/patches/0077-Support-Llama4-and-Mistral4-runtime-slices.patchthird_party/llama.cpp/patches/0078-Contain-chat-grammar-sampler-exceptions.patchthird_party/llama.cpp/patches/0079-Trigger-lazy-chat-grammar-before-trailing-whitespace.patchthird_party/llama.cpp/patches/0080-Expose-skippy-backend-device-enumeration-ABI.patchthird_party/llama.cpp/patches/0081-Add-min-p-sampler-to-skippy-sampling-chain.patchthird_party/llama.cpp/patches/0082-Compact-KV-cache-cells-during-optimized-memory-updates.patchthird_party/llama.cpp/patches/0083-skippy-add-model-open-runtime-events-ABI.patchthird_party/llama.cpp/patches/0084-Add-GLM-native-MTP-graph-support.patchthird_party/llama.cpp/patches/0085-Add-Skippy-batched-stage-execution-ABI.patchthird_party/llama.cpp/patches/0086-Add-Skippy-native-MTP-n1-sidecar.patchthird_party/llama.cpp/patches/0087-Add-Skippy-batched-MTP-verification-drafts.patchthird_party/llama.cpp/patches/0088-Align-Skippy-sampling-and-detokenization-parity.patchthird_party/llama.cpp/patches/0089-Remove-unused-Skippy-ABI-exports.patchthird_party/llama.cpp/patches/0090-Fix-Skippy-MTP-stage-layer-count.patchthird_party/llama.cpp/patches/0091-Avoid-copying-VerifySpan-activation-inputs.patchthird_party/llama.cpp/patches/0092-Add-Skippy-greedy-sampling-fast-path.patchthird_party/llama.cpp/patches/0093-Add-GLM-chat-template-fallback.patchthird_party/llama.cpp/patches/0094-Support-GLM-DSA-IndexShare-layers.patchthird_party/llama.cpp/patches/0095-Skip-native-MTP-sidecar-when-split-stage-lacks-tensors.patchthird_party/llama.cpp/patches/0110-Support-GLM-DSA-IndexShare-layers.patchthird_party/llama.cpp/upstream.txt
💤 Files with no reviewable changes (1)
- third_party/llama.cpp/patches/0110-Support-GLM-DSA-IndexShare-layers.patch
Comment on lines
+58
to
+62
| + shared_top_k = top_k; | ||
| + } else { | ||
| + GGML_ASSERT(shared_top_k != nullptr && "DSA IndexShare layer requires a previous top-k"); | ||
| + top_k = shared_top_k; | ||
| + cb(top_k, "top_k_shared", il); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
IndexShare slices can crash at stage boundaries.
shared_top_k is only populated when an earlier layer in the same graph has a real indexer. In the staged runtime, a slice can legitimately start on an IndexShare layer, so this assert turns a valid model into a hard failure. This needs an explicit boundary contract: either reject those layer_start values during slice validation or carry the predecessor top_k across the stage boundary.
🤖 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/0094-Support-GLM-DSA-IndexShare-layers.patch`
around lines 58 - 62, The IndexShare handling in the graph build path assumes
`shared_top_k` always exists, but staged slices can start at an `IndexShare`
layer and hit the assert. Update the `top_k`/`shared_top_k` logic in the
affected DSA `IndexShare` branch so stage boundaries are handled explicitly:
either validate and reject unsupported `layer_start` values before execution, or
preserve and pass the predecessor `top_k` into the slice so `shared_top_k` is
initialized before it is reused.
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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
e12a0128ab0be91d500d25d18db05067d3f7b4e8Validation
scripts/prepare-llama.sh pinnedin a fresh temporary checkoutjust buildcargo test -p mesh-llm --libSummary by CodeRabbit