Make quantized layer-package builds resumable - #995
Conversation
📝 WalkthroughWalkthroughPackage writing now supports pre-metadata transformation and resumable artifacts. Quantization adds resumable package creation, deterministic hook outputs, atomic result installation, and wiring through the new transformation command. ChangesArtifact pipeline
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant QuantizeLayerPackage
participant SkippyModelPackage
participant TransformHook
participant Quantizer
participant PackageDirectory
QuantizeLayerPackage->>SkippyModelPackage: write-package with transform and resume options
SkippyModelPackage->>PackageDirectory: reuse existing artifact when enabled
SkippyModelPackage->>TransformHook: transform artifact
TransformHook->>Quantizer: run quantization with manifest
Quantizer-->>TransformHook: completed GGUF result
TransformHook->>PackageDirectory: atomically install result
SkippyModelPackage->>PackageDirectory: read transformed artifact metadata
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/skippy-quantize/src/main.rs (1)
773-774: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFix relative path symlink resolution.
The symlink creation for the hook script on line 776 (
ln -s "$artifact" "$single_source/model.gguf") will produce a broken symlink ifargs.package_dir(and therefore$artifact) is a relative path. This happens because a symlink target is resolved relative to the symlink's own parent directory rather than the working directory.Since
$single_source(${artifact}.quant-src) is always a sibling directory of$artifact, you can fix this robustly for both relative and absolute paths by pointing the symlink to../$(basename "$artifact").♻️ Proposed refactor (including the unchanged `ln -s` line)
script.push_str("single_source=\"${artifact}.quant-src\"\n"); script.push_str("rm -rf \"$single_source\"\n"); - script.push_str("mkdir -p \"$single_source\"\n"); - script.push_str("ln -s \"$artifact\" \"$single_source/model.gguf\"\n"); + script.push_str("mkdir -p \"$single_source\"\n"); + script.push_str("ln -s \"../$(basename \"$artifact\")\" \"$single_source/model.gguf\"\n");🤖 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/skippy-quantize/src/main.rs` around lines 773 - 774, Update the hook script construction around the symlink creation to use a target relative to the symlink directory: point `model.gguf` to the sibling artifact via `../$(basename "$artifact")` instead of `$artifact`. Preserve the existing `single_source` setup and `ln -s` destination.
🤖 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.
Nitpick comments:
In `@crates/skippy-quantize/src/main.rs`:
- Around line 773-774: Update the hook script construction around the symlink
creation to use a target relative to the symlink directory: point `model.gguf`
to the sibling artifact via `../$(basename "$artifact")` instead of `$artifact`.
Preserve the existing `single_source` setup and `ln -s` destination.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a3fd4677-b10d-4faa-a9e7-e83daf21aeb2
📒 Files selected for processing (4)
crates/skippy-model-package/README.mdcrates/skippy-model-package/src/main.rscrates/skippy-quantize/README.mdcrates/skippy-quantize/src/main.rs
2 participants
Title
Make quantized layer-package builds resumable
Original problem
Building a quantized Skippy layer package required materializing an intermediate package and then quantizing it in a second pass. That duplicated model-sized storage, made interrupted jobs expensive to restart, and made it unsafe to transform artifacts in the existing post-write hook because package hashes and sizes had already been recorded.
Diagnostics
The GLM package-production workflow exposed three generic failure modes: model-sized duplicate output, no artifact-level resume point, and metadata capture occurring before an after-artifact transformation.
A full
cargo test -p skippy-quantizealso exposes two pre-existing main-branch catalog failures because the pinned llama quantizer now advertisesQ2_0while the Rust quant catalog does not. The focused tests for this change pass.Fix
Add a dedicated pre-metadata artifact transform hook to
skippy-model-package, so package metadata always describes the final transformed artifact.Add resumable package production to
skippy-quantizewith persistent per-artifact results, atomic finalization, hard-linking with copy fallback, and source-stage removal after successful conversion. Scratch state is retained until package preflight succeeds.All new behavior is opt-in; existing package and quantization commands retain their current defaults.
Validation
cargo check -p skippy-model-package -p skippy-quantizecargo test -p skippy-model-package(25 passed)cargo test -p skippy-quantize layer_package_hook(4 passed)cargo clippy -p skippy-model-package -p skippy-quantize --all-targets --no-deps -- -D warningscargo fmt --all --checkgit diff --checkSummary by CodeRabbit
New Features
--transform-artifact-commandto transform artifacts before metadata is recorded.--resume-packageto continue interrupted package builds using existing artifacts.Documentation