You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
test(skippy-ffi): update llama_context_params layout for b12199 - #2987
llama_draft::tests::mirrors_match_the_pinned_upstream_layouts fails on main: size_of::<LlamaContextParams>() is 176, the test expects 168. The b12199 upgrade (#2953) regenerated bindings/llama.rs with the new moe_cache_cpu_share field of llama_context_params, and the expected sizes in this test were not updated. CI did not catch it because ci-rust-tests-slice.yml skips skippy-ffi in its test loop and the Windows unit row doesn't list it; I hit it in a full cargo test --workspace --lib pass on Windows.
Diagnostics
The bindings are right, the test values are stale. llama.h at the pinned upstream revision (372dc5ca1, skippy/llama_cpp/upstream.txt) declares float moe_cache_cpu_share after moe_cache_size, and the patch queue doesn't touch the struct. Compiled with MSVC against those headers: llama_context_params is 176 bytes, moe_cache_cpu_share at 120, abort_callback at 128, embeddings at 144, samplers at 152, ctx_other at 168. llama_batch (56) and llama_model_params (80, unchanged offsets) still match.
Fix
Update the llama_context_params expectations to those measured values, and also pin moe_cache_cpu_share and abort_callback so the next field change shows up at the field rather than only in the total.
Validation
cargo test -p skippy-ffi --lib --features skippy-ffi/dynamic-runtime on Windows: 13 passed. cargo fmt --all --check clean.
Summary by CodeRabbit
Tests
Updated layout checks to reflect the current size and field offsets of LlamaContextParams.
No actionable comments were generated in the recent review.
Walkthrough
The pinned-layout test updates its expected LlamaContextParams size to 176 bytes. It adds offset checks for moe_cache_cpu_share and abort_callback, and updates the expected offsets of embeddings, samplers, and ctx_other.
The layout test expects a 176-byte LlamaContextParams, checks the offsets of moe_cache_cpu_share and abort_callback, and updates the offsets of embeddings, samplers, and ctx_other.
Mesh-LLM/mesh-llm#2761: Replaces local struct mirrors with generated upstream binding types and changes how ABI layout is verified.
Merge Risk: Minimal · up to 9787b
The layout assertions are updated; no concrete merge-blocking issue was found.
Pre-merge checks: 5
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly describes the main change: updating the llama_context_params layout test for b12199.
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 726082d8-b20c-4649-a13e-e9ddeab6bc99
Commits
Reviewing files that changed from the base of the PR and between bdadd4b and 9787bfa.
Files selected for processing: 1
skippy/crates/skippy-ffi/src/llama_draft.rs
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
…kage (#296)
Three canaries fail on upstream main for reasons unrelated to the
nixpkgs recipes under review. Each fix stays in the canary's overlay, so
the pinned release builds don't change.
- **buzz-app** (#215): main dropped the `/usr/bin/printenv` git shim in
`crates/plugin-manager/src/imports.rs` (block/buzz-app#2665, which
supersedes block/buzz-app#2417), so the recipe's `--replace-fail`
stopped matching in patchPhase. The canary turns that substitution into
`--replace-quiet`. The 0.7.16 recipe keeps it as is; it can go once the
recipe's buzz-app version contains #2665.
- **mesh-llm** (#253): `skippy-ffi`
`llama_draft::tests::mirrors_match_the_pinned_upstream_layouts` fails on
every platform (176 vs 168). That isn't our llama.cpp: the size comes
from the committed bindgen output (`src/bindings/llama.rs`), which
gained `moe_cache_cpu_share` with the b12199 upgrade
(Mesh-LLM/mesh-llm#2953) while the test kept the old numbers. Upstream
CI doesn't run that crate's tests. Skipped in the canary. upstream:
Mesh-LLM/mesh-llm#2987
- **Buzz** (#291): a new buzz-cli test (block/buzz#9270) checks its
child with `/bin/kill`, which the Linux sandbox lacks (darwin passed).
The canary substitutes util-linux's `kill` on Linux. tracking: #295
Checked by evaluating each overlay against `kleinbem/nixpkgs`
buzz-app-prep, mesh-llm-dev and buzz-next. I'll start the three canaries
on this branch to confirm.
Part of #215, #253, #291.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->
## Summary by CodeRabbit
* **Chores**
* Updated packaging adjustments for Buzz, Buzz CLI and Mesh LLM. These
changes affect build and test handling and do not add or change end-user
features.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Co-authored-by: Claude Opus 5.5 <[email protected]>
The reason will be displayed to describe this comment to others. Learn more.
Verdict: content-good — ready for owner approval (owner gate applies).
Head 9787bfa001. Test-only: refreshes the LlamaContextParams ABI mirror to the b12199 pin's layout (size 168→176, moe_cache_cpu_share@120, abort_callback@128) — the struct fields came with #2953's upgrade and this catches the mirror up. Coherent +8 growth with the two new fields exactly where expected; CI green at head across 67 lanes including the platforms these layout tests run on.
Note: #2962 carries the same realignment as commit ea99a7c0a — coordinate so only one lands with it.
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
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.
i386
left a comment
Original problem
llama_draft::tests::mirrors_match_the_pinned_upstream_layoutsfails on main:size_of::<LlamaContextParams>()is 176, the test expects 168. The b12199 upgrade (#2953) regeneratedbindings/llama.rswith the newmoe_cache_cpu_sharefield ofllama_context_params, and the expected sizes in this test were not updated. CI did not catch it becauseci-rust-tests-slice.ymlskipsskippy-ffiin its test loop and the Windows unit row doesn't list it; I hit it in a fullcargo test --workspace --libpass on Windows.Diagnostics
The bindings are right, the test values are stale.
llama.hat the pinned upstream revision (372dc5ca1,skippy/llama_cpp/upstream.txt) declaresfloat moe_cache_cpu_shareaftermoe_cache_size, and the patch queue doesn't touch the struct. Compiled with MSVC against those headers:llama_context_paramsis 176 bytes,moe_cache_cpu_shareat 120,abort_callbackat 128,embeddingsat 144,samplersat 152,ctx_otherat 168.llama_batch(56) andllama_model_params(80, unchanged offsets) still match.Fix
Update the
llama_context_paramsexpectations to those measured values, and also pinmoe_cache_cpu_shareandabort_callbackso the next field change shows up at the field rather than only in the total.Validation
cargo test -p skippy-ffi --lib --features skippy-ffi/dynamic-runtimeon Windows: 13 passed.cargo fmt --all --checkclean.Summary by CodeRabbit
LlamaContextParams.