Add native MTP generation metadata to layer packages - #888
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an optional ChangesNative MTP Speculative Decoding Generation Configuration and Execution
Model Package Prepare Model ID Resolution
Sequence Diagram(s)sequenceDiagram
participant ConfigResolver
participant SpeculativeResolver
participant StageLoader
participant BinaryTransport
participant StageRuntime
ConfigResolver->>SpeculativeResolver: strategy from config/defaults
SpeculativeResolver->>SpeculativeResolver: validate(strategy)
SpeculativeResolver->>SpeculativeResolver: compute native_mtp_enabled from package generation
SpeculativeResolver->>StageLoader: ResolvedSpeculativeConfig{strategy, native_mtp_enabled}
StageLoader->>StageLoader: populate StageLoadRequest/StageConfig
StageLoader->>BinaryTransport: StageLoadRequest{native_mtp_enabled}
BinaryTransport->>BinaryTransport: deserialize proto LoadStage
BinaryTransport->>BinaryTransport: compute effective native_mtp_enabled
BinaryTransport->>BinaryTransport: create BinaryStageExecutionOptions
alt native_mtp_enabled == true
BinaryTransport->>StageRuntime: decode_frame_sampled_mtp_n1(capacity)
else
BinaryTransport->>StageRuntime: decode_frame_sampled(capacity)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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
🤖 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 `@crates/skippy-model-package/src/preflight.rs`:
- Around line 91-100: The PreflightSpeculativeStrategy struct is missing the
window_policy field that is validated by validate_window_policy, preventing the
validated policy details from being included in the serialized output. Add a
window_policy field to the PreflightSpeculativeStrategy struct with appropriate
serde attributes to ensure it is properly serialized when present, similar to
how other optional fields like prediction_depth are handled. Also apply the same
change to any other related struct mentioned in the comment that handles
preflight strategy output.
🪄 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: fc489ff7-20df-4267-bff2-435e40041caf
📒 Files selected for processing (4)
crates/skippy-model-package/src/main.rscrates/skippy-model-package/src/preflight.rscrates/skippy-runtime/src/package.rsdocs/specs/layer-package-repos.md
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
crates/mesh-llm-host-runtime/src/inference/skippy/resolver/tests.rs (1)
778-836: ⚡ Quick winAdd one assertion path for
to_model_load_options()propagation.Current tests validate
to_stage_config(...)andto_embedded_openai_args(...), but not the direct model-load options path. A small regression test there would close the remaining propagation gap.🤖 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 `@crates/mesh-llm-host-runtime/src/inference/skippy/resolver/tests.rs` around lines 778 - 836, The test functions speculative_strategy_defaults_to_native_mtp_enabled and speculative_strategy_disabled_reaches_stage_and_openai_args validate that native_mtp_enabled propagates through to_stage_config and to_embedded_openai_args, but they do not test the to_model_load_options method. Add assertions in both test functions to call to_model_load_options on the resolved object and verify that the native_mtp_enabled property is correctly propagated (should be true in the first test and false in the second test), matching the existing assertion patterns.
🤖 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 `@crates/mesh-llm-host-runtime/src/inference/skippy/deployment.rs`:
- Line 60: The hard-coded `native_mtp_enabled: true` at line 60 and line 109 in
the stage deployment builders overrides the resolved deployment strategy.
Instead of hard-coding this value, retrieve the `native_mtp_enabled` setting
from the deployment input state and propagate it through to the deployment
builder configuration. This ensures the actual strategy semantics are preserved
rather than being overwritten by the hard-coded value.
In `@crates/skippy-server/src/binary_transport.rs`:
- Around line 2442-2450: The BinaryStageExecutionOptions::new call in the
restore-prefill-decode path at line 2449 uses config.native_mtp_enabled
directly, but it should use the same effective native MTP gate as the main path
at line 204, which combines both options.native_mtp_enabled (runtime override)
and config.native_mtp_enabled (configuration). Replace the direct
config.native_mtp_enabled argument with the combined condition
options.native_mtp_enabled && config.native_mtp_enabled to ensure the
restore-prefill-decode path respects the same runtime/environment overrides as
the normal decode path.
---
Nitpick comments:
In `@crates/mesh-llm-host-runtime/src/inference/skippy/resolver/tests.rs`:
- Around line 778-836: The test functions
speculative_strategy_defaults_to_native_mtp_enabled and
speculative_strategy_disabled_reaches_stage_and_openai_args validate that
native_mtp_enabled propagates through to_stage_config and
to_embedded_openai_args, but they do not test the to_model_load_options method.
Add assertions in both test functions to call to_model_load_options on the
resolved object and verify that the native_mtp_enabled property is correctly
propagated (should be true in the first test and false in the second test),
matching the existing assertion patterns.
🪄 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: 70c443ed-8028-42f0-a571-e5833288e617
📒 Files selected for processing (37)
crates/mesh-llm-config/src/model.rscrates/mesh-llm-config/src/model/built_in_schema.rscrates/mesh-llm-config/src/validate.rscrates/mesh-llm-host-runtime/src/inference/skippy/deployment.rscrates/mesh-llm-host-runtime/src/inference/skippy/family_policy.rscrates/mesh-llm-host-runtime/src/inference/skippy/materialization.rscrates/mesh-llm-host-runtime/src/inference/skippy/mod.rscrates/mesh-llm-host-runtime/src/inference/skippy/resolver/speculative.rscrates/mesh-llm-host-runtime/src/inference/skippy/resolver/tests.rscrates/mesh-llm-host-runtime/src/inference/skippy/resolver/translation.rscrates/mesh-llm-host-runtime/src/inference/skippy/resolver/types.rscrates/mesh-llm-host-runtime/src/inference/skippy/stage/mod.rscrates/mesh-llm-host-runtime/src/inference/skippy/stage/tests.rscrates/mesh-llm-host-runtime/src/inference/skippy/stage/types.rscrates/mesh-llm-host-runtime/src/mesh/mod.rscrates/mesh-llm-host-runtime/src/mesh/tests.rscrates/mesh-llm-host-runtime/src/runtime/local.rscrates/skippy-prompt/src/prompt_cli/stage_config.rscrates/skippy-protocol/proto/stage.protocrates/skippy-protocol/src/lib.rscrates/skippy-server/src/binary_transport.rscrates/skippy-server/src/binary_transport/forwarding.rscrates/skippy-server/src/binary_transport/options.rscrates/skippy-server/src/binary_transport/tests.rscrates/skippy-server/src/frontend.rscrates/skippy-server/src/frontend/embedded_execution.rscrates/skippy-server/src/frontend/embedded_generation.rscrates/skippy-server/src/frontend/generation_flow.rscrates/skippy-server/src/frontend/prefix_cache.rscrates/skippy-server/src/frontend/tests.rscrates/skippy-server/src/kv_integration/activation.rscrates/skippy-server/src/kv_integration/config.rscrates/skippy-server/src/kv_integration/identity.rscrates/skippy-server/src/runtime_state.rsdocs/USAGE.mddocs/skippy/CONFIGURATION.mddocs/specs/layer-package-repos.md
✅ Files skipped from review due to trivial changes (8)
- crates/mesh-llm-host-runtime/src/inference/skippy/stage/tests.rs
- crates/skippy-server/src/kv_integration/config.rs
- crates/skippy-prompt/src/prompt_cli/stage_config.rs
- docs/skippy/CONFIGURATION.md
- crates/mesh-llm-host-runtime/src/inference/skippy/family_policy.rs
- docs/USAGE.md
- docs/specs/layer-package-repos.md
- crates/skippy-server/src/runtime_state.rs
ndizazzo
reviewed
Jun 21, 2026
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/mesh-llm-host-runtime/src/inference/skippy/resolver/tests.rs (1)
824-952: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftSplit the new speculative-strategy test block into a dedicated module.
This file is already over 1k LoC, and this change adds a separable responsibility (native-MTP strategy resolution/propagation tests). Please extract this block (and helper(s) it owns) into a semantically named submodule to keep the oversized file from growing.
As per coding guidelines, "Apply the 1k LoC refactoring rule: when touching a source file that is already over 1,000 lines, ... split that responsibility into a semantically named module as part of the change."
🤖 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 `@crates/mesh-llm-host-runtime/src/inference/skippy/resolver/tests.rs` around lines 824 - 952, The file is already over 1,000 lines of code and the new test block adds multiple related tests for speculative strategy and native-MTP resolution. Extract the four test functions speculative_strategy_auto_without_package_generation_disables_native_mtp, speculative_strategy_auto_uses_package_native_mtp_default, speculative_strategy_native_mtp_rejects_package_without_native_mtp_metadata, and speculative_strategy_disabled_reaches_stage_and_openai_args, along with any helper functions they depend on, into a new semantically named submodule that reflects the responsibility of testing native-MTP strategy resolution and propagation.Source: Coding guidelines
🤖 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 `@crates/mesh-llm-host-runtime/src/inference/skippy/resolver/tests.rs`:
- Around line 824-952: The file is already over 1,000 lines of code and the new
test block adds multiple related tests for speculative strategy and native-MTP
resolution. Extract the four test functions
speculative_strategy_auto_without_package_generation_disables_native_mtp,
speculative_strategy_auto_uses_package_native_mtp_default,
speculative_strategy_native_mtp_rejects_package_without_native_mtp_metadata, and
speculative_strategy_disabled_reaches_stage_and_openai_args, along with any
helper functions they depend on, into a new semantically named submodule that
reflects the responsibility of testing native-MTP strategy resolution and
propagation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8dbcf018-28d6-4d7b-9c9c-34ee41e6acca
📒 Files selected for processing (13)
crates/mesh-llm-host-runtime/src/inference/skippy/certification.rscrates/mesh-llm-host-runtime/src/inference/skippy/deployment.rscrates/mesh-llm-host-runtime/src/inference/skippy/materialization.rscrates/mesh-llm-host-runtime/src/inference/skippy/mod.rscrates/mesh-llm-host-runtime/src/inference/skippy/package.rscrates/mesh-llm-host-runtime/src/inference/skippy/resolver/resolution.rscrates/mesh-llm-host-runtime/src/inference/skippy/resolver/speculative.rscrates/mesh-llm-host-runtime/src/inference/skippy/resolver/tests.rscrates/mesh-llm-host-runtime/src/inference/skippy/resolver/types.rscrates/mesh-llm-host-runtime/src/inference/skippy/topology.rscrates/mesh-llm-host-runtime/src/runtime/local.rscrates/mesh-llm-host-runtime/src/runtime/split_planning.rscrates/skippy-runtime/src/package.rs
✅ Files skipped from review due to trivial changes (1)
- crates/mesh-llm-host-runtime/src/runtime/split_planning.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- crates/mesh-llm-host-runtime/src/inference/skippy/resolver/speculative.rs
- crates/mesh-llm-host-runtime/src/inference/skippy/deployment.rs
- crates/mesh-llm-host-runtime/src/inference/skippy/mod.rs
- crates/skippy-runtime/src/package.rs
ndizazzo
self-requested a review
June 21, 2026 23:06
michaelneale
added a commit
that referenced
this pull request
Jun 23, 2026
* origin/main: update guides for dev loop (#895) Add native MTP generation metadata to layer packages (#888) upgrade iroh to 1.0 (#894) fix(runtime): support relocating shared libs Improve LAN direct-path discovery and connection reliability (#853) Add GLM chat template fallback in llama (#890) # Conflicts: # crates/mesh-llm-config/src/model/built_in_schema.rs # crates/mesh-llm-config/src/validate.rs # crates/mesh-llm-host-runtime/src/inference/skippy/deployment.rs # crates/mesh-llm-host-runtime/src/inference/skippy/materialization.rs # crates/mesh-llm-host-runtime/src/inference/skippy/resolver/speculative.rs # crates/mesh-llm-host-runtime/src/inference/skippy/resolver/translation.rs # crates/mesh-llm-host-runtime/src/inference/skippy/stage/mod.rs # crates/mesh-llm-host-runtime/src/inference/skippy/stage/tests.rs # crates/mesh-llm-host-runtime/src/inference/skippy/stage/types.rs # crates/mesh-llm-host-runtime/src/mesh/mod.rs # crates/mesh-llm-host-runtime/src/mesh/tests.rs # crates/mesh-llm-host-runtime/src/runtime/local.rs # crates/skippy-protocol/proto/stage.proto # crates/skippy-server/src/binary_transport.rs # crates/skippy-server/src/frontend.rs # crates/skippy-server/src/frontend/embedded_execution.rs # crates/skippy-server/src/frontend/embedded_generation.rs # crates/skippy-server/src/frontend/generation_flow.rs # crates/skippy-server/src/frontend/prefix_cache.rs # docs/skippy/CONFIGURATION.md
This was referenced Jun 30, 2026
Merged
This was referenced Jul 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
generation.speculative_decodingmetadata to layer package manifestsnative-mtp-n1strategy when GGUF tensors include native MTP/NextN blocksautostrategy resolutionConfig behavior
Layer packages can now advertise a default speculative decoding strategy, and operators can override it in
config.tomlwithspeculative.strategyat the defaults or per-model scope.Supported values:
auto: enable native MTP only when the layer package metadata advertises a defaultnative-mtpstrategy withprediction_depth = 1; direct GGUF/no-metadata loads do not enable native MTP automaticallynative-mtp-n1: explicitly select native one-token MTP; package-backed loads must advertise matchingnative-mtp-n1generation metadatadisabled: disable package/default native MTP for that model loadThe resolved flag is carried through Skippy model load options, stage load protobuf/config, embedded OpenAI execution, and binary transport. Direct
skippy-serverusage still keeps the existingSKIPPY_NATIVE_MTP_ENABLEDenvironment fallback.Updated MTP layer packages
Updated the published layer package manifests with native MTP strategy metadata:
GLM 4.7 Flash:
GLM 5.2 Q2_K MTP Q8:
Review fixes
window_policydetails for speculative strategiesnative_mtp_enabledfrom deployment context instead of hard-codingtrueautostrategy now reads packagegeneration.speculative_decodingmetadata instead of hard-coding native MTP onto_model_load_options()as well as stage/OpenAI argsdefaults.speculative.strategyValidation
cargo test -p mesh-llm-config --libcargo test -p mesh-llm-host-runtime --libcargo test -p skippy-server --libcargo test -p skippy-model-package --bin skippy-model-packagecargo test -p skippy-runtime --libcargo check -p skippy-promptcargo clippy -p mesh-llm-config --all-targets -- -D warningscargo clippy -p skippy-server --all-targets -- -D warningscargo clippy -p mesh-llm-host-runtime --all-targets -- -D warningscargo clippy -p skippy-prompt --all-targets -- -D warningscargo clippy -p skippy-protocol --all-targets -- -D warningscargo clippy -p skippy-model-package --all-targets -- -D warningscargo clippy -p skippy-runtime --all-targets -- -D warningscargo fmt --all --checkLatest review-fix validation after rebasing on
origin/jd/jianyang-model-package-mtp:cargo test -p mesh-llm-host-runtime --lib speculative_strategycargo test -p mesh-llm-host-runtime --lib split_generation_load_settings_consumes_resolved_skippy_configcargo test -p skippy-runtime --lib packagecargo clippy -p mesh-llm-host-runtime --all-targets -- -D warningscargo clippy -p skippy-runtime --all-targets -- -D warningscargo fmt --all --checkHF manifest validation:
meshllm/GLM-5.2-Q2_K-MTP-Q8-layers/model-package.jsonafter upload and verifiedgeneration.speculative_decoding.default = "native-mtp-n1"with the same strategy block as GLM 4.7 FlashSummary by CodeRabbit
Release Notes
New Features
generationblock to layer-package manifests to declare speculative-decoding defaults, including native-MTP settings, and expose them during package preflight.native_mtp_enabledas a propagated execution flag across stage loading and embedded execution.Validation & Configuration
speculative.strategy(auto,disabled,native-mtp-n1) with validation.Documentation
generationdefaults andspeculative.strategy.