Skip to content

fix: enforce model identity and native header rebuilds - #26

Merged
zatevakhin merged 1 commit into
mainfrom
fix/v0.0.2-model-identity-and-rebuilds
Oct 27, 2026
Merged

zatevakhin merged 1 commit into
mainfrom
fix/v0.0.2-model-identity-and-rebuilds

Conversation

@zatevakhin

@zatevakhin zatevakhin commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Fix immutable model resolution and native header rebuilds, and simplify fixture parsing and validation without changing vllm.cpp 0.0.2 support.

@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3f44ea98-89dd-458a-aa1c-13d3e2d99fde
Commits

Reviewing files that changed from the base of the PR and between a7c743f and 75635b6.

Files selected for processing: 10
  • CHANGELOG.md
  • Justfile
  • vllm-cpp-sys/build.rs
  • vllm-cpp-sys/src/build_config.rs
  • vllm-cpp-sys/tests/build_config.rs
  • vllm-cpp/README.md
  • vllm-cpp/examples/README.md
  • vllm-cpp/src/hf.rs
  • vllm-cpp/src/params.rs
  • vllm-cpp/tests/qwen3.rs
Files with no reviewable changes: 2
  • vllm-cpp-sys/tests/build_config.rs
  • vllm-cpp-sys/src/build_config.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The changes validate full commit SHAs and cached revisions during GGUF and Safetensors resolution. They also update native build and test checks, adjust sampling-parameter validation, and restrict WAV parsing tests to the committed fixture layout.

Changes: Pinned Hub resolution

Layer / File(s) Summary
Commit validation and pinned resolution
vllm-cpp/src/hf.rs, vllm-cpp/README.md, vllm-cpp/examples/README.md
Resolution checks require full commit SHAs in online metadata and validate explicit SHA revisions. Safetensors retrieval repairs the pinned cache ref after failures. The documentation describes the SHA and cache-ref requirements.
Hub mock and resolution tests
vllm-cpp/src/hf.rs
The mock Hub serves multiple configured files. Tests cover indexed and unsharded Safetensors, cached revisions, offline resolution, failed downloads, and mutable aliases.

Native build and test validation

Layer / File(s) Summary
Build inputs and validation gates
vllm-cpp-sys/build.rs, vllm-cpp-sys/src/build_config.rs, vllm-cpp-sys/tests/build_config.rs, Justfile, CHANGELOG.md
Cargo watches the full native include directory. CUTLASS and MLX validation no longer require canonicalized paths to be absolute. Link-mode checks run the layout and symbols tests, the watched-archive rerun builds the crate, and the native C API gate checks the single-test CTest result.
Sampling parameter validation
vllm-cpp/src/params.rs
The custom logits-processor check rejects an unbounded token limit; zero remains rejected by the earlier conversion.
Fixed-layout WAV fixture parsing
vllm-cpp/tests/qwen3.rs
The parser checks the fixed 44-byte PCM fixture layout and exact data sizes. Tests use the committed fixture and check mutations, truncation, and trailing bytes.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Resolver
  participant HuggingFaceHub
  participant SnapshotCache
  Resolver->>HuggingFaceHub: request metadata and file list
  HuggingFaceHub-->>Resolver: return commit SHA and repository files
  Resolver->>HuggingFaceHub: download files at the validated commit
  HuggingFaceHub-->>Resolver: return file contents
  Resolver->>SnapshotCache: verify and store the snapshot
  Resolver->>SnapshotCache: restore pinned ref if retrieval fails
Loading
Merge Risk: Minimal · up to 75635

This change tightens pinned Hugging Face model resolution, native header rebuild tracking, and some test and build gates. No actionable merge-blocking issue was found. The WAV fixture compatibility with the new parser could not be confirmed here and is covered by the test itself when run in CI.

Pre-merge checks: Passed 4
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the two main changes: enforcing model identity and rebuilding native headers.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@zatevakhin
zatevakhin added this pull request to stack #25 October 24, 2026 20:27
@zatevakhin
zatevakhin marked this pull request as ready for review October 25, 2026 02:14
@zatevakhin
zatevakhin force-pushed the fix/v0.0.2-model-identity-and-rebuilds branch from 33eb40e to 2915113 Compare October 25, 2026 03:10
@zatevakhin
zatevakhin force-pushed the fix/v0.0.2-model-identity-and-rebuilds branch from 2915113 to a67af4f Compare October 26, 2026 23:17
Base automatically changed from chore/simplify-v0.0.2-bindings to main October 27, 2026 00:24
@zatevakhin
zatevakhin force-pushed the fix/v0.0.2-model-identity-and-rebuilds branch from a67af4f to 75635b6 Compare October 27, 2026 00:24
@zatevakhin
zatevakhin merged commit 1179fb3 into main Oct 27, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant