From d64e978b674c44d04fea4d8495683c474ece4886 Mon Sep 17 00:00:00 2001 From: Paul Hogan <5004d00b753726762516a66df75936687319095eacf0e9f7a4710f7a0ff12098@meshllm.communities.buzz.xyz> Date: Wed, 9 Sep 2026 17:50:10 +1000 Subject: [PATCH 01/22] feat(queue): add --republish to replace existing layer packages Replacement policy for v2: --republish re-queues a model whose layer package is already published, catalogued, or previously failed, so the new job overwrites the existing meshllm/--layers repo in place. Recently-queued (in-flight) jobs still skip to avoid duplicate submissions. Ratchet allowlist renumbered mechanically (31 entries). --- .../src/bin/queue-unsloth-layer-packages.rs | 76 +++++++++++-------- tools/xtask/data/console_print_allowlist.json | 62 +++++++-------- 2 files changed, 77 insertions(+), 61 deletions(-) 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..8a6f4b2ac3 100644 --- a/crates/model-package/src/bin/queue-unsloth-layer-packages.rs +++ b/crates/model-package/src/bin/queue-unsloth-layer-packages.rs @@ -38,6 +38,7 @@ struct Args { wait_for_jobs: bool, job_poll_interval: Duration, catalog_direct: bool, + republish: bool, confirm: bool, dry_run: bool, } @@ -215,36 +216,47 @@ 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; + if args.republish + && matches!( + status, + QueueStatus::Published { .. } + | QueueStatus::Cataloged { .. } + | QueueStatus::Failed { .. } + ) + { + // Replacement policy: republish v2 over the existing package in place. + } else { + 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 => {} } - QueueStatus::Missing | QueueStatus::StaleQueued => {} } let source_total_bytes = candidate_source_total_bytes(&candidate); @@ -353,6 +365,7 @@ impl Args { wait_for_jobs: false, job_poll_interval: Duration::from_secs(60), catalog_direct: true, + republish: false, confirm: false, dry_run: true, }; @@ -403,6 +416,7 @@ impl Args { } "--catalog-direct" => args.catalog_direct = true, "--no-catalog-direct" => args.catalog_direct = false, + "--republish" => args.republish = true, "--confirm" => { args.confirm = true; args.dry_run = false; @@ -465,7 +479,8 @@ 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)" ); } @@ -1378,6 +1393,7 @@ mod tests { wait_for_jobs: true, job_poll_interval: Duration::from_secs(60), catalog_direct: true, + republish: false, confirm: true, dry_run: false, }; diff --git a/tools/xtask/data/console_print_allowlist.json b/tools/xtask/data/console_print_allowlist.json index cf79c6119d..5776df5fb9 100644 --- a/tools/xtask/data/console_print_allowlist.json +++ b/tools/xtask/data/console_print_allowlist.json @@ -3347,127 +3347,127 @@ ], "crates/model-package/src/bin/queue-unsloth-layer-packages.rs": [ { - "line": 154, + "line": 155, "macro_name": "println!" }, { - "line": 159, + "line": 160, "macro_name": "println!" }, { - "line": 160, + "line": 161, "macro_name": "println!" }, { - "line": 164, + "line": 165, "macro_name": "println!" }, { - "line": 168, + "line": 169, "macro_name": "println!" }, { - "line": 191, + "line": 192, "macro_name": "println!" }, { - "line": 211, + "line": 212, "macro_name": "println!" }, { - "line": 220, + "line": 231, "macro_name": "println!" }, { - "line": 227, + "line": 238, "macro_name": "println!" }, { - "line": 234, + "line": 245, "macro_name": "println!" }, { - "line": 241, + "line": 252, "macro_name": "println!" }, { - "line": 264, + "line": 276, "macro_name": "println!" }, { - "line": 298, + "line": 310, "macro_name": "println!" }, { - "line": 312, + "line": 324, "macro_name": "println!" }, { - "line": 321, + "line": 333, "macro_name": "println!" }, { - "line": 448, + "line": 462, "macro_name": "println!" }, { - "line": 476, + "line": 491, "macro_name": "eprintln!" }, { - "line": 484, + "line": 499, "macro_name": "eprintln!" }, { - "line": 488, + "line": 503, "macro_name": "eprintln!" }, { - "line": 492, + "line": 507, "macro_name": "eprintln!" }, { - "line": 510, + "line": 525, "macro_name": "println!" }, { - "line": 525, + "line": 540, "macro_name": "println!" }, { - "line": 545, + "line": 560, "macro_name": "eprintln!" }, { - "line": 549, + "line": 564, "macro_name": "eprintln!" }, { - "line": 558, + "line": 573, "macro_name": "println!" }, { - "line": 583, + "line": 598, "macro_name": "println!" }, { - "line": 677, + "line": 692, "macro_name": "eprintln!" }, { - "line": 681, + "line": 696, "macro_name": "eprintln!" }, { - "line": 698, + "line": 713, "macro_name": "eprintln!" }, { - "line": 713, + "line": 728, "macro_name": "eprintln!" }, { - "line": 728, + "line": 743, "macro_name": "eprintln!" } ], From 7f925f77486ab6c838344e10d05ad8c0a1bc42c1 Mon Sep 17 00:00:00 2001 From: Paul Hogan <5004d00b753726762516a66df75936687319095eacf0e9f7a4710f7a0ff12098@meshllm.communities.buzz.xyz> Date: Thu, 10 Sep 2026 06:20:51 +1000 Subject: [PATCH 02/22] fix(model-package): accept descriptor-only GGUF shards and stage packages on the bucket Two independent failures from the Batch 1 v2 republish runs: 1. read_gguf_catalog rejected any artifact whose aligned data_start lands beyond EOF. Split models legitimately ship a descriptor-only first shard (n_t=0, all model metadata, zero tensors) whose metadata table can end past the alignment boundary: unsloth/inkling-GGUF shard 1 ends at 12,986,167 bytes while 32-byte alignment rounds data_start to 12,986,176. Kimi-K3's equivalent shard passes only because its table end happens to be exactly 32-aligned. Descriptor-only shards now only require the metadata table itself to fit; payload-bearing artifacts keep the original bounds checks. 2. split-model-job.sh staged the package workspace under /tmp, which is capped by the HF Jobs per-container 50G ephemeral-storage limit (a kubelet eviction threshold, not disk size - the node had 1.3T free when the pod was evicted). The package must stay fully materialized until the final metadata-carrier pass, so PACKAGE_DIR now defaults to the bucket workspace (/bucket/job-work/...) and the job refuses to run with the package workspace on the container root filesystem. --- crates/model-package/src/script.rs | 8 ++-- .../src/scripts/split-model-job.sh | 14 +++++-- crates/skippy-model/src/gguf_catalog.rs | 41 +++++++++++++++++-- 3 files changed, 52 insertions(+), 11 deletions(-) diff --git a/crates/model-package/src/script.rs b/crates/model-package/src/script.rs index 91010c52a1..32aa459f62 100644 --- a/crates/model-package/src/script.rs +++ b/crates/model-package/src/script.rs @@ -165,11 +165,13 @@ 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}""#) + EMBEDDED_SCRIPT.contains(r#"PACKAGE_DIR="${PACKAGE_DIR:-${JOB_WORK_DIR}/package}""#) ); + assert!(EMBEDDED_SCRIPT.contains( + "package workspace is on the container root filesystem, which is capped at 50G" + )); + assert!(EMBEDDED_SCRIPT.contains(r#"HF_XET_CACHE="${HF_XET_CACHE:-${HF_HOME}/xet}""#)); 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!( diff --git a/crates/model-package/src/scripts/split-model-job.sh b/crates/model-package/src/scripts/split-model-job.sh index 63ac241961..c0f8dbabb2 100755 --- a/crates/model-package/src/scripts/split-model-job.sh +++ b/crates/model-package/src/scripts/split-model-job.sh @@ -41,8 +41,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 written to the bucket workspace: container-local +# ephemeral storage is capped (50G on HF Jobs) and the full package — the +# artifacts that still await the final metadata-carrier pass plus the +# per-artifact shard scratch — must be allowed to exceed it. 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}-$$}" @@ -52,7 +54,7 @@ if [ -z "${JOB_WORK_DIR:-}" ]; then else CLEANUP_JOB_WORK_DIR="${CLEANUP_JOB_WORK_DIR:-false}" fi -PACKAGE_DIR="${PACKAGE_DIR:-${LOCAL_WORK_DIR}/package}" +PACKAGE_DIR="${PACKAGE_DIR:-${JOB_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}" @@ -344,10 +346,14 @@ echo " Hugging Face cache: $HF_HUB_CACHE" echo " Package workspace: $PACKAGE_DIR" echo " Temporary workspace: $TMPDIR" log_storage_snapshot "before write-package" +# The package workspace must NOT sit on the container root filesystem: HF Jobs +# evicts the pod once container-local ephemeral storage exceeds 50G, and a full +# package plus shard scratch can far exceed that. 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 + echo "ERROR: package workspace is on the container root filesystem, which is capped at 50G of ephemeral storage; refusing to continue (set PACKAGE_DIR to the bucket workspace)." >&2 + exit 1 fi if [ -n "${ESTIMATED_BUCKET_BYTES:-}" ]; then PACKAGE_AVAILABLE_BYTES="$(df -Pk "$PACKAGE_DIR" | awk 'NR==2 {printf "%.0f", $4 * 1024}')" diff --git a/crates/skippy-model/src/gguf_catalog.rs b/crates/skippy-model/src/gguf_catalog.rs index 5086961497..75288f884d 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 { @@ -538,6 +549,28 @@ mod tests { assert_eq!(catalog.tensors.len(), 1); assert_eq!(catalog.tensors[0].name, "weight"); assert!(read_gguf_catalog(&path).is_err()); + 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(); } From adc389e922d83e8c27e6235cf9e7a6c9113074a2 Mon Sep 17 00:00:00 2001 From: Paul Hogan <5004d00b753726762516a66df75936687319095eacf0e9f7a4710f7a0ff12098@meshllm.communities.buzz.xyz> Date: Thu, 10 Sep 2026 06:35:50 +1000 Subject: [PATCH 03/22] fix(model-package): re-create the package workspace before the root-FS gate On the HF bucket FUSE mount, empty directories are not backed by an object and can vanish between the job's initial mkdir and the storage gate, so 'df -P $PACKAGE_DIR' failed under set -e and every retried job died at the gate before writing anything. Re-create the directory immediately before the check; write-package creates parents itself from there. --- crates/model-package/src/scripts/split-model-job.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/model-package/src/scripts/split-model-job.sh b/crates/model-package/src/scripts/split-model-job.sh index c0f8dbabb2..63ec6de0f5 100755 --- a/crates/model-package/src/scripts/split-model-job.sh +++ b/crates/model-package/src/scripts/split-model-job.sh @@ -348,7 +348,10 @@ echo " Temporary workspace: $TMPDIR" log_storage_snapshot "before write-package" # The package workspace must NOT sit on the container root filesystem: HF Jobs # evicts the pod once container-local ephemeral storage exceeds 50G, and a full -# package plus shard scratch can far exceed that. +# package plus shard scratch can far exceed that. Re-create the directory here: +# on the bucket FUSE mount, empty directories are not backed by an object and +# can disappear between the initial mkdir and this point. +mkdir -p "$PACKAGE_DIR" 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 From 35e393cb6494bd98f3043ce6ccb172d24cba9c10 Mon Sep 17 00:00:00 2001 From: Paul Hogan <5004d00b753726762516a66df75936687319095eacf0e9f7a4710f7a0ff12098@meshllm.communities.buzz.xyz> Date: Thu, 10 Sep 2026 13:35:31 +1000 Subject: [PATCH 04/22] fix(model-package): upload payload artifacts as they are verified The v2 writer emitted every payload artifact with no upload hook and ran the hook in one final pass, because the metadata carrier was built from the on-disk parts. A full package therefore accumulated in the package workspace before any upload: Kimi-K3, DeepSeek-V4-Flash-0731, and Inkling were all evicted by the HF Jobs 50G per-container ephemeral storage limit mid-write (the canary only survived at 18.8G). Each verified artifact is now uploaded and deleted immediately. The metadata carrier needs only the parts' descriptor tables and tensor locators, which live entirely in each part's header (bytes before its aligned data start), so the writer captures a header-only stub per artifact before the hook runs and builds shared/metadata.gguf from the stubs. Peak workspace usage drops from the full package to one artifact plus kilobytes of headers. Validated end-to-end on a real model: write-package with a copy-then-delete hook uploads every artifact, leaves only model-package.json locally, and the reconstructed package passes verify-package-v2 against the independent source (35 artifacts, 427 tensors). --- crates/skippy-model-package/src/package_v2.rs | 119 +++++++++++++----- 1 file changed, 89 insertions(+), 30 deletions(-) diff --git a/crates/skippy-model-package/src/package_v2.rs b/crates/skippy-model-package/src/package_v2.rs index ee49cfdf0c..ee42204b21 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, @@ -52,7 +53,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,6 +60,16 @@ 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 are uploaded (and locally deleted) as soon as they are + // verified, so the metadata carrier cannot be built from the full parts at + // 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( @@ -73,6 +83,21 @@ 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)?; + // The upload hook publishes each verified part immediately and deletes + // its local copy. If a hook left the part in place we still delete it: + // keeping it would re-accumulate the full package in the workspace and + // re-trigger the HF Jobs 50G ephemeral-storage eviction. + if artifact_hook.command.is_some() && path.exists() { + fs::remove_file(&path) + .with_context(|| format!("remove uploaded artifact {}", path.display()))?; + } progress.finish_step(&format!( "{} {}", artifact.path, @@ -100,7 +125,7 @@ pub(crate) fn write_package( &inventory, &manifest, &out_dir, - &no_hook, + &header_stubs, resume_existing_artifacts, )?; progress.finish_step(&format!( @@ -122,10 +147,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 upload hook publish (and delete) it. + 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)?; @@ -280,9 +318,9 @@ fn source_tensors_by_name(inventory: &SourceInventory) -> Result Result { let relative = "shared/metadata.gguf"; @@ -290,25 +328,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 +364,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) } @@ -449,6 +472,42 @@ fn emit_payload_artifact( Ok((artifact, bound)) } +/// Path of the header-only stub kept for artifact `id` while its full payload +/// is being uploaded and deleted. +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), From d28ff8f8a4b93aeb9b9fbd73c98e2a6561be0b9f Mon Sep 17 00:00:00 2001 From: Paul Hogan <5004d00b753726762516a66df75936687319095eacf0e9f7a4710f7a0ff12098@meshllm.communities.buzz.xyz> Date: Thu, 10 Sep 2026 13:45:16 +1000 Subject: [PATCH 05/22] chore(xtask): regenerate console print ratchet for package_v2 line moves --- tools/xtask/data/console_print_allowlist.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/xtask/data/console_print_allowlist.json b/tools/xtask/data/console_print_allowlist.json index 5776df5fb9..40dc1c3998 100644 --- a/tools/xtask/data/console_print_allowlist.json +++ b/tools/xtask/data/console_print_allowlist.json @@ -3767,7 +3767,7 @@ ], "crates/skippy-model-package/src/package_v2.rs": [ { - "line": 169, + "line": 207, "macro_name": "println!" } ], From 227f7699a42e9cccd283a9048770ebb6de10bf30 Mon Sep 17 00:00:00 2001 From: Paul Hogan <5004d00b753726762516a66df75936687319095eacf0e9f7a4710f7a0ff12098@meshllm.communities.buzz.xyz> Date: Thu, 10 Sep 2026 13:47:26 +1000 Subject: [PATCH 06/22] feat(queue): add --exclude-repo to quarantine broken sources MiniMax-H3's GGUF family (unsloth/qwen3vl_32b_minimax_h3-GGUF and the pruned variants) are raw tensor dumps with no general.architecture, block_count, or tokenizer metadata; packaging them cannot succeed until upstream fixes the files. --exclude-repo lets the queue skip named source repos explicitly instead of relying on the failed-job heuristic, and is repeatable for the whole quarantined family. --- .../src/bin/queue-unsloth-layer-packages.rs | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) 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 8a6f4b2ac3..e9d9c16a7c 100644 --- a/crates/model-package/src/bin/queue-unsloth-layer-packages.rs +++ b/crates/model-package/src/bin/queue-unsloth-layer-packages.rs @@ -41,6 +41,7 @@ struct Args { republish: bool, confirm: bool, dry_run: bool, + exclude_repos: Vec, } #[derive(Debug, Clone)] @@ -368,6 +369,7 @@ impl Args { republish: false, confirm: false, dry_run: true, + exclude_repos: Vec::new(), }; let mut iter = std::env::args().skip(1); @@ -416,6 +418,12 @@ 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; @@ -480,7 +488,8 @@ fn print_help() { --job-poll-seconds N\n\ --split-candidate-vram-gib GiB\n\ --no-catalog-direct\n\ - --republish (re-queue even if published/catalogued; replaces the existing package in place)" + --republish (re-queue even if published/catalogued; replaces the existing package in place)\n\ + --exclude-repo org/repo (skip a quarantined source repo; repeatable)" ); } @@ -688,6 +697,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); From 66d9b11b4e54adae63634572d3903a7f1c87c44b Mon Sep 17 00:00:00 2001 From: Paul Hogan <5004d00b753726762516a66df75936687319095eacf0e9f7a4710f7a0ff12098@meshllm.communities.buzz.xyz> Date: Thu, 10 Sep 2026 13:48:01 +1000 Subject: [PATCH 07/22] fix(queue): initialize exclude_repos in the job-plan test --- crates/model-package/src/bin/queue-unsloth-layer-packages.rs | 1 + 1 file changed, 1 insertion(+) 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 e9d9c16a7c..3137588217 100644 --- a/crates/model-package/src/bin/queue-unsloth-layer-packages.rs +++ b/crates/model-package/src/bin/queue-unsloth-layer-packages.rs @@ -1416,6 +1416,7 @@ mod tests { republish: false, confirm: true, dry_run: false, + exclude_repos: Vec::new(), }; let job_plan = CpuJobPlan { flavor: "cpu-upgrade".to_string(), From fff94de8b57cea68d7d2f76ff2d8dd8081c6d096 Mon Sep 17 00:00:00 2001 From: Paul Hogan <5004d00b753726762516a66df75936687319095eacf0e9f7a4710f7a0ff12098@meshllm.communities.buzz.xyz> Date: Thu, 10 Sep 2026 14:27:06 +1000 Subject: [PATCH 08/22] chore(xtask): regenerate console print ratchet after queue tool line moves --- tools/xtask/data/console_print_allowlist.json | 66 ++++++++++--------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/tools/xtask/data/console_print_allowlist.json b/tools/xtask/data/console_print_allowlist.json index 40dc1c3998..939e540813 100644 --- a/tools/xtask/data/console_print_allowlist.json +++ b/tools/xtask/data/console_print_allowlist.json @@ -3347,127 +3347,131 @@ ], "crates/model-package/src/bin/queue-unsloth-layer-packages.rs": [ { - "line": 155, + "line": 156, "macro_name": "println!" }, { - "line": 160, + "line": 161, "macro_name": "println!" }, { - "line": 161, + "line": 162, "macro_name": "println!" }, { - "line": 165, + "line": 166, "macro_name": "println!" }, { - "line": 169, + "line": 170, "macro_name": "println!" }, { - "line": 192, + "line": 193, "macro_name": "println!" }, { - "line": 212, + "line": 213, "macro_name": "println!" }, { - "line": 231, + "line": 232, "macro_name": "println!" }, { - "line": 238, + "line": 239, "macro_name": "println!" }, { - "line": 245, + "line": 246, "macro_name": "println!" }, { - "line": 252, + "line": 253, "macro_name": "println!" }, { - "line": 276, + "line": 277, "macro_name": "println!" }, { - "line": 310, + "line": 311, "macro_name": "println!" }, { - "line": 324, + "line": 325, "macro_name": "println!" }, { - "line": 333, + "line": 334, "macro_name": "println!" }, { - "line": 462, + "line": 470, "macro_name": "println!" }, { - "line": 491, + "line": 500, "macro_name": "eprintln!" }, { - "line": 499, + "line": 508, "macro_name": "eprintln!" }, { - "line": 503, + "line": 512, "macro_name": "eprintln!" }, { - "line": 507, + "line": 516, "macro_name": "eprintln!" }, { - "line": 525, + "line": 534, "macro_name": "println!" }, { - "line": 540, + "line": 549, "macro_name": "println!" }, { - "line": 560, + "line": 569, "macro_name": "eprintln!" }, { - "line": 564, + "line": 573, "macro_name": "eprintln!" }, { - "line": 573, + "line": 582, "macro_name": "println!" }, { - "line": 598, + "line": 607, "macro_name": "println!" }, { - "line": 692, + "line": 705, "macro_name": "eprintln!" }, { - "line": 696, + "line": 712, "macro_name": "eprintln!" }, { - "line": 713, + "line": 716, + "macro_name": "eprintln!" + }, + { + "line": 733, "macro_name": "eprintln!" }, { - "line": 728, + "line": 748, "macro_name": "eprintln!" }, { - "line": 743, + "line": 763, "macro_name": "eprintln!" } ], From dcbcf772b38d2f2dab36e6e581175c14b8d54d72 Mon Sep 17 00:00:00 2001 From: Paul Hogan Date: Thu, 10 Sep 2026 14:27:08 +1000 Subject: [PATCH 09/22] fix(queue): make --republish idempotent and extract the skip policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review fixes for PR #1718 (ndizazzo + CodeRabbit): - candidate_status() now checks the target repo's queue marker FIRST, so QueueStatus::Queued can never be shadowed by the cataloged/published/ failure returns. Previously a second --republish run re-submitted (and paid for) the same in-flight HF job, because the marker check was unreachable for exactly the statuses --republish bypasses. - Extracted the skip decision into should_skip(status, republish): Queued always skips (even under --republish — an in-flight job already covers the candidate); Published/Cataloged/Failed re-queue only with --republish; Missing/StaleQueued always queue. Replaces the empty then-branch if/else. - write_queue_marker() now deletes the stale automation/failure.json when re-queueing under --republish so the repo doesn't carry contradictory markers. - Tests: double-submit regression (two consecutive --republish skips the second), skip policy matrix, queue-marker freshness via last_modified. Note: CodeRabbit's outside-diff claim that allowlist entries landed under the wrong path key was verified as a false positive (per ndizazzo) and not chased. --- .../src/bin/queue-unsloth-layer-packages.rs | 268 ++++++++++++++---- 1 file changed, 205 insertions(+), 63 deletions(-) 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 3137588217..e533c79727 100644 --- a/crates/model-package/src/bin/queue-unsloth-layer-packages.rs +++ b/crates/model-package/src/bin/queue-unsloth-layer-packages.rs @@ -217,47 +217,14 @@ async fn main() -> Result<()> { continue; } let status = candidate_status(&hf_client, &candidate, args.retry_queued_after).await?; - if args.republish - && matches!( - status, - QueueStatus::Published { .. } - | QueueStatus::Cataloged { .. } - | QueueStatus::Failed { .. } - ) - { - // Replacement policy: republish v2 over the existing package in place. - } else { - 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); @@ -832,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 }); } @@ -842,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") @@ -859,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() @@ -1001,9 +1029,36 @@ 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(err) => Err(err) + .with_context(|| format!("delete queue failure marker from {}", candidate.target_repo)), + } +} + async fn write_queue_failure_marker( client: &HFClient, submitted: &SubmittedJob, @@ -1334,14 +1389,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"); From 6111692b2505c62c1f5ba3b7c4e57f832f47a8e3 Mon Sep 17 00:00:00 2001 From: Paul Hogan <5004d00b753726762516a66df75936687319095eacf0e9f7a4710f7a0ff12098@meshllm.communities.buzz.xyz> Date: Thu, 10 Sep 2026 14:47:04 +1000 Subject: [PATCH 10/22] chore(xtask): regenerate console print ratchet for queue tool line moves d93d3333d moved lines in queue-unsloth-layer-packages.rs without a ratchet regen; CI contracts and consistency failed on stale line pins. --- tools/xtask/data/console_print_allowlist.json | 56 ++++++++----------- 1 file changed, 22 insertions(+), 34 deletions(-) diff --git a/tools/xtask/data/console_print_allowlist.json b/tools/xtask/data/console_print_allowlist.json index 939e540813..c4b1eaa18c 100644 --- a/tools/xtask/data/console_print_allowlist.json +++ b/tools/xtask/data/console_print_allowlist.json @@ -3375,103 +3375,91 @@ "macro_name": "println!" }, { - "line": 232, - "macro_name": "println!" - }, - { - "line": 239, - "macro_name": "println!" - }, - { - "line": 246, - "macro_name": "println!" - }, - { - "line": 253, + "line": 226, "macro_name": "println!" }, { - "line": 277, + "line": 244, "macro_name": "println!" }, { - "line": 311, + "line": 278, "macro_name": "println!" }, { - "line": 325, + "line": 292, "macro_name": "println!" }, { - "line": 334, + "line": 301, "macro_name": "println!" }, { - "line": 470, + "line": 437, "macro_name": "println!" }, { - "line": 500, + "line": 467, "macro_name": "eprintln!" }, { - "line": 508, + "line": 475, "macro_name": "eprintln!" }, { - "line": 512, + "line": 479, "macro_name": "eprintln!" }, { - "line": 516, + "line": 483, "macro_name": "eprintln!" }, { - "line": 534, + "line": 501, "macro_name": "println!" }, { - "line": 549, + "line": 516, "macro_name": "println!" }, { - "line": 569, + "line": 536, "macro_name": "eprintln!" }, { - "line": 573, + "line": 540, "macro_name": "eprintln!" }, { - "line": 582, + "line": 549, "macro_name": "println!" }, { - "line": 607, + "line": 574, "macro_name": "println!" }, { - "line": 705, + "line": 672, "macro_name": "eprintln!" }, { - "line": 712, + "line": 679, "macro_name": "eprintln!" }, { - "line": 716, + "line": 683, "macro_name": "eprintln!" }, { - "line": 733, + "line": 700, "macro_name": "eprintln!" }, { - "line": 748, + "line": 715, "macro_name": "eprintln!" }, { - "line": 763, + "line": 730, "macro_name": "eprintln!" } ], From 8734d9f117109e0248d6a2d4fa9ecdf511d9421d Mon Sep 17 00:00:00 2001 From: Paul Hogan <5004d00b753726762516a66df75936687319095eacf0e9f7a4710f7a0ff12098@meshllm.communities.buzz.xyz> Date: Thu, 10 Sep 2026 15:14:45 +1000 Subject: [PATCH 11/22] fix(queue): tolerate missing target repo when clearing failure markers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit delete_queue_failure_marker only tolerated file-level 404s; a candidate whose target repo was never created (no run has completed yet) fails the re-queue path with RepoNotFound before the job is submitted. A missing repo has nothing to delete — treat it as success. --- crates/model-package/src/bin/queue-unsloth-layer-packages.rs | 1 + 1 file changed, 1 insertion(+) 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 e533c79727..9f7703de78 100644 --- a/crates/model-package/src/bin/queue-unsloth-layer-packages.rs +++ b/crates/model-package/src/bin/queue-unsloth-layer-packages.rs @@ -1054,6 +1054,7 @@ async fn delete_queue_failure_marker(client: &HFClient, candidate: &Candidate) - { 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)), } From f715cdbdf597d6cae0f4addf7eba35ca97476748 Mon Sep 17 00:00:00 2001 From: Paul Hogan <5004d00b753726762516a66df75936687319095eacf0e9f7a4710f7a0ff12098@meshllm.communities.buzz.xyz> Date: Thu, 10 Sep 2026 15:48:11 +1000 Subject: [PATCH 12/22] fix(model-package): ride out sustained Xet upload outages with 8-attempt exponential backoff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two production jobs died when a sustained HF Xet I/O error (os error 5) outlasted the upload hook's 4 attempts x <=60s backoff (V4-Flash layer-00010, Inkling layer-00008; Inkling had already recovered from an identical single-attempt blip earlier in the same run). Raise the default to 8 attempts with exponential backoff capped at 300s — ~22 minutes of total retry window per artifact, inside the 12h job budget. --- crates/model-package/src/scripts/split-model-job.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/model-package/src/scripts/split-model-job.sh b/crates/model-package/src/scripts/split-model-job.sh index 63ec6de0f5..61f5ab1994 100755 --- a/crates/model-package/src/scripts/split-model-job.sh +++ b/crates/model-package/src/scripts/split-model-job.sh @@ -246,7 +246,7 @@ 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")) +max_attempts = int(os.environ.get("ARTIFACT_UPLOAD_ATTEMPTS", "8")) api = HfApi(token=os.environ["HF_TOKEN"]) last_error = None @@ -265,7 +265,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...", From 6e0f0ed2c4aa9e3ae7fd1ca29cb4cec039a0e111 Mon Sep 17 00:00:00 2001 From: Paul Hogan <5004d00b753726762516a66df75936687319095eacf0e9f7a4710f7a0ff12098@meshllm.communities.buzz.xyz> Date: Thu, 10 Sep 2026 16:10:05 +1000 Subject: [PATCH 13/22] fix(skippy-model-package): treat hook-deleted artifacts as unchanged on FUSE mounts Kimi-K3 job 6aa229b42 died after uploading layer-00001 (16.4 GB): verify_hook_result checked path.exists() and then hashed the file, but the /bucket FUSE mount kept reporting the freshly unlinked artifact as present via a stale attr cache, so file_sha256 failed with ENOENT and took the whole job down ~100 layers before completion. An upload hook removing the artifact is the expected outcome of this design, so 'can no longer be opened' now reads as unchanged; only a file that opens but differs from its record (hook mutated it) fails. Applies uniformly to payload artifacts, the metadata carrier, and projectors. Regression test covers present/mutated/deleted. --- crates/skippy-model-package/src/package_v2.rs | 33 +++++++++++++-- .../src/package_v2/tests.rs | 42 +++++++++++++++++++ 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/crates/skippy-model-package/src/package_v2.rs b/crates/skippy-model-package/src/package_v2.rs index ee42204b21..05d68be495 100644 --- a/crates/skippy-model-package/src/package_v2.rs +++ b/crates/skippy-model-package/src/package_v2.rs @@ -175,7 +175,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" ); } @@ -554,8 +554,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 ); @@ -563,6 +562,34 @@ fn verify_hook_result(artifact: &Artifact, path: &Path, hook: &ArtifactHook) -> Ok(()) } +/// Whether the on-disk artifact still matches its record. +/// +/// An upload hook removing the artifact is the expected outcome, and 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/tests.rs b/crates/skippy-model-package/src/package_v2/tests.rs index aba19f4f0c..645b95307b 100644 --- a/crates/skippy-model-package/src/package_v2/tests.rs +++ b/crates/skippy-model-package/src/package_v2/tests.rs @@ -462,3 +462,45 @@ fn upload_hook_can_delete_verified_copies_without_losing_inventory() { ); assert!(source.exists()); } + +#[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(); +} From f2446f65a5a9e487ecc32e1faf7cb16cea958bd9 Mon Sep 17 00:00:00 2001 From: Paul Hogan <5004d00b753726762516a66df75936687319095eacf0e9f7a4710f7a0ff12098@meshllm.communities.buzz.xyz> Date: Thu, 10 Sep 2026 16:14:17 +1000 Subject: [PATCH 14/22] fix(model-package): retry raw-SHA fetch in split job Two Inkling jobs died 90 seconds in with 'upload-pack: not our ref' (exit 128): GitHub's ref advertisement for allow-any-SHA fetches lags freshly-pushed commits, and MESH_LLM_REF is always a raw branch-tip SHA in this pipeline. fetch_ref_with_retry polls up to 10 x 60s before giving up. --- .../src/scripts/split-model-job.sh | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/crates/model-package/src/scripts/split-model-job.sh b/crates/model-package/src/scripts/split-model-job.sh index 61f5ab1994..352a1996c7 100755 --- a/crates/model-package/src/scripts/split-model-job.sh +++ b/crates/model-package/src/scripts/split-model-job.sh @@ -176,17 +176,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 From cf02a6fabc244b7d4d6a2f59dee8786d951e7822 Mon Sep 17 00:00:00 2001 From: Paul Hogan <5004d00b753726762516a66df75936687319095eacf0e9f7a4710f7a0ff12098@meshllm.communities.buzz.xyz> Date: Thu, 10 Sep 2026 19:16:48 +1000 Subject: [PATCH 15/22] fix(model-package): stage package artifacts and Xet cache on local SSD Three hardened split jobs died on sustained Xet upload I/O errors (os error 5) while re-reading artifacts through the writable /bucket FUSE mount: Inkling at layer 5, V4-Flash at layer 13, Kimi stuck at layer 1. Xet is healthy from outside HF Jobs (2.15GB probe at 146-172MB/s), so the failures are specific to reading freshly-written files back through the FUSE mount in the job container. Default the package workspace and the Xet chunk cache to the local work dir and refuse a non-local PACKAGE_DIR unless PACKAGE_DIR_ALLOW_BUCKET is set. The per-artifact upload+delete interleave keeps peak local usage at one artifact plus its shard scratch, which fits the 50G ephemeral cap; the bucket is still used for the script, source-cache fallback, and job-work bookkeeping. The capacity warning now checks /bucket free space for the fallback path instead of the package dir. --- crates/model-package/src/script.rs | 9 ++-- .../src/scripts/split-model-job.sh | 52 ++++++++++++------- 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/crates/model-package/src/script.rs b/crates/model-package/src/script.rs index 32aa459f62..75b6ab64a6 100644 --- a/crates/model-package/src/script.rs +++ b/crates/model-package/src/script.rs @@ -166,12 +166,15 @@ mod tests { assert!(EMBEDDED_SCRIPT.contains("Estimated fallback /bucket cache needed")); assert!(EMBEDDED_SCRIPT.contains("estimate_bucket_workspace_bytes")); assert!( - EMBEDDED_SCRIPT.contains(r#"PACKAGE_DIR="${PACKAGE_DIR:-${JOB_WORK_DIR}/package}""#) + EMBEDDED_SCRIPT.contains(r#"PACKAGE_DIR="${PACKAGE_DIR:-${LOCAL_WORK_DIR}/package}""#) ); assert!(EMBEDDED_SCRIPT.contains( - "package workspace is on the container root filesystem, which is capped at 50G" + "refusing to continue (unset PACKAGE_DIR_ALLOW_BUCKET to require local staging)" )); - assert!(EMBEDDED_SCRIPT.contains(r#"HF_XET_CACHE="${HF_XET_CACHE:-${HF_HOME}/xet}""#)); + assert!( + EMBEDDED_SCRIPT + .contains(r#"HF_XET_CACHE="${HF_XET_CACHE:-${LOCAL_WORK_DIR}/xet-cache}""#) + ); 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!( diff --git a/crates/model-package/src/scripts/split-model-job.sh b/crates/model-package/src/scripts/split-model-job.sh index 352a1996c7..c38d836c89 100755 --- a/crates/model-package/src/scripts/split-model-job.sh +++ b/crates/model-package/src/scripts/split-model-job.sh @@ -41,10 +41,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 written to the bucket workspace: container-local -# ephemeral storage is capped (50G on HF Jobs) and the full package — the -# artifacts that still await the final metadata-carrier pass plus the -# per-artifact shard scratch — must be allowed to exceed it. +# 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}-$$}" @@ -54,10 +54,15 @@ if [ -z "${JOB_WORK_DIR:-}" ]; then else CLEANUP_JOB_WORK_DIR="${CLEANUP_JOB_WORK_DIR:-false}" fi -PACKAGE_DIR="${PACKAGE_DIR:-${JOB_WORK_DIR}/package}" +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}" +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}" @@ -363,23 +368,34 @@ echo " Hugging Face cache: $HF_HUB_CACHE" echo " Package workspace: $PACKAGE_DIR" echo " Temporary workspace: $TMPDIR" log_storage_snapshot "before write-package" -# The package workspace must NOT sit on the container root filesystem: HF Jobs -# evicts the pod once container-local ephemeral storage exceeds 50G, and a full -# package plus shard scratch can far exceed that. Re-create the directory here: -# on the bucket FUSE mount, empty directories are not backed by an object and -# can disappear between the initial mkdir and this point. +# 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" -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 is on the container root filesystem, which is capped at 50G of ephemeral storage; refusing to continue (set PACKAGE_DIR to the bucket workspace)." >&2 - exit 1 +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)" From 1a28147007e5cd10192c1a560760a2a007795f8b Mon Sep 17 00:00:00 2001 From: Paul Hogan <5004d00b753726762516a66df75936687319095eacf0e9f7a4710f7a0ff12098@meshllm.communities.buzz.xyz> Date: Fri, 11 Sep 2026 08:23:50 +1000 Subject: [PATCH 16/22] fix(model-package): default split jobs to the HTTP upload path With package artifacts and the Xet chunk cache already on local SSD, split jobs still hit sustained I/O errors (os error 5) on the Xet-CAS channel from inside HF Jobs containers; the same uploads run clean from outside the cluster. Per-layer GGUF artifacts gain nothing from chunk deduplication, so default the job environment to HF_HUB_DISABLE_XET=1 and upload through the classic HTTP path, which uses different egress. Set HF_HUB_DISABLE_XET=0 to restore the Xet uploader. --- crates/model-package/src/script.rs | 1 + crates/model-package/src/scripts/split-model-job.sh | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/model-package/src/script.rs b/crates/model-package/src/script.rs index 75b6ab64a6..0d6ef3573a 100644 --- a/crates/model-package/src/script.rs +++ b/crates/model-package/src/script.rs @@ -175,6 +175,7 @@ mod tests { 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!( diff --git a/crates/model-package/src/scripts/split-model-job.sh b/crates/model-package/src/scripts/split-model-job.sh index c38d836c89..179c74752c 100755 --- a/crates/model-package/src/scripts/split-model-job.sh +++ b/crates/model-package/src/scripts/split-model-job.sh @@ -62,6 +62,12 @@ HF_HUB_CACHE="${HF_HUB_CACHE:-${HF_HOME}/hub}" # 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}" @@ -78,7 +84,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() { From 290c0ec9393e6b5d7b1b75f1d73c53bb2d0ff947 Mon Sep 17 00:00:00 2001 From: Paul Hogan <5004d00b753726762516a66df75936687319095eacf0e9f7a4710f7a0ff12098@meshllm.communities.buzz.xyz> Date: Fri, 11 Sep 2026 18:05:59 +1000 Subject: [PATCH 17/22] fix(skippy): consume merge parts to fit HF Jobs ephemeral budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Kimi-K3's sharded split (32 shards) stages one per-shard part per layer (~16G) then merges them into the layer GGUF while every part is still on disk — a ~33G staging peak per layer that tipped the HF Jobs 50G container-local ephemeral-storage eviction at layer 9 (job 6aa368302, uploads themselves were clean). Add llama patch 0024: skippy_write_gguf_from_parts_consuming, an opt-in materializing variant that unlinks each input part as soon as its tensors are absorbed into the output (sources are opened metadata-only, so nothing pins the file). The default from-parts writer is byte-identical for bench materialization and metadata carriers. ABI patch 53->54 paired in skippy-ffi. The sharded stage writer now uses the consuming variant, so peak staging drops to ~1x layer size. --- crates/skippy-ffi/src/dynamic.rs | 1 + crates/skippy-ffi/src/lib.rs | 6 +- crates/skippy-ffi/src/static_bindings.rs | 7 + crates/skippy-model-package/src/write.rs | 4 +- crates/skippy-runtime/src/gguf_writer.rs | 44 ++++- crates/skippy-runtime/src/lib.rs | 3 +- ...ts-during-from-parts-materialization.patch | 165 ++++++++++++++++++ 7 files changed, 219 insertions(+), 11 deletions(-) create mode 100644 third_party/llama.cpp/patches/0024-skippy-consume-merge-parts-during-from-parts-materialization.patch 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 58ee6bf20c..cdd8435c97 100644 --- a/crates/skippy-ffi/src/lib.rs +++ b/crates/skippy-ffi/src/lib.rs @@ -130,7 +130,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")] @@ -180,5 +181,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/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-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 d90966144d..ac19c1399b 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/third_party/llama.cpp/patches/0024-skippy-consume-merge-parts-during-from-parts-materialization.patch b/third_party/llama.cpp/patches/0024-skippy-consume-merge-parts-during-from-parts-materialization.patch new file mode 100644 index 0000000000..15c13195a9 --- /dev/null +++ b/third_party/llama.cpp/patches/0024-skippy-consume-merge-parts-during-from-parts-materialization.patch @@ -0,0 +1,165 @@ +From d5c31ea86b0f114b78930edfba5ea5d93b127255 Mon Sep 17 00:00:00 2001 +From: Mesh-LLM CI +Date: Fri, 11 Sep 2026 17:33:13 +1000 +Subject: [PATCH 24/24] 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 53->54. +--- + include/skippy/common.h | 2 +- + include/skippy/model_package.h | 11 +++++++ + src/skippy/model_package.cpp | 53 ++++++++++++++++++++++++++++++++-- + 3 files changed, 62 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 53 ++#define SKIPPY_ABI_VERSION_PATCH 54 + + #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,17 @@ LLAMA_API enum skippy_status skippy_write_gguf_from_parts( + const char * output_path, + struct skippy_error ** out_error); + ++// Materializing variant that unlinks each input part file as soon as its ++// tensors have been absorbed into the output. Use from staging pipelines whose ++// parts are scratch files (for example the HF Jobs sharded split writer, where ++// parts and output coexisting would double local disk usage); never use it on ++// 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) + From 96f35bf1edc4474642e7ed94da57069fc14cdfb7 Mon Sep 17 00:00:00 2001 From: Paul Hogan <5004d00b753726762516a66df75936687319095eacf0e9f7a4710f7a0ff12098@meshllm.communities.buzz.xyz> Date: Sun, 13 Sep 2026 08:49:13 +1000 Subject: [PATCH 18/22] fix(skippy): drop zero-element tensors from native inventory cross-check Mirrors the #1807 catalog-reader change: the reader skips zero-dim placeholder tensors (e.g. Unsloth diffusion __index_timestep_zero__), so the native-vs-GGUF count comparison in source inspection and direct planning must filter zero-element native tensors too. Without this, Qwen-Image-Edit-2511 passes the source read and dies at write-package with 'native and GGUF tensor inventories disagree' (1934 vs 1933). --- .../mesh-llm-host-runtime/src/inference/skippy/package.rs | 7 ++++++- crates/skippy-model-package/src/source_inventory.rs | 8 +++++++- 2 files changed, 13 insertions(+), 2 deletions(-) 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 438de395cf..abe7252a1f 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/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)) } From 509155aa242021e2358949e8e01d786459361f9f Mon Sep 17 00:00:00 2001 From: scama Date: Sun, 13 Sep 2026 09:49:03 +1000 Subject: [PATCH 19/22] fix(package): publish replacements atomically --- .../src/bin/queue-unsloth-layer-packages.rs | 17 ++- crates/model-package/src/script.rs | 3 + .../src/scripts/split-model-job.sh | 35 +++++- crates/skippy-model-package/src/package_v2.rs | 33 ++---- .../src/package_v2/tests.rs | 30 +++++ scripts/promote_layer_package_snapshot.py | 106 ++++++++++++++++++ .../test_promote_layer_package_snapshot.py | 91 +++++++++++++++ ...ts-during-from-parts-materialization.patch | 20 ++-- tools/xtask/data/console_print_allowlist.json | 2 +- 9 files changed, 303 insertions(+), 34 deletions(-) create mode 100644 scripts/promote_layer_package_snapshot.py create mode 100644 scripts/tests/test_promote_layer_package_snapshot.py 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 9f7703de78..890afe5db9 100644 --- a/crates/model-package/src/bin/queue-unsloth-layer-packages.rs +++ b/crates/model-package/src/bin/queue-unsloth-layer-packages.rs @@ -1157,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()); @@ -1538,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, @@ -1594,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") @@ -1611,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 0d6ef3573a..9fdf6c63a9 100644 --- a/crates/model-package/src/script.rs +++ b/crates/model-package/src/script.rs @@ -211,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 179c74752c..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##*:}" @@ -75,6 +77,7 @@ 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}" @@ -243,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 / @@ -265,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 @@ -274,6 +292,7 @@ import time path = Path(os.environ["SKIPPY_PACKAGE_ARTIFACT_PATH"]) relative = os.environ["SKIPPY_PACKAGE_ARTIFACT_RELATIVE_PATH"] target_repo = os.environ["TARGET_REPO"] +target_revision = os.environ["TARGET_UPLOAD_REVISION"] max_attempts = int(os.environ.get("ARTIFACT_UPLOAD_ATTEMPTS", "8")) api = HfApi(token=os.environ["HF_TOKEN"]) @@ -285,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 @@ -466,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' @@ -475,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-model-package/src/package_v2.rs b/crates/skippy-model-package/src/package_v2.rs index 05d68be495..c270bceed4 100644 --- a/crates/skippy-model-package/src/package_v2.rs +++ b/crates/skippy-model-package/src/package_v2.rs @@ -61,12 +61,12 @@ pub(crate) fn write_package( .unwrap_or_default(); let mut catalog = Vec::with_capacity(source_tensors.len()); let no_hook = ArtifactHook { command: None }; - // Payload artifacts are uploaded (and locally deleted) as soon as they are - // verified, so the metadata carrier cannot be built from the full parts at - // 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. + // 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()); @@ -90,14 +90,6 @@ pub(crate) fn write_package( header_stubs.push(header); run_artifact_hook(&artifact_hook, &path, &artifact.path)?; verify_hook_result(&artifact, &path, &artifact_hook)?; - // The upload hook publishes each verified part immediately and deletes - // its local copy. If a hook left the part in place we still delete it: - // keeping it would re-accumulate the full package in the workspace and - // re-trigger the HF Jobs 50G ephemeral-storage eviction. - if artifact_hook.command.is_some() && path.exists() { - fs::remove_file(&path) - .with_context(|| format!("remove uploaded artifact {}", path.display()))?; - } progress.finish_step(&format!( "{} {}", artifact.path, @@ -148,7 +140,7 @@ pub(crate) fn write_package( "metadata carrier tensor inventory differs from the independently verified payloads" ); // The carrier is fully verified against the manifest while it is still on - // disk; only then does the upload hook publish (and delete) it. + // 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( @@ -473,7 +465,7 @@ fn emit_payload_artifact( } /// Path of the header-only stub kept for artifact `id` while its full payload -/// is being uploaded and deleted. +/// 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() @@ -564,11 +556,10 @@ fn verify_hook_result(artifact: &Artifact, path: &Path, hook: &ArtifactHook) -> /// Whether the on-disk artifact still matches its record. /// -/// An upload hook removing the artifact is the expected outcome, and 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. +/// 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 { diff --git a/crates/skippy-model-package/src/package_v2/tests.rs b/crates/skippy-model-package/src/package_v2/tests.rs index 645b95307b..ffb836dd87 100644 --- a/crates/skippy-model-package/src/package_v2/tests.rs +++ b/crates/skippy-model-package/src/package_v2/tests.rs @@ -463,6 +463,36 @@ 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, + ) + .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; 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/third_party/llama.cpp/patches/0024-skippy-consume-merge-parts-during-from-parts-materialization.patch b/third_party/llama.cpp/patches/0024-skippy-consume-merge-parts-during-from-parts-materialization.patch index 15c13195a9..8f481eec8b 100644 --- a/third_party/llama.cpp/patches/0024-skippy-consume-merge-parts-during-from-parts-materialization.patch +++ b/third_party/llama.cpp/patches/0024-skippy-consume-merge-parts-during-from-parts-materialization.patch @@ -15,9 +15,9 @@ behavior for bench materialization and metadata carriers. Bump SKIPPY_ABI_VERSION_PATCH 53->54. --- include/skippy/common.h | 2 +- - include/skippy/model_package.h | 11 +++++++ + include/skippy/model_package.h | 14 ++++++++++ src/skippy/model_package.cpp | 53 ++++++++++++++++++++++++++++++++-- - 3 files changed, 62 insertions(+), 4 deletions(-) + 3 files changed, 65 insertions(+), 4 deletions(-) diff --git a/include/skippy/common.h b/include/skippy/common.h index 65cfa2ce8..5853db327 100644 @@ -36,15 +36,18 @@ 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,17 @@ LLAMA_API enum skippy_status skippy_write_gguf_from_parts( +@@ -103,6 +103,20 @@ LLAMA_API enum skippy_status skippy_write_gguf_from_parts( const char * output_path, struct skippy_error ** out_error); -+// Materializing variant that unlinks each input part file as soon as its -+// tensors have been absorbed into the output. Use from staging pipelines whose -+// parts are scratch files (for example the HF Jobs sharded split writer, where -+// parts and output coexisting would double local disk usage); never use it on -+// published package files that must survive. ++/** ++ * @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, @@ -162,4 +165,3 @@ index 6df18689e..acd468ef1 100644 +} -- 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 c4b1eaa18c..e9ce2360e0 100644 --- a/tools/xtask/data/console_print_allowlist.json +++ b/tools/xtask/data/console_print_allowlist.json @@ -3759,7 +3759,7 @@ ], "crates/skippy-model-package/src/package_v2.rs": [ { - "line": 207, + "line": 199, "macro_name": "println!" } ], From bd36e5573d52714c74c9fd132315dc4df7bb4280 Mon Sep 17 00:00:00 2001 From: Paul Hogan <5004d00b753726762516a66df75936687319095eacf0e9f7a4710f7a0ff12098@meshllm.communities.buzz.xyz> Date: Sun, 13 Sep 2026 13:37:46 +1000 Subject: [PATCH 20/22] feat(package): split oversized layers into byte-balanced part artifacts HF Jobs evicts split-package pods above 50G of container-local ephemeral storage; DeepSeek-V4-Pro-0813 died deterministically writing layer-00000 (12.9 GiB single layer) because the v2 planner had no per-artifact byte budget and the sharded writer needs ~2x transient for parts+merge. Plan and write oversized layers as byte-balanced part artifacts instead: - layout: plan_artifacts_with_budget subdivides groups above max_artifact_bytes (default 8 GiB) using the proven byte_balanced_split_boundaries algorithm; unsplit layers keep the legacy layers/layer-N.gguf path, split layers emit layers/layer-N-partMM.gguf with -partNN ids. A single tensor larger than the budget stays whole - it is the smallest indivisible unit. - part_writer: new byte-preserving Rust GGUF writer that streams tensor payloads directly from the source shards at exact catalog offsets - no native ABI call, no parts+merge staging, no 2x transient copy. Source metadata KV entries are copied as raw wire bytes (llama.cpp type-checks general.alignment as u32; a serde_json round trip widens it to u64 and the native reader rejects the file). - package_v2: --max-artifact-bytes CLI, part-aware emit path, and physical-content check (parts hold exactly their planned tensors; native stage slices duplicate common tensors, parts do not). - verify_v2/package_carrier: layer-ordinal parsers accept -partNN paths. - Manifest validation, stage admission, materialization and carrier resolution bind tensors by tensor-id -> (artifact, offset, length) and are artifact-grouping-agnostic, so splitting a layer across artifacts needs no loader or native changes. Verified end to end by tests: tiny-budget package splits a 3-tensor layer into 3 parts, all pass manifest validation and verify_v2 against the independent source. --- crates/skippy-model-package/src/cli.rs | 4 + crates/skippy-model-package/src/main.rs | 3 + crates/skippy-model-package/src/package_v2.rs | 40 +- .../src/package_v2/layout.rs | 136 ++++++- .../src/package_v2/layout/tests.rs | 78 ++++ .../src/package_v2/tests.rs | 66 ++++ .../skippy-model-package/src/part_writer.rs | 363 ++++++++++++++++++ .../src/part_writer/tests.rs | 95 +++++ crates/skippy-model-package/src/preflight.rs | 1 + crates/skippy-model-package/src/verify_v2.rs | 10 +- .../src/verify_v2/tests.rs | 1 + crates/skippy-model/src/package_carrier.rs | 13 +- tools/xtask/data/console_print_allowlist.json | 12 +- 13 files changed, 789 insertions(+), 33 deletions(-) create mode 100644 crates/skippy-model-package/src/part_writer.rs create mode 100644 crates/skippy-model-package/src/part_writer/tests.rs 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 c270bceed4..843e1ad6a3 100644 --- a/crates/skippy-model-package/src/package_v2.rs +++ b/crates/skippy-model-package/src/package_v2.rs @@ -26,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, @@ -36,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(), @@ -45,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!( @@ -75,6 +78,7 @@ pub(crate) fn write_package( let (artifact, mut tensors) = emit_payload_artifact( &source, &source_tensors, + &inventory, artifact_plan, &common_names, inventory.layer_count, @@ -380,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, @@ -391,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, @@ -418,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() 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 ffb836dd87..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 = @@ -480,6 +484,7 @@ fn successful_artifact_hook_may_leave_verified_copies_for_rechecking() { ArtifactHook { command: None }, explicit(&source), false, + None, ) .unwrap(); let manifest = read_manifest(&out); @@ -534,3 +539,64 @@ fn hook_verification_treats_deleted_artifact_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/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/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/tools/xtask/data/console_print_allowlist.json b/tools/xtask/data/console_print_allowlist.json index e9ce2360e0..be80bc27b8 100644 --- a/tools/xtask/data/console_print_allowlist.json +++ b/tools/xtask/data/console_print_allowlist.json @@ -3737,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": 199, + "line": 203, "macro_name": "println!" } ], From 7209cb5645a3762e4992008d3c751e13a317428c Mon Sep 17 00:00:00 2001 From: scama Date: Sun, 13 Sep 2026 14:40:36 +1000 Subject: [PATCH 21/22] fix(ci): authorize runtime gate model cadences --- scripts/tests/test_runtime_events_native_gate.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scripts/tests/test_runtime_events_native_gate.py b/scripts/tests/test_runtime_events_native_gate.py index f08ef81c3e..02cba91616 100644 --- a/scripts/tests/test_runtime_events_native_gate.py +++ b/scripts/tests/test_runtime_events_native_gate.py @@ -343,6 +343,13 @@ def test_family_certification_has_one_complete_model_list(self) -> None: manifest = json.loads((ROOT / "ci/llama-canary/family-certified.json").read_text()) self.assertEqual(83, len(manifest["models"])) self.assertTrue(all("cadences" not in model for model in manifest["models"])) + 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 From 4091dad9553b621595c3266892397f27cc743697 Mon Sep 17 00:00:00 2001 From: scama Date: Sun, 13 Sep 2026 14:58:14 +1000 Subject: [PATCH 22/22] fix(ci): share runtime event evidence path --- crates/skippy-model/src/gguf_catalog.rs | 5 +++++ scripts/ci-runtime-events-native-gate.sh | 6 +++++ .../tests/test_runtime_events_native_gate.py | 22 ++++++++++++++++++- 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/crates/skippy-model/src/gguf_catalog.rs b/crates/skippy-model/src/gguf_catalog.rs index 75288f884d..fab2514b00 100644 --- a/crates/skippy-model/src/gguf_catalog.rs +++ b/crates/skippy-model/src/gguf_catalog.rs @@ -549,6 +549,11 @@ mod tests { assert_eq!(catalog.tensors.len(), 1); assert_eq!(catalog.tensors[0].name, "weight"); assert!(read_gguf_catalog(&path).is_err()); + + 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(); 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/tests/test_runtime_events_native_gate.py b/scripts/tests/test_runtime_events_native_gate.py index 02cba91616..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. @@ -343,6 +360,9 @@ def test_family_certification_has_one_complete_model_list(self) -> None: manifest = json.loads((ROOT / "ci/llama-canary/family-certified.json").read_text()) 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