diff --git a/crates/mesh-llm-host-runtime/src/inference/skippy/package.rs b/crates/mesh-llm-host-runtime/src/inference/skippy/package.rs index 549dd56007..d4801811f7 100644 --- a/crates/mesh-llm-host-runtime/src/inference/skippy/package.rs +++ b/crates/mesh-llm-host-runtime/src/inference/skippy/package.rs @@ -1001,7 +1001,12 @@ pub(crate) fn direct_gguf_planning_manifest_from_identity( let native = skippy_runtime::ModelInfo::open(path) .with_context(|| format!("open direct GGUF metadata {}", path.display()))? .tensors() - .with_context(|| format!("read direct GGUF tensors {}", path.display()))?; + .with_context(|| format!("read direct GGUF tensors {}", path.display()))? + .into_iter() + // Zero-element placeholders are skipped by the GGUF catalog + // reader; drop them from the native side so counts agree. + .filter(|tensor| tensor.element_count > 0) + .collect::>(); let shard_tensors = direct_planning_tensor_catalog(&directory, &native, &artifact_id)?; total_tensors = total_tensors .checked_add(shard_tensors.entries.len()) diff --git a/crates/mesh-llm-host-runtime/src/inference/skippy/split-certified.json b/crates/mesh-llm-host-runtime/src/inference/skippy/split-certified.json index 51f5ee5743..58b1e8e1ef 100644 --- a/crates/mesh-llm-host-runtime/src/inference/skippy/split-certified.json +++ b/crates/mesh-llm-host-runtime/src/inference/skippy/split-certified.json @@ -2,8 +2,8 @@ "schema_version": 1, "native_recipe": { "llama_upstream_sha": "661643e43079a4ee6faab4c1895291767b67ea8d", - "skippy_abi": "0.1.56", - "patch_queue_sha256": "2ef672b91ce3b2a0bd13590bb985895cbf6b30df4c8815376df1482bd25c3dcd" + "skippy_abi": "0.1.57", + "patch_queue_sha256": "8b55597e18ffda86b0c0b9edf0edc3dfcbf2d91b4f07126a7d2ed04672cf2e59" }, "models": [ { diff --git a/crates/model-package/src/bin/queue-unsloth-layer-packages.rs b/crates/model-package/src/bin/queue-unsloth-layer-packages.rs index b3f0b02ac5..890afe5db9 100644 --- a/crates/model-package/src/bin/queue-unsloth-layer-packages.rs +++ b/crates/model-package/src/bin/queue-unsloth-layer-packages.rs @@ -38,8 +38,10 @@ struct Args { wait_for_jobs: bool, job_poll_interval: Duration, catalog_direct: bool, + republish: bool, confirm: bool, dry_run: bool, + exclude_repos: Vec, } #[derive(Debug, Clone)] @@ -215,36 +217,14 @@ async fn main() -> Result<()> { continue; } let status = candidate_status(&hf_client, &candidate, args.retry_queued_after).await?; - match status { - QueueStatus::Published { repo } => { - println!( - "skip {}: already has published layer package {}", - candidate.model.repo_id, repo - ); - continue; - } - QueueStatus::Cataloged { repo } => { - println!( - "skip {}: already has layer package {} in meshllm/catalog", - candidate.model.repo_id, repo - ); - continue; - } - QueueStatus::Queued { repo } => { - println!( - "skip {}: already recently queued layer package {}", - candidate.model.repo_id, repo - ); - continue; - } - QueueStatus::Failed { repo } => { - println!( - "skip {}: layer package job previously failed for {}", - candidate.model.repo_id, repo - ); - continue; - } - QueueStatus::Missing | QueueStatus::StaleQueued => {} + // `Queued` is authoritative regardless of `--republish`: a fresh + // queue marker means an in-flight (paid) job already covers this + // candidate, and re-submitting would double-spend. The republish + // replacement policy only re-queues candidates whose previous job + // is finished (published/catalogued/failed). + if let Some(skip_message) = should_skip(&status, args.republish) { + println!("skip {}: {}", candidate.model.repo_id, skip_message); + continue; } let source_total_bytes = candidate_source_total_bytes(&candidate); @@ -353,8 +333,10 @@ impl Args { wait_for_jobs: false, job_poll_interval: Duration::from_secs(60), catalog_direct: true, + republish: false, confirm: false, dry_run: true, + exclude_repos: Vec::new(), }; let mut iter = std::env::args().skip(1); @@ -403,6 +385,13 @@ impl Args { } "--catalog-direct" => args.catalog_direct = true, "--no-catalog-direct" => args.catalog_direct = false, + "--exclude-repo" => { + let value = next_value(&mut iter, &flag)?; + if !value.is_empty() && !args.exclude_repos.contains(&value) { + args.exclude_repos.push(value); + } + } + "--republish" => args.republish = true, "--confirm" => { args.confirm = true; args.dry_run = false; @@ -465,7 +454,9 @@ fn print_help() { --wait-for-jobs\n\ --job-poll-seconds N\n\ --split-candidate-vram-gib GiB\n\ - --no-catalog-direct" + --no-catalog-direct\n\ + --republish (re-queue even if published/catalogued; replaces the existing package in place)\n\ + --exclude-repo org/repo (skip a quarantined source repo; repeatable)" ); } @@ -673,6 +664,17 @@ async fn build_candidate( model: RankedModel, args: &Args, ) -> Result> { + if args + .exclude_repos + .iter() + .any(|excluded| excluded.eq_ignore_ascii_case(&model.repo_id)) + { + eprintln!( + "skip {}: excluded by --exclude-repo (quarantined)", + model.repo_id + ); + return Ok(None); + } let Some(source_info) = model_repo_info(client, &model.repo_id).await? else { eprintln!("skip {}: source repo no longer exists", model.repo_id); return Ok(None); @@ -797,6 +799,14 @@ async fn candidate_status( candidate: &Candidate, retry_queued_after: Duration, ) -> Result { + // Check the target repo's queue marker FIRST: a recent marker means an + // in-flight (paid) job already covers this candidate. Without this, the + // cataloged/published/failure returns below would shadow `Queued`, and a + // second `--republish` run would submit (and pay for) the same job again. + if let Some(repo) = recently_queued_target_repo(client, candidate, retry_queued_after).await? { + return Ok(QueueStatus::Queued { repo }); + } + if let Some(repo) = catalog_layer_package_repo(client, candidate).await? { return Ok(QueueStatus::Cataloged { repo }); } @@ -807,7 +817,7 @@ async fn candidate_status( continue; }; - let siblings = repo_info.siblings.unwrap_or_default(); + let siblings = repo_info.siblings.clone().unwrap_or_default(); if siblings .iter() .any(|sibling| sibling.rfilename == "model-package.json") @@ -824,31 +834,84 @@ async fn candidate_status( if has_failure_marker && repo == &candidate.target_repo { return Ok(QueueStatus::Failed { repo: repo.clone() }); } - if has_queue_marker { - let queued_recently = repo_info - .last_modified - .as_deref() - .and_then(parse_hf_datetime) - .map(|last_modified| { - Utc::now() - .signed_duration_since(last_modified) - .to_std() - .unwrap_or_default() - < retry_queued_after - }) - .unwrap_or(true); - if queued_recently { - return Ok(QueueStatus::Queued { repo: repo.clone() }); - } - if repo == &candidate.target_repo { - exact_status = QueueStatus::StaleQueued; - } + if has_queue_marker && repo_queued_recently(&repo_info, retry_queued_after) { + return Ok(QueueStatus::Queued { repo: repo.clone() }); + } + if has_queue_marker && repo == &candidate.target_repo { + exact_status = QueueStatus::StaleQueued; } } Ok(exact_status) } +/// True when the repo's `last_modified` is within `retry_queued_after` of now +/// (or is unknown, which we treat as recent to stay on the safe side). +fn repo_queued_recently(repo_info: &ModelInfo, retry_queued_after: Duration) -> bool { + repo_info + .last_modified + .as_deref() + .and_then(parse_hf_datetime) + .map(|last_modified| { + Utc::now() + .signed_duration_since(last_modified) + .to_std() + .unwrap_or_default() + < retry_queued_after + }) + .unwrap_or(true) +} + +/// Check the candidate's target repo for a fresh queue marker before any +/// cataloged/published/failure state, so `QueueStatus::Queued` can never be +/// shadowed by them (the double-submit bug in the `--republish` review). +async fn recently_queued_target_repo( + client: &HFClient, + candidate: &Candidate, + retry_queued_after: Duration, +) -> Result> { + let Some(repo_info) = model_repo_info(client, &candidate.target_repo).await? else { + return Ok(None); + }; + let has_queue_marker = repo_info + .siblings + .as_deref() + .unwrap_or_default() + .iter() + .any(|sibling| sibling.rfilename == "automation/queue.json"); + if has_queue_marker && repo_queued_recently(&repo_info, retry_queued_after) { + return Ok(Some(candidate.target_repo.clone())); + } + Ok(None) +} + +/// Replacement/skip policy for a candidate. Returns the skip message when the +/// candidate must NOT be queued, or `None` when it should be. +/// +/// `Queued` always skips — even under `--republish` — because a recent queue +/// marker means an in-flight (paid) job already covers the candidate. +fn should_skip(status: &QueueStatus, republish: bool) -> Option { + match status { + QueueStatus::Missing | QueueStatus::StaleQueued => None, + QueueStatus::Queued { repo } => Some(format!( + "already recently queued layer package {repo} (--republish cannot re-submit an in-flight job)" + )), + QueueStatus::Published { repo } if !republish => { + Some(format!("already has published layer package {repo}")) + } + QueueStatus::Cataloged { repo } if !republish => Some(format!( + "already has layer package {repo} in meshllm/catalog" + )), + QueueStatus::Failed { repo } if !republish => { + Some(format!("layer package job previously failed for {repo}")) + } + // Replacement policy: republish re-queues finished packages in place. + QueueStatus::Published { .. } + | QueueStatus::Cataloged { .. } + | QueueStatus::Failed { .. } => None, + } +} + fn model_pipeline_tag(info: &ModelInfo) -> Option { info.pipeline_tag .as_deref() @@ -966,9 +1029,37 @@ async fn write_queue_marker(client: &HFClient, candidate: &Candidate, args: &Arg .send() .await .with_context(|| format!("upload queue marker to {}", candidate.target_repo))?; + // A republished job supersedes any earlier failure: leave the repo without + // a stale failure marker so a future status check cannot read the old job's + // outcome as the current state. + if args.republish { + delete_queue_failure_marker(client, candidate).await?; + } Ok(()) } +/// Remove a stale `automation/failure.json` from the candidate's target repo. +/// A 404 (nothing to delete) is fine. +async fn delete_queue_failure_marker(client: &HFClient, candidate: &Candidate) -> Result<()> { + let repo = model_repo(client, &candidate.target_repo)?; + match repo + .delete_file() + .path_in_repo("automation/failure.json") + .commit_message(format!( + "Clear stale failure marker for {}", + candidate.model_id + )) + .send() + .await + { + Ok(_) => Ok(()), + Err(HFError::Http { context }) if context.status.as_u16() == 404 => Ok(()), + Err(HFError::RepoNotFound { .. }) => Ok(()), + Err(err) => Err(err) + .with_context(|| format!("delete queue failure marker from {}", candidate.target_repo)), + } +} + async fn write_queue_failure_marker( client: &HFClient, submitted: &SubmittedJob, @@ -1066,6 +1157,10 @@ fn job_spec_with_token( "CATALOG_CREATE_PR".into(), if args.catalog_direct { "false" } else { "true" }.into(), ); + environment.insert( + "REPUBLISH".into(), + if args.republish { "true" } else { "false" }.into(), + ); let mut secrets = HashMap::new(); secrets.insert("HF_TOKEN".into(), hf_token.to_string()); @@ -1299,14 +1394,101 @@ fn parse_duration_seconds(input: &str) -> Result { mod tests { use std::time::Duration; + use chrono::Utc; use model_package::jobs::CpuJobPlan; use super::{ - Args, Candidate, DiscoveredProjector, DiscoveredQuant, RankedModel, + Args, Candidate, DiscoveredProjector, DiscoveredQuant, ModelInfo, QueueStatus, RankedModel, estimated_bucket_workspace_bytes, job_spec_with_token, json_layer_package_repo, - model_family_key, model_layer_repos, + model_family_key, model_layer_repos, repo_queued_recently, should_skip, }; + #[test] + fn should_skip_queues_missing_and_stale_candidates() { + assert!(should_skip(&QueueStatus::Missing, false).is_none()); + assert!(should_skip(&QueueStatus::StaleQueued, false).is_none()); + assert!(should_skip(&QueueStatus::Missing, true).is_none()); + assert!(should_skip(&QueueStatus::StaleQueued, true).is_none()); + } + + #[test] + fn should_skip_blocks_finished_states_without_republish() { + for status in [ + QueueStatus::Published { + repo: "meshllm/foo".to_string(), + }, + QueueStatus::Cataloged { + repo: "meshllm/foo".to_string(), + }, + QueueStatus::Failed { + repo: "meshllm/foo".to_string(), + }, + ] { + let message = should_skip(&status, false).expect("must skip without --republish"); + assert!(message.contains("meshllm/foo")); + } + } + + #[test] + fn should_skip_allows_republish_over_finished_states() { + for status in [ + QueueStatus::Published { + repo: "meshllm/foo".to_string(), + }, + QueueStatus::Cataloged { + repo: "meshllm/foo".to_string(), + }, + QueueStatus::Failed { + repo: "meshllm/foo".to_string(), + }, + ] { + assert!( + should_skip(&status, true).is_none(), + "republish must re-queue finished states" + ); + } + } + + #[test] + fn republish_cannot_resubmit_an_in_flight_job() { + // Regression (PR #1718 review): run 1 `--republish --confirm` writes + // automation/queue.json and submits the job; run 2 must skip it, not + // submit (and pay for) the same job again. + let status = QueueStatus::Queued { + repo: "meshllm/foo".to_string(), + }; + assert!(should_skip(&status, false).is_some()); + let message = + should_skip(&status, true).expect("--republish must still skip an in-flight job"); + assert!(message.contains("--republish cannot re-submit an in-flight job")); + } + + #[test] + fn repo_queued_recently_treats_unknown_timestamp_as_recent() { + let info = model_info_for_queued_recently("meshllm/foo", None); + assert!(repo_queued_recently(&info, Duration::from_secs(1))); + } + + #[test] + fn repo_queued_recently_uses_last_modified() { + let fresh = model_info_for_queued_recently("meshllm/foo", Some(Utc::now().to_rfc3339())); + assert!(repo_queued_recently(&fresh, Duration::from_secs(3600))); + let stale = model_info_for_queued_recently( + "meshllm/foo", + Some((Utc::now() - chrono::Duration::hours(2)).to_rfc3339()), + ); + assert!(!repo_queued_recently(&stale, Duration::from_secs(3600))); + } + + /// Build a minimal `ModelInfo` — the type has no `Default`. + fn model_info_for_queued_recently(id: &str, last_modified: Option) -> ModelInfo { + ModelInfo { + id: id.to_string(), + last_modified, + ..serde_json::from_value(serde_json::json!({ "id": id })).unwrap() + } + } + #[test] fn model_family_key_collapses_common_unsloth_families() { assert_eq!(model_family_key("unsloth/Kimi-K2-Instruct-GGUF"), "kimi"); @@ -1360,7 +1542,7 @@ mod tests { model_id: "unsloth/GLM-5-GGUF:UD-Q4_K_XL".to_string(), family: "glm".to_string(), }; - let args = Args { + let mut args = Args { author: "unsloth".to_string(), search: "GGUF".to_string(), recent_limit: 1, @@ -1378,8 +1560,10 @@ mod tests { wait_for_jobs: true, job_poll_interval: Duration::from_secs(60), catalog_direct: true, + republish: false, confirm: true, dry_run: false, + exclude_repos: Vec::new(), }; let job_plan = CpuJobPlan { flavor: "cpu-upgrade".to_string(), @@ -1414,6 +1598,10 @@ mod tests { spec.environment.get("SOURCE_REVISION").map(String::as_str), Some("0123456789abcdef") ); + assert_eq!( + spec.environment.get("REPUBLISH").map(String::as_str), + Some("false") + ); assert_eq!( spec.environment .get("SOURCE_PROJECTOR_FILES") @@ -1431,6 +1619,13 @@ mod tests { ); assert_eq!(spec.volumes[1].mount_path, "/source"); assert_eq!(spec.volumes[1].read_only, Some(true)); + + args.republish = true; + let replacement = job_spec_with_token(&candidate, &args, "hf_test", &job_plan).unwrap(); + assert_eq!( + replacement.environment.get("REPUBLISH").map(String::as_str), + Some("true") + ); } #[test] diff --git a/crates/model-package/src/script.rs b/crates/model-package/src/script.rs index 91010c52a1..9fdf6c63a9 100644 --- a/crates/model-package/src/script.rs +++ b/crates/model-package/src/script.rs @@ -165,11 +165,17 @@ mod tests { assert!(EMBEDDED_SCRIPT.contains("SOURCE_TOTAL_BYTES")); assert!(EMBEDDED_SCRIPT.contains("Estimated fallback /bucket cache needed")); assert!(EMBEDDED_SCRIPT.contains("estimate_bucket_workspace_bytes")); - assert!(EMBEDDED_SCRIPT.contains(r#"HF_HUB_CACHE="${HF_HUB_CACHE:-${HF_HOME}/hub}""#)); - assert!(EMBEDDED_SCRIPT.contains(r#"HF_XET_CACHE="${HF_XET_CACHE:-${HF_HOME}/xet}""#)); assert!( EMBEDDED_SCRIPT.contains(r#"PACKAGE_DIR="${PACKAGE_DIR:-${LOCAL_WORK_DIR}/package}""#) ); + assert!(EMBEDDED_SCRIPT.contains( + "refusing to continue (unset PACKAGE_DIR_ALLOW_BUCKET to require local staging)" + )); + assert!( + EMBEDDED_SCRIPT + .contains(r#"HF_XET_CACHE="${HF_XET_CACHE:-${LOCAL_WORK_DIR}/xet-cache}""#) + ); + assert!(EMBEDDED_SCRIPT.contains(r#"HF_HUB_DISABLE_XET="${HF_HUB_DISABLE_XET:-1}""#)); assert!(EMBEDDED_SCRIPT.contains(r#"JOB_TMP_DIR="${JOB_TMP_DIR:-${LOCAL_WORK_DIR}/tmp}""#)); assert!(EMBEDDED_SCRIPT.contains(r#"LOCAL_WORK_DIR="${LOCAL_WORK_DIR:-/tmp/"#)); assert!( @@ -205,6 +211,9 @@ mod tests { assert!(EMBEDDED_SCRIPT.contains(r#"--after-artifact-command "$ARTIFACT_UPLOAD_HOOK""#)); assert!(EMBEDDED_SCRIPT.contains("Uploaded and removed")); assert!(!EMBEDDED_SCRIPT.contains("api.upload_folder")); + assert!(EMBEDDED_SCRIPT.contains("TARGET_UPLOAD_REVISION")); + assert!(EMBEDDED_SCRIPT.contains("promote_layer_package_snapshot.py")); + assert!(EMBEDDED_SCRIPT.contains("Atomically promoted replacement snapshot to main")); assert!(EMBEDDED_SCRIPT.contains(r#"MOUNTED_SOURCE_PATH="/source/${SOURCE_FILE}""#)); assert!(EMBEDDED_SCRIPT.contains(r#"WRITE_PACKAGE_INPUT="$MOUNTED_SOURCE_PATH""#)); assert!(EMBEDDED_SCRIPT.contains(r#"--source-file "$SOURCE_FILE""#)); diff --git a/crates/model-package/src/scripts/split-model-job.sh b/crates/model-package/src/scripts/split-model-job.sh index 63ac241961..06d352c2d0 100755 --- a/crates/model-package/src/scripts/split-model-job.sh +++ b/crates/model-package/src/scripts/split-model-job.sh @@ -10,6 +10,7 @@ set -euo pipefail # SOURCE_PIPELINE_TAG — source model pipeline tag for the published model card # MESH_LLM_REF — git ref to build from (default: main) # CATALOG_CREATE_PR — "true" to open a PR for catalog updates (non-org members) +# REPUBLISH — "true" to stage a replacement and atomically promote it to main # PACKAGE_EXPERIMENTAL — "true" to label the public package as not runtime-certified # HF_TOKEN — injected as a secret by HF Jobs # @@ -19,6 +20,7 @@ set -euo pipefail MESH_LLM_REF="${MESH_LLM_REF:-main}" SOURCE_REVISION="${SOURCE_REVISION:-main}" SOURCE_QUANT="${SOURCE_QUANT:-}" +REPUBLISH="${REPUBLISH:-false}" : "${SOURCE_REPO:?SOURCE_REPO is required}" if [ -z "$SOURCE_QUANT" ] && [[ "${MODEL_ID:-}" == *:* ]]; then SOURCE_QUANT="${MODEL_ID##*:}" @@ -41,8 +43,10 @@ echo "" # Keep executable toolchains/build products on local ephemeral storage: # HF bucket mounts can be unsuitable for dynamic loader/toolchain execution. -# Package artifacts are also written locally, uploaded one at a time, and -# removed immediately so the job never accumulates a full 400GB+ package. +# Package artifacts are also written to the local work dir: the per-artifact +# upload+delete interleave keeps peak usage at one artifact plus its shard +# scratch, which fits the 50G ephemeral cap — and the upload hook re-reading +# artifacts through the writable /bucket FUSE mount surfaces I/O errors. JOB_WORK_ROOT="${JOB_WORK_ROOT:-/bucket/job-work}" SAFE_TARGET_REPO="$(printf '%s' "$TARGET_REPO" | tr -c '[:alnum:]._-' '_')" LOCAL_WORK_DIR="${LOCAL_WORK_DIR:-/tmp/meshllm-layer-job-${SAFE_TARGET_REPO}-$$}" @@ -55,13 +59,25 @@ fi PACKAGE_DIR="${PACKAGE_DIR:-${LOCAL_WORK_DIR}/package}" HF_HOME="${HF_HOME:-${JOB_WORK_DIR}/hf-home}" HF_HUB_CACHE="${HF_HUB_CACHE:-${HF_HOME}/hub}" -HF_XET_CACHE="${HF_XET_CACHE:-${HF_HOME}/xet}" +# The Xet chunk cache must live on the container's local SSD: on network +# filesystems it performs poorly and surfaces I/O errors (the July convert +# wrapper learned this; the same os error 5 killed three split jobs through +# the /bucket FUSE mount on 2026-09-10). +HF_XET_CACHE="${HF_XET_CACHE:-${LOCAL_WORK_DIR}/xet-cache}" +# Route uploads through the classic HTTP path instead of Xet-CAS: HF Jobs +# containers hit sustained I/O errors (os error 5) on the Xet channel that +# do not reproduce outside the cluster, and per-layer GGUF artifacts do not +# benefit from chunk deduplication anyway. Set HF_HUB_DISABLE_XET=0 to +# restore the Xet uploader. +HF_HUB_DISABLE_XET="${HF_HUB_DISABLE_XET:-1}" +PACKAGE_DIR_ALLOW_BUCKET="${PACKAGE_DIR_ALLOW_BUCKET:-}" JOB_TMP_DIR="${JOB_TMP_DIR:-${LOCAL_WORK_DIR}/tmp}" BUILD_DIR="${BUILD_DIR:-${LOCAL_WORK_DIR}/build}" TOOL_DIR="${TOOL_DIR:-${LOCAL_WORK_DIR}/tools}" VENV_DIR="${VENV_DIR:-${LOCAL_WORK_DIR}/venv}" ARTIFACT_UPLOAD_SCRIPT="${ARTIFACT_UPLOAD_SCRIPT:-${LOCAL_WORK_DIR}/upload-package-artifact.py}" ARTIFACT_UPLOAD_HOOK="${ARTIFACT_UPLOAD_HOOK:-${LOCAL_WORK_DIR}/upload-package-artifact.sh}" +SNAPSHOT_PROMOTER="${SNAPSHOT_PROMOTER:-${TOOL_DIR}/promote_layer_package_snapshot.py}" CARGO_HOME="${CARGO_HOME:-${LOCAL_WORK_DIR}/cargo-home}" RUSTUP_HOME="${RUSTUP_HOME:-${LOCAL_WORK_DIR}/rustup-home}" CARGO_TARGET_DIR="${CARGO_TARGET_DIR:-${LOCAL_WORK_DIR}/cargo-target}" @@ -71,7 +87,7 @@ BUILD_TMP_DIR="${BUILD_TMP_DIR:-${LOCAL_WORK_DIR}/tmp}" TMPDIR="$BUILD_TMP_DIR" TEMP="$BUILD_TMP_DIR" TMP="$BUILD_TMP_DIR" -export JOB_WORK_DIR PACKAGE_DIR HF_HOME HF_HUB_CACHE HF_XET_CACHE VENV_DIR ARTIFACT_UPLOAD_SCRIPT +export JOB_WORK_DIR PACKAGE_DIR HF_HOME HF_HUB_CACHE HF_XET_CACHE HF_HUB_DISABLE_XET VENV_DIR ARTIFACT_UPLOAD_SCRIPT export TMPDIR TEMP TMP CARGO_HOME RUSTUP_HOME CARGO_TARGET_DIR XDG_CACHE_HOME PIP_CACHE_DIR cleanup_job_work_dir() { @@ -174,17 +190,34 @@ curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y > /dev/n # shellcheck source=/dev/null source "${CARGO_HOME}/env" +# Fetch a raw commit SHA with retries: GitHub's ref advertisement for +# allow-any-SHA fetches ("upload-pack: not our ref") lags behind the push for +# freshly-pushed commits, which killed two jobs 90 seconds in. Poll until the +# commit is servable, up to 10 minutes. +fetch_ref_with_retry() { + local ref="$1" attempt + for attempt in 1 2 3 4 5 6 7 8 9 10; do + if git fetch --depth 1 origin "$ref"; then + return 0 + fi + echo " fetch of ${ref} failed (attempt ${attempt}/10); retrying in 60s..." >&2 + sleep 60 + done + echo "ERROR: could not fetch ${ref} from origin after 10 attempts" >&2 + return 1 +} + echo "=== [3/9] Cloning mesh-llm and building skippy-model-package ===" git clone --filter=blob:none https://github.com/Mesh-LLM/mesh-llm.git "$BUILD_DIR" cd "$BUILD_DIR" if git ls-remote --exit-code --heads origin "$MESH_LLM_REF" >/dev/null 2>&1 || \ git ls-remote --exit-code --tags origin "$MESH_LLM_REF" >/dev/null 2>&1; then - git fetch --depth 1 origin "$MESH_LLM_REF" + fetch_ref_with_retry "$MESH_LLM_REF" git checkout --detach FETCH_HEAD elif git cat-file -e "$MESH_LLM_REF^{commit}" 2>/dev/null; then git checkout --detach "$MESH_LLM_REF" else - git fetch --depth 1 origin "$MESH_LLM_REF" + fetch_ref_with_retry "$MESH_LLM_REF" git checkout --detach FETCH_HEAD fi @@ -213,6 +246,7 @@ if [ ! -f "$SLICER" ]; then exit 1 fi cp "$SLICER" "${TOOL_DIR}/skippy-model-package" +cp scripts/promote_layer_package_snapshot.py "$SNAPSHOT_PROMOTER" SLICER="${TOOL_DIR}/skippy-model-package" chmod +x "$SLICER" cd / @@ -235,6 +269,20 @@ import os api = HfApi(token=os.environ["HF_TOKEN"]) api.create_repo(os.environ["TARGET_REPO"], exist_ok=True) PYTHON +TARGET_UPLOAD_REVISION="main" +TARGET_MAIN_PARENT="" +if [ "$REPUBLISH" = "true" ]; then + mapfile -t SNAPSHOT_STATE < <( + "$VENV_DIR/bin/python3" "$SNAPSHOT_PROMOTER" prepare \ + --repo "$TARGET_REPO" \ + --source-revision "$SOURCE_REVISION" \ + --token "$(date -u +%Y%m%d%H%M%S)-$$" + ) + TARGET_UPLOAD_REVISION="${SNAPSHOT_STATE[0]:?missing staging revision}" + TARGET_MAIN_PARENT="${SNAPSHOT_STATE[1]:?missing target main parent}" + echo " Staging replacement on ${TARGET_UPLOAD_REVISION} from ${TARGET_MAIN_PARENT}" +fi +export TARGET_UPLOAD_REVISION TARGET_MAIN_PARENT cat > "$ARTIFACT_UPLOAD_SCRIPT" <<'PYTHON' from huggingface_hub import HfApi from pathlib import Path @@ -244,7 +292,8 @@ import time path = Path(os.environ["SKIPPY_PACKAGE_ARTIFACT_PATH"]) relative = os.environ["SKIPPY_PACKAGE_ARTIFACT_RELATIVE_PATH"] target_repo = os.environ["TARGET_REPO"] -max_attempts = int(os.environ.get("ARTIFACT_UPLOAD_ATTEMPTS", "4")) +target_revision = os.environ["TARGET_UPLOAD_REVISION"] +max_attempts = int(os.environ.get("ARTIFACT_UPLOAD_ATTEMPTS", "8")) api = HfApi(token=os.environ["HF_TOKEN"]) last_error = None @@ -255,6 +304,7 @@ for attempt in range(1, max_attempts + 1): path_or_fileobj=str(path), path_in_repo=relative, repo_type="model", + revision=target_revision, commit_message=f"Add package artifact {relative}", ) last_error = None @@ -263,7 +313,7 @@ for attempt in range(1, max_attempts + 1): last_error = err if attempt == max_attempts: break - delay = min(60, 5 * attempt) + delay = min(300, 10 * 2 ** (attempt - 1)) print( f" Upload failed for {relative} on attempt {attempt}/{max_attempts}: {err}. " f"Retrying in {delay}s...", @@ -344,16 +394,34 @@ echo " Hugging Face cache: $HF_HUB_CACHE" echo " Package workspace: $PACKAGE_DIR" echo " Temporary workspace: $TMPDIR" log_storage_snapshot "before write-package" -ROOT_FS="$(df -P / | awk 'NR==2 {print $1}')" -PACKAGE_FS="$(df -P "$PACKAGE_DIR" | awk 'NR==2 {print $1}')" -if [ -n "$ROOT_FS" ] && [ "$ROOT_FS" = "$PACKAGE_FS" ]; then - echo "WARNING: package workspace is on the container root filesystem; very large splits may hit the HF Jobs 50G ephemeral storage limit." >&2 +# The package workspace must live on the container's local SSD. Two reasons: +# (1) HF Jobs evicts the pod once container-local ephemeral storage exceeds +# 50G, and writes through the /bucket FUSE mount count against that same +# budget ~1:1, so staging on /bucket never avoided the wall — only the +# per-artifact upload+delete interleave does; (2) the artifact upload hook +# re-reads each artifact through the writable FUSE mount, which surfaces +# I/O errors (os error 5) mid-upload. Re-create the directory here: empty +# directories on the bucket FUSE mount are not backed by an object and can +# disappear between the initial mkdir and this point. +mkdir -p "$PACKAGE_DIR" +if [ -n "$PACKAGE_DIR_ALLOW_BUCKET" ]; then + echo " NOTE: PACKAGE_DIR placement override active; /bucket EIO risk accepted." >&2 +else + ROOT_FS="$(df -P / | awk 'NR==2 {print $1}')" + PACKAGE_FS="$(df -P "$PACKAGE_DIR" | awk 'NR==2 {print $1}')" + if [ -n "$ROOT_FS" ] && [ "$ROOT_FS" != "$PACKAGE_FS" ]; then + echo "ERROR: package workspace $PACKAGE_DIR is not on the container root filesystem. The /bucket FUSE mount counts writes against the same 50G ephemeral cap AND surfaces upload I/O errors; refusing to continue (unset PACKAGE_DIR_ALLOW_BUCKET to require local staging)." >&2 + exit 1 + fi fi if [ -n "${ESTIMATED_BUCKET_BYTES:-}" ]; then - PACKAGE_AVAILABLE_BYTES="$(df -Pk "$PACKAGE_DIR" | awk 'NR==2 {printf "%.0f", $4 * 1024}')" + # This estimate covers the HF-cache fallback (full source under + # HF_HUB_CACHE, which lives on /bucket). The local package workspace only + # needs one artifact plus its shard scratch at a time. + PACKAGE_AVAILABLE_BYTES="$(df -Pk /bucket | awk 'NR==2 {printf "%.0f", $4 * 1024}')" if [ -n "$PACKAGE_AVAILABLE_BYTES" ] && [ "$PACKAGE_AVAILABLE_BYTES" -gt 0 ] && \ [ "$PACKAGE_AVAILABLE_BYTES" -lt "$ESTIMATED_BUCKET_BYTES" ]; then - echo "WARNING: package workspace has $(format_bytes "$PACKAGE_AVAILABLE_BYTES") available, below estimated need $(format_bytes "$ESTIMATED_BUCKET_BYTES")." >&2 + echo "WARNING: /bucket has $(format_bytes "$PACKAGE_AVAILABLE_BYTES") available for the source-cache fallback, below estimated need $(format_bytes "$ESTIMATED_BUCKET_BYTES")." >&2 fi fi echo " Starting write-package at $(date -u +%Y-%m-%dT%H:%M:%SZ)" @@ -418,6 +486,7 @@ from pathlib import Path api = HfApi(token=os.environ['HF_TOKEN']) target_repo = os.environ['TARGET_REPO'] +target_revision = os.environ['TARGET_UPLOAD_REVISION'] source_repo = os.environ['SOURCE_REPO'] model_id = os.environ.get('MODEL_ID', '') manifest_path = Path(os.environ['PACKAGE_DIR']) / 'model-package.json' @@ -427,17 +496,27 @@ api.upload_file( path_or_fileobj=str(manifest_path), path_in_repo='model-package.json', repo_type='model', + revision=target_revision, commit_message=f'Add layer package manifest from {source_repo} ({model_id})', ) -# Print summary manifest = json.load(open(manifest_path)) -print(f' ✓ Published: https://huggingface.co/{target_repo}') +action = 'Staged replacement' if target_revision != 'main' else 'Published' +print(f' ✓ {action}: https://huggingface.co/{target_repo}/tree/{target_revision}') print(f' Model: {manifest["model_id"]}') print(f' Layers: {manifest["layer_count"]}') print(f' Schema: {manifest["schema_version"]}') PYTHON +if [ "$REPUBLISH" = "true" ]; then + "$VENV_DIR/bin/python3" "$SNAPSHOT_PROMOTER" promote \ + --repo "$TARGET_REPO" \ + --manifest "$PACKAGE_DIR/model-package.json" \ + --staging-revision "$TARGET_UPLOAD_REVISION" \ + --parent-commit "$TARGET_MAIN_PARENT" + echo " ✓ Atomically promoted replacement snapshot to main" +fi + # ─── Update catalog ─────────────────────────────────────────────────────── echo "" echo "=== [7/9] Updating meshllm/catalog ===" diff --git a/crates/skippy-ffi/src/dynamic.rs b/crates/skippy-ffi/src/dynamic.rs index d2b8492596..0482a89d69 100644 --- a/crates/skippy-ffi/src/dynamic.rs +++ b/crates/skippy-ffi/src/dynamic.rs @@ -239,6 +239,7 @@ dynamic_symbols! { skippy_write_slice_gguf(info: *mut ModelInfo, plan: *const SlicePlan, stage_index: i32, output_path: *const c_char, out_error: *mut *mut Error) -> Status; skippy_write_gguf_metadata_from_parts(input_paths: *const *const c_char, input_count: usize, output_path: *const c_char, out_error: *mut *mut Error) -> Status; skippy_write_gguf_from_parts(input_paths: *const *const c_char, input_count: usize, output_path: *const c_char, out_error: *mut *mut Error) -> Status; + skippy_write_gguf_from_parts_consuming(input_paths: *const *const c_char, input_count: usize, output_path: *const c_char, out_error: *mut *mut Error) -> Status; skippy_stage_planner_create_v1(config: *const StagePlannerConfigV1, out_planner: *mut *mut StagePlanner, out_error: *mut *mut Error) -> Status; skippy_stage_planner_free(planner: *mut StagePlanner); skippy_stage_planner_realize_v1(planner: *const StagePlanner, layer_start: i32, layer_end: i32, out_plan: *mut *mut StagePlan, out_error: *mut *mut Error) -> Status; diff --git a/crates/skippy-ffi/src/lib.rs b/crates/skippy-ffi/src/lib.rs index ff55fee7ea..628bb79f7a 100644 --- a/crates/skippy-ffi/src/lib.rs +++ b/crates/skippy-ffi/src/lib.rs @@ -5,7 +5,7 @@ mod dynamic_library; // without compiling the crate to determine native-runtime compatibility. pub const ABI_VERSION_MAJOR: u32 = 0; pub const ABI_VERSION_MINOR: u32 = 1; -pub const ABI_VERSION_PATCH: u32 = 56; +pub const ABI_VERSION_PATCH: u32 = 57; // Propagate static native archive changes through Cargo dependency metadata so // final binaries are relinked after CMake rebuilds llama.cpp. @@ -135,7 +135,8 @@ pub use dynamic::{ skippy_stage_planner_create_v1, skippy_stage_planner_free, skippy_stage_planner_realize_v1, skippy_token_is_eog, skippy_tokenize, skippy_trim_session, skippy_verify_tokens, skippy_verify_tokens_frame_sampled, skippy_write_gguf_from_parts, - skippy_write_gguf_metadata_from_parts, skippy_write_slice_gguf, + skippy_write_gguf_from_parts_consuming, skippy_write_gguf_metadata_from_parts, + skippy_write_slice_gguf, }; #[cfg(feature = "dynamic-runtime")] @@ -185,5 +186,6 @@ pub use static_bindings::{ skippy_stage_planner_create_v1, skippy_stage_planner_free, skippy_stage_planner_realize_v1, skippy_token_is_eog, skippy_tokenize, skippy_trim_session, skippy_verify_tokens, skippy_verify_tokens_frame_sampled, skippy_write_gguf_from_parts, - skippy_write_gguf_metadata_from_parts, skippy_write_slice_gguf, + skippy_write_gguf_from_parts_consuming, skippy_write_gguf_metadata_from_parts, + skippy_write_slice_gguf, }; diff --git a/crates/skippy-ffi/src/static_bindings.rs b/crates/skippy-ffi/src/static_bindings.rs index a40b48ac0c..ecd97c7c6f 100644 --- a/crates/skippy-ffi/src/static_bindings.rs +++ b/crates/skippy-ffi/src/static_bindings.rs @@ -636,6 +636,13 @@ unsafe extern "C" { out_error: *mut *mut Error, ) -> Status; + pub fn skippy_write_gguf_from_parts_consuming( + input_paths: *const *const c_char, + input_count: usize, + output_path: *const c_char, + out_error: *mut *mut Error, + ) -> Status; + pub fn skippy_stage_planner_create_v1( config: *const StagePlannerConfigV1, out_planner: *mut *mut StagePlanner, diff --git a/crates/skippy-model-package/src/cli.rs b/crates/skippy-model-package/src/cli.rs index 6e5db5db5a..d01447de07 100644 --- a/crates/skippy-model-package/src/cli.rs +++ b/crates/skippy-model-package/src/cli.rs @@ -66,6 +66,10 @@ pub(crate) enum Command { source_file: Option, #[arg(long)] resume_existing_artifacts: bool, + /// Maximum payload bytes per artifact; oversized layers are split into + /// byte-balanced part artifacts. Defaults to 8 GiB. + #[arg(long)] + max_artifact_bytes: Option, }, /// Verify byte-preserving v2 packages against independent local source files. VerifyPackageV2 { diff --git a/crates/skippy-model-package/src/main.rs b/crates/skippy-model-package/src/main.rs index 726543edfe..3cafb82722 100644 --- a/crates/skippy-model-package/src/main.rs +++ b/crates/skippy-model-package/src/main.rs @@ -9,6 +9,7 @@ mod hash; mod inspect; mod package; mod package_v2; +mod part_writer; mod plan; mod preflight; mod progress; @@ -104,6 +105,7 @@ fn run(args: Args) -> Result<()> { source_revision, source_file, resume_existing_artifacts, + max_artifact_bytes, } => package_v2::write_package( model, out_dir, @@ -121,6 +123,7 @@ fn run(args: Args) -> Result<()> { source_file, }, resume_existing_artifacts, + max_artifact_bytes, ), Command::VerifyPackageV2 { package, diff --git a/crates/skippy-model-package/src/package_v2.rs b/crates/skippy-model-package/src/package_v2.rs index ee49cfdf0c..843e1ad6a3 100644 --- a/crates/skippy-model-package/src/package_v2.rs +++ b/crates/skippy-model-package/src/package_v2.rs @@ -1,11 +1,12 @@ //! Source-complete v2 creation. Shards are physical containers, not stage owners. use std::collections::{BTreeMap, BTreeSet}; use std::fs::{self, File, OpenOptions}; -use std::io; +use std::io::{self, Read}; use std::path::{Path, PathBuf}; use std::time::{SystemTime, UNIX_EPOCH}; use anyhow::{Context, Result, ensure}; +use skippy_model::gguf_catalog::read_gguf_metadata_catalog; use skippy_model::package_carrier::resolve_package_carrier; use skippy_package_format::{ Artifact, ArtifactCatalog, PACKAGE_SCHEMA_VERSION, PackageManifest, Sidecar, SidecarKind, @@ -25,8 +26,9 @@ use crate::write::{ModelSource, create_parent_dir, write_json_file, write_stage_ mod layout; -use layout::{PlannedArtifact, PlannedArtifactKind, plan_artifacts}; +use layout::{PlannedArtifact, PlannedArtifactKind, plan_artifacts_with_budget}; +#[allow(clippy::too_many_arguments)] pub(crate) fn write_package( model: String, out_dir: PathBuf, @@ -35,6 +37,7 @@ pub(crate) fn write_package( artifact_transform: ArtifactHook, explicit: ExplicitSourceIdentity, resume_existing_artifacts: bool, + max_artifact_bytes: Option, ) -> Result<()> { ensure!( artifact_transform.command.is_none(), @@ -44,7 +47,8 @@ pub(crate) fn write_package( let inventory = SourceInventory::read(&input)?; let source = ModelSource::open(&input.model_path)?; ensure_native_inventory_matches(&inventory, &source)?; - let planned = plan_artifacts(&source.tensors)?; + let budget = max_artifact_bytes.unwrap_or(layout::DEFAULT_MAX_ARTIFACT_BYTES); + let planned = plan_artifacts_with_budget(&source.tensors, budget)?; let mut manifest = manifest_from_source(&input, &inventory)?; fs::create_dir_all(&out_dir)?; ensure!( @@ -52,7 +56,6 @@ pub(crate) fn write_package( "output already contains model-package.json; use a new directory for v2 creation" ); let mut progress = PackageProgress::new(planned.len() + projectors.len() + 2); - let no_hook = ArtifactHook { command: None }; let source_tensors = source_tensors_by_name(&inventory)?; let common_names = planned .iter() @@ -60,11 +63,22 @@ pub(crate) fn write_package( .map(|artifact| artifact.tensor_names.iter().cloned().collect()) .unwrap_or_default(); let mut catalog = Vec::with_capacity(source_tensors.len()); + let no_hook = ArtifactHook { command: None }; + // Payload artifacts may be uploaded and locally deleted by their hook as + // soon as they are verified, so the metadata carrier cannot depend on the + // full parts surviving until the end. Each artifact's header — everything + // before its aligned data start — fully determines its descriptor table and + // tensor offsets, so a header-only stub of each part carries the same + // locators while occupying kilobytes instead of the full payload. + let headers_dir = out_dir.join(".headers"); + fs::create_dir_all(&headers_dir)?; + let mut header_stubs = Vec::with_capacity(planned.len()); for (stage_index, artifact_plan) in planned.iter().enumerate() { progress.start_step(&artifact_plan.path)?; let (artifact, mut tensors) = emit_payload_artifact( &source, &source_tensors, + &inventory, artifact_plan, &common_names, inventory.layer_count, @@ -73,6 +87,13 @@ pub(crate) fn write_package( &no_hook, resume_existing_artifacts, )?; + let path = out_dir.join(&artifact.path); + // Capture the header stub before the upload hook can delete the part. + let header = header_stub_path(&headers_dir, &artifact.id); + write_header_stub(&path, &header, artifact.byte_size)?; + header_stubs.push(header); + run_artifact_hook(&artifact_hook, &path, &artifact.path)?; + verify_hook_result(&artifact, &path, &artifact_hook)?; progress.finish_step(&format!( "{} {}", artifact.path, @@ -100,7 +121,7 @@ pub(crate) fn write_package( &inventory, &manifest, &out_dir, - &no_hook, + &header_stubs, resume_existing_artifacts, )?; progress.finish_step(&format!( @@ -122,10 +143,23 @@ pub(crate) fn write_package( resolved.tensor_catalog == manifest.tensor_catalog, "metadata carrier tensor inventory differs from the independently verified payloads" ); - for artifact in &manifest.artifact_catalog.entries { - let path = out_dir.join(&artifact.path); - run_artifact_hook(&artifact_hook, &path, &artifact.path)?; - verify_hook_result(artifact, &path, &artifact_hook)?; + // The carrier is fully verified against the manifest while it is still on + // disk; only then does the optional artifact hook run. + let carrier_path = out_dir.join("shared/metadata.gguf"); + run_artifact_hook(&artifact_hook, &carrier_path, "shared/metadata.gguf")?; + verify_hook_result( + manifest + .artifact_catalog + .entries + .first() + .expect("carrier entry"), + &carrier_path, + &artifact_hook, + )?; + // Header stubs are an internal working set; the published package root + // contains only the manifest and its catalogued artifacts. + if !header_stubs.is_empty() { + let _ = fs::remove_dir_all(&headers_dir); } for (index, projector) in projectors.iter().enumerate() { let artifact = copy_projector(projector, index, &out_dir, resume_existing_artifacts)?; @@ -137,7 +171,7 @@ pub(crate) fn write_package( )?; if artifact_hook.command.is_some() && out_dir.join(&artifact.path).exists() { ensure!( - file_sha256(&out_dir.join(&artifact.path))? == artifact.sha256, + artifact_unchanged_on_disk(&artifact, &out_dir.join(&artifact.path))?, "projector changed after artifact hook" ); } @@ -280,9 +314,9 @@ fn source_tensors_by_name(inventory: &SourceInventory) -> Result Result { let relative = "shared/metadata.gguf"; @@ -290,25 +324,12 @@ fn emit_metadata_artifact( create_parent_dir(&path)?; ensure_not_source_file(source, &path)?; if !path.exists() { - let sidecars = manifest - .sidecars - .iter() - .map(|sidecar| sidecar.artifact_id.as_str()) - .collect::>(); - let mut payloads = manifest - .artifact_catalog - .entries - .iter() - .filter(|artifact| { - artifact.id != manifest.source_model.metadata_artifact_id - && !sidecars.contains(artifact.id.as_str()) - }) - .collect::>(); - payloads.sort_by(|left, right| left.id.cmp(&right.id)); - let payload_paths = payloads - .iter() - .map(|artifact| out_dir.join(&artifact.path)) - .collect::>(); + // Payload parts may already be uploaded and deleted; the header-only + // stubs carry the exact descriptor tables and locators of the full + // parts, so the carrier is built from them instead. Stub order must + // match the id-sorted payload artifacts the carrier locators index. + let mut payload_paths: Vec<&Path> = header_stubs.iter().map(PathBuf::as_path).collect(); + payload_paths.sort(); write_gguf_metadata_from_parts(&payload_paths, &path) .with_context(|| format!("write GGUF metadata carrier {}", path.display()))?; } else { @@ -339,8 +360,6 @@ fn emit_metadata_artifact( "metadata carrier descriptor table differs from independent source inventory" ); let artifact = artifact_record("metadata", relative, &path)?; - run_artifact_hook(artifact_hook, &path, relative)?; - verify_hook_result(&artifact, &path, artifact_hook)?; Ok(artifact) } @@ -365,6 +384,7 @@ fn metadata_descriptors_match( fn emit_payload_artifact( source: &ModelSource, source_tensors: &BTreeMap, + inventory: &SourceInventory, planned: &PlannedArtifact, common_names: &BTreeSet, layer_count: u32, @@ -376,8 +396,12 @@ fn emit_payload_artifact( let path = out_dir.join(&planned.path); ensure_not_source_file(source, &path)?; if !path.exists() { - let stage = stage_plan(planned, stage_index, layer_count); - write_stage_artifact(source, &stage, &path)?; + if planned.is_part() { + crate::part_writer::write_part(inventory, &planned.tensor_names, &path)?; + } else { + let stage = stage_plan(planned, stage_index, layer_count); + write_stage_artifact(source, &stage, &path)?; + } } else { ensure!( resume, @@ -403,12 +427,23 @@ fn emit_payload_artifact( ) }) .collect::>(); - let expected_physical = planned - .tensor_names - .iter() - .chain(common_names) - .map(String::as_str) - .collect::>(); + let expected_physical = if planned.is_part() { + // Part artifacts are written by the Rust part writer and hold exactly + // their planned tensors; unlike native stage slices they carry no + // duplicated common tensors. + planned + .tensor_names + .iter() + .map(String::as_str) + .collect::>() + } else { + planned + .tensor_names + .iter() + .chain(common_names) + .map(String::as_str) + .collect::>() + }; ensure!( emitted_by_name .keys() @@ -449,6 +484,42 @@ fn emit_payload_artifact( Ok((artifact, bound)) } +/// Path of the header-only stub kept for artifact `id` while its full payload +/// may be uploaded and deleted by its hook. +fn header_stub_path(headers_dir: &Path, artifact_id: &str) -> PathBuf { + let safe_id = artifact_id + .chars() + .filter(|c| c.is_ascii_alphanumeric() || *c == '-' || *c == '_' || *c == '.') + .collect::(); + headers_dir.join(format!("{safe_id}.gguf")) +} + +/// Copy the header of the verified artifact at `path` — every byte before its +/// aligned tensor data start — to `header`. The stub re-opens as a legal +/// descriptor-only GGUF whose tensor offsets equal the full artifact's, which +/// is what the metadata carrier records. +fn write_header_stub(path: &Path, header: &Path, byte_size: u64) -> Result<()> { + let catalog = read_gguf_metadata_catalog(path)?; + ensure!( + catalog.data_start <= byte_size && catalog.data_start <= catalog.artifact_bytes, + "artifact {} header extends beyond its recorded size", + path.display() + ); + let length = usize::try_from(catalog.data_start) + .with_context(|| format!("artifact {} header length overflows usize", path.display()))?; + let input = + File::open(path).with_context(|| format!("open verified artifact {}", path.display()))?; + let mut output = + File::create(header).with_context(|| format!("create header stub {}", header.display()))?; + io::copy( + &mut input.take(u64::try_from(length).context("header length to u64")?), + &mut output, + ) + .with_context(|| format!("copy header stub {}", header.display()))?; + output.sync_all()?; + Ok(()) +} + fn stage_plan(planned: &PlannedArtifact, stage_index: usize, layer_count: u32) -> StagePlan { let (layer_start, layer_end, includes_embeddings, includes_output) = match planned.kind { PlannedArtifactKind::Common => (0, 0, false, false), @@ -495,8 +566,7 @@ fn ensure_not_source_file(source: &ModelSource, path: &Path) -> Result<()> { fn verify_hook_result(artifact: &Artifact, path: &Path, hook: &ArtifactHook) -> Result<()> { if hook.command.is_some() && path.exists() { ensure!( - fs::metadata(path)?.len() == artifact.byte_size - && file_sha256(path)? == artifact.sha256, + artifact_unchanged_on_disk(artifact, path)?, "artifact {:?} changed after artifact hook", artifact.id ); @@ -504,6 +574,33 @@ fn verify_hook_result(artifact: &Artifact, path: &Path, hook: &ArtifactHook) -> Ok(()) } +/// Whether the on-disk artifact still matches its record. +/// +/// A hook may retain or remove the artifact. A FUSE bucket mount can keep +/// reporting a freshly unlinked file as present via a stale attr cache, so a +/// file that can no longer be opened counts as unchanged rather than corrupted. +/// Only a file that opens but differs (the hook mutated it) fails. +fn artifact_unchanged_on_disk(artifact: &Artifact, path: &Path) -> Result { + let probe = + || -> Result { + Ok(fs::metadata(path)?.len() == artifact.byte_size + && file_sha256(path)? == artifact.sha256) + }; + match probe() { + Ok(same) => Ok(same), + Err(err) if is_not_found(&err) => Ok(true), + Err(err) => Err(err), + } +} + +fn is_not_found(err: &anyhow::Error) -> bool { + err.chain().any(|cause| { + cause + .downcast_ref::() + .is_some_and(|io| io.kind() == std::io::ErrorKind::NotFound) + }) +} + fn copy_artifact(source: &Path, output: &Path, resume: bool) -> Result<()> { if resume && output.is_file() { ensure!( diff --git a/crates/skippy-model-package/src/package_v2/layout.rs b/crates/skippy-model-package/src/package_v2/layout.rs index bf5fbb472d..a377cd3f02 100644 --- a/crates/skippy-model-package/src/package_v2/layout.rs +++ b/crates/skippy-model-package/src/package_v2/layout.rs @@ -10,13 +10,20 @@ use anyhow::Result; use skippy_ffi::TensorRole; use skippy_runtime::TensorInfo; +/// Default per-artifact payload ceiling for HF Jobs split packages. The HF +/// Jobs kubelet evicts pods above 50G of container-local ephemeral storage, so +/// payload artifacts are kept well below that even while several parts and +/// their upload buffers coexist on local disk. Oversized model layers are +/// subdivided into byte-balanced part artifacts instead. +pub(crate) const DEFAULT_MAX_ARTIFACT_BYTES: u64 = 8 * 1024 * 1024 * 1024; + /// A payload artifact the writer must emit, with the source tensor names that /// bind to it. `layer` is set only for per-layer artifacts. #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct PlannedArtifact { pub(crate) id: String, /// Relative package path; the verifier derives layer ordinals from - /// `layers/layer-N.gguf` naming. + /// `layers/layer-N.gguf` (and `layers/layer-N-partMM.gguf`) naming. pub(crate) path: String, pub(crate) kind: PlannedArtifactKind, /// Canonical tensor names bound to this artifact. Repeated copies emitted @@ -24,6 +31,15 @@ pub(crate) struct PlannedArtifact { pub(crate) tensor_names: Vec, } +impl PlannedArtifact { + /// Whether this artifact is one of several byte-balanced parts of an + /// oversized layer, written by the Rust part writer rather than the native + /// whole-layer slice writer. + pub(crate) fn is_part(&self) -> bool { + self.id.contains("-part") + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum PlannedArtifactKind { /// Metadata/Tokenizer/Unknown-role payload tensors. Emitted only when the @@ -37,12 +53,19 @@ pub(crate) enum PlannedArtifactKind { }, } -/// Assign every native tensor to exactly one payload artifact. +/// Assign every native tensor to exactly one payload artifact, subdividing +/// artifacts whose payload would exceed `max_artifact_bytes`. /// /// `tensors` must be the complete native inventory of the source. Fails if any /// tensor cannot be bound, so the writer never produces a package with an -/// unowned tensor. -pub(crate) fn plan_artifacts(tensors: &[TensorInfo]) -> Result> { +/// unowned tensor. A single tensor larger than the budget stays whole: it is +/// already the smallest indivisible unit, and failing the package on it would +/// publish nothing at all. +pub(crate) fn plan_artifacts_with_budget( + tensors: &[TensorInfo], + max_artifact_bytes: u64, +) -> Result> { + ensure_budget(max_artifact_bytes)?; let mut common: Vec = Vec::new(); let mut embeddings: Vec = Vec::new(); let mut output: Vec = Vec::new(); @@ -93,15 +116,108 @@ pub(crate) fn plan_artifacts(tensors: &[TensorInfo]) -> Result Result>> { + let bytes_of = |name: &str| -> u64 { + tensors + .iter() + .find(|tensor| tensor.name == name) + .map(|tensor| tensor.byte_size) + .unwrap_or(0) + }; + let total: u64 = names.iter().map(|name| bytes_of(name)).sum(); + if total <= max_artifact_bytes { + return Ok(vec![names.to_vec()]); + } + anyhow::ensure!( + !names.is_empty(), + "layer {ordinal} has no tensors to subdivide" + ); + // A dominating tensor is the smallest indivisible unit: keep it whole + // instead of emitting parts that still exceed the budget. + if names.len() == 1 || bytes_of(&names[0]) > max_artifact_bytes { + return Ok(vec![names.to_vec()]); + } + let split_count = usize::try_from(total.div_ceil(max_artifact_bytes)) + .unwrap_or(usize::MAX) + .clamp(2, names.len()); + // Byte-balanced boundaries mirroring the safetensors GGUF splitter + // (`byte_balanced_split_boundaries`): close part k once accumulated bytes + // reach k/split_count of the layer total, keeping enough tensors back to + // keep every remaining part nonempty. + let mut boundaries = vec![0_usize]; + let mut accumulated: u128 = 0; + for (index, name) in names.iter().enumerate() { + accumulated += u128::from(bytes_of(name)); + let remaining_tensors = names.len() - (index + 1); + let remaining_splits = split_count - (boundaries.len() - 1); + if boundaries.len() < split_count && remaining_tensors >= remaining_splits { + let target = u128::from(total) * boundaries.len() as u128 / split_count as u128; + if accumulated >= target { + boundaries.push(index + 1); + } + } + } + while boundaries.len() < split_count { + let next = boundaries.last().copied().unwrap_or(0) + 1; + boundaries.push(next); + } + boundaries.push(names.len()); + let mut parts = Vec::with_capacity(split_count); + for window in boundaries.windows(2) { + let part = &names[window[0]..window[1]]; + anyhow::ensure!( + !part.is_empty(), + "layer {ordinal} byte-balanced split produced an empty part" + ); + parts.push(part.to_vec()); + } + let assigned: usize = parts.iter().map(Vec::len).sum(); + anyhow::ensure!( + assigned == names.len(), + "layer {ordinal} byte-balanced split dropped tensors" + ); + Ok(parts) +} + +fn ensure_budget(max_artifact_bytes: u64) -> Result<()> { + anyhow::ensure!( + max_artifact_bytes > 0, + "max artifact bytes must be greater than zero" + ); + Ok(()) +} + #[cfg(test)] mod tests; diff --git a/crates/skippy-model-package/src/package_v2/layout/tests.rs b/crates/skippy-model-package/src/package_v2/layout/tests.rs index c1394e7808..53c28ad7d5 100644 --- a/crates/skippy-model-package/src/package_v2/layout/tests.rs +++ b/crates/skippy-model-package/src/package_v2/layout/tests.rs @@ -12,6 +12,10 @@ fn info(name: &str, role: TensorRole, layer: i32) -> TensorInfo { } } +fn plan_artifacts(tensors: &[TensorInfo]) -> Result> { + plan_artifacts_with_budget(tensors, DEFAULT_MAX_ARTIFACT_BYTES) +} + #[test] fn assigns_every_role_to_its_canonical_artifact() { let planned = plan_artifacts(&[ @@ -99,3 +103,77 @@ fn every_tensor_is_bound_exactly_once() { expected.sort(); assert_eq!(bound, expected); } + +#[test] +fn oversized_layer_splits_into_byte_balanced_parts() { + let tensors = [ + info("blk.0.attn_q.weight", TensorRole::Layer, 0), + info("blk.0.attn_k.weight", TensorRole::Layer, 0), + info("blk.0.attn_v.weight", TensorRole::Layer, 0), + info("blk.0.ffn_down.weight", TensorRole::Layer, 0), + ] + .map(|mut tensor| { + tensor.byte_size = 10; + tensor + }); + let planned = plan_artifacts_with_budget(&tensors, 21).unwrap(); + let paths: Vec<&str> = planned.iter().map(|a| a.path.as_str()).collect(); + assert_eq!( + paths, + [ + "layers/layer-00000-part00.gguf", + "layers/layer-00000-part01.gguf", + ] + ); + assert!(planned.iter().all(PlannedArtifact::is_part)); + // Two byte-balanced parts of a 40-byte layer hold 20/20 within budget. + for part in &planned { + let bytes: u64 = part + .tensor_names + .iter() + .map(|name| tensors.iter().find(|t| t.name == *name).unwrap().byte_size) + .sum(); + assert!(bytes <= 21); + } + let mut bound: Vec<&str> = planned + .iter() + .flat_map(|artifact| artifact.tensor_names.iter().map(String::as_str)) + .collect(); + bound.sort_unstable(); + assert_eq!( + bound, + [ + "blk.0.attn_k.weight", + "blk.0.attn_q.weight", + "blk.0.attn_v.weight", + "blk.0.ffn_down.weight", + ] + ); +} + +#[test] +fn dominating_tensor_stays_whole_rather_than_exceeding_budget_in_parts() { + let tensors = [ + info("blk.0.attn_q.weight", TensorRole::Layer, 0), + info("blk.0.ffn_down.weight", TensorRole::Layer, 0), + ] + .map(|mut tensor| { + tensor.byte_size = 100; + tensor + }); + let planned = plan_artifacts_with_budget(&tensors, 21).unwrap(); + // The oversized tensor is indivisible: keep it whole instead of emitting + // parts that still exceed the budget. + assert_eq!(planned.len(), 1); + assert_eq!(planned[0].path, "layers/layer-00000.gguf"); + assert!(!planned[0].is_part()); + assert_eq!(planned[0].tensor_names.len(), 2); +} + +#[test] +fn zero_budget_fails_closed() { + assert!( + plan_artifacts_with_budget(&[info("blk.0.attn_q.weight", TensorRole::Layer, 0)], 0) + .is_err() + ); +} diff --git a/crates/skippy-model-package/src/package_v2/tests.rs b/crates/skippy-model-package/src/package_v2/tests.rs index aba19f4f0c..1bc21b1ec2 100644 --- a/crates/skippy-model-package/src/package_v2/tests.rs +++ b/crates/skippy-model-package/src/package_v2/tests.rs @@ -37,6 +37,7 @@ fn write(source: &Path, out: &Path, resume: bool) -> Result<()> { ArtifactHook { command: None }, explicit(source), resume, + None, ) } @@ -374,6 +375,7 @@ fn refuses_transform_hooks_and_existing_completion_marker() { }, explicit(&source), false, + None, ); assert!( result @@ -405,6 +407,7 @@ fn verified_resume_and_projector_sidecar_round_trip() { ArtifactHook { command: None }, explicit(&source), true, + None, ) .unwrap(); let manifest = read_manifest(&out); @@ -447,6 +450,7 @@ fn upload_hook_can_delete_verified_copies_without_losing_inventory() { ArtifactHook { command: None }, explicit(&source), false, + None, ) .unwrap(); let manifest: PackageManifest = @@ -462,3 +466,137 @@ fn upload_hook_can_delete_verified_copies_without_losing_inventory() { ); assert!(source.exists()); } + +#[cfg(unix)] +#[test] +fn successful_artifact_hook_may_leave_verified_copies_for_rechecking() { + let temp = tempfile::tempdir().unwrap(); + let source = temp.path().join("source.gguf"); + fixture(&source, &[tensor("first", 0), tensor("second", 32)], None); + let out = temp.path().join("package"); + write_package( + source.display().to_string(), + out.clone(), + Vec::new(), + ArtifactHook { + command: Some("/usr/bin/true".into()), + }, + ArtifactHook { command: None }, + explicit(&source), + false, + None, + ) + .unwrap(); + let manifest = read_manifest(&out); + manifest.validate().unwrap(); + assert!( + manifest + .artifact_catalog + .entries + .iter() + .all(|artifact| out.join(&artifact.path).is_file()) + ); +} + +#[test] +fn hook_verification_treats_deleted_artifact_as_unchanged() { + use skippy_package_format::Artifact; + + fn artifact_for(path: &std::path::Path) -> Artifact { + Artifact { + id: "artifact".to_string(), + path: path.display().to_string(), + byte_size: std::fs::metadata(path).unwrap().len(), + sha256: crate::hash::file_sha256(path).unwrap(), + } + } + + let temp = tempfile::tempdir().unwrap(); + let present = temp.path().join("present.gguf"); + let mutated = temp.path().join("mutated.gguf"); + let gone = temp.path().join("gone.gguf"); + fs::write(&present, b"payload").unwrap(); + fs::write(&mutated, b"payload").unwrap(); + fs::write(&gone, b"payload").unwrap(); + + let hook = ArtifactHook { + command: Some(temp.path().join("upload.sh")), + }; + + // Unchanged on disk -> unchanged. + let record = artifact_for(&present); + super::verify_hook_result(&record, &present, &hook).unwrap(); + + // Mutated after the hook -> rejected (content differs from the record). + fs::write(&mutated, b"tampered").unwrap(); + let record = artifact_for(&gone); + let mut tampered_record = record.clone(); + tampered_record.path = mutated.display().to_string(); + assert!(super::verify_hook_result(&tampered_record, &mutated, &hook).is_err()); + + // Deleted by the hook (a FUSE attr cache can still report it present via + // path.exists()) -> opening it fails ENOENT, which must read as unchanged. + fs::remove_file(&gone).unwrap(); + super::verify_hook_result(&record, &gone, &hook).unwrap(); +} + +#[test] +fn oversized_layer_splits_into_verified_part_artifacts_end_to_end() { + let temp = tempfile::tempdir().unwrap(); + let source = temp.path().join("model.gguf"); + fixture( + &source, + &[ + tensor("blk.0.attn_q.weight", 0), + tensor("blk.0.attn_k.weight", 32), + tensor("blk.0.attn_v.weight", 64), + tensor("unknown-global", 96), + ], + None, + ); + let out = temp.path().join("package"); + // 16-byte fixture tensors: a 17-byte budget forces the 48-byte layer to + // split into ceil(48/17)=3 single-tensor parts, each under budget. + write_package( + source.display().to_string(), + out.clone(), + Vec::new(), + ArtifactHook { command: None }, + ArtifactHook { command: None }, + explicit(&source), + false, + Some(17), + ) + .unwrap(); + let manifest = read_manifest(&out); + manifest.validate().unwrap(); + let paths: Vec<&str> = manifest + .artifact_catalog + .entries + .iter() + .map(|artifact| artifact.path.as_str()) + .collect(); + assert_eq!( + paths, + [ + "shared/metadata.gguf", + "shared/common.gguf", + "layers/layer-00000-part00.gguf", + "layers/layer-00000-part01.gguf", + "layers/layer-00000-part02.gguf", + ] + ); + // Split-layer tensors keep their layer ordinal and resolve through the + // carrier to their physical part artifacts. + for tensor in &manifest.tensor_catalog.entries { + if tensor.layer_ordinal.is_some() { + assert_eq!(tensor.layer_ordinal, Some(0)); + let TensorStorage::Owned { artifact_id, .. } = &tensor.storage else { + panic!("part tensors own storage"); + }; + assert!(artifact_id.starts_with("layer-00000-part")); + } + } + // The independent verifier accepts part artifacts and their paths. + crate::verify_v2::verify_package(&out, &source, None, &[]).unwrap(); +} diff --git a/crates/skippy-model-package/src/part_writer.rs b/crates/skippy-model-package/src/part_writer.rs new file mode 100644 index 0000000000..5fae9ba35c --- /dev/null +++ b/crates/skippy-model-package/src/part_writer.rs @@ -0,0 +1,363 @@ +//! Byte-preserving payload-part writer for v2 packages. +//! +//! The native slice writer emits one whole-layer GGUF per artifact, which is +//! the right shape for ordinary layers. HF Jobs split packages must also cap +//! per-artifact bytes (the kubelet evicts pods above 50G of container-local +//! ephemeral storage), so oversized layers are subdivided into part artifacts +//! by the Rust planner. This module writes those parts directly from the +//! independent source inventory, streaming exact tensor payload bytes — no +//! per-shard scratch parts, no merge step, no transient 2x copy — while +//! emitting a legal GGUF whose descriptor table the native carrier writer and +//! the verifier both consume unchanged. +//! +//! Source metadata KV entries are copied as raw wire bytes rather than +//! re-encoded from parsed values: llama.cpp type-checks well-known keys +//! (`general.alignment` must be u32), and a serde_json round trip widens +//! them to u64, which the native reader rejects. +use std::collections::BTreeMap; +use std::fs::File; +use std::io::{Read, Seek, SeekFrom, Write}; +use std::path::{Path, PathBuf}; + +use anyhow::{Context, Result, ensure}; +use skippy_model::gguf_catalog::GgufTensor; +use skippy_package_format::{Tensor, TensorStorage}; + +use crate::source_inventory::SourceInventory; + +const GGUF_MAGIC: &[u8; 4] = b"GGUF"; +const GGUF_VERSION: u32 = 3; +const GGUF_TYPE_U8: u32 = 0; +const GGUF_TYPE_I8: u32 = 1; +const GGUF_TYPE_U16: u32 = 2; +const GGUF_TYPE_I16: u32 = 3; +const GGUF_TYPE_U32: u32 = 4; +const GGUF_TYPE_I32: u32 = 5; +const GGUF_TYPE_F32: u32 = 6; +const GGUF_TYPE_BOOL: u32 = 7; +const GGUF_TYPE_STRING: u32 = 8; +const GGUF_TYPE_ARRAY: u32 = 9; +const GGUF_TYPE_U64: u32 = 10; +const GGUF_TYPE_I64: u32 = 11; +const GGUF_TYPE_F64: u32 = 12; +const COPY_BUFFER_BYTES: usize = 4 * 1024 * 1024; +const GENERAL_ALIGNMENT_KEY: &str = "general.alignment"; +const SPLIT_BOOKKEEPING_KEYS: [&str; 3] = ["split.no", "split.count", "split.tensors.count"]; + +/// The primary shard's metadata section re-encoded as raw GGUF wire bytes with +/// split bookkeeping keys removed and `general.alignment` guaranteed present. +struct SourceMetadataWire { + entry_count: u64, + entries: Vec, +} + +fn source_metadata_wire_bytes( + shard: &crate::source_inventory::SourceShard, +) -> Result { + let mut reader = WireReader::open(&shard.path)?; + let magic = reader.read_bytes(4)?; + ensure!( + magic == GGUF_MAGIC, + "{} is not a GGUF file", + shard.path.display() + ); + reader.read_u32()?; // version; the inventory already validated it + reader.read_u64()?; // tensor count + let metadata_count = reader.read_u64()?; + let mut wire = SourceMetadataWire { + entry_count: 0, + entries: Vec::new(), + }; + let mut has_alignment = false; + for _ in 0..metadata_count { + let key_wire = reader.read_string_wire()?; + let key = std::str::from_utf8(&key_wire[8..]) + .context("GGUF metadata key is not UTF-8")? + .to_string(); + let value_type = reader.read_u32()?; + let value_wire = reader.read_value_wire(value_type)?; + if SPLIT_BOOKKEEPING_KEYS.contains(&key.as_str()) { + continue; + } + has_alignment |= key == GENERAL_ALIGNMENT_KEY; + wire.entries.extend_from_slice(&key_wire); + extend_u32(&mut wire.entries, value_type); + wire.entries.extend_from_slice(&value_wire); + wire.entry_count += 1; + } + if !has_alignment { + // The catalog reader defaults a missing alignment to 32; the part + // inherits the same guarantee with the u32 type llama.cpp expects. + write_string(&mut wire.entries, GENERAL_ALIGNMENT_KEY); + extend_u32(&mut wire.entries, GGUF_TYPE_U32); + extend_u32(&mut wire.entries, shard.directory.alignment as u32); + wire.entry_count += 1; + } + Ok(wire) +} + +/// Minimal sequential GGUF reader that keeps every consumed value as its exact +/// wire bytes. The source was already validated by the catalog reader when the +/// inventory was built, so this walker only needs to agree on value sizes. +struct WireReader { + reader: File, +} + +impl WireReader { + fn open(path: &Path) -> Result { + Ok(Self { + reader: File::open(path).with_context(|| format!("open source {}", path.display()))?, + }) + } + + fn read_bytes(&mut self, length: usize) -> Result> { + let mut bytes = vec![0_u8; length]; + self.reader + .read_exact(&mut bytes) + .with_context(|| format!("read {} wire bytes", length))?; + Ok(bytes) + } + + fn read_u32(&mut self) -> Result { + Ok(u32::from_le_bytes(self.read_bytes(4)?.try_into().unwrap())) + } + + fn read_u64(&mut self) -> Result { + Ok(u64::from_le_bytes(self.read_bytes(8)?.try_into().unwrap())) + } + + /// A length-prefixed string, returned as its complete wire encoding + /// (length bytes included). + fn read_string_wire(&mut self) -> Result> { + let length = self.read_u64()?; + let length = usize::try_from(length).context("GGUF string length exceeds usize")?; + let mut wire = Vec::with_capacity(8 + length); + wire.extend_from_slice(&(length as u64).to_le_bytes()); + wire.extend_from_slice(&self.read_bytes(length)?); + Ok(wire) + } + + fn read_value_wire(&mut self, value_type: u32) -> Result> { + match value_type { + GGUF_TYPE_U8 | GGUF_TYPE_I8 | GGUF_TYPE_BOOL => self.read_bytes(1), + GGUF_TYPE_U16 | GGUF_TYPE_I16 => self.read_bytes(2), + GGUF_TYPE_U32 | GGUF_TYPE_I32 | GGUF_TYPE_F32 => self.read_bytes(4), + GGUF_TYPE_U64 | GGUF_TYPE_I64 | GGUF_TYPE_F64 => self.read_bytes(8), + GGUF_TYPE_STRING => self.read_string_wire(), + GGUF_TYPE_ARRAY => { + let element_type = self.read_u32()?; + ensure!( + element_type != GGUF_TYPE_ARRAY, + "nested GGUF metadata arrays are unsupported" + ); + let count = self.read_u64()?; + let mut wire = Vec::new(); + extend_u32(&mut wire, element_type); + wire.extend_from_slice(&count.to_le_bytes()); + for _ in 0..count { + wire.extend_from_slice(&self.read_value_wire(element_type)?); + } + Ok(wire) + } + other => anyhow::bail!("unsupported GGUF metadata type {other}"), + } + } +} + +/// A tensor extent in the independent source, resolved through the +/// package-format catalog so aliases copy the target's bytes exactly once. +#[derive(Debug, Clone)] +struct SourceExtent { + path: PathBuf, + data_offset: u64, + stored_length: u64, +} + +/// Write one payload part holding exactly `tensor_names`, copying bytes from +/// the source shards. The output is a self-contained GGUF carrying the source +/// metadata KV (minus split bookkeeping) and one aligned payload extent per +/// tensor. Tensors are laid out in sorted-name order. +pub(crate) fn write_part( + inventory: &SourceInventory, + tensor_names: &[String], + out: &Path, +) -> Result<()> { + ensure!( + !tensor_names.is_empty(), + "a payload part must bind at least one tensor" + ); + let alignment = inventory.shards[0].directory.alignment; + ensure!(alignment > 0, "source alignment must be positive"); + let mut descriptors = BTreeMap::new(); + for shard in &inventory.shards { + for tensor in &shard.tensors.entries { + let gguf_tensor = shard + .directory + .tensors + .iter() + .find(|candidate| candidate.name == tensor.id) + .with_context(|| { + format!( + "tensor {:?} missing from the shard descriptor table", + tensor.id + ) + })?; + ensure!( + descriptors + .insert( + tensor.id.clone(), + (gguf_tensor.clone(), shard.path.clone(), tensor.clone()), + ) + .is_none(), + "duplicate source tensor {:?} across shards", + tensor.id + ); + } + } + let mut selected: BTreeMap = BTreeMap::new(); + for name in tensor_names { + let (_, path, tensor) = descriptors + .get(name) + .with_context(|| format!("part tensor {name:?} is absent from the source inventory"))?; + let extent = owned_extent(&descriptors, &tensor.storage, path)?; + selected.insert(name.clone(), extent); + } + + if let Some(parent) = out.parent() + && !parent.as_os_str().is_empty() + { + std::fs::create_dir_all(parent) + .with_context(|| format!("create part parent directory {}", parent.display()))?; + } + let mut writer = File::create(out).with_context(|| format!("create part {}", out.display()))?; + + // Header: magic, version, tensor count, metadata count, KV pairs, then the + // tensor descriptor table with freshly assigned aligned offsets. Metadata + // KV entries are copied from the primary source shard as raw wire bytes + // (minus split bookkeeping), preserving their exact GGUF types. + let mut header: Vec = Vec::new(); + header.extend_from_slice(GGUF_MAGIC); + extend_u32(&mut header, GGUF_VERSION); + extend_u64(&mut header, selected.len() as u64); + let metadata_bytes = source_metadata_wire_bytes(&inventory.shards[0])?; + extend_u64(&mut header, metadata_bytes.entry_count); + header.extend_from_slice(&metadata_bytes.entries); + let mut offset: u64 = 0; + let mut table: Vec<(&String, &GgufTensor, u64)> = Vec::new(); + for (name, extent) in &selected { + let (descriptor, _, _) = &descriptors[name]; + table.push((name, descriptor, offset)); + offset = align_to(offset + extent.stored_length, alignment); + } + for (name, descriptor, tensor_offset) in &table { + write_string(&mut header, name); + extend_u32(&mut header, descriptor.dimensions.len() as u32); + for dimension in &descriptor.dimensions { + extend_u64(&mut header, *dimension); + } + extend_u32(&mut header, descriptor.ggml_type); + extend_u64(&mut header, *tensor_offset); + } + let data_start = align_to(header.len() as u64, alignment); + header.resize(data_start as usize, 0); + writer + .write_all(&header) + .with_context(|| format!("write part header {}", out.display()))?; + + // Payload: stream each extent in table order; table offsets are already + // aligned, so pad the cursor to each tensor's offset before copying and + // consecutive writes land exactly where the table points. + let mut cursor: u64 = 0; + let zeros = vec![0_u8; COPY_BUFFER_BYTES]; + for (name, _, tensor_offset) in &table { + while cursor < *tensor_offset { + let gap = (*tensor_offset - cursor).min(zeros.len() as u64); + writer.write_all(&zeros[..gap as usize])?; + cursor += gap; + } + let extent = &selected[name.as_str()]; + copy_exact_extent( + &extent.path, + extent.data_offset, + extent.stored_length, + &mut writer, + ) + .with_context(|| format!("copy payload of tensor {name:?} into part"))?; + cursor += extent.stored_length; + } + writer + .sync_all() + .with_context(|| format!("sync part {}", out.display()))?; + Ok(()) +} + +/// Resolve the byte extent that stores `storage`. Owned storage copies its own +/// extent; alias storage copies the target's extent (the alias shares bytes, it +/// does not own a second copy). +fn owned_extent( + descriptors: &BTreeMap, + storage: &TensorStorage, + path: &Path, +) -> Result { + match storage { + TensorStorage::Owned { + data_offset, + stored_length, + .. + } => Ok(SourceExtent { + path: path.to_path_buf(), + data_offset: *data_offset, + stored_length: *stored_length, + }), + TensorStorage::Alias { target_tensor_id } => { + let (_, target_path, tensor) = descriptors + .get(target_tensor_id) + .with_context(|| format!("alias target {target_tensor_id:?} is missing"))?; + owned_extent(descriptors, &tensor.storage, target_path) + } + } +} + +fn copy_exact_extent(source: &Path, offset: u64, length: u64, writer: &mut File) -> Result<()> { + ensure!(length > 0, "refusing to copy a zero-length extent"); + let mut file = + File::open(source).with_context(|| format!("open source {}", source.display()))?; + file.seek(SeekFrom::Start(offset)) + .with_context(|| format!("seek source {}", source.display()))?; + let mut buffer = + vec![0_u8; COPY_BUFFER_BYTES.min(length.try_into().unwrap_or(COPY_BUFFER_BYTES))]; + let mut remaining = length; + while remaining > 0 { + let want = buffer + .len() + .min(usize::try_from(remaining).unwrap_or(buffer.len())); + file.read_exact(&mut buffer[..want]) + .with_context(|| format!("read source {}", source.display()))?; + writer.write_all(&buffer[..want])?; + remaining -= want as u64; + } + Ok(()) +} + +fn extend_u32(bytes: &mut Vec, value: u32) { + bytes.extend_from_slice(&value.to_le_bytes()); +} + +fn extend_u64(bytes: &mut Vec, value: u64) { + bytes.extend_from_slice(&value.to_le_bytes()); +} + +fn write_string(bytes: &mut Vec, value: &str) { + extend_u64(bytes, value.len() as u64); + bytes.extend_from_slice(value.as_bytes()); +} + +fn align_to(value: u64, alignment: u64) -> u64 { + if alignment == 0 { + return value; + } + value.div_ceil(alignment) * alignment +} + +#[cfg(test)] +mod tests; diff --git a/crates/skippy-model-package/src/part_writer/tests.rs b/crates/skippy-model-package/src/part_writer/tests.rs new file mode 100644 index 0000000000..4a97fed8d0 --- /dev/null +++ b/crates/skippy-model-package/src/part_writer/tests.rs @@ -0,0 +1,95 @@ +use super::*; +use crate::source_inventory::SourceInventory; +use crate::tensor_payload::TensorLocation; +use crate::test_gguf::{explicit, fixture, tensor}; +use skippy_model::gguf_catalog::read_gguf_catalog; + +fn inventory_for(source: &Path) -> SourceInventory { + let input = + crate::package::resolve_package_input(source.display().to_string(), explicit(source)) + .unwrap(); + SourceInventory::read(&input).unwrap() +} + +#[test] +fn part_round_trips_exact_tensor_payloads_from_the_source() { + let temp = tempfile::tempdir().unwrap(); + let source = temp.path().join("model.gguf"); + fixture( + &source, + &[ + tensor("blk.0.attn_q.weight", 0), + tensor("blk.0.attn_k.weight", 32), + tensor("blk.0.attn_v.weight", 64), + ], + None, + ); + let inventory = inventory_for(&source); + let out = temp.path().join("layers/layer-00000-part00.gguf"); + write_part( + &inventory, + &[ + "blk.0.attn_k.weight".to_string(), + "blk.0.attn_q.weight".to_string(), + ], + &out, + ) + .unwrap(); + + // The part is a legal GGUF catalog: metadata preserved, split bookkeeping + // keys dropped, alignment metadata present, offsets aligned. + let directory = read_gguf_catalog(&out).unwrap(); + assert_eq!(directory.tensors.len(), 2); + assert_eq!( + directory + .tensors + .iter() + .map(|tensor| tensor.name.as_str()) + .collect::>(), + ["blk.0.attn_k.weight", "blk.0.attn_q.weight"] + ); + assert!(directory.metadata.contains_key("general.alignment")); + assert!(!directory.metadata.contains_key("split.no")); + + // Payload bytes are byte-identical to the independent source extents. + let (_, emitted) = crate::source_inventory::inspect(&out, "layer-00000-part00").unwrap(); + let mut emitted_locations = BTreeMap::new(); + for tensor_entry in &emitted.entries { + emitted_locations.insert( + tensor_entry.name.clone(), + TensorLocation { + path: out.clone(), + tensor: tensor_entry.clone(), + }, + ); + } + let mut source_locations = BTreeMap::new(); + for shard in &inventory.shards { + for tensor_entry in &shard.tensors.entries { + source_locations.insert( + tensor_entry.name.clone(), + TensorLocation { + path: shard.path.clone(), + tensor: tensor_entry.clone(), + }, + ); + } + } + for name in ["blk.0.attn_k.weight", "blk.0.attn_q.weight"] { + crate::tensor_payload::compare_tensor_payload(name, &source_locations, &emitted_locations) + .unwrap(); + } + // The unbound tensor stays absent. + assert!(!emitted_locations.contains_key("blk.0.attn_v.weight")); +} + +#[test] +fn a_part_requires_at_least_one_tensor() { + let temp = tempfile::tempdir().unwrap(); + let source = temp.path().join("model.gguf"); + fixture(&source, &[tensor("blk.0.attn_q.weight", 0)], None); + let inventory = inventory_for(&source); + let out = temp.path().join("layers/layer-00000-part00.gguf"); + assert!(write_part(&inventory, &[], &out).is_err()); + assert!(!out.exists()); +} diff --git a/crates/skippy-model-package/src/preflight.rs b/crates/skippy-model-package/src/preflight.rs index 0691a566f5..d6cf7f3c04 100644 --- a/crates/skippy-model-package/src/preflight.rs +++ b/crates/skippy-model-package/src/preflight.rs @@ -678,6 +678,7 @@ mod tests { ArtifactHook { command: None }, explicit(&source), false, + None, ) .unwrap(); package diff --git a/crates/skippy-model-package/src/source_inventory.rs b/crates/skippy-model-package/src/source_inventory.rs index c9f04eda3a..9b42657da1 100644 --- a/crates/skippy-model-package/src/source_inventory.rs +++ b/crates/skippy-model-package/src/source_inventory.rs @@ -182,9 +182,15 @@ fn validate_shards(shards: &[SourceShard]) -> Result<()> { pub(crate) fn inspect(path: &Path, artifact_id: &str) -> Result<(GgufCatalog, TensorCatalog)> { let directory = read_gguf_catalog(path)?; + // Zero-element placeholders (e.g. Unsloth diffusion `__index_timestep_zero__`) + // are skipped by the GGUF catalog reader; drop them from the native side too + // so the inventories agree. let native = ModelInfo::open(path) .with_context(|| format!("native GGUF inspection failed for {}; shared-offset aliases/non-contiguous storage require a native inspection extension", path.display()))? - .tensors()?; + .tensors()? + .into_iter() + .filter(|tensor| tensor.element_count > 0) + .collect::>(); let tensors = catalog_from_inspection(&directory, &native, artifact_id)?; Ok((directory, tensors)) } diff --git a/crates/skippy-model-package/src/verify_v2.rs b/crates/skippy-model-package/src/verify_v2.rs index 228bebac6a..e3ffea7d6f 100644 --- a/crates/skippy-model-package/src/verify_v2.rs +++ b/crates/skippy-model-package/src/verify_v2.rs @@ -170,9 +170,13 @@ pub(crate) fn verify_package( } fn layer_ordinal_from_artifact_path(path: &str) -> Option { - path.strip_prefix("layers/layer-") - .and_then(|value| value.strip_suffix(".gguf")) - .and_then(|value| value.parse().ok()) + let stem = path.strip_prefix("layers/layer-")?.strip_suffix(".gguf")?; + // Oversized layers are split into byte-balanced part artifacts + // (`layer-00042-part01`); every part still belongs to the layer ordinal. + stem.split_once("-part") + .map_or(stem, |(ordinal, _)| ordinal) + .parse() + .ok() } fn source_tensor_locations( diff --git a/crates/skippy-model-package/src/verify_v2/tests.rs b/crates/skippy-model-package/src/verify_v2/tests.rs index 852cdb7a2a..5983390494 100644 --- a/crates/skippy-model-package/src/verify_v2/tests.rs +++ b/crates/skippy-model-package/src/verify_v2/tests.rs @@ -36,6 +36,7 @@ impl Case { ArtifactHook { command: None }, explicit(&self.source), false, + None, ) .unwrap(); } diff --git a/crates/skippy-model-package/src/write.rs b/crates/skippy-model-package/src/write.rs index 25738229dc..f843a54756 100644 --- a/crates/skippy-model-package/src/write.rs +++ b/crates/skippy-model-package/src/write.rs @@ -6,7 +6,7 @@ use anyhow::{Context, Result, bail}; use model_artifact::ModelArtifactFile; use model_ref::split_gguf_shard_info; use serde::Serialize; -use skippy_runtime::{ModelInfo, TensorInfo, write_gguf_from_parts}; +use skippy_runtime::{ModelInfo, TensorInfo, write_gguf_from_parts_consuming}; use crate::hash::file_sha256; use crate::plan::{ @@ -352,7 +352,7 @@ fn write_sharded_stage_artifact(source: &ModelSource, stage: &StagePlan, out: &P })?; parts.push(part_path); } - write_gguf_from_parts(&parts, out) + write_gguf_from_parts_consuming(&parts, out) .with_context(|| format!("merge split-GGUF shard slices into {}", out.display())) })(); diff --git a/crates/skippy-model/src/gguf_catalog.rs b/crates/skippy-model/src/gguf_catalog.rs index 5086961497..fab2514b00 100644 --- a/crates/skippy-model/src/gguf_catalog.rs +++ b/crates/skippy-model/src/gguf_catalog.rs @@ -140,10 +140,21 @@ fn read_gguf_catalog_with_mode( } let tensor_table_end = reader.position()?; let data_start = align_to(tensor_table_end, alignment).context("GGUF data offset overflow")?; - ensure!( - data_start <= artifact_bytes, - "GGUF tensor data starts beyond the artifact" - ); + if tensors.is_empty() { + // A descriptor-only GGUF (zero tensors) is a legal metadata carrier: + // split models commonly ship a first shard holding only model + // metadata, and nothing requires its table end to be aligned or the + // file to extend past it. + ensure!( + tensor_table_end <= artifact_bytes, + "GGUF metadata table ends beyond the artifact" + ); + } else { + ensure!( + data_start <= artifact_bytes, + "GGUF tensor data starts beyond the artifact" + ); + } let mut names = std::collections::BTreeSet::new(); for tensor in &mut tensors { @@ -542,6 +553,33 @@ mod tests { fs::remove_file(path).unwrap(); } + #[test] + fn reads_descriptor_only_shard_with_unaligned_table_end() { + let path = temp_path("descriptor-only"); + let mut bytes = Vec::new(); + bytes.extend_from_slice(GGUF_MAGIC); + bytes.extend_from_slice(&3_u32.to_le_bytes()); + bytes.extend_from_slice(&0_u64.to_le_bytes()); // zero tensors + bytes.extend_from_slice(&2_u64.to_le_bytes()); // two metadata keys + write_string(&mut bytes, "general.architecture"); + bytes.extend_from_slice(&GGUF_TYPE_STRING.to_le_bytes()); + write_string(&mut bytes, "inkling"); + write_string(&mut bytes, "inkling.block_count"); + bytes.extend_from_slice(&GGUF_TYPE_UINT32.to_le_bytes()); + bytes.extend_from_slice(&66_u32.to_le_bytes()); + // File ends immediately after the metadata table, unaligned. + fs::write(&path, bytes).unwrap(); + + let catalog = read_gguf_catalog(&path).unwrap(); + assert_eq!(catalog.tensors.len(), 0); + assert_eq!(catalog.metadata["inkling.block_count"], Value::from(66)); + // data_start is still the aligned offset a payload-bearing companion + // shard would use; the file simply ends before it. + assert!(catalog.data_start > catalog.artifact_bytes); + + fs::remove_file(path).unwrap(); + } + fn write_string(bytes: &mut Vec, value: &str) { bytes .write_all(&(value.len() as u64).to_le_bytes()) diff --git a/crates/skippy-model/src/package_carrier.rs b/crates/skippy-model/src/package_carrier.rs index b14f8626cd..be9d6dcdcb 100644 --- a/crates/skippy-model/src/package_carrier.rs +++ b/crates/skippy-model/src/package_carrier.rs @@ -199,9 +199,14 @@ fn u64_array(metadata: &BTreeMap, key: &str) -> Result> } fn layer_ordinal(artifact: &Artifact) -> Option { - artifact + let stem = artifact .path - .strip_prefix("layers/layer-") - .and_then(|value| value.strip_suffix(".gguf")) - .and_then(|value| value.parse().ok()) + .strip_prefix("layers/layer-")? + .strip_suffix(".gguf")?; + // Oversized layers are split into byte-balanced part artifacts + // (`layer-00042-part01`); every part still belongs to the layer ordinal. + stem.split_once("-part") + .map_or(stem, |(ordinal, _)| ordinal) + .parse() + .ok() } diff --git a/crates/skippy-runtime/src/gguf_writer.rs b/crates/skippy-runtime/src/gguf_writer.rs index 2473052ea3..0336ca0c1d 100644 --- a/crates/skippy-runtime/src/gguf_writer.rs +++ b/crates/skippy-runtime/src/gguf_writer.rs @@ -166,6 +166,29 @@ impl Drop for SlicePlan { pub fn write_gguf_from_parts( input_paths: &[impl AsRef], output_path: impl AsRef, +) -> Result<()> { + write_gguf_from_parts_impl(input_paths, output_path, false) +} + +/// Materialize GGUF parts into one file, unlinking each input part as soon as +/// its tensors have been absorbed into the output. +/// +/// Use this only when the inputs are scratch files owned by the caller. The +/// per-artifact staging peak drops from parts-plus-output to roughly one +/// output file, which keeps sharded splits inside ephemeral-storage budgets +/// such as the HF Jobs 50G container limit. Bench materialization reads +/// published package files and must keep using [`write_gguf_from_parts`]. +pub fn write_gguf_from_parts_consuming( + input_paths: &[impl AsRef], + output_path: impl AsRef, +) -> Result<()> { + write_gguf_from_parts_impl(input_paths, output_path, true) +} + +fn write_gguf_from_parts_impl( + input_paths: &[impl AsRef], + output_path: impl AsRef, + consume_inputs: bool, ) -> Result<()> { if input_paths.is_empty() { return Err(anyhow!("at least one GGUF part path is required")); @@ -182,12 +205,21 @@ pub fn write_gguf_from_parts( let output_path = path_to_cstring(output_path.as_ref(), "output path")?; let mut error = ptr::null_mut(); let status = unsafe { - skippy_ffi::skippy_write_gguf_from_parts( - input_ptrs.as_ptr(), - input_ptrs.len(), - output_path.as_ptr(), - &mut error, - ) + if consume_inputs { + skippy_ffi::skippy_write_gguf_from_parts_consuming( + input_ptrs.as_ptr(), + input_ptrs.len(), + output_path.as_ptr(), + &mut error, + ) + } else { + skippy_ffi::skippy_write_gguf_from_parts( + input_ptrs.as_ptr(), + input_ptrs.len(), + output_path.as_ptr(), + &mut error, + ) + } }; ensure_ok(status, error) } diff --git a/crates/skippy-runtime/src/lib.rs b/crates/skippy-runtime/src/lib.rs index 113634cfa0..5453d5f366 100644 --- a/crates/skippy-runtime/src/lib.rs +++ b/crates/skippy-runtime/src/lib.rs @@ -35,7 +35,8 @@ pub use config::{ pub use devices::{BackendDevice, BackendDeviceType, backend_devices}; pub(crate) use error::ensure_ok; pub use gguf_writer::{ - ModelInfo, SlicePlan, write_gguf_from_parts, write_gguf_metadata_from_parts, + ModelInfo, SlicePlan, write_gguf_from_parts, write_gguf_from_parts_consuming, + write_gguf_metadata_from_parts, }; pub use logging::{ LLAMA_LOG_LEVEL_DEBUG, NativeLogEvent, NativeLogParserMode, NativeLogParserPolicy, diff --git a/scripts/ci-runtime-events-native-gate.sh b/scripts/ci-runtime-events-native-gate.sh index 312b5d0e18..c813ad417f 100755 --- a/scripts/ci-runtime-events-native-gate.sh +++ b/scripts/ci-runtime-events-native-gate.sh @@ -73,6 +73,12 @@ if [[ ! -s "$MODEL_PATH" ]]; then exit 1 fi +# Cargo runs integration-test binaries from the package directory. Resolve a +# caller-relative evidence path before invoking Cargo so the test and this +# wrapper always read the same file. +if [[ "$EVIDENCE_FILE" != /* ]]; then + EVIDENCE_FILE="$PWD/$EVIDENCE_FILE" +fi mkdir -p "$(dirname "$EVIDENCE_FILE")" # Cargo runs integration tests from the crate directory. Keep the writer and # the check below pointed at the same file regardless of that working directory. diff --git a/scripts/promote_layer_package_snapshot.py b/scripts/promote_layer_package_snapshot.py new file mode 100644 index 0000000000..bb25993176 --- /dev/null +++ b/scripts/promote_layer_package_snapshot.py @@ -0,0 +1,106 @@ +#!/usr/bin/env python3 +"""Atomically promote a staged Hugging Face layer-package snapshot.""" + +from __future__ import annotations + +import argparse +import json +import re +import sys +from pathlib import Path +from typing import Any, Callable + + +def snapshot_paths(manifest: dict[str, Any]) -> list[str]: + entries = manifest.get("artifact_catalog", {}).get("entries") + if not isinstance(entries, list) or not entries: + raise ValueError("manifest artifact catalog is empty") + paths = [entry.get("path") for entry in entries if isinstance(entry, dict)] + if len(paths) != len(entries) or any(not isinstance(path, str) or not path for path in paths): + raise ValueError("manifest artifact catalog contains an invalid path") + paths.append("model-package.json") + if len(paths) != len(set(paths)): + raise ValueError("manifest snapshot paths are not unique") + return paths + + +def prepare_snapshot(api: Any, repo_id: str, source_revision: str, token: str) -> tuple[str, str]: + parent = api.model_info(repo_id, revision="main").sha + if not isinstance(parent, str) or not re.fullmatch(r"[0-9a-f]{40}", parent): + raise ValueError("target main did not resolve to an immutable commit") + safe_token = re.sub(r"[^A-Za-z0-9._-]", "-", token).strip("-") + if not safe_token: + raise ValueError("snapshot token is empty") + revision = f"automation/republish-{source_revision[:12]}-{safe_token}" + api.create_branch(repo_id, branch=revision, revision=parent, repo_type="model") + return revision, parent + + +def promote_snapshot( + api: Any, + copy_operation: Callable[..., Any], + repo_id: str, + manifest_path: Path, + staging_revision: str, + parent_commit: str, +) -> None: + manifest = json.loads(manifest_path.read_text(encoding="utf-8")) + operations = [ + copy_operation( + src_path_in_repo=path, + path_in_repo=path, + src_revision=staging_revision, + ) + for path in snapshot_paths(manifest) + ] + api.create_commit( + repo_id=repo_id, + repo_type="model", + revision="main", + parent_commit=parent_commit, + operations=operations, + commit_message=f"Atomically promote layer package from {staging_revision}", + ) + try: + api.delete_branch(repo_id, branch=staging_revision, repo_type="model") + except Exception as error: # Promotion is complete; branch cleanup is best effort. + print(f"WARNING: could not delete staging branch {staging_revision}: {error}", file=sys.stderr) + + +def parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser() + subparsers = parser.add_subparsers(dest="command", required=True) + prepare = subparsers.add_parser("prepare") + prepare.add_argument("--repo", required=True) + prepare.add_argument("--source-revision", required=True) + prepare.add_argument("--token", required=True) + promote = subparsers.add_parser("promote") + promote.add_argument("--repo", required=True) + promote.add_argument("--manifest", type=Path, required=True) + promote.add_argument("--staging-revision", required=True) + promote.add_argument("--parent-commit", required=True) + return parser.parse_args() + + +def main() -> None: + args = parse_args() + from huggingface_hub import CommitOperationCopy, HfApi + + api = HfApi() + if args.command == "prepare": + revision, parent = prepare_snapshot(api, args.repo, args.source_revision, args.token) + print(revision) + print(parent) + return + promote_snapshot( + api, + CommitOperationCopy, + args.repo, + args.manifest, + args.staging_revision, + args.parent_commit, + ) + + +if __name__ == "__main__": + main() diff --git a/scripts/tests/test_promote_layer_package_snapshot.py b/scripts/tests/test_promote_layer_package_snapshot.py new file mode 100644 index 0000000000..19ab5d42f5 --- /dev/null +++ b/scripts/tests/test_promote_layer_package_snapshot.py @@ -0,0 +1,91 @@ +from __future__ import annotations + +import importlib.util +import json +import tempfile +import unittest +from dataclasses import dataclass +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[2] +MODULE_PATH = ROOT / "scripts" / "promote_layer_package_snapshot.py" +SPEC = importlib.util.spec_from_file_location("promote_layer_package_snapshot", MODULE_PATH) +assert SPEC and SPEC.loader +MODULE = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(MODULE) + + +@dataclass +class CopyOperation: + src_path_in_repo: str + path_in_repo: str + src_revision: str + + +class FakeApi: + def __init__(self) -> None: + self.parent = "a" * 40 + self.refs = { + "main": { + "shared/metadata.gguf": b"old-artifact", + "model-package.json": b"old-manifest", + } + } + + def model_info(self, repo_id: str, revision: str): + del repo_id + assert revision == "main" + return type("Info", (), {"sha": self.parent})() + + def create_branch(self, repo_id: str, branch: str, revision: str, repo_type: str) -> None: + del repo_id, repo_type + assert revision == self.parent + self.refs[branch] = dict(self.refs["main"]) + + def create_commit(self, **kwargs) -> None: + assert kwargs["revision"] == "main" + assert kwargs["parent_commit"] == self.parent + promoted = dict(self.refs["main"]) + for operation in kwargs["operations"]: + promoted[operation.path_in_repo] = self.refs[operation.src_revision][ + operation.src_path_in_repo + ] + self.refs["main"] = promoted + + def delete_branch(self, repo_id: str, branch: str, repo_type: str) -> None: + del repo_id, repo_type + del self.refs[branch] + + +class SnapshotPromotionTests(unittest.TestCase): + def test_partial_or_failed_replacement_leaves_main_readable(self) -> None: + api = FakeApi() + revision, _ = MODULE.prepare_snapshot(api, "meshllm/model", "b" * 40, "run-1") + api.refs[revision]["shared/metadata.gguf"] = b"new-artifact" + + self.assertEqual(api.refs["main"]["shared/metadata.gguf"], b"old-artifact") + self.assertEqual(api.refs["main"]["model-package.json"], b"old-manifest") + + def test_complete_snapshot_promotes_in_one_parent_guarded_commit(self) -> None: + api = FakeApi() + revision, parent = MODULE.prepare_snapshot(api, "meshllm/model", "b" * 40, "run-2") + manifest = { + "artifact_catalog": {"entries": [{"path": "shared/metadata.gguf"}]} + } + api.refs[revision]["shared/metadata.gguf"] = b"new-artifact" + api.refs[revision]["model-package.json"] = json.dumps(manifest).encode() + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "model-package.json" + path.write_text(json.dumps(manifest), encoding="utf-8") + MODULE.promote_snapshot( + api, CopyOperation, "meshllm/model", path, revision, parent + ) + + self.assertEqual(api.refs["main"]["shared/metadata.gguf"], b"new-artifact") + self.assertEqual(api.refs["main"]["model-package.json"], json.dumps(manifest).encode()) + self.assertNotIn(revision, api.refs) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/tests/test_runtime_events_native_gate.py b/scripts/tests/test_runtime_events_native_gate.py index f08ef81c3e..59f5ecf61b 100644 --- a/scripts/tests/test_runtime_events_native_gate.py +++ b/scripts/tests/test_runtime_events_native_gate.py @@ -83,6 +83,7 @@ def run_gate( bundle: str | None = None, model: str | None = None, evidence_seed: str | None = None, + relative_evidence: bool = False, evidence_path: str | None = None, ) -> subprocess.CompletedProcess[str]: stub_bin = root / "stub-bin" @@ -103,6 +104,7 @@ def run_gate( evidence = root / "evidence.txt" if evidence_seed is not None: evidence.write_text(evidence_seed, encoding="utf-8") + evidence_arg = evidence.name if relative_evidence else str(evidence) return subprocess.run( [ @@ -113,7 +115,7 @@ def run_gate( "--model", model, "--evidence", - evidence_path if evidence_path is not None else str(evidence), + evidence_path if evidence_path is not None else evidence_arg, ], cwd=root, capture_output=True, @@ -162,6 +164,21 @@ def test_relative_evidence_survives_cargo_working_directory(self) -> None: ) self.assertFalse((root / "stub-bin/nested evidence").exists()) + def test_a_relative_evidence_path_is_resolved_before_cargo_runs(self) -> None: + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + result = self.run_gate( + root, + cargo_body=( + "#!/usr/bin/env bash\n" + 'case "$MESH_LLM_RUNTIME_EVENTS_EVIDENCE_FILE" in /*) ;; *) exit 2 ;; esac\n' + 'printf \'executed\\n\' >> "$MESH_LLM_RUNTIME_EVENTS_EVIDENCE_FILE"\n' + ), + relative_evidence=True, + ) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual((root / "evidence.txt").read_text(), "executed\n") + def test_a_blocked_gate_fails_even_though_the_test_exits_zero(self) -> None: """The whole reason the script checks the marker. @@ -344,6 +361,16 @@ def test_family_certification_has_one_complete_model_list(self) -> None: self.assertEqual(83, len(manifest["models"])) self.assertTrue(all("cadences" not in model for model in manifest["models"])) + def test_gate_model_cadences_cover_pr_and_main(self) -> None: + step = self.steps["Restore runtime-event gate model"] + manifest = yaml.safe_load((ROOT / step["with"]["model_manifest"]).read_text()) + artifact = next( + artifact + for artifact in manifest["artifacts"] + if artifact["id"] == step["with"]["model_artifact_id"] + ) + self.assertTrue({"pull-request", "main"}.issubset(artifact["cadences"])) + def test_evidence_is_uploaded_even_when_the_gate_fails(self) -> None: """The evidence file is how a failure is diagnosed, so it must survive one.""" diff --git a/third_party/llama.cpp/patches/0057-skippy-consume-merge-parts-during-from-parts-materialization.patch b/third_party/llama.cpp/patches/0057-skippy-consume-merge-parts-during-from-parts-materialization.patch new file mode 100644 index 0000000000..01327b97ff --- /dev/null +++ b/third_party/llama.cpp/patches/0057-skippy-consume-merge-parts-during-from-parts-materialization.patch @@ -0,0 +1,167 @@ +From d5c31ea86b0f114b78930edfba5ea5d93b127255 Mon Sep 17 00:00:00 2001 +From: Mesh-LLM CI +Date: Fri, 11 Sep 2026 17:33:13 +1000 +Subject: [PATCH 57/57] skippy: consume merge parts during from-parts materialization + +HF Jobs pods are evicted once container-local ephemeral storage crosses 50G. +The sharded split writer stages one per-shard part per layer (~16G for Kimi-K3) +and then merges them into the layer GGUF while every part is still on disk, so +parts+output coexistence doubles the per-layer peak and tips the eviction +budget mid-merge. Add skippy_write_gguf_from_parts_consuming, an opt-in +materializing variant that unlinks each input part as soon as its tensors have +been absorbed into the output, halving the staging peak. The default +skippy_write_gguf_from_parts keeps its byte-identical, inputs-preserved +behavior for bench materialization and metadata carriers. Bump +SKIPPY_ABI_VERSION_PATCH 56->57. +--- + include/skippy/common.h | 2 +- + include/skippy/model_package.h | 14 ++++++++++ + src/skippy/model_package.cpp | 53 ++++++++++++++++++++++++++++++++-- + 3 files changed, 65 insertions(+), 4 deletions(-) + +diff --git a/include/skippy/common.h b/include/skippy/common.h +index 65cfa2ce8..5853db327 100644 +--- a/include/skippy/common.h ++++ b/include/skippy/common.h +@@ -46,7 +46,7 @@ extern "C" { + + #define SKIPPY_ABI_VERSION_MAJOR 0 + #define SKIPPY_ABI_VERSION_MINOR 1 +-#define SKIPPY_ABI_VERSION_PATCH 56 ++#define SKIPPY_ABI_VERSION_PATCH 57 + + #if defined(_MSC_VER) + #define SKIPPY_DEPRECATED(message) __declspec(deprecated(message)) +diff --git a/include/skippy/model_package.h b/include/skippy/model_package.h +index 6ea9b395f..1c19572b9 100644 +--- a/include/skippy/model_package.h ++++ b/include/skippy/model_package.h +@@ -103,6 +103,20 @@ LLAMA_API enum skippy_status skippy_write_gguf_from_parts( + const char * output_path, + struct skippy_error ** out_error); + ++/** ++ * @brief Materialize a GGUF while consuming scratch input parts. ++ * ++ * Each input is unlinked after its last selected tensor is copied. If the ++ * operation fails, earlier inputs may already be deleted and the output may be ++ * incomplete; later inputs remain available. Use only with disposable staging ++ * parts, never published package files that must survive. ++ */ ++LLAMA_API enum skippy_status skippy_write_gguf_from_parts_consuming( ++ const char * const * input_paths, ++ size_t input_count, ++ const char * output_path, ++ struct skippy_error ** out_error); ++ + #ifdef __cplusplus + } + #endif +diff --git a/src/skippy/model_package.cpp b/src/skippy/model_package.cpp +index 6df18689e..acd468ef1 100644 +--- a/src/skippy/model_package.cpp ++++ b/src/skippy/model_package.cpp +@@ -356,6 +356,7 @@ static enum skippy_status skippy_copy_source_tensors( + const std::vector & selected, + const char * output_path, + gguf_context * out_ctx, ++ bool consume_sources, + struct skippy_error ** out_error) { + if (!gguf_write_to_file(out_ctx, output_path, true)) { + skippy_set_error(out_error, SKIPPY_STATUS_IO_ERROR, "failed to write output GGUF metadata"); +@@ -373,6 +374,20 @@ static enum skippy_status skippy_copy_source_tensors( + std::vector zeroes(output_alignment, 0); + bool ok = true; + ++ // When consuming, each source part is unlinked as soon as the last of its ++ // tensors has been absorbed into the output. `selected` is grouped by ++ // source part (the part loaders preserve input order), so a single ++ // trailing scan after each tensor copy is enough: any source whose last ++ // selected tensor was just copied can be released. Sources are opened ++ // metadata-only (no_alloc), so nothing pins the file after the per-tensor ++ // read handle is closed. ++ std::set consumed; ++ const auto try_consume_source = [&consumed](const skippy_model_info * source) { ++ if (consumed.insert(source).second) { ++ std::remove(source->path.c_str()); ++ } ++ }; ++ + for (const skippy_source_tensor & item : selected) { + FILE * input = ggml_fopen(item.source->path.c_str(), "rb"); + if (input == nullptr) { +@@ -433,6 +448,17 @@ static enum skippy_status skippy_copy_source_tensors( + ok = false; + break; + } ++ ++ if (consume_sources) { ++ const bool more_from_this_source = std::any_of( ++ &item + 1, selected.data() + selected.size(), ++ [&item](const skippy_source_tensor & candidate) { ++ return candidate.source == item.source; ++ }); ++ if (!more_from_this_source) { ++ try_consume_source(item.source); ++ } ++ } + } + + if (std::fclose(output) != 0) { +@@ -640,7 +666,7 @@ enum skippy_status skippy_write_slice_gguf( + } + } + +- const enum skippy_status status = skippy_copy_source_tensors(selected, output_path, out_ctx, out_error); ++ const enum skippy_status status = skippy_copy_source_tensors(selected, output_path, out_ctx, false, out_error); + ggml_free(ggml_ctx); + gguf_free(out_ctx); + if (status != SKIPPY_STATUS_OK) { +@@ -778,10 +804,15 @@ enum skippy_status skippy_write_gguf_metadata_from_parts( + return skippy_success(out_error); + } + +-enum skippy_status skippy_write_gguf_from_parts( ++// Shared implementation for the from-parts writers. `consume_inputs` deletes ++// each input part file as soon as its tensors have been absorbed into the ++// output; callers that need the inputs to survive (bench materialization from ++// published package files, metadata carriers) pass false. ++static enum skippy_status skippy_write_gguf_parts_impl( + const char * const * input_paths, + size_t input_count, + const char * output_path, ++ bool consume_inputs, + struct skippy_error ** out_error) { + if (input_paths == nullptr || input_count == 0 || output_path == nullptr) { + skippy_set_error(out_error, SKIPPY_STATUS_INVALID_ARGUMENT, "input paths and output_path are required"); +@@ -862,7 +893,7 @@ enum skippy_status skippy_write_gguf_from_parts( + } + } + +- const enum skippy_status status = skippy_copy_source_tensors(selected, output_path, out_ctx, out_error); ++ const enum skippy_status status = skippy_copy_source_tensors(selected, output_path, out_ctx, consume_inputs, out_error); + ggml_free(ggml_ctx); + gguf_free(out_ctx); + for (skippy_model_info * source : sources) { +@@ -874,3 +905,19 @@ enum skippy_status skippy_write_gguf_from_parts( + + return skippy_success(out_error); + } ++ ++enum skippy_status skippy_write_gguf_from_parts( ++ const char * const * input_paths, ++ size_t input_count, ++ const char * output_path, ++ struct skippy_error ** out_error) { ++ return skippy_write_gguf_parts_impl(input_paths, input_count, output_path, false, out_error); ++} ++ ++enum skippy_status skippy_write_gguf_from_parts_consuming( ++ const char * const * input_paths, ++ size_t input_count, ++ const char * output_path, ++ struct skippy_error ** out_error) { ++ return skippy_write_gguf_parts_impl(input_paths, input_count, output_path, true, out_error); ++} +-- +2.54.0 (Apple Git-157) diff --git a/tools/xtask/data/console_print_allowlist.json b/tools/xtask/data/console_print_allowlist.json index a2489892f3..ea363eae05 100644 --- a/tools/xtask/data/console_print_allowlist.json +++ b/tools/xtask/data/console_print_allowlist.json @@ -3347,127 +3347,119 @@ ], "crates/model-package/src/bin/queue-unsloth-layer-packages.rs": [ { - "line": 154, - "macro_name": "println!" - }, - { - "line": 159, - "macro_name": "println!" - }, - { - "line": 160, - "macro_name": "println!" - }, - { - "line": 164, + "line": 156, "macro_name": "println!" }, { - "line": 168, + "line": 161, "macro_name": "println!" }, { - "line": 191, + "line": 162, "macro_name": "println!" }, { - "line": 211, + "line": 166, "macro_name": "println!" }, { - "line": 220, + "line": 170, "macro_name": "println!" }, { - "line": 227, + "line": 193, "macro_name": "println!" }, { - "line": 234, + "line": 213, "macro_name": "println!" }, { - "line": 241, + "line": 226, "macro_name": "println!" }, { - "line": 264, + "line": 244, "macro_name": "println!" }, { - "line": 298, + "line": 278, "macro_name": "println!" }, { - "line": 312, + "line": 292, "macro_name": "println!" }, { - "line": 321, + "line": 301, "macro_name": "println!" }, { - "line": 448, + "line": 437, "macro_name": "println!" }, { - "line": 476, + "line": 467, "macro_name": "eprintln!" }, { - "line": 484, + "line": 475, "macro_name": "eprintln!" }, { - "line": 488, + "line": 479, "macro_name": "eprintln!" }, { - "line": 492, + "line": 483, "macro_name": "eprintln!" }, { - "line": 510, + "line": 501, "macro_name": "println!" }, { - "line": 525, + "line": 516, "macro_name": "println!" }, { - "line": 545, + "line": 536, "macro_name": "eprintln!" }, { - "line": 549, + "line": 540, "macro_name": "eprintln!" }, { - "line": 558, + "line": 549, "macro_name": "println!" }, { - "line": 583, + "line": 574, "macro_name": "println!" }, { - "line": 677, + "line": 672, "macro_name": "eprintln!" }, { - "line": 681, + "line": 679, "macro_name": "eprintln!" }, { - "line": 698, + "line": 683, "macro_name": "eprintln!" }, { - "line": 713, + "line": 700, "macro_name": "eprintln!" }, { - "line": 728, + "line": 715, + "macro_name": "eprintln!" + }, + { + "line": 730, "macro_name": "eprintln!" } ], @@ -3745,29 +3737,29 @@ ], "crates/skippy-model-package/src/main.rs": [ { - "line": 32, + "line": 33, "macro_name": "eprintln!" }, { - "line": 40, + "line": 41, "macro_name": "eprintln!" }, { - "line": 71, + "line": 72, "macro_name": "println!" }, { - "line": 137, + "line": 140, "macro_name": "println!" }, { - "line": 157, + "line": 160, "macro_name": "println!" } ], "crates/skippy-model-package/src/package_v2.rs": [ { - "line": 169, + "line": 203, "macro_name": "println!" } ], diff --git a/website/src/docs/pages/skippy-api.md b/website/src/docs/pages/skippy-api.md index f014c42eb0..4c09e86043 100644 --- a/website/src/docs/pages/skippy-api.md +++ b/website/src/docs/pages/skippy-api.md @@ -9,7 +9,7 @@ description: Generated reference for the capability-oriented Skippy C ABI. This reference is generated from the patched llama.cpp public headers. It documents the native C ABI used by Skippy's Rust FFI layer and staged runtime. The ABI is experimental and versioned for lockstep native/Rust builds. -Current generated surface: **15 headers** and **99 exported functions**. +Current generated surface: **15 headers** and **100 exported functions**. ## Quick navigation @@ -74,7 +74,7 @@ Current generated surface: **15 headers** and **99 exported functions**.
- model_package.h10 functions + model_package.h11 functions
@@ -813,6 +814,19 @@ LLAMA_API enum skippy_status skippy_write_gguf_from_parts( struct skippy_error ** out_error); ``` + +#### `skippy_write_gguf_from_parts_consuming` + +Materialize a GGUF while consuming scratch input parts. Each input is unlinked after its last selected tensor is copied. If the operation fails, earlier inputs may already be deleted and the output may be incomplete; later inputs remain available. Use only with disposable staging parts, never published package files that must survive. + +```cpp +LLAMA_API enum skippy_status skippy_write_gguf_from_parts_consuming( + const char * const * input_paths, + size_t input_count, + const char * output_path, + struct skippy_error ** out_error); +``` + ↩ Back to function index @@ -1674,7 +1688,7 @@ SKIPPY_COMMON_API enum skippy_status skippy_parse_chat_response_json( The headers also define the following enums, structs, opaque handles, and ABI constants: - `activation.h`: `skippy_activation_part_desc`, `skippy_activation_boundary_desc`, `skippy_activation_desc`, `SKIPPY_ACTIVATION_FRAME_VERSION = 2`, `SKIPPY_ACTIVATION_BOUNDARY_DESC_VERSION = 2`, `SKIPPY_ACTIVATION_IDENTITY_BYTES = 32`, `SKIPPY_ACTIVATION_MAX_DIMS = 4`, `SKIPPY_ACTIVATION_MAX_PARTS = 16`, `SKIPPY_ACTIVATION_PART_OPTIONAL = (UINT32_C(1) << 0)` -- `common.h`: `skippy_feature`, `skippy_status`, `skippy_error`, `skippy_abi_version`, `SKIPPY_ABI_VERSION_MAJOR = 0`, `SKIPPY_ABI_VERSION_MINOR = 1`, `SKIPPY_ABI_VERSION_PATCH = 56`, `SKIPPY_FEATURE_RUNTIME_EVENT_REPORTER = ((uint64_t)1 << 31)`, `SKIPPY_FEATURE_MODEL_LOAD_EVENTS_V2 = ((uint64_t)1 << 32)`, `SKIPPY_FEATURE_KV_EVENTS = ((uint64_t)1 << 33)`, `SKIPPY_FEATURE_DEVICE_EVENTS = ((uint64_t)1 << 34)`, `SKIPPY_FEATURE_DIAGNOSTIC_EVENTS = ((uint64_t)1 << 35)`, `SKIPPY_FEATURE_UNLOAD_EVENTS = ((uint64_t)1 << 36)` +- `common.h`: `skippy_feature`, `skippy_status`, `skippy_error`, `skippy_abi_version`, `SKIPPY_ABI_VERSION_MAJOR = 0`, `SKIPPY_ABI_VERSION_MINOR = 1`, `SKIPPY_ABI_VERSION_PATCH = 57`, `SKIPPY_FEATURE_RUNTIME_EVENT_REPORTER = ((uint64_t)1 << 31)`, `SKIPPY_FEATURE_MODEL_LOAD_EVENTS_V2 = ((uint64_t)1 << 32)`, `SKIPPY_FEATURE_KV_EVENTS = ((uint64_t)1 << 33)`, `SKIPPY_FEATURE_DEVICE_EVENTS = ((uint64_t)1 << 34)`, `SKIPPY_FEATURE_DIAGNOSTIC_EVENTS = ((uint64_t)1 << 35)`, `SKIPPY_FEATURE_UNLOAD_EVENTS = ((uint64_t)1 << 36)` - `devices.h`: `skippy_backend_device_type`, `skippy_backend_device_cap`, `skippy_backend_device` - `events.h`: `skippy_runtime_event_v1`, `skippy_runtime_event_reporter_v1`, `SKIPPY_RUNTIME_EVENT_V1_ABI_VERSION = 1` - `execution.h`: `skippy_iteration_request`