From 8398e74931e9a6a28a211f49ea0d2e587150f615 Mon Sep 17 00:00:00 2001 From: Mengsheng Wu Date: Wed, 29 Jul 2026 14:01:01 +0800 Subject: [PATCH 01/14] refactor(sdk): drop default-quant fallback from model-manager core Signed-off-by: Mengsheng Wu --- sdk/model-manager/crates/core/src/error.rs | 3 - sdk/model-manager/crates/core/src/lib.rs | 6 +- sdk/model-manager/crates/core/src/logging.rs | 18 -- sdk/model-manager/crates/core/src/manifest.rs | 6 - .../crates/core/src/manifest_builder.rs | 158 ++++------------ sdk/model-manager/crates/core/src/paths.rs | 179 ------------------ sdk/model-manager/crates/core/src/query.rs | 52 ++--- sdk/model-manager/crates/core/src/store.rs | 74 +++++++- .../crates/core/tests/pull_resume.rs | 2 +- 9 files changed, 143 insertions(+), 355 deletions(-) delete mode 100644 sdk/model-manager/crates/core/src/paths.rs diff --git a/sdk/model-manager/crates/core/src/error.rs b/sdk/model-manager/crates/core/src/error.rs index 00acf01aa..b2c4bea1b 100644 --- a/sdk/model-manager/crates/core/src/error.rs +++ b/sdk/model-manager/crates/core/src/error.rs @@ -25,9 +25,6 @@ pub enum Error { #[error("quantization '{0}' exists but is not downloaded for model '{1}'")] QuantNotDownloaded(String, String), - #[error("no downloaded quantization found for model '{0}'")] - NoDownloadedQuant(String), - #[error("model manager not initialized; call geniex_model_init() first")] NotInitialized, diff --git a/sdk/model-manager/crates/core/src/lib.rs b/sdk/model-manager/crates/core/src/lib.rs index 7165ae9b1..02a548a16 100644 --- a/sdk/model-manager/crates/core/src/lib.rs +++ b/sdk/model-manager/crates/core/src/lib.rs @@ -8,11 +8,11 @@ pub mod logging; pub mod manifest; pub mod manifest_builder; pub mod mapping; -pub mod paths; pub mod pull; pub mod query; -pub mod resume; pub mod source; pub mod store; pub mod transport; -pub mod validation; + +mod resume; +mod validation; diff --git a/sdk/model-manager/crates/core/src/logging.rs b/sdk/model-manager/crates/core/src/logging.rs index d9d1eaee2..573f86314 100644 --- a/sdk/model-manager/crates/core/src/logging.rs +++ b/sdk/model-manager/crates/core/src/logging.rs @@ -38,25 +38,7 @@ pub fn log(level: Level, msg: &str) { } } -#[inline] -#[allow(dead_code)] -pub fn debug(msg: &str) { - log(Level::Debug, msg); -} - -#[inline] -#[allow(dead_code)] -pub fn info(msg: &str) { - log(Level::Info, msg); -} - #[inline] pub fn warn(msg: &str) { log(Level::Warn, msg); } - -#[inline] -#[allow(dead_code)] -pub fn error(msg: &str) { - log(Level::Error, msg); -} diff --git a/sdk/model-manager/crates/core/src/manifest.rs b/sdk/model-manager/crates/core/src/manifest.rs index 0494cbb10..8d9abd6f1 100644 --- a/sdk/model-manager/crates/core/src/manifest.rs +++ b/sdk/model-manager/crates/core/src/manifest.rs @@ -98,9 +98,3 @@ impl ModelManifest { total } } - -#[derive(Debug, Clone)] -pub struct DownloadInfo { - pub total_downloaded: i64, - pub total_size: i64, -} diff --git a/sdk/model-manager/crates/core/src/manifest_builder.rs b/sdk/model-manager/crates/core/src/manifest_builder.rs index 18482d8df..6049d008c 100644 --- a/sdk/model-manager/crates/core/src/manifest_builder.rs +++ b/sdk/model-manager/crates/core/src/manifest_builder.rs @@ -31,30 +31,11 @@ pub struct ManifestHint { pub config_json_bytes: Option>, } -/// Quantization priority order (earlier = preferred). Prefers the smaller, -/// faster `Q4_0` first — the historical Go CLI default that all bindings now -/// share. -pub(crate) const QUANT_PRIORITY: &[&str] = &["Q4_0", "Q4_K_M", "Q8_0"]; - -/// Bucket key for GGUFs whose filename carries no recognizable quant tag -/// (e.g. an untagged FP16 export). Deliberately lower-case: it is a -/// sentinel, not a real quant tag, and it is user-visible — in the -/// precision picker, in "available:" error lists, and as a `:default` -/// pull spec. -pub const DEFAULT_QUANT: &str = "default"; - -/// Canonicalize a user-supplied quant so it matches manifest keys: real -/// tags are upper-cased (keys come from [`extract_quant`], which -/// upper-cases), while any casing of [`DEFAULT_QUANT`] folds to the -/// lower-case sentinel. Blanket upper-casing turned `:default` into -/// `"DEFAULT"`, which no manifest ever keys (#1202). -pub fn normalize_quant_tag(tag: &str) -> String { - if tag.eq_ignore_ascii_case(DEFAULT_QUANT) { - DEFAULT_QUANT.to_string() - } else { - tag.to_ascii_uppercase() - } -} +/// Quantization priority order (earlier = preferred). Consumed by +/// [`extract_quant`] as a tiebreaker among composite tags in one filename +/// and by [`crate::query`] to sort candidates so callers can grab the +/// head as the recommended pick. +pub const QUANT_PRIORITY: &[&str] = &["Q4_0", "Q4_K_M", "Q8_0"]; /// Infer a manifest by scanning `src_dir` for model files. pub fn infer_manifest_from_dir( @@ -109,9 +90,8 @@ pub fn infer_manifest_from_names( if is_mmproj_filename(&lname) { mmprojs.push(n); } else if lname.contains("mtp") { - // TODO: MTP draft models can't load standalone; skip for now. - } else { - let quant = extract_quant(n).unwrap_or_else(|| DEFAULT_QUANT.to_string()); + // MTP draft models can't load standalone; skip. + } else if let Some(quant) = extract_quant(n) { ggufs.entry(quant).or_default().push(n); } } else if lname.ends_with("tokenizer.json") { @@ -154,16 +134,7 @@ pub fn infer_manifest_from_names( let mut shard_extras: Vec<&String> = Vec::new(); for (quant, mut files) in ggufs { files.sort(); - let entry = files[0]; - let entry_size = sizes.get(entry.as_str()).copied().unwrap_or(0); - model_file.insert( - quant, - ModelFileInfo { - name: entry.clone(), - downloaded: true, - size: entry_size, - }, - ); + model_file.insert(quant, file_info(files[0], sizes)); shard_extras.extend_from_slice(&files[1..]); } @@ -171,55 +142,27 @@ pub fn infer_manifest_from_names( let mmproj_file = match mmprojs.len() { 0 => { if onnx_files.len() == 1 { - let n = onnx_files[0]; - ModelFileInfo { - name: n.clone(), - downloaded: true, - size: sizes.get(n.as_str()).copied().unwrap_or(0), - } + file_info(onnx_files[0], sizes) } else if geniex_files.len() == 1 { - let n = geniex_files[0]; - ModelFileInfo { - name: n.clone(), - downloaded: true, - size: sizes.get(n.as_str()).copied().unwrap_or(0), - } + file_info(geniex_files[0], sizes) } else { ModelFileInfo::default() } } - 1 => { - let n = mmprojs[0]; - ModelFileInfo { - name: n.clone(), - downloaded: true, - size: sizes.get(n.as_str()).copied().unwrap_or(0), - } - } + 1 => file_info(mmprojs[0], sizes), _ => { let chosen = mmprojs .iter() .max_by_key(|n| sizes.get(n.as_str()).copied().unwrap_or(0)) .unwrap(); - ModelFileInfo { - name: (*chosen).clone(), - downloaded: true, - size: sizes.get(chosen.as_str()).copied().unwrap_or(0), - } + file_info(chosen, sizes) } }; // Tokenizer: 0 -> none; 1 -> use; >1 -> error (ambiguous). let tokenizer_file = match tokenizers.len() { 0 => ModelFileInfo::default(), - 1 => { - let n = tokenizers[0]; - ModelFileInfo { - name: n.clone(), - downloaded: true, - size: sizes.get(n.as_str()).copied().unwrap_or(0), - } - } + 1 => file_info(tokenizers[0], sizes), _ => { return Err(Error::ManifestInferenceFailed(format!( "multiple tokenizer files found: {:?}", @@ -231,29 +174,14 @@ pub fn infer_manifest_from_names( // ExtraFiles: trailing GGUF shards (the entrypoint shard lives in // model_file) + all .npy + .geniex not used as mmproj. let mut extra_files: Vec = Vec::new(); - for n in &shard_extras { - extra_files.push(ModelFileInfo { - name: (*n).clone(), - downloaded: true, - size: sizes.get(n.as_str()).copied().unwrap_or(0), - }); - } - for n in &npy_files { - extra_files.push(ModelFileInfo { - name: (*n).clone(), - downloaded: true, - size: sizes.get(n.as_str()).copied().unwrap_or(0), - }); - } - for n in &geniex_files { - if mmproj_file.name != **n { - extra_files.push(ModelFileInfo { - name: (*n).clone(), - downloaded: true, - size: sizes.get(n.as_str()).copied().unwrap_or(0), - }); - } - } + extra_files.extend(shard_extras.iter().map(|n| file_info(n, sizes))); + extra_files.extend(npy_files.iter().map(|n| file_info(n, sizes))); + extra_files.extend( + geniex_files + .iter() + .filter(|n| mmproj_file.name != ***n) + .map(|n| file_info(n, sizes)), + ); let model_type = infer_model_type(&hint, file_names); @@ -533,6 +461,14 @@ fn is_token_start(bytes: &[u8], i: usize) -> bool { matches!(bytes[i - 1], b'-' | b'_' | b'.' | b'/' | b'\\') } +fn file_info(name: &str, sizes: &HashMap) -> ModelFileInfo { + ModelFileInfo { + name: name.to_string(), + downloaded: true, + size: sizes.get(name).copied().unwrap_or(0), + } +} + #[cfg(test)] mod tests { use super::*; @@ -724,35 +660,23 @@ mod tests { } #[test] - fn normalize_quant_tag_folds_default_and_upper_cases_tags() { - assert_eq!(normalize_quant_tag("q4_0"), "Q4_0"); - assert_eq!(normalize_quant_tag("mxfp4"), "MXFP4"); - assert_eq!(normalize_quant_tag("default"), DEFAULT_QUANT); - assert_eq!(normalize_quant_tag("DEFAULT"), DEFAULT_QUANT); - assert_eq!(normalize_quant_tag("Default"), DEFAULT_QUANT); - } - - #[test] - fn quant_hint_default_selects_untagged_gguf() { - // #1202: Qwen/Qwen2-1.5B-Instruct-GGUF ships tagged quants plus an - // untagged fp16 export that lands in the "default" bucket. Pulling - // `:default` — the exact key the picker and error list advertise — - // must select that bucket, not fail with QuantNotFound. + fn untagged_gguf_is_silently_dropped() { + // Untagged fp16 exports (like Qwen/Qwen2-1.5B-Instruct-GGUF's stray + // shard) no longer land in a synthetic bucket — they're excluded from + // model_file so callers see only real quant tags. let (names, sizes) = sizes_of(&[ ("qwen2-1_5b-instruct-q4_0.gguf", 1_000_000), ("qwen2-1_5b-instruct-fp16.gguf", 3_100_000), ]); - let hint = ManifestHint { - quant: Some(DEFAULT_QUANT.to_string()), - ..Default::default() - }; - let m = infer_manifest_from_names("Qwen/Qwen2-1.5B-Instruct-GGUF", &names, &sizes, hint) - .unwrap(); + let m = infer_manifest_from_names( + "Qwen/Qwen2-1.5B-Instruct-GGUF", + &names, + &sizes, + ManifestHint::default(), + ) + .unwrap(); assert_eq!(m.model_file.len(), 1); - assert_eq!( - m.model_file.get(DEFAULT_QUANT).map(|f| f.name.as_str()), - Some("qwen2-1_5b-instruct-fp16.gguf") - ); + assert!(m.model_file.contains_key("Q4_0")); } #[test] diff --git a/sdk/model-manager/crates/core/src/paths.rs b/sdk/model-manager/crates/core/src/paths.rs deleted file mode 100644 index e657548e8..000000000 --- a/sdk/model-manager/crates/core/src/paths.rs +++ /dev/null @@ -1,179 +0,0 @@ -// Copyright (c) 2026 Qualcomm Technologies, Inc. and/or its subsidiaries. -// SPDX-License-Identifier: BSD-3-Clause - -use crate::error::{Error, Result}; -use crate::manifest::{ModelManifest, ModelType}; -use crate::manifest_builder::QUANT_PRIORITY; -use std::path::{Path, PathBuf}; - -#[derive(Debug, Clone)] -pub struct ModelPaths { - /// Absolute path to the main model file. - pub model_path: PathBuf, - pub mmproj_path: Option, - pub tokenizer_path: Option, - pub model_dir: PathBuf, - pub model_name: String, - pub plugin_id: String, - pub model_type: ModelType, -} - -/// Resolve file paths from a manifest + local base directory + optional quant hint. -/// -/// Replicates the logic in cli/server/service/keepalive.go:121-141: -/// - If `quant` is Some, look up that exact key; error if not downloaded. -/// - If `quant` is None, prefer the highest-ranked entry in -/// [`QUANT_PRIORITY`]; fall back to lexicographic min when none of the -/// downloaded quants appear in the priority list. -pub fn resolve_model_paths( - manifest: &ModelManifest, - base_dir: &Path, - quant: Option<&str>, -) -> Result<(String, ModelPaths)> { - let model_dir = base_dir.to_path_buf(); - - let (resolved_quant, model_path) = { - let (q, file_info) = if let Some(q) = quant { - let fi = manifest - .model_file - .get(q) - .ok_or_else(|| Error::QuantNotFound(q.to_string(), manifest.name.clone()))?; - if !fi.downloaded { - return Err(Error::QuantNotDownloaded( - q.to_string(), - manifest.name.clone(), - )); - } - (q.to_string(), fi) - } else { - let downloaded: Vec<&str> = manifest - .model_file - .iter() - .filter(|(_, v)| v.downloaded) - .map(|(k, _)| k.as_str()) - .collect(); - if downloaded.is_empty() { - return Err(Error::NoDownloadedQuant(manifest.name.clone())); - } - let q = pick_default_quant(&downloaded).to_string(); - let fi = &manifest.model_file[&q]; - (q, fi) - }; - (q, model_dir.join(&file_info.name)) - }; - - let mmproj_path = if !manifest.mmproj_file.name.is_empty() { - Some(model_dir.join(&manifest.mmproj_file.name)) - } else { - None - }; - - let tokenizer_path = if !manifest.tokenizer_file.name.is_empty() { - Some(model_dir.join(&manifest.tokenizer_file.name)) - } else { - None - }; - - Ok(( - resolved_quant, - ModelPaths { - model_path, - mmproj_path, - tokenizer_path, - model_dir, - model_name: manifest.model_name.clone(), - plugin_id: manifest.plugin_id.clone(), - model_type: manifest.model_type.clone(), - }, - )) -} - -/// Pick a default quant from a non-empty slice of available quants. -/// `QUANT_PRIORITY` wins; otherwise lexicographic min keeps the legacy -/// `slices.Min` behavior for unrecognised quants. -pub(crate) fn pick_default_quant<'a>(available: &'a [&'a str]) -> &'a str { - for pref in QUANT_PRIORITY { - if let Some(hit) = available.iter().find(|q| **q == *pref) { - return hit; - } - } - available.iter().min().copied().expect("non-empty") -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::manifest::{ModelFileInfo, ModelManifest, ModelType}; - use std::collections::HashMap; - use std::path::PathBuf; - - #[test] - fn pick_default_quant_uses_priority() { - assert_eq!(pick_default_quant(&["Q4_0", "Q4_K_M", "Q8_0"]), "Q4_0"); - assert_eq!(pick_default_quant(&["Q4_K_M", "Q8_0"]), "Q4_K_M"); - } - - #[test] - fn pick_default_quant_falls_back_to_lex_min() { - assert_eq!(pick_default_quant(&["Q6_K", "Q5_K_M"]), "Q5_K_M"); - assert_eq!(pick_default_quant(&["IQ4_XS"]), "IQ4_XS"); - } - - #[test] - fn pick_default_quant_partial_priority() { - // Q4_0 is in priority, Q5_K_M is not — priority wins regardless of - // lex order (Q4_0 < Q5_K_M lexicographically anyway, but that is - // incidental). - assert_eq!(pick_default_quant(&["Q4_0", "Q5_K_M"]), "Q4_0"); - } - - fn manifest_with(quants: &[(&str, bool)]) -> ModelManifest { - let mut model_file: HashMap = HashMap::new(); - for (q, downloaded) in quants { - model_file.insert( - (*q).to_string(), - ModelFileInfo { - name: format!("model-{q}.gguf"), - downloaded: *downloaded, - size: 0, - }, - ); - } - ModelManifest { - name: "owner/repo".to_string(), - model_name: "repo".to_string(), - model_type: ModelType::Llm, - plugin_id: "llama_cpp".to_string(), - precision: String::new(), - model_file, - mmproj_file: ModelFileInfo::default(), - tokenizer_file: ModelFileInfo::default(), - extra_files: Vec::new(), - } - } - - #[test] - fn resolve_paths_no_quant_picks_priority() { - let m = manifest_with(&[("Q4_0", true), ("Q4_K_M", true), ("Q8_0", true)]); - let (q, paths) = resolve_model_paths(&m, &PathBuf::from("/cache"), None).unwrap(); - assert_eq!(q, "Q4_0"); - assert_eq!( - paths.model_path, - PathBuf::from("/cache").join("model-Q4_0.gguf") - ); - } - - #[test] - fn resolve_paths_no_quant_falls_back_to_lex_min() { - let m = manifest_with(&[("Q6_K", true), ("Q5_K_M", true)]); - let (q, _) = resolve_model_paths(&m, &PathBuf::from("/cache"), None).unwrap(); - assert_eq!(q, "Q5_K_M"); - } - - #[test] - fn resolve_paths_no_quant_skips_undownloaded_priority_member() { - let m = manifest_with(&[("Q4_0", false), ("Q4_K_M", true), ("Q8_0", true)]); - let (q, _) = resolve_model_paths(&m, &PathBuf::from("/cache"), None).unwrap(); - assert_eq!(q, "Q4_K_M"); - } -} diff --git a/sdk/model-manager/crates/core/src/query.rs b/sdk/model-manager/crates/core/src/query.rs index caf6beebe..9148dc11b 100644 --- a/sdk/model-manager/crates/core/src/query.rs +++ b/sdk/model-manager/crates/core/src/query.rs @@ -12,20 +12,21 @@ use std::sync::Arc; use crate::error::Result; use crate::manifest::ModelType; +use crate::manifest_builder::QUANT_PRIORITY; use crate::mapping::canonicalize_model_name; -use crate::paths::pick_default_quant; use crate::pull::{build_source, PullRequest}; use crate::source::Plan; use crate::store::Store; use crate::transport::{HttpTransport, ReqwestTransport}; use crate::validation::validate_model_name; -/// One quantization the source advertises for a model. +/// One quantization the source advertises for a model. Candidates in +/// [`ModelQuery::candidates`] are sorted so callers can grab the head as +/// the recommended pick. #[derive(Debug, Clone)] pub struct QuantCandidate { pub quant: String, pub size: i64, - pub is_default: bool, } /// Result of a plan-only query against a hub. @@ -60,14 +61,12 @@ pub fn query_blocking( } fn model_query_from_plan(model_name: String, plan: Plan) -> ModelQuery { - let quants: Vec<&str> = plan - .manifest - .model_file - .keys() - .map(String::as_str) - .collect(); - let default = (!quants.is_empty()).then(|| pick_default_quant(&quants).to_string()); - + let priority_idx = |q: &str| { + QUANT_PRIORITY + .iter() + .position(|p| *p == q) + .unwrap_or(usize::MAX) + }; let mut candidates: Vec = plan .manifest .model_file @@ -75,10 +74,14 @@ fn model_query_from_plan(model_name: String, plan: Plan) -> ModelQuery { .map(|(quant, fi)| QuantCandidate { quant: quant.clone(), size: fi.size, - is_default: Some(quant) == default.as_ref(), }) .collect(); - candidates.sort_by(|a, b| b.size.cmp(&a.size).then_with(|| a.quant.cmp(&b.quant))); + candidates.sort_by(|a, b| { + priority_idx(&a.quant) + .cmp(&priority_idx(&b.quant)) + .then_with(|| a.size.cmp(&b.size)) + .then_with(|| a.quant.cmp(&b.quant)) + }); ModelQuery { model_name, @@ -124,22 +127,19 @@ mod tests { } #[test] - fn marks_priority_default_and_sorts_by_size() { + fn sorts_priority_first_then_size() { + // Q4_0 → Q4_K_M → Q8_0 by priority; unknown quants after, size-asc. let q = model_query_from_plan( "Org/Repo".to_string(), - plan_with(&[("Q4_0", 900), ("Q4_K_M", 1_000), ("Q8_0", 1_800)]), + plan_with(&[ + ("Q8_0", 1_800), + ("Q5_K_M", 1_200), + ("Q4_K_M", 1_000), + ("Q4_0", 900), + ]), ); - // Sorted size-desc. - assert_eq!(q.candidates[0].quant, "Q8_0"); - assert_eq!(q.candidates[2].quant, "Q4_0"); - // Q4_0 is the priority default even though it is the smallest. - let default: Vec<&str> = q - .candidates - .iter() - .filter(|c| c.is_default) - .map(|c| c.quant.as_str()) - .collect(); - assert_eq!(default, vec!["Q4_0"]); + let order: Vec<&str> = q.candidates.iter().map(|c| c.quant.as_str()).collect(); + assert_eq!(order, vec!["Q4_0", "Q4_K_M", "Q8_0", "Q5_K_M"]); } #[test] diff --git a/sdk/model-manager/crates/core/src/store.rs b/sdk/model-manager/crates/core/src/store.rs index 329c48f27..d63008b32 100644 --- a/sdk/model-manager/crates/core/src/store.rs +++ b/sdk/model-manager/crates/core/src/store.rs @@ -3,17 +3,28 @@ use std::fs; use std::io::Read; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use fs2::FileExt; use crate::config::StoreConfig; use crate::error::{Error, Result}; use crate::manifest::{ModelManifest, ModelType}; +use crate::manifest_builder::QUANT_PRIORITY; use crate::mapping::canonicalize_model_name; -use crate::paths::{resolve_model_paths, ModelPaths}; use crate::validation::{validate_model_name, validate_relative_file}; +#[derive(Debug, Clone)] +pub struct ModelPaths { + pub model_path: PathBuf, + pub mmproj_path: Option, + pub tokenizer_path: Option, + pub model_dir: PathBuf, + pub model_name: String, + pub plugin_id: String, + pub model_type: ModelType, +} + pub const MANIFEST_FILE: &str = "geniex.json"; /// Maximum allowed size for a geniex.json manifest, to prevent OOM via @@ -249,6 +260,65 @@ impl Store { } } +/// Look up the file paths for one quant of a cached model. +/// +/// When `quant` is `None` we fall back to the highest-priority downloaded +/// quant. That fallback is a *cache-side* convenience — it picks among +/// what's already local, not what a hub offers — and exists because +/// keepalive / auto-load paths pass bare names. +pub(crate) fn resolve_model_paths( + manifest: &ModelManifest, + base_dir: &Path, + quant: Option<&str>, +) -> Result<(String, ModelPaths)> { + let quant = match quant { + Some(q) => q.to_string(), + None => pick_downloaded_by_priority(manifest)?, + }; + let fi = manifest + .model_file + .get(&quant) + .ok_or_else(|| Error::QuantNotFound(quant.clone(), manifest.name.clone()))?; + if !fi.downloaded { + return Err(Error::QuantNotDownloaded(quant, manifest.name.clone())); + } + let model_path = base_dir.join(&fi.name); + let mmproj_path = + (!manifest.mmproj_file.name.is_empty()).then(|| base_dir.join(&manifest.mmproj_file.name)); + let tokenizer_path = (!manifest.tokenizer_file.name.is_empty()) + .then(|| base_dir.join(&manifest.tokenizer_file.name)); + Ok(( + quant, + ModelPaths { + model_path, + mmproj_path, + tokenizer_path, + model_dir: base_dir.to_path_buf(), + model_name: manifest.model_name.clone(), + plugin_id: manifest.plugin_id.clone(), + model_type: manifest.model_type.clone(), + }, + )) +} + +fn pick_downloaded_by_priority(manifest: &ModelManifest) -> Result { + let downloaded: Vec<&str> = manifest + .model_file + .iter() + .filter(|(_, v)| v.downloaded) + .map(|(k, _)| k.as_str()) + .collect(); + if downloaded.is_empty() { + return Err(Error::ModelNotFound(manifest.name.clone())); + } + for pref in QUANT_PRIORITY { + if let Some(hit) = downloaded.iter().find(|q| **q == *pref) { + return Ok((*hit).to_string()); + } + } + Ok(downloaded.iter().min().copied().unwrap().to_string()) +} + /// Read and parse `geniex.json` with a hard size cap. fn read_manifest(path: &std::path::Path) -> Result { let file = fs::File::open(path)?; diff --git a/sdk/model-manager/crates/core/tests/pull_resume.rs b/sdk/model-manager/crates/core/tests/pull_resume.rs index 21e88f0b1..39f23dc7c 100644 --- a/sdk/model-manager/crates/core/tests/pull_resume.rs +++ b/sdk/model-manager/crates/core/tests/pull_resume.rs @@ -58,7 +58,7 @@ async fn pull_resumes_after_mid_download_failure() { let server = MockServer::start().await; let body = make_body(64 * 1024, 0xA5); let repo = "test/tiny"; - let file_name = "weights.gguf"; + let file_name = "weights-Q4_0.gguf"; let api_body = serde_json::json!({ "siblings": [{ "rfilename": file_name, "size": body.len() }] From b8201bfa3e9efe0b3cd80a6e87098b172437e803 Mon Sep 17 00:00:00 2001 From: Mengsheng Wu Date: Wed, 29 Jul 2026 14:03:41 +0800 Subject: [PATCH 02/14] refactor(sdk): drop is_default from model-manager FFI Signed-off-by: Mengsheng Wu --- sdk/model-manager/crates/ffi/src/logging.rs | 13 --------- sdk/model-manager/crates/ffi/src/pull.rs | 19 ++++++------- sdk/model-manager/crates/ffi/src/query.rs | 2 -- sdk/model-manager/crates/ffi/src/types.rs | 31 +++------------------ sdk/model-manager/include/geniex_model.h | 17 +++++------ 5 files changed, 22 insertions(+), 60 deletions(-) diff --git a/sdk/model-manager/crates/ffi/src/logging.rs b/sdk/model-manager/crates/ffi/src/logging.rs index 60d845f92..4c484e9d7 100644 --- a/sdk/model-manager/crates/ffi/src/logging.rs +++ b/sdk/model-manager/crates/ffi/src/logging.rs @@ -60,23 +60,10 @@ pub fn install_core_sink() { } #[inline] -#[allow(dead_code)] -pub fn trace(msg: &str) { - log(GenieXLogLevel::Trace, msg); -} - -#[inline] -#[allow(dead_code)] pub fn debug(msg: &str) { log(GenieXLogLevel::Debug, msg); } -#[inline] -#[allow(dead_code)] -pub fn info(msg: &str) { - log(GenieXLogLevel::Info, msg); -} - #[inline] pub fn warn(msg: &str) { log(GenieXLogLevel::Warn, msg); diff --git a/sdk/model-manager/crates/ffi/src/pull.rs b/sdk/model-manager/crates/ffi/src/pull.rs index 2b069e6f0..9121dd9cd 100644 --- a/sdk/model-manager/crates/ffi/src/pull.rs +++ b/sdk/model-manager/crates/ffi/src/pull.rs @@ -5,7 +5,7 @@ use std::os::raw::{c_char, c_void}; use std::path::PathBuf; use model_manager_core::config::StoreConfig; -use model_manager_core::manifest_builder::{normalize_quant_tag, ManifestHint}; +use model_manager_core::manifest_builder::ManifestHint; use model_manager_core::mapping::{ aihub_display_name_from_repo, canonicalize_model_name, docker_hub_repo_from_name, is_docker_hub_reference, @@ -276,15 +276,14 @@ pub extern "C" fn geniex_model_pull(input: *const GenieXModelPullInput) -> i32 { Err(c) => return c, }; - // Thread `quant` into the manifest hint so `pull` only fetches - // the requested quantization instead of every GGUF in the repo. - // Normalized here so the lookup in `manifest_builder::infer_*` - // (against keys produced by `extract_quant`, which upper-cases) - // succeeds for bindings that don't normalize themselves. The - // untagged-GGUF bucket key is the lower-case "default" sentinel, - // so normalization folds its casing rather than blanket - // upper-casing (#1202: `pull :default` failed as "DEFAULT"). - let quant = unsafe { cstr_to_str(inp.quant) }.map(normalize_quant_tag); + // Thread `quant` into the manifest hint so `pull` only fetches the + // requested quantization instead of every GGUF in the repo. Upper- + // cased here so the lookup in `manifest_builder::infer_*` (against + // keys produced by `extract_quant`, which upper-cases) succeeds for + // bindings that don't normalize themselves. + let quant = unsafe { cstr_to_str(inp.quant) } + .filter(|s| !s.is_empty()) + .map(str::to_ascii_uppercase); // -1 (GENIEX_MODEL_TYPE_AUTO) leaves detection to the inferer; 0/1 force // the type so the manifest is written correctly in one shot. let model_type = match inp.model_type { diff --git a/sdk/model-manager/crates/ffi/src/query.rs b/sdk/model-manager/crates/ffi/src/query.rs index 2b873b77e..d34297ee1 100644 --- a/sdk/model-manager/crates/ffi/src/query.rs +++ b/sdk/model-manager/crates/ffi/src/query.rs @@ -17,7 +17,6 @@ use crate::types::*; pub struct GenieXQuantCandidate { pub quant: *mut c_char, pub size: i64, - pub is_default: bool, } /// Result of `geniex_model_query`. Mirrors `geniex_ModelQueryOutput`. @@ -83,7 +82,6 @@ pub extern "C" fn geniex_model_query( .map(|c| GenieXQuantCandidate { quant: str_to_cptr(&c.quant), size: c.size, - is_default: c.is_default, }) .collect(); cands.shrink_to_fit(); diff --git a/sdk/model-manager/crates/ffi/src/types.rs b/sdk/model-manager/crates/ffi/src/types.rs index 21e2bb1ef..b0b6f6041 100644 --- a/sdk/model-manager/crates/ffi/src/types.rs +++ b/sdk/model-manager/crates/ffi/src/types.rs @@ -3,11 +3,10 @@ use std::cell::RefCell; use std::ffi::{CStr, CString}; -use std::os::raw::{c_char, c_void}; +use std::os::raw::c_char; use std::panic::{catch_unwind, AssertUnwindSafe}; use model_manager_core::error::Error; -use model_manager_core::manifest_builder::normalize_quant_tag; use crate::logging; @@ -72,7 +71,6 @@ pub fn err_to_code(e: &Error) -> i32 { Error::HubModelNotFound(_) => GENIEX_ERROR_COMMON_HUB_MODEL_NOT_FOUND, Error::QuantNotFound(_, _) | Error::QuantNotDownloaded(_, _) - | Error::NoDownloadedQuant(_) | Error::InvalidModelName(_) | Error::InvalidFileName(_) => GENIEX_ERROR_COMMON_INVALID_INPUT, // Split HTTP status into actionable buckets; everything else (other @@ -162,24 +160,17 @@ pub unsafe fn free_cptr(ptr: *mut c_char) { /// Canonicalize the `:QUANT` suffix of a model name. Manifest keys are /// produced by `extract_quant`, which upper-cases (so `q4_0` -> `Q4_0`); /// without matching the lookup side, `pull :q4_0` fails for callers -/// (Python, JNI) whose bindings don't already upper-case. The untagged-GGUF -/// bucket is keyed by the lower-case "default" sentinel, so that spelling -/// folds down instead of up (#1202). Done here at the FFI boundary so the -/// invariant is a single point of enforcement. +/// (Python, JNI) whose bindings don't already upper-case. Done here at +/// the FFI boundary so the invariant is a single point of enforcement. pub fn normalize_quant_suffix(name: &str) -> String { match name.rsplit_once(':') { Some((base, quant)) if !quant.is_empty() => { - format!("{base}:{}", normalize_quant_tag(quant)) + format!("{base}:{}", quant.to_ascii_uppercase()) } _ => name.to_string(), } } -// Silence unused warning for c_void import when building without features that -// use it; pull.rs re-imports c_void directly when it needs it. -#[allow(dead_code)] -pub(crate) type VoidPtr = *mut c_void; - #[cfg(test)] mod tests { use super::*; @@ -245,20 +236,6 @@ mod tests { assert_eq!(normalize_quant_suffix("Org/Repo:"), "Org/Repo:"); } - #[test] - fn normalize_quant_suffix_folds_default_sentinel() { - // #1202: the untagged-GGUF bucket is keyed "default" (lower-case); - // upper-casing the suffix made `:default` unmatchable. - assert_eq!( - normalize_quant_suffix("Qwen/Qwen2-1.5B-Instruct-GGUF:default"), - "Qwen/Qwen2-1.5B-Instruct-GGUF:default" - ); - assert_eq!( - normalize_quant_suffix("Qwen/Qwen2-1.5B-Instruct-GGUF:DEFAULT"), - "Qwen/Qwen2-1.5B-Instruct-GGUF:default" - ); - } - #[test] fn not_found_variants_are_distinct() { assert_eq!( diff --git a/sdk/model-manager/include/geniex_model.h b/sdk/model-manager/include/geniex_model.h index 968d0aa1c..956ef4b60 100644 --- a/sdk/model-manager/include/geniex_model.h +++ b/sdk/model-manager/include/geniex_model.h @@ -107,7 +107,8 @@ typedef struct { * @brief Get resolved file paths for a model. * * @param model_name "org/repo" or "org/repo:quant". - * If quant is omitted the first downloaded quantization is used. + * If quant is omitted the highest-priority downloaded + * quantization is used (Q4_0 > Q4_K_M > Q8_0 > others). * @param out_paths Populated on success. Call geniex_model_paths_free() when done. * @return GENIEX_SUCCESS, or a negative geniex_ErrorCode. */ @@ -253,10 +254,11 @@ typedef struct { uint32_t struct_size; const char* model_name; /**< "org/repo" or short alias */ /** - * Quantization hint for HuggingFace / AI Hub pulls (NULL for - * auto-select). Doubles as the Docker tag or `sha256:` digest - * when `hub == GENIEX_HUB_DOCKER` (or GENIEX_HUB_AUTO resolves to - * Docker); NULL or empty then means the `latest` tag. + * Quantization filter for HuggingFace / AI Hub pulls. When set only + * that quant is fetched; NULL pulls every quant the repo publishes. + * Doubles as the Docker tag or `sha256:` digest when + * `hub == GENIEX_HUB_DOCKER` (or GENIEX_HUB_AUTO resolves to Docker); + * NULL or empty then means the `latest` tag. */ const char* quant; geniex_HubSource hub; /**< Use GENIEX_HUB_AUTO for automatic selection */ @@ -324,9 +326,8 @@ GENIEX_API int32_t geniex_model_pull(const geniex_ModelPullInput* input); * `quant` is heap-allocated; freed by geniex_model_query_free(). */ typedef struct { - char* quant; /**< Quantization name, e.g. "Q4_K_M". */ - int64_t size; /**< Size in bytes of the largest file for this quant. */ - bool is_default; /**< True for the quant the SDK would auto-select. */ + char* quant; /**< Quantization name, e.g. "Q4_K_M". */ + int64_t size; /**< Size in bytes of the largest file for this quant. */ } geniex_QuantCandidate; /** From a857c58867fdb52f9915fc7ccb59414e1c46d226 Mon Sep 17 00:00:00 2001 From: Mengsheng Wu Date: Wed, 29 Jul 2026 14:05:39 +0800 Subject: [PATCH 03/14] refactor(bindings): drop is_default from precision candidate wrappers Signed-off-by: Mengsheng Wu --- .../app/src/main/cpp/model_manager_jni.cpp | 8 ++------ .../com/geniex/sdk/bean/PrecisionCandidate.kt | 1 - bindings/go/model_manager.go | 6 +++--- bindings/python/geniex/_ffi/_types.py | 1 - bindings/python/geniex/model_manager.py | 18 ++++++++++-------- 5 files changed, 15 insertions(+), 19 deletions(-) diff --git a/bindings/android/app/src/main/cpp/model_manager_jni.cpp b/bindings/android/app/src/main/cpp/model_manager_jni.cpp index 1b540b221..dfb54f4b4 100644 --- a/bindings/android/app/src/main/cpp/model_manager_jni.cpp +++ b/bindings/android/app/src/main/cpp/model_manager_jni.cpp @@ -219,15 +219,11 @@ jobject build_model_detail(JNIEnv* env, const geniex_ModelDetail& d) { // Build a com.geniex.sdk.bean.ModelQuery from a geniex_ModelQueryOutput. jobject build_model_query(JNIEnv* env, const geniex_ModelQueryOutput& out) { jclass candCls = env->FindClass("com/geniex/sdk/bean/PrecisionCandidate"); - jmethodID candCtor = env->GetMethodID(candCls, "", "(Ljava/lang/String;JZ)V"); + jmethodID candCtor = env->GetMethodID(candCls, "", "(Ljava/lang/String;J)V"); jobjectArray jCandidates = env->NewObjectArray(out.candidate_count, candCls, nullptr); for (int32_t i = 0; i < out.candidate_count; ++i) { jstring jQuant = env->NewStringUTF(out.candidates[i].quant ? out.candidates[i].quant : ""); - jobject item = env->NewObject(candCls, - candCtor, - jQuant, - static_cast(out.candidates[i].size), - static_cast(out.candidates[i].is_default)); + jobject item = env->NewObject(candCls, candCtor, jQuant, static_cast(out.candidates[i].size)); env->SetObjectArrayElement(jCandidates, i, item); env->DeleteLocalRef(item); env->DeleteLocalRef(jQuant); diff --git a/bindings/android/app/src/main/java/com/geniex/sdk/bean/PrecisionCandidate.kt b/bindings/android/app/src/main/java/com/geniex/sdk/bean/PrecisionCandidate.kt index 04ff219dd..3f31d33fb 100644 --- a/bindings/android/app/src/main/java/com/geniex/sdk/bean/PrecisionCandidate.kt +++ b/bindings/android/app/src/main/java/com/geniex/sdk/bean/PrecisionCandidate.kt @@ -4,5 +4,4 @@ package com.geniex.sdk.bean data class PrecisionCandidate( val precision: String, val size: Long, - val is_default: Boolean, ) diff --git a/bindings/go/model_manager.go b/bindings/go/model_manager.go index faae92e0f..a19312f2c 100644 --- a/bindings/go/model_manager.go +++ b/bindings/go/model_manager.go @@ -365,11 +365,12 @@ func ModelGetPaths(name string) (*ModelPaths, error) { }, nil } -// PrecisionCandidate mirrors geniex_QuantCandidate. +// PrecisionCandidate mirrors geniex_QuantCandidate. Candidates in +// ModelQueryResult.Candidates are sorted by SDK priority — grab the head +// for the recommended pick. type PrecisionCandidate struct { Precision string Size int64 - IsDefault bool } // ModelQueryResult mirrors geniex_ModelQueryOutput. @@ -428,7 +429,6 @@ func ModelQuery(input ModelPullInput) (*ModelQueryResult, error) { result.Candidates[i] = PrecisionCandidate{ Precision: C.GoString(c.quant), Size: int64(c.size), - IsDefault: bool(c.is_default), } } } diff --git a/bindings/python/geniex/_ffi/_types.py b/bindings/python/geniex/_ffi/_types.py index 4f4a12216..98c94b59a 100644 --- a/bindings/python/geniex/_ffi/_types.py +++ b/bindings/python/geniex/_ffi/_types.py @@ -393,7 +393,6 @@ class geniex_QuantCandidate(Structure): _fields_ = [ ('quant', c_char_p), ('size', c_int64), - ('is_default', c_bool), ] diff --git a/bindings/python/geniex/model_manager.py b/bindings/python/geniex/model_manager.py index c93cce1c2..af20046f5 100644 --- a/bindings/python/geniex/model_manager.py +++ b/bindings/python/geniex/model_manager.py @@ -102,12 +102,15 @@ class PrecisionCandidate: precision: str size: int - is_default: bool @dataclass(frozen=True) class ModelQuery: - """Result of a plan-only :func:`query`.""" + """Result of a plan-only :func:`query`. + + ``candidates`` is sorted by SDK priority — grab index 0 for the + recommended pick. + """ model_name: str runtime: str @@ -359,7 +362,6 @@ def query( PrecisionCandidate( precision=out.candidates[i].quant.decode() if out.candidates[i].quant else '', size=out.candidates[i].size, - is_default=bool(out.candidates[i].is_default), ) for i in range(out.candidate_count) ] @@ -524,14 +526,14 @@ def ensure_cached( except GenieXError: full_name = name_part - # No precision + remote source: resolve the hub default before pulling so - # only one variant is downloaded instead of all of them. + # No precision + remote source: pick the head of the SDK's priority- + # sorted candidate list so only one variant is downloaded instead of + # every quant the repo publishes. if precision is None and local_path is None: try: result = query(full_name, hub=hub, hf_token=hf_token) - default = next((c.precision for c in result.candidates if c.is_default), None) - if default: - precision = default + if result.candidates: + precision = result.candidates[0].precision except GenieXError: pass # offline or unsupported hub; let pull decide From 9d3c5de8cf4a3bec3bc64683d8f6c0aba3ff8acb Mon Sep 17 00:00:00 2001 From: Mengsheng Wu Date: Wed, 29 Jul 2026 14:06:08 +0800 Subject: [PATCH 04/14] refactor(cli): choosePrecision uses SDK-sorted head as pre-select Signed-off-by: Mengsheng Wu --- cli/cmd/geniex/model.go | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/cli/cmd/geniex/model.go b/cli/cmd/geniex/model.go index 5dd9b41d3..fe1c4bfc1 100644 --- a/cli/cmd/geniex/model.go +++ b/cli/cmd/geniex/model.go @@ -478,8 +478,9 @@ func pullModel(ctx context.Context, name string, quant string) error { } // choosePrecision picks a precision from the remote candidates: the only one -// when there's a single option, otherwise an interactive picker that defaults -// to the SDK-recommended quant. +// when there's a single option, otherwise an interactive picker pre-filled +// with candidates[0] (the SDK sorts by priority, so head is the recommended +// pick). func choosePrecision(candidates []geniex_sdk.PrecisionCandidate) (string, error) { if len(candidates) == 0 { return "", fmt.Errorf("no precision available for this model") @@ -488,24 +489,17 @@ func choosePrecision(candidates []geniex_sdk.PrecisionCandidate) (string, error) return candidates[0].Precision, nil } - var defaultQuant string - var options []huh.Option[string] + options := make([]huh.Option[string], 0, len(candidates)) for _, c := range candidates { - var sz string + sz := "—" if c.Size > 0 { sz = humanize.IBytes(uint64(c.Size)) - } else { - sz = "—" } label := fmt.Sprintf("%-10s [%7s]", c.Precision, sz) - if c.IsDefault { - label += " (default)" - defaultQuant = c.Precision - } options = append(options, huh.NewOption(label, c.Precision)) } - chosen := defaultQuant + chosen := candidates[0].Precision if err := huh.NewSelect[string](). Title("Choose a precision version to download"). Options(options...). From 9fb7cc142167eac7c584611a7ae8c95139c115d4 Mon Sep 17 00:00:00 2001 From: Mengsheng Wu Date: Wed, 29 Jul 2026 14:07:14 +0800 Subject: [PATCH 05/14] test(python): rename default_precision suite to ensure_cached Signed-off-by: Mengsheng Wu --- ...ult_precision.py => test_ensure_cached.py} | 35 +++++-------------- 1 file changed, 9 insertions(+), 26 deletions(-) rename bindings/python/tests/unit/{test_default_precision.py => test_ensure_cached.py} (63%) diff --git a/bindings/python/tests/unit/test_default_precision.py b/bindings/python/tests/unit/test_ensure_cached.py similarity index 63% rename from bindings/python/tests/unit/test_default_precision.py rename to bindings/python/tests/unit/test_ensure_cached.py index 1beffea0c..88b486329 100644 --- a/bindings/python/tests/unit/test_default_precision.py +++ b/bindings/python/tests/unit/test_ensure_cached.py @@ -1,10 +1,10 @@ # Copyright 2024-2026 Qualcomm Technologies, Inc. and/or its subsidiaries. # SPDX-License-Identifier: BSD-3-Clause -"""Unit tests for the ensure_cached default-precision resolution (#1098). +"""Unit tests for the ensure_cached precision-selection flow (#1098). -When precision=None, ensure_cached must call query() to pick the hub's -default precision and pass it to pull() — so only one variant is downloaded. +When precision=None, ensure_cached must call query() and pass the SDK's +priority-sorted head precision to pull() so only one variant is downloaded. """ from __future__ import annotations @@ -14,13 +14,12 @@ from geniex.model_manager import ModelPaths, ModelQuery, PrecisionCandidate -def _make_query(*precisions: tuple[str, bool]) -> ModelQuery: - """Build a ModelQuery with the given (precision, is_default) pairs.""" +def _make_query(*precisions: str) -> ModelQuery: return ModelQuery( model_name='org/repo', runtime='llama_cpp', model_type='llm', - candidates=[PrecisionCandidate(precision=p, size=0, is_default=d) for p, d in precisions], + candidates=[PrecisionCandidate(precision=p, size=0) for p in precisions], ) @@ -34,11 +33,11 @@ def _fake_paths() -> ModelPaths: ) -def test_ensure_cached_resolves_default_precision(monkeypatch): - """pull() receives the is_default precision when none is specified.""" +def test_ensure_cached_pulls_head_precision(monkeypatch): + """pull() receives candidates[0].precision when none is specified.""" pulled = {} - monkeypatch.setattr(mm, 'query', lambda name, **_kw: _make_query(('Q8_0', False), ('Q4_0', True))) + monkeypatch.setattr(mm, 'query', lambda name, **_kw: _make_query('Q4_0', 'Q4_K_M', 'Q8_0')) monkeypatch.setattr(mm, 'pull', lambda name, *, precision=None, **_kw: pulled.update(precision=precision)) monkeypatch.setattr(mm, 'get_paths', lambda _key: _fake_paths()) monkeypatch.setattr(mm, 'resolve_alias', lambda name: name) @@ -53,7 +52,7 @@ def test_ensure_cached_explicit_precision_skips_query(monkeypatch): """An explicit precision bypasses query() entirely.""" queried = [] - monkeypatch.setattr(mm, 'query', lambda *_a, **_kw: queried.append(True) or _make_query(('Q4_0', True))) + monkeypatch.setattr(mm, 'query', lambda *_a, **_kw: queried.append(True) or _make_query('Q4_0')) monkeypatch.setattr(mm, 'pull', lambda *_a, **_kw: None) monkeypatch.setattr(mm, 'get_paths', lambda _key: _fake_paths()) monkeypatch.setattr(mm, 'resolve_alias', lambda name: name) @@ -64,21 +63,6 @@ def test_ensure_cached_explicit_precision_skips_query(monkeypatch): assert queried == [], 'query() should not be called when precision is explicit' -def test_ensure_cached_local_path_skips_query(monkeypatch): - """A local_path pull bypasses query() — the manifest is already on disk.""" - queried = [] - - monkeypatch.setattr(mm, 'query', lambda *_a, **_kw: queried.append(True) or _make_query(('Q4_0', True))) - monkeypatch.setattr(mm, 'pull', lambda *_a, **_kw: None) - monkeypatch.setattr(mm, 'get_paths', lambda _key: _fake_paths()) - monkeypatch.setattr(mm, 'resolve_alias', lambda name: name) - monkeypatch.setattr(mm, '_ensure_init', lambda: None) - - mm.ensure_cached('org/repo', local_path='/some/path') - - assert queried == [] - - def test_ensure_cached_query_failure_falls_through(monkeypatch): """A query() GenieXError is swallowed and pull() is still called.""" pulled = {} @@ -91,5 +75,4 @@ def test_ensure_cached_query_failure_falls_through(monkeypatch): mm.ensure_cached('org/repo') - # precision stays None — pull decides on its own assert pulled['precision'] is None From 5b4a867acdfc68d1e6442559da89b8e3ac1d01fb Mon Sep 17 00:00:00 2001 From: Mengsheng Wu Date: Wed, 29 Jul 2026 16:36:16 +0800 Subject: [PATCH 06/14] refactor(sdk): simplify FFI boundary with ? and into_c_array Signed-off-by: Mengsheng Wu --- sdk/model-manager/crates/ffi/src/chipset.rs | 116 ++++------ sdk/model-manager/crates/ffi/src/init.rs | 27 +-- sdk/model-manager/crates/ffi/src/mapping.rs | 32 +-- sdk/model-manager/crates/ffi/src/pull.rs | 132 +++++------ sdk/model-manager/crates/ffi/src/query.rs | 44 +--- sdk/model-manager/crates/ffi/src/store.rs | 232 ++++++-------------- sdk/model-manager/crates/ffi/src/types.rs | 65 ++++-- 7 files changed, 237 insertions(+), 411 deletions(-) diff --git a/sdk/model-manager/crates/ffi/src/chipset.rs b/sdk/model-manager/crates/ffi/src/chipset.rs index b8d1ab3a2..8e001f71c 100644 --- a/sdk/model-manager/crates/ffi/src/chipset.rs +++ b/sdk/model-manager/crates/ffi/src/chipset.rs @@ -27,36 +27,33 @@ pub struct GenieXChipsetList { pub count: i32, } +fn ai_hub_cfg_for_chipset_query(store: &model_manager_core::store::Store) -> AiHubConfig { + AiHubConfig::new( + StoreConfig::ai_hub_base_url(), + StoreConfig::ai_hub_version(), + String::new(), + store.config().ai_hub_cache_dir(), + false, + ) +} + #[no_mangle] pub extern "C" fn geniex_model_list_chipsets(out: *mut GenieXChipsetList) -> i32 { ffi_guard(|| { if out.is_null() { - return GENIEX_ERROR_COMMON_INVALID_INPUT; + return Err(GENIEX_ERROR_COMMON_INVALID_INPUT); } - let store = match get_store() { - Ok(s) => s, - Err(c) => return c, - }; - let cfg = AiHubConfig::new( - StoreConfig::ai_hub_base_url(), - StoreConfig::ai_hub_version(), - String::new(), - store.config().ai_hub_cache_dir(), - false, - ); - let chipsets = match runtime_handle().block_on(list_supported_chipsets(&cfg)) { - Ok(c) => c, - Err(e) => return report(&e), - }; + let cfg = ai_hub_cfg_for_chipset_query(get_store()?); + let chipsets = runtime_handle() + .block_on(list_supported_chipsets(&cfg)) + .map_err(|e| report(&e))?; - let mut infos: Vec = chipsets + let infos: Vec = chipsets .iter() .map(|c| { - // Surface the reference device (e.g. "Snapdragon X Elite - // CRD") as `name`, demoting the canonical chipset id - // ("qualcomm-snapdragon-x-elite") into the alias list so - // callers can still resolve / display it. Fall back to the - // canonical id when the bucket omits a reference device. + // Surface reference_device as `name` and demote the canonical + // chipset id into the alias list; fall back to the id when no + // reference device exists. let display = if c.reference_device.is_empty() { c.name.as_str() } else { @@ -67,36 +64,22 @@ pub extern "C" fn geniex_model_list_chipsets(out: *mut GenieXChipsetList) -> i32 alias_strs.push(c.name.as_str()); } alias_strs.extend(c.aliases.iter().map(String::as_str)); - let mut aliases: Vec<*mut c_char> = + let aliases_vec: Vec<*mut c_char> = alias_strs.iter().map(|a| str_to_cptr(a)).collect(); - aliases.shrink_to_fit(); - let alias_count = aliases.len() as i32; - let aliases_ptr = if aliases.is_empty() { - std::ptr::null_mut() - } else { - aliases.as_mut_ptr() - }; - std::mem::forget(aliases); + let (aliases, alias_count) = into_c_array(aliases_vec); GenieXChipsetInfo { name: str_to_cptr(display), - aliases: aliases_ptr, + aliases, alias_count, } }) .collect(); - infos.shrink_to_fit(); - let count = infos.len() as i32; - let chipsets_ptr = if infos.is_empty() { - std::ptr::null_mut() - } else { - infos.as_mut_ptr() - }; - std::mem::forget(infos); + let (chipsets_ptr, count) = into_c_array(infos); unsafe { (*out).chipsets = chipsets_ptr; (*out).count = count; } - GENIEX_SUCCESS + Ok(GENIEX_SUCCESS) }) } @@ -106,28 +89,15 @@ pub unsafe extern "C" fn geniex_model_list_chipsets_free(out: *mut GenieXChipset return; } let o = &mut *out; - if !o.chipsets.is_null() { - let slice = std::slice::from_raw_parts_mut(o.chipsets, o.count as usize); - for info in slice.iter_mut() { + if let Some(mut infos) = from_c_array(o.chipsets, o.count) { + for info in infos.iter_mut() { free_cptr(info.name); - if !info.aliases.is_null() { - let aliases = - std::slice::from_raw_parts_mut(info.aliases, info.alias_count as usize); - for a in aliases.iter_mut() { - free_cptr(*a); + if let Some(aliases) = from_c_array(info.aliases, info.alias_count) { + for a in aliases { + free_cptr(a); } - drop(Vec::from_raw_parts( - info.aliases, - info.alias_count as usize, - info.alias_count as usize, - )); } } - drop(Vec::from_raw_parts( - o.chipsets, - o.count as usize, - o.count as usize, - )); } o.chipsets = std::ptr::null_mut(); o.count = 0; @@ -139,26 +109,14 @@ pub unsafe extern "C" fn geniex_model_list_chipsets_free(out: *mut GenieXChipset pub extern "C" fn geniex_model_detect_chipset(out_chipset: *mut *mut c_char) -> i32 { ffi_guard(|| { if out_chipset.is_null() { - return GENIEX_ERROR_COMMON_INVALID_INPUT; - } - let store = match get_store() { - Ok(s) => s, - Err(c) => return c, - }; - let cfg = AiHubConfig::new( - StoreConfig::ai_hub_base_url(), - StoreConfig::ai_hub_version(), - String::new(), - store.config().ai_hub_cache_dir(), - false, - ); - let ptr = match runtime_handle().block_on(detect_host_chipset_reference(&cfg)) { - Some(s) => str_to_cptr(&s), - None => std::ptr::null_mut(), - }; - unsafe { - *out_chipset = ptr; + return Err(GENIEX_ERROR_COMMON_INVALID_INPUT); } - GENIEX_SUCCESS + let cfg = ai_hub_cfg_for_chipset_query(get_store()?); + let ptr = runtime_handle() + .block_on(detect_host_chipset_reference(&cfg)) + .map(|s| str_to_cptr(&s)) + .unwrap_or(std::ptr::null_mut()); + unsafe { *out_chipset = ptr }; + Ok(GENIEX_SUCCESS) }) } diff --git a/sdk/model-manager/crates/ffi/src/init.rs b/sdk/model-manager/crates/ffi/src/init.rs index 31416c7b6..89ccb2432 100644 --- a/sdk/model-manager/crates/ffi/src/init.rs +++ b/sdk/model-manager/crates/ffi/src/init.rs @@ -62,38 +62,27 @@ pub extern "C" fn geniex_model_init(data_dir: *const c_char) -> i32 { ffi_guard(|| { logging::install_core_sink(); - let _guard = match INIT_LOCK.lock() { - Ok(g) => g, - // Mutex is poisoned only if a previous init panicked. Treat - // as "try again" — the panic path already logged via ffi_guard. - Err(poisoned) => poisoned.into_inner(), - }; + // Poisoned mutex ⇒ a previous init panicked; treat as "try again". + let _guard = INIT_LOCK.lock().unwrap_or_else(|p| p.into_inner()); if STORE.get().is_some() { logging::warn( "geniex_model_init called after the model manager was already initialized; \ call geniex_model_deinit first", ); - return GENIEX_ERROR_COMMON_ALREADY_INITIALIZED; + return Err(GENIEX_ERROR_COMMON_ALREADY_INITIALIZED); } - let data_dir_override = unsafe { cstr_to_str(data_dir).map(PathBuf::from) }; - let mut cfg = StoreConfig::from_env(); - if let Some(dir) = data_dir_override { - cfg.data_dir = dir; + if let Ok(s) = unsafe { cstr_to_str(data_dir) } { + cfg.data_dir = PathBuf::from(s); } - let store = match Store::new(cfg) { - Ok(s) => s, - Err(e) => return report(&e), - }; - - // Holding INIT_LOCK means no other thread is between the get() - // check and set() call, so this always succeeds. + let store = Store::new(cfg).map_err(|e| report(&e))?; + // INIT_LOCK is held ⇒ this set() never races. let _ = STORE.set(store); logging::debug("geniex model manager initialized"); - GENIEX_SUCCESS + Ok(GENIEX_SUCCESS) }) } diff --git a/sdk/model-manager/crates/ffi/src/mapping.rs b/sdk/model-manager/crates/ffi/src/mapping.rs index 276893a54..4bb129709 100644 --- a/sdk/model-manager/crates/ffi/src/mapping.rs +++ b/sdk/model-manager/crates/ffi/src/mapping.rs @@ -25,20 +25,15 @@ pub extern "C" fn geniex_model_resolve_hub( ) -> i32 { ffi_guard(|| { if out_hub.is_null() { - return GENIEX_ERROR_COMMON_INVALID_INPUT; + return Err(GENIEX_ERROR_COMMON_INVALID_INPUT); } - let name = match unsafe { cstr_to_str(model_name) } { - Some(s) => s, - None => return GENIEX_ERROR_COMMON_INVALID_INPUT, - }; + let name = unsafe { cstr_to_str(model_name) }?; let resolved = match hub_in { GenieXHubSource::Auto if is_docker_hub_reference(name) => GenieXHubSource::Docker, other => other, }; - unsafe { - *out_hub = resolved; - } - GENIEX_SUCCESS + unsafe { *out_hub = resolved }; + Ok(GENIEX_SUCCESS) }) } @@ -49,20 +44,11 @@ pub extern "C" fn geniex_model_resolve_alias( ) -> i32 { ffi_guard(|| { if out_full_name.is_null() { - return GENIEX_ERROR_COMMON_INVALID_INPUT; - } - let alias_str = match unsafe { cstr_to_str(alias) } { - Some(s) => s, - None => return GENIEX_ERROR_COMMON_INVALID_INPUT, - }; - match resolve_alias(alias_str) { - Some(full) => { - unsafe { - *out_full_name = str_to_cptr(&full); - } - GENIEX_SUCCESS - } - None => GENIEX_ERROR_COMMON_INVALID_INPUT, + return Err(GENIEX_ERROR_COMMON_INVALID_INPUT); } + let alias_str = unsafe { cstr_to_str(alias) }?; + let full = resolve_alias(alias_str).ok_or(GENIEX_ERROR_COMMON_INVALID_INPUT)?; + unsafe { *out_full_name = str_to_cptr(&full) }; + Ok(GENIEX_SUCCESS) }) } diff --git a/sdk/model-manager/crates/ffi/src/pull.rs b/sdk/model-manager/crates/ffi/src/pull.rs index 9121dd9cd..6e8a58704 100644 --- a/sdk/model-manager/crates/ffi/src/pull.rs +++ b/sdk/model-manager/crates/ffi/src/pull.rs @@ -165,43 +165,37 @@ pub(crate) unsafe fn extract_name_and_intent( } let inp = &*input; + let raw_model_name = cstr_to_str(inp.model_name)?; - let raw_model_name = cstr_to_str(inp.model_name).ok_or(GENIEX_ERROR_COMMON_INVALID_INPUT)?; - - // Docker Hub routing is decided on the *original* string, before the - // generic HF/AI-Hub canonicalisation below would discard the - // hub-identifying prefix — mirrors docker/model-runner's - // `IsHuggingFaceReference(originalReference)` check ahead of its own - // name normalisation. `--model-hub docker` (or GENIEX_HUB_DOCKER) works - // on a bare "org/repo" too, since the hub is then already unambiguous. + // Docker Hub routing is decided on the *original* string, before generic + // HF/AI-Hub canonicalisation below would discard the hub-identifying + // prefix — mirrors docker/model-runner's `IsHuggingFaceReference` check. let use_docker = matches!(inp.hub, GenieXHubSource::Docker) || (matches!(inp.hub, GenieXHubSource::Auto) && is_docker_hub_reference(raw_model_name)); if use_docker { let repo = canonicalize_model_name(&docker_hub_repo_from_name(raw_model_name)); - // `quant` doubles as the Docker tag/digest for this hub (no GGUF - // quant filtering happens for Docker pulls); empty means "latest". + // `quant` doubles as the Docker tag/digest; empty ⇒ "latest". let reference = cstr_to_str(inp.quant) + .ok() .filter(|s| !s.is_empty()) .unwrap_or("latest") .to_string(); return Ok((repo.clone(), PullIntent::DockerHub { repo, reference })); } - // Bare names (no '/') are treated as AI Hub model ids and stored under - // `qualcomm/`; anything with '/' is passed through. + // Bare names (no '/') land under `qualcomm/`. let model_name = canonicalize_model_name(raw_model_name); - // Explicit token wins; env var is the fallback; anonymous otherwise. let hf_token = cstr_to_str(inp.hf_token) + .ok() .map(str::to_string) .or_else(StoreConfig::hf_token_from_env); - // chipset / display_name are only meaningful for AI Hub; read up front so - // both the explicit-AiHub and Auto-→-AiHub paths share them. let chipset = cstr_to_str(inp.chipset).unwrap_or("").to_string(); let explicit_display_name = cstr_to_str(inp.display_name) + .ok() .map(str::to_string) .filter(|s| !s.is_empty()); - let local_path = cstr_to_str(inp.local_path).map(PathBuf::from); + let local_path = cstr_to_str(inp.local_path).ok().map(PathBuf::from); let intent = build_pull_intent( &inp.hub, @@ -218,18 +212,12 @@ pub(crate) unsafe fn extract_name_and_intent( pub extern "C" fn geniex_model_pull(input: *const GenieXModelPullInput) -> i32 { ffi_guard(|| { if input.is_null() { - return GENIEX_ERROR_COMMON_INVALID_INPUT; + return Err(GENIEX_ERROR_COMMON_INVALID_INPUT); } - let (model_name, intent) = - match unsafe { extract_name_and_intent(input, "geniex_model_pull") } { - Ok(v) => v, - Err(c) => return c, - }; + let (model_name, intent) = unsafe { extract_name_and_intent(input, "geniex_model_pull") }?; let inp = unsafe { &*input }; - // Build a Rust closure that re-marshals Rust FileProgress → C array - // and invokes the caller's function pointer. struct CCallback { cb: unsafe extern "C" fn(*const GenieXFileProgress, i32, *mut c_void) -> bool, user_data: *mut c_void, @@ -237,76 +225,60 @@ pub extern "C" fn geniex_model_pull(input: *const GenieXModelPullInput) -> i32 { unsafe impl Send for CCallback {} unsafe impl Sync for CCallback {} - let progress_cb: Option = if let Some(cb) = - inp.on_progress - { - let cc = std::sync::Arc::new(CCallback { - cb, - user_data: inp.user_data, + let progress_cb: Option = + inp.on_progress.map(|cb| { + let cc = std::sync::Arc::new(CCallback { + cb, + user_data: inp.user_data, + }); + Box::new( + move |files: &[model_manager_core::executor::FileProgress]| -> bool { + let cstrings: Vec = files + .iter() + .map(|f| { + std::ffi::CString::new(f.file_name.as_bytes()).unwrap_or_default() + }) + .collect(); + let ffi_entries: Vec = files + .iter() + .zip(cstrings.iter()) + .map(|(f, cs)| GenieXFileProgress { + file_name: cs.as_ptr(), + downloaded_bytes: f.downloaded_bytes, + total_bytes: f.total_bytes, + }) + .collect(); + let result = unsafe { + (cc.cb)(ffi_entries.as_ptr(), ffi_entries.len() as i32, cc.user_data) + }; + let _ = cstrings; + result + }, + ) as model_manager_core::executor::ProgressCallback }); - Some(Box::new( - move |files: &[model_manager_core::executor::FileProgress]| -> bool { - let cstrings: Vec = files - .iter() - .map(|f| std::ffi::CString::new(f.file_name.as_bytes()).unwrap_or_default()) - .collect(); - let ffi_entries: Vec = files - .iter() - .zip(cstrings.iter()) - .map(|(f, cs)| GenieXFileProgress { - file_name: cs.as_ptr(), - downloaded_bytes: f.downloaded_bytes, - total_bytes: f.total_bytes, - }) - .collect(); - - let result = unsafe { - (cc.cb)(ffi_entries.as_ptr(), ffi_entries.len() as i32, cc.user_data) - }; - let _ = cstrings; - result - }, - )) - } else { - None - }; - - let store = match get_store() { - Ok(s) => s, - Err(c) => return c, - }; - // Thread `quant` into the manifest hint so `pull` only fetches the - // requested quantization instead of every GGUF in the repo. Upper- - // cased here so the lookup in `manifest_builder::infer_*` (against - // keys produced by `extract_quant`, which upper-cases) succeeds for - // bindings that don't normalize themselves. + // Upper-cased so lookup against `extract_quant`-produced keys matches + // for bindings that don't normalize themselves. let quant = unsafe { cstr_to_str(inp.quant) } + .ok() .filter(|s| !s.is_empty()) .map(str::to_ascii_uppercase); - // -1 (GENIEX_MODEL_TYPE_AUTO) leaves detection to the inferer; 0/1 force - // the type so the manifest is written correctly in one shot. let model_type = match inp.model_type { 0 => Some(model_manager_core::manifest::ModelType::Llm), 1 => Some(model_manager_core::manifest::ModelType::Vlm), _ => None, }; - let hint = ManifestHint { - quant, - model_type, - ..ManifestHint::default() - }; - let req = PullRequest { model_name, intent, on_progress: progress_cb, - hint, + hint: ManifestHint { + quant, + model_type, + ..ManifestHint::default() + }, }; - - match pull_blocking(&runtime_handle(), store, req) { - Ok(()) => GENIEX_SUCCESS, - Err(e) => report(&e), - } + pull_blocking(&runtime_handle(), get_store()?, req).map_err(|e| report(&e))?; + Ok(GENIEX_SUCCESS) }) } diff --git a/sdk/model-manager/crates/ffi/src/query.rs b/sdk/model-manager/crates/ffi/src/query.rs index d34297ee1..42817ad73 100644 --- a/sdk/model-manager/crates/ffi/src/query.rs +++ b/sdk/model-manager/crates/ffi/src/query.rs @@ -48,35 +48,22 @@ pub extern "C" fn geniex_model_query( ) -> i32 { ffi_guard(|| { if input.is_null() || out.is_null() { - return GENIEX_ERROR_COMMON_INVALID_INPUT; + return Err(GENIEX_ERROR_COMMON_INVALID_INPUT); } // Query reuses the pull input struct; the quant / callback / model_type // fields are simply ignored here. - let (model_name, intent) = - match unsafe { extract_name_and_intent(input, "geniex_model_query") } { - Ok(v) => v, - Err(c) => return c, - }; - - let store = match get_store() { - Ok(s) => s, - Err(c) => return c, - }; - + let (model_name, intent) = unsafe { extract_name_and_intent(input, "geniex_model_query") }?; let req = PullRequest { model_name, intent, on_progress: None, hint: ManifestHint::default(), }; + let result = + query_blocking(&runtime_handle(), get_store()?, req).map_err(|e| report(&e))?; - let result = match query_blocking(&runtime_handle(), store, req) { - Ok(r) => r, - Err(e) => return report(&e), - }; - - let mut cands: Vec = result + let cands: Vec = result .candidates .iter() .map(|c| GenieXQuantCandidate { @@ -84,14 +71,7 @@ pub extern "C" fn geniex_model_query( size: c.size, }) .collect(); - cands.shrink_to_fit(); - let candidate_count = cands.len() as i32; - let candidates = if cands.is_empty() { - std::ptr::null_mut() - } else { - cands.as_mut_ptr() - }; - std::mem::forget(cands); + let (candidates, candidate_count) = into_c_array(cands); unsafe { (*out).model_name = str_to_cptr(&result.model_name); @@ -100,7 +80,7 @@ pub extern "C" fn geniex_model_query( (*out).candidates = candidates; (*out).candidate_count = candidate_count; } - GENIEX_SUCCESS + Ok(GENIEX_SUCCESS) }) } @@ -112,16 +92,10 @@ pub unsafe extern "C" fn geniex_model_query_free(out: *mut GenieXModelQueryOutpu let o = &mut *out; free_cptr(o.model_name); free_cptr(o.plugin_id); - if !o.candidates.is_null() { - let slice = std::slice::from_raw_parts_mut(o.candidates, o.candidate_count as usize); - for c in slice.iter_mut() { + if let Some(cands) = from_c_array(o.candidates, o.candidate_count) { + for c in cands { free_cptr(c.quant); } - drop(Vec::from_raw_parts( - o.candidates, - o.candidate_count as usize, - o.candidate_count as usize, - )); } *out = GenieXModelQueryOutput::null(); } diff --git a/sdk/model-manager/crates/ffi/src/store.rs b/sdk/model-manager/crates/ffi/src/store.rs index 7e3dd00ff..3208ecb81 100644 --- a/sdk/model-manager/crates/ffi/src/store.rs +++ b/sdk/model-manager/crates/ffi/src/store.rs @@ -56,39 +56,25 @@ pub extern "C" fn geniex_model_get_paths( ) -> i32 { ffi_guard(|| { if out_paths.is_null() { - return GENIEX_ERROR_COMMON_INVALID_INPUT; + return Err(GENIEX_ERROR_COMMON_INVALID_INPUT); } - let name = match unsafe { cstr_to_str(model_name) } { - Some(s) => normalize_quant_suffix(s), - None => return GENIEX_ERROR_COMMON_INVALID_INPUT, + let name = normalize_quant_suffix(unsafe { cstr_to_str(model_name) }?); + let store = get_store()?; + let (_, paths) = store.get_paths(&name).map_err(|e| report(&e))?; + let opt_path = |p: Option<&std::path::PathBuf>| { + p.map(|p| str_to_cptr(&p.to_string_lossy())) + .unwrap_or(std::ptr::null_mut()) }; - let store = match get_store() { - Ok(s) => s, - Err(c) => return c, - }; - match store.get_paths(&name) { - Ok((_, paths)) => { - unsafe { - (*out_paths).model_path = str_to_cptr(&paths.model_path.to_string_lossy()); - (*out_paths).model_dir = str_to_cptr(&paths.model_dir.to_string_lossy()); - (*out_paths).model_name = str_to_cptr(&paths.model_name); - (*out_paths).plugin_id = str_to_cptr(&paths.plugin_id); - (*out_paths).mmproj_path = paths - .mmproj_path - .as_ref() - .map(|p| str_to_cptr(&p.to_string_lossy())) - .unwrap_or(std::ptr::null_mut()); - (*out_paths).tokenizer_path = paths - .tokenizer_path - .as_ref() - .map(|p| str_to_cptr(&p.to_string_lossy())) - .unwrap_or(std::ptr::null_mut()); - (*out_paths).model_type = to_ffi_type(paths.model_type); - } - GENIEX_SUCCESS - } - Err(e) => report(&e), + unsafe { + (*out_paths).model_path = str_to_cptr(&paths.model_path.to_string_lossy()); + (*out_paths).model_dir = str_to_cptr(&paths.model_dir.to_string_lossy()); + (*out_paths).model_name = str_to_cptr(&paths.model_name); + (*out_paths).plugin_id = str_to_cptr(&paths.plugin_id); + (*out_paths).mmproj_path = opt_path(paths.mmproj_path.as_ref()); + (*out_paths).tokenizer_path = opt_path(paths.tokenizer_path.as_ref()); + (*out_paths).model_type = to_ffi_type(paths.model_type); } + Ok(GENIEX_SUCCESS) }) } @@ -112,39 +98,20 @@ pub unsafe extern "C" fn geniex_model_paths_free(paths: *mut GenieXModelPaths) { #[no_mangle] pub extern "C" fn geniex_model_remove(model_name: *const c_char) -> i32 { ffi_guard(|| { - let name = match unsafe { cstr_to_str(model_name) } { - Some(s) => normalize_quant_suffix(s), - None => return GENIEX_ERROR_COMMON_INVALID_INPUT, - }; - let store = match get_store() { - Ok(s) => s, - Err(c) => return c, - }; - match store.remove(&name) { - Ok(()) => GENIEX_SUCCESS, - Err(e) => report(&e), - } + let name = normalize_quant_suffix(unsafe { cstr_to_str(model_name) }?); + get_store()?.remove(&name).map_err(|e| report(&e))?; + Ok(GENIEX_SUCCESS) }) } #[no_mangle] pub extern "C" fn geniex_model_clean(removed_count: *mut i32) -> i32 { ffi_guard(|| { - let store = match get_store() { - Ok(s) => s, - Err(c) => return c, - }; - match store.clean() { - Ok(n) => { - if !removed_count.is_null() { - unsafe { - *removed_count = n; - } - } - GENIEX_SUCCESS - } - Err(e) => report(&e), + let n = get_store()?.clean().map_err(|e| report(&e))?; + if !removed_count.is_null() { + unsafe { *removed_count = n }; } + Ok(GENIEX_SUCCESS) }) } @@ -157,25 +124,12 @@ pub extern "C" fn geniex_model_get_type( ) -> i32 { ffi_guard(|| { if out_type.is_null() { - return GENIEX_ERROR_COMMON_INVALID_INPUT; - } - let name = match unsafe { cstr_to_str(model_name) } { - Some(s) => s, - None => return GENIEX_ERROR_COMMON_INVALID_INPUT, - }; - let store = match get_store() { - Ok(s) => s, - Err(c) => return c, - }; - match store.get_model_type(name) { - Ok(t) => { - unsafe { - *out_type = to_ffi_type(t); - } - GENIEX_SUCCESS - } - Err(e) => report(&e), + return Err(GENIEX_ERROR_COMMON_INVALID_INPUT); } + let name = unsafe { cstr_to_str(model_name) }?; + let t = get_store()?.get_model_type(name).map_err(|e| report(&e))?; + unsafe { *out_type = to_ffi_type(t) }; + Ok(GENIEX_SUCCESS) }) } @@ -187,22 +141,15 @@ pub extern "C" fn geniex_model_set_type( model_type: GenieXModelType, ) -> i32 { ffi_guard(|| { - let name = match unsafe { cstr_to_str(model_name) } { - Some(s) => s, - None => return GENIEX_ERROR_COMMON_INVALID_INPUT, - }; - let store = match get_store() { - Ok(s) => s, - Err(c) => return c, - }; + let name = unsafe { cstr_to_str(model_name) }?; let t = match model_type { GenieXModelType::Llm => ModelType::Llm, GenieXModelType::Vlm => ModelType::Vlm, }; - match store.set_model_type(name, t) { - Ok(()) => GENIEX_SUCCESS, - Err(e) => report(&e), - } + get_store()? + .set_model_type(name, t) + .map_err(|e| report(&e))?; + Ok(GENIEX_SUCCESS) }) } @@ -230,69 +177,47 @@ pub struct GenieXModelListDetailedOutput { pub extern "C" fn geniex_model_list_detailed(output: *mut GenieXModelListDetailedOutput) -> i32 { ffi_guard(|| { if output.is_null() { - return GENIEX_ERROR_COMMON_INVALID_INPUT; + return Err(GENIEX_ERROR_COMMON_INVALID_INPUT); } - let store = match get_store() { - Ok(s) => s, - Err(c) => return c, - }; - match store.list() { - Ok(manifests) => { - let mut details: Vec = manifests + let manifests = get_store()?.list().map_err(|e| report(&e))?; + let details: Vec = manifests + .iter() + .map(|m| { + // QAIRT manifests key model_file under "N/A" and carry the + // real precision (e.g. "W4A16") on the top-level field — + // surface that to bindings instead of the placeholder. + let downloaded: Vec<&str> = m + .model_file .iter() - .map(|m| { - // QAIRT manifests key model_file under "N/A" and carry the - // real precision (e.g. "W4A16") on the top-level field — - // surface that to bindings instead of the placeholder. - let downloaded_quants: Vec<&str> = m - .model_file - .iter() - .filter(|(_, fi)| fi.downloaded) - .map(|(q, _)| q.as_str()) - .collect(); - let use_top_level = !m.precision.is_empty() - && !downloaded_quants.is_empty() - && downloaded_quants.iter().all(|q| *q == "N/A"); - let mut precs: Vec<*mut c_char> = if use_top_level { - vec![str_to_cptr(&m.precision)] - } else { - downloaded_quants.iter().map(|q| str_to_cptr(q)).collect() - }; - precs.shrink_to_fit(); - let precision_count = precs.len() as i32; - let precisions = if precs.is_empty() { - std::ptr::null_mut() - } else { - precs.as_mut_ptr() - }; - std::mem::forget(precs); - GenieXModelDetail { - name: str_to_cptr(&m.name), - model_name: str_to_cptr(&m.model_name), - plugin_id: str_to_cptr(&m.plugin_id), - model_type: to_ffi_type(m.model_type.clone()), - total_size: m.total_size(), - precisions, - precision_count, - } - }) + .filter(|(_, fi)| fi.downloaded) + .map(|(q, _)| q.as_str()) .collect(); - details.shrink_to_fit(); - let count = details.len() as i32; - let models = if details.is_empty() { - std::ptr::null_mut() + let precs: Vec<*mut c_char> = if !m.precision.is_empty() + && !downloaded.is_empty() + && downloaded.iter().all(|q| *q == "N/A") + { + vec![str_to_cptr(&m.precision)] } else { - details.as_mut_ptr() + downloaded.iter().map(|q| str_to_cptr(q)).collect() }; - std::mem::forget(details); - unsafe { - (*output).models = models; - (*output).count = count; + let (precisions, precision_count) = into_c_array(precs); + GenieXModelDetail { + name: str_to_cptr(&m.name), + model_name: str_to_cptr(&m.model_name), + plugin_id: str_to_cptr(&m.plugin_id), + model_type: to_ffi_type(m.model_type.clone()), + total_size: m.total_size(), + precisions, + precision_count, } - GENIEX_SUCCESS - } - Err(e) => report(&e), + }) + .collect(); + let (models, count) = into_c_array(details); + unsafe { + (*output).models = models; + (*output).count = count; } + Ok(GENIEX_SUCCESS) }) } @@ -304,30 +229,17 @@ pub unsafe extern "C" fn geniex_model_list_detailed_free( return; } let o = &mut *output; - if !o.models.is_null() { - let slice = std::slice::from_raw_parts_mut(o.models, o.count as usize); - for d in slice.iter_mut() { + if let Some(mut details) = from_c_array(o.models, o.count) { + for d in details.iter_mut() { free_cptr(d.name); free_cptr(d.model_name); free_cptr(d.plugin_id); - if !d.precisions.is_null() { - let precs = - std::slice::from_raw_parts_mut(d.precisions, d.precision_count as usize); - for p in precs.iter_mut() { - free_cptr(*p); + if let Some(precs) = from_c_array(d.precisions, d.precision_count) { + for p in precs { + free_cptr(p); } - drop(Vec::from_raw_parts( - d.precisions, - d.precision_count as usize, - d.precision_count as usize, - )); } } - drop(Vec::from_raw_parts( - o.models, - o.count as usize, - o.count as usize, - )); } o.models = std::ptr::null_mut(); o.count = 0; diff --git a/sdk/model-manager/crates/ffi/src/types.rs b/sdk/model-manager/crates/ffi/src/types.rs index b0b6f6041..f4500c76c 100644 --- a/sdk/model-manager/crates/ffi/src/types.rs +++ b/sdk/model-manager/crates/ffi/src/types.rs @@ -109,25 +109,27 @@ pub fn report(e: &Error) -> i32 { } /// Wrap an FFI entry point so panics can't cross the C boundary. -/// Any panic is logged and converted to `GENIEX_ERROR_COMMON_UNKNOWN`. +/// +/// The body returns `Result`: `Ok(code)` (usually `GENIEX_SUCCESS`) +/// is returned as-is, `Err(code)` is returned as-is — this lets bodies use +/// `?` on `cstr_to_str` / `get_store` / etc. Any panic is logged and +/// converted to `GENIEX_ERROR_COMMON_UNKNOWN`. pub fn ffi_guard(f: F) -> i32 where - F: FnOnce() -> i32, + F: FnOnce() -> Result, { // Clear any message left by a prior call on this thread so // geniex_model_last_error_message only ever reflects THIS call's outcome // (set again by report() / the panic arm below on failure). clear_last_error(); match catch_unwind(AssertUnwindSafe(f)) { - Ok(code) => code, + Ok(Ok(code)) | Ok(Err(code)) => code, Err(payload) => { - let msg = if let Some(s) = payload.downcast_ref::<&'static str>() { - (*s).to_string() - } else if let Some(s) = payload.downcast_ref::() { - s.clone() - } else { - "panic in FFI boundary".to_string() - }; + let msg = payload + .downcast_ref::<&'static str>() + .map(|s| (*s).to_string()) + .or_else(|| payload.downcast_ref::().cloned()) + .unwrap_or_else(|| "panic in FFI boundary".to_string()); logging::error(&format!("panic caught at FFI boundary: {msg}")); set_last_error(&msg); GENIEX_ERROR_COMMON_UNKNOWN @@ -135,13 +137,19 @@ where } } -/// Convert a raw C string pointer to a &str. Returns None if ptr is null or invalid UTF-8. -pub unsafe fn cstr_to_str<'a>(ptr: *const c_char) -> Option<&'a str> { +/// Convert a raw C string pointer to a `&str`. `Err(INVALID_INPUT)` if +/// the pointer is null or the bytes aren't UTF-8. +/// +/// # Safety +/// `ptr` must be null or point to a NUL-terminated C string valid for +/// the lifetime `'a`. +pub unsafe fn cstr_to_str<'a>(ptr: *const c_char) -> Result<&'a str, i32> { if ptr.is_null() { - None - } else { - CStr::from_ptr(ptr).to_str().ok() + return Err(GENIEX_ERROR_COMMON_INVALID_INPUT); } + CStr::from_ptr(ptr) + .to_str() + .map_err(|_| GENIEX_ERROR_COMMON_INVALID_INPUT) } /// Allocate a CString from a Rust &str and return its raw pointer. @@ -157,6 +165,33 @@ pub unsafe fn free_cptr(ptr: *mut c_char) { } } +/// Hand ownership of `v` off to C: shrink, forget, and hand back `(ptr, len)`. +/// Empty vec becomes `(null_mut, 0)`. Every geniex_model_*_free must reclaim +/// via [`from_c_array`] with the same `(T, len)`. +pub fn into_c_array(mut v: Vec) -> (*mut T, i32) { + v.shrink_to_fit(); + let len = v.len() as i32; + if v.is_empty() { + return (std::ptr::null_mut(), 0); + } + let ptr = v.as_mut_ptr(); + std::mem::forget(v); + (ptr, len) +} + +/// Reclaim a Vec previously handed to C by [`into_c_array`]. +/// +/// # Safety +/// `ptr` must either be null, or come from [`into_c_array`] with the same +/// `T` and matching `count`; the caller must guarantee it isn't freed twice. +pub unsafe fn from_c_array(ptr: *mut T, count: i32) -> Option> { + if ptr.is_null() { + return None; + } + let len = count as usize; + Some(Vec::from_raw_parts(ptr, len, len)) +} + /// Canonicalize the `:QUANT` suffix of a model name. Manifest keys are /// produced by `extract_quant`, which upper-cases (so `q4_0` -> `Q4_0`); /// without matching the lookup side, `pull :q4_0` fails for callers From 1fc23d2c5947deabd8725578977650af4936f53b Mon Sep 17 00:00:00 2001 From: Mengsheng Wu Date: Wed, 29 Jul 2026 16:43:54 +0800 Subject: [PATCH 07/14] refactor(sdk): fold local_kind detection into localfs source Signed-off-by: Mengsheng Wu --- .../crates/core/src/source/local_kind.rs | 209 ------------------ .../crates/core/src/source/localfs.rs | 165 ++++++++++++-- .../crates/core/src/source/mod.rs | 1 - 3 files changed, 150 insertions(+), 225 deletions(-) delete mode 100644 sdk/model-manager/crates/core/src/source/local_kind.rs diff --git a/sdk/model-manager/crates/core/src/source/local_kind.rs b/sdk/model-manager/crates/core/src/source/local_kind.rs deleted file mode 100644 index b84b66350..000000000 --- a/sdk/model-manager/crates/core/src/source/local_kind.rs +++ /dev/null @@ -1,209 +0,0 @@ -// Copyright (c) 2026 Qualcomm Technologies, Inc. and/or its subsidiaries. -// SPDX-License-Identifier: BSD-3-Clause - -//! Auto-detect the layout of a `local_path` passed to [`LocalFsSource`]. -//! -//! Three known shapes today: -//! -//! - `HfGguf` — a directory whose model weights are GGUF (the existing -//! pre-AI-Hub path). -//! - `AiHubExtracted` — a directory matching what `aihub.ExtractFlat` / -//! the user produces by unzipping an AI Hub asset: at least one `.bin` -//! shard and a `metadata.json` at the root. -//! - `AiHubZip` — a `.zip` file straight off the AI Hub website. -//! -//! Order of detection matters: a `.zip` file is identified by its -//! extension before we look inside, and AI Hub directories take -//! precedence over GGUF because nothing prevents an AI Hub release from -//! eventually shipping a sibling GGUF file alongside `.bin` shards. - -use std::ffi::OsStr; -use std::fs; -use std::path::Path; - -use crate::error::{Error, Result}; - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum LocalKind { - HfGguf, - AiHubExtracted, - AiHubZip, -} - -const AIHUB_METADATA_FILE: &str = "metadata.json"; - -/// Inspect `path` and decide which loader the [`LocalFsSource`] should -/// dispatch to. -/// -/// Returns [`Error::Hub`] when `path` is neither a recognised directory -/// layout nor a `.zip` file; the message lists the three shapes the -/// loader knows how to handle so a user pointing at the wrong directory -/// gets actionable feedback. -pub fn detect(path: &Path) -> Result { - let meta = fs::metadata(path).map_err(|e| { - Error::Hub(format!( - "local path {} is not accessible: {e}", - path.display() - )) - })?; - - if meta.is_file() { - if has_extension(path, "zip") { - return Ok(LocalKind::AiHubZip); - } - return Err(Error::Hub(format!( - "local path {} is a file but not a .zip; expected an AI Hub archive or a directory", - path.display() - ))); - } - - if !meta.is_dir() { - return Err(Error::Hub(format!( - "local path {} is neither a regular file nor a directory", - path.display() - ))); - } - - let mut has_bin = false; - let mut has_metadata = false; - let mut has_gguf = false; - let mut has_safetensors = false; - for entry in fs::read_dir(path)?.flatten() { - let ft = match entry.file_type() { - Ok(t) => t, - Err(_) => continue, - }; - if !ft.is_file() { - continue; - } - let Some(name) = entry.file_name().to_str().map(str::to_string) else { - continue; - }; - let lower = name.to_ascii_lowercase(); - if lower == AIHUB_METADATA_FILE { - has_metadata = true; - } else if lower.ends_with(".bin") { - has_bin = true; - } else if lower.ends_with(".gguf") { - has_gguf = true; - } else if lower.ends_with(".safetensors") { - has_safetensors = true; - } - } - - if has_metadata && has_bin { - return Ok(LocalKind::AiHubExtracted); - } - if has_gguf { - return Ok(LocalKind::HfGguf); - } - if has_safetensors { - return Err(Error::Hub(format!( - "local path {} looks like a HuggingFace safetensors snapshot, \ - which is not supported as a local pull source yet", - path.display() - ))); - } - Err(Error::Hub(format!( - "local path {} did not match any known layout: \ - expected a directory with *.gguf (HF GGUF), \ - a directory with metadata.json + *.bin (AI Hub extracted), \ - or a .zip file (AI Hub archive)", - path.display() - ))) -} - -fn has_extension(path: &Path, ext: &str) -> bool { - path.extension() - .and_then(OsStr::to_str) - .map(|e| e.eq_ignore_ascii_case(ext)) - .unwrap_or(false) -} - -#[cfg(test)] -mod tests { - use super::*; - use std::fs; - - #[test] - fn detects_aihub_zip_by_extension() { - let tmp = tempfile::tempdir().unwrap(); - let p = tmp.path().join("model.zip"); - fs::write(&p, b"PK\x03\x04not-a-real-zip").unwrap(); - assert_eq!(detect(&p).unwrap(), LocalKind::AiHubZip); - } - - #[test] - fn detects_aihub_zip_case_insensitive() { - let tmp = tempfile::tempdir().unwrap(); - let p = tmp.path().join("Model.ZIP"); - fs::write(&p, b"x").unwrap(); - assert_eq!(detect(&p).unwrap(), LocalKind::AiHubZip); - } - - #[test] - fn detects_aihub_extracted_via_metadata_and_bin() { - let tmp = tempfile::tempdir().unwrap(); - fs::write(tmp.path().join("metadata.json"), b"{}").unwrap(); - fs::write(tmp.path().join("weights_part_1.bin"), b"x").unwrap(); - fs::write(tmp.path().join("weights_part_2.bin"), b"y").unwrap(); - assert_eq!(detect(tmp.path()).unwrap(), LocalKind::AiHubExtracted); - } - - #[test] - fn detects_hf_gguf_dir() { - let tmp = tempfile::tempdir().unwrap(); - fs::write(tmp.path().join("model-Q4_K_M.gguf"), b"x").unwrap(); - assert_eq!(detect(tmp.path()).unwrap(), LocalKind::HfGguf); - } - - #[test] - fn aihub_extracted_wins_over_gguf_sibling() { - // Forward-compat: if AI Hub ever ships both, `metadata.json + .bin` - // is the load-bearing signal — GGUF inference would get plugin_id - // wrong. - let tmp = tempfile::tempdir().unwrap(); - fs::write(tmp.path().join("metadata.json"), b"{}").unwrap(); - fs::write(tmp.path().join("weights.bin"), b"x").unwrap(); - fs::write(tmp.path().join("extra.gguf"), b"y").unwrap(); - assert_eq!(detect(tmp.path()).unwrap(), LocalKind::AiHubExtracted); - } - - #[test] - fn safetensors_only_dir_returns_unsupported_error() { - let tmp = tempfile::tempdir().unwrap(); - fs::write(tmp.path().join("config.json"), b"{}").unwrap(); - fs::write(tmp.path().join("model.safetensors"), b"x").unwrap(); - let err = detect(tmp.path()).unwrap_err(); - let msg = format!("{err}"); - assert!(msg.contains("safetensors"), "msg: {msg}"); - } - - #[test] - fn unknown_dir_lists_known_layouts() { - let tmp = tempfile::tempdir().unwrap(); - fs::write(tmp.path().join("readme.txt"), b"hi").unwrap(); - let err = detect(tmp.path()).unwrap_err(); - let msg = format!("{err}"); - assert!(msg.contains("AI Hub extracted"), "msg: {msg}"); - assert!(msg.contains("HF GGUF"), "msg: {msg}"); - assert!(msg.contains(".zip"), "msg: {msg}"); - } - - #[test] - fn nonexistent_path_returns_helpful_error() { - let err = detect(Path::new("/nonexistent/path/12345xyz")).unwrap_err(); - let msg = format!("{err}"); - assert!(msg.contains("not accessible"), "msg: {msg}"); - } - - #[test] - fn non_zip_file_rejected() { - let tmp = tempfile::tempdir().unwrap(); - let p = tmp.path().join("model.tar.gz"); - fs::write(&p, b"x").unwrap(); - let err = detect(&p).unwrap_err(); - let msg = format!("{err}"); - assert!(msg.contains("not a .zip"), "msg: {msg}"); - } -} diff --git a/sdk/model-manager/crates/core/src/source/localfs.rs b/sdk/model-manager/crates/core/src/source/localfs.rs index 232586c18..faf2ba2fb 100644 --- a/sdk/model-manager/crates/core/src/source/localfs.rs +++ b/sdk/model-manager/crates/core/src/source/localfs.rs @@ -5,22 +5,16 @@ //! //! Given a `source_dir` (or a local archive file), reads its layout and //! produces a [`Plan`] whose [`BytesSource`]s point at on-disk bytes. -//! Three layouts are recognised by [`local_kind::detect`]: +//! Three layouts are recognised by [`detect_local_kind`]: //! -//! - HF GGUF directory — existing path: read shipped `geniex.json` if -//! present, else infer via [`infer_manifest_from_names`]. Files emit -//! as [`BytesSource::Local`]. -//! - AI Hub extracted directory — `metadata.json` + `.bin` shards. -//! Plugin id is forced to `qairt`, modality comes from -//! [`classify_from_metadata_json`], lex-first `.bin` is the -//! entrypoint. Mirrors what the remote AI Hub source produces. -//! - AI Hub local `.zip` — feed the existing -//! [`fetch_central_directory`] parser through a [`LocalFileTransport`] -//! adapter, then emit [`BytesSource::LocalRange`] for STORED entries -//! and [`BytesSource::LocalDeflate`] for DEFLATE entries. +//! - HF GGUF directory +//! - AI Hub extracted directory (`metadata.json` + `.bin` shards) +//! - AI Hub local `.zip` use std::collections::HashMap; -use std::path::PathBuf; +use std::ffi::OsStr; +use std::fs; +use std::path::{Path, PathBuf}; use std::sync::Arc; use async_trait::async_trait; @@ -34,7 +28,6 @@ use crate::transport::HttpTransport; use super::ai_hub::local_transport::LocalFileTransport; use super::ai_hub::remote_zip::{fetch_central_directory, Method}; use super::ai_hub::{classify_from_metadata_json, prepare_flat_entries}; -use super::local_kind::{detect, LocalKind}; use super::{BytesSource, FileSpec, ModelSource, Plan}; const MANIFEST_FILE: &str = "geniex.json"; @@ -42,6 +35,91 @@ const CONFIG_FILE: &str = "config.json"; const AIHUB_METADATA_FILE: &str = "metadata.json"; const QAIRT_PLUGIN_ID: &str = "qairt"; +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum LocalKind { + HfGguf, + AiHubExtracted, + AiHubZip, +} + +/// Inspect `path` and decide which loader to dispatch to. Returns +/// [`Error::Hub`] with an actionable message when nothing matches. +fn detect_local_kind(path: &Path) -> Result { + let meta = fs::metadata(path).map_err(|e| { + Error::Hub(format!( + "local path {} is not accessible: {e}", + path.display() + )) + })?; + + if meta.is_file() { + if has_extension(path, "zip") { + return Ok(LocalKind::AiHubZip); + } + return Err(Error::Hub(format!( + "local path {} is a file but not a .zip; expected an AI Hub archive or a directory", + path.display() + ))); + } + if !meta.is_dir() { + return Err(Error::Hub(format!( + "local path {} is neither a regular file nor a directory", + path.display() + ))); + } + + let mut has_bin = false; + let mut has_metadata = false; + let mut has_gguf = false; + let mut has_safetensors = false; + for entry in fs::read_dir(path)?.flatten() { + let Ok(ft) = entry.file_type() else { continue }; + if !ft.is_file() { + continue; + } + let Some(name) = entry.file_name().to_str().map(str::to_ascii_lowercase) else { + continue; + }; + if name == AIHUB_METADATA_FILE { + has_metadata = true; + } else if name.ends_with(".bin") { + has_bin = true; + } else if name.ends_with(".gguf") { + has_gguf = true; + } else if name.ends_with(".safetensors") { + has_safetensors = true; + } + } + + if has_metadata && has_bin { + return Ok(LocalKind::AiHubExtracted); + } + if has_gguf { + return Ok(LocalKind::HfGguf); + } + if has_safetensors { + return Err(Error::Hub(format!( + "local path {} looks like a HuggingFace safetensors snapshot, \ + which is not supported as a local pull source yet", + path.display() + ))); + } + Err(Error::Hub(format!( + "local path {} did not match any known layout: \ + expected a directory with *.gguf (HF GGUF), \ + a directory with metadata.json + *.bin (AI Hub extracted), \ + or a .zip file (AI Hub archive)", + path.display() + ))) +} + +fn has_extension(path: &Path, ext: &str) -> bool { + path.extension() + .and_then(OsStr::to_str) + .map(|e| e.eq_ignore_ascii_case(ext)) + .unwrap_or(false) +} + pub struct LocalFsSource { source_dir: PathBuf, model_name: String, @@ -61,7 +139,7 @@ impl LocalFsSource { #[async_trait] impl ModelSource for LocalFsSource { async fn plan(&self) -> Result { - match detect(&self.source_dir)? { + match detect_local_kind(&self.source_dir)? { LocalKind::HfGguf => self.plan_hf_gguf(), LocalKind::AiHubExtracted => self.plan_ai_hub_extracted(), LocalKind::AiHubZip => self.plan_ai_hub_zip().await, @@ -681,4 +759,61 @@ mod tests { let msg = format!("{err}"); assert!(msg.contains("safetensors"), "msg: {msg}"); } + + // ---------------- detect_local_kind ---------------- + + #[test] + fn detect_local_kind_covers_layouts() { + let tmp = tempfile::tempdir().unwrap(); + + let zip = tmp.path().join("model.zip"); + fs::write(&zip, b"PK\x03\x04").unwrap(); + assert_eq!(detect_local_kind(&zip).unwrap(), LocalKind::AiHubZip); + + let zip_upper = tmp.path().join("Model.ZIP"); + fs::write(&zip_upper, b"x").unwrap(); + assert_eq!(detect_local_kind(&zip_upper).unwrap(), LocalKind::AiHubZip); + + let aihub = tmp.path().join("aihub"); + fs::create_dir_all(&aihub).unwrap(); + fs::write(aihub.join("metadata.json"), b"{}").unwrap(); + fs::write(aihub.join("weights_part_1.bin"), b"x").unwrap(); + // AI Hub wins even with a sibling GGUF. + fs::write(aihub.join("extra.gguf"), b"y").unwrap(); + assert_eq!( + detect_local_kind(&aihub).unwrap(), + LocalKind::AiHubExtracted + ); + + let gguf = tmp.path().join("gguf"); + fs::create_dir_all(&gguf).unwrap(); + fs::write(gguf.join("model-Q4_K_M.gguf"), b"x").unwrap(); + assert_eq!(detect_local_kind(&gguf).unwrap(), LocalKind::HfGguf); + } + + #[test] + fn detect_local_kind_error_messages_are_actionable() { + let tmp = tempfile::tempdir().unwrap(); + + let missing = detect_local_kind(Path::new("/nonexistent/path/12345xyz")).unwrap_err(); + assert!(format!("{missing}").contains("not accessible")); + + let non_zip = tmp.path().join("model.tar.gz"); + fs::write(&non_zip, b"x").unwrap(); + assert!(format!("{}", detect_local_kind(&non_zip).unwrap_err()).contains("not a .zip")); + + let safetensors = tmp.path().join("st"); + fs::create_dir_all(&safetensors).unwrap(); + fs::write(safetensors.join("config.json"), b"{}").unwrap(); + fs::write(safetensors.join("model.safetensors"), b"x").unwrap(); + assert!( + format!("{}", detect_local_kind(&safetensors).unwrap_err()).contains("safetensors") + ); + + let unknown = tmp.path().join("unk"); + fs::create_dir_all(&unknown).unwrap(); + fs::write(unknown.join("readme.txt"), b"hi").unwrap(); + let msg = format!("{}", detect_local_kind(&unknown).unwrap_err()); + assert!(msg.contains("AI Hub extracted") && msg.contains("HF GGUF") && msg.contains(".zip")); + } } diff --git a/sdk/model-manager/crates/core/src/source/mod.rs b/sdk/model-manager/crates/core/src/source/mod.rs index 16ecabd65..30a63ee17 100644 --- a/sdk/model-manager/crates/core/src/source/mod.rs +++ b/sdk/model-manager/crates/core/src/source/mod.rs @@ -18,7 +18,6 @@ pub mod ai_hub; pub mod dockerhub; pub mod hf; -pub mod local_kind; pub mod localfs; use std::path::PathBuf; From 8827c3a6075e68ecc60ff52d4733e737c0e61715 Mon Sep 17 00:00:00 2001 From: Mengsheng Wu Date: Wed, 29 Jul 2026 16:46:00 +0800 Subject: [PATCH 08/14] refactor(sdk): fold local_transport into remote_zip module Signed-off-by: Mengsheng Wu --- .../core/src/source/ai_hub/local_transport.rs | 169 ------------------ .../crates/core/src/source/ai_hub/mod.rs | 1 - .../core/src/source/ai_hub/remote_zip.rs | 125 ++++++++++++- .../crates/core/src/source/localfs.rs | 3 +- 4 files changed, 124 insertions(+), 174 deletions(-) delete mode 100644 sdk/model-manager/crates/core/src/source/ai_hub/local_transport.rs diff --git a/sdk/model-manager/crates/core/src/source/ai_hub/local_transport.rs b/sdk/model-manager/crates/core/src/source/ai_hub/local_transport.rs deleted file mode 100644 index 2133dfe0e..000000000 --- a/sdk/model-manager/crates/core/src/source/ai_hub/local_transport.rs +++ /dev/null @@ -1,169 +0,0 @@ -// Copyright (c) 2026 Qualcomm Technologies, Inc. and/or its subsidiaries. -// SPDX-License-Identifier: BSD-3-Clause - -//! [`HttpTransport`] adapter backed by a single local file. -//! -//! [`fetch_central_directory`](super::remote_zip::fetch_central_directory) -//! is written against the [`HttpTransport`] trait so any byte source — -//! reqwest, wiremock, or a local file — can drive it. This module -//! provides the third option: when the user pulls from a `.zip` already -//! sitting on disk, we wrap the file as a transport whose `head` -//! returns its length and whose `get_range` becomes a `seek` + `read`. -//! The `Url` parameter is ignored. - -use std::path::{Path, PathBuf}; - -use async_trait::async_trait; -use tokio::io::{AsyncSeekExt, AsyncWrite, AsyncWriteExt}; -use url::Url; - -use crate::error::{Error, Result}; -use crate::transport::{HeadInfo, HttpTransport}; - -#[derive(Debug)] -pub struct LocalFileTransport { - path: PathBuf, - len: u64, -} - -impl LocalFileTransport { - pub fn open(path: &Path) -> Result { - let meta = std::fs::metadata(path).map_err(|e| { - Error::Hub(format!( - "local archive {} is not accessible: {e}", - path.display() - )) - })?; - if !meta.is_file() { - return Err(Error::Hub(format!( - "local archive {} is not a regular file", - path.display() - ))); - } - Ok(Self { - path: path.to_path_buf(), - len: meta.len(), - }) - } - - pub fn path(&self) -> &Path { - &self.path - } -} - -#[async_trait] -impl HttpTransport for LocalFileTransport { - async fn head(&self, _url: &Url, _auth: Option<&str>) -> Result { - Ok(HeadInfo { - size: self.len, - accepts_ranges: true, - etag: None, - }) - } - - async fn get_range( - &self, - _url: &Url, - _auth: Option<&str>, - offset: u64, - len: u64, - sink: &mut (dyn AsyncWrite + Unpin + Send), - ) -> Result<()> { - if len == 0 { - return Ok(()); - } - if offset - .checked_add(len) - .map(|end| end > self.len) - .unwrap_or(true) - { - return Err(Error::Hub(format!( - "local range {offset}+{len} exceeds archive size {}", - self.len - ))); - } - - let mut file = tokio::fs::File::open(&self.path).await?; - file.seek(std::io::SeekFrom::Start(offset)).await?; - let mut remaining = len; - let mut buf = vec![0u8; 64 * 1024]; - while remaining > 0 { - let want = remaining.min(buf.len() as u64) as usize; - let n = tokio::io::AsyncReadExt::read(&mut file, &mut buf[..want]).await?; - if n == 0 { - return Err(Error::Hub(format!( - "local archive {}: unexpected EOF at offset {}", - self.path.display(), - offset + (len - remaining) - ))); - } - sink.write_all(&buf[..n]) - .await - .map_err(|e| Error::Http(format!("write sink: {e}")))?; - remaining -= n as u64; - } - sink.flush() - .await - .map_err(|e| Error::Http(format!("flush sink: {e}")))?; - Ok(()) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use std::sync::Arc; - - async fn read_all(t: &Arc, off: u64, len: u64) -> Vec { - let dummy = Url::parse("file:///dummy").unwrap(); - let mut buf: Vec = Vec::new(); - t.get_range(&dummy, None, off, len, &mut buf).await.unwrap(); - buf - } - - #[tokio::test] - async fn head_returns_file_length() { - let tmp = tempfile::tempdir().unwrap(); - let p = tmp.path().join("a.bin"); - std::fs::write(&p, b"abcdef").unwrap(); - let t = LocalFileTransport::open(&p).unwrap(); - let dummy = Url::parse("file:///dummy").unwrap(); - let info = t.head(&dummy, None).await.unwrap(); - assert_eq!(info.size, 6); - assert!(info.accepts_ranges); - } - - #[tokio::test] - async fn get_range_returns_exact_slice() { - let tmp = tempfile::tempdir().unwrap(); - let p = tmp.path().join("a.bin"); - std::fs::write(&p, b"abcdef").unwrap(); - let t: Arc = Arc::new(LocalFileTransport::open(&p).unwrap()); - assert_eq!(read_all(&t, 0, 3).await, b"abc"); - assert_eq!(read_all(&t, 2, 4).await, b"cdef"); - assert_eq!(read_all(&t, 5, 1).await, b"f"); - } - - #[tokio::test] - async fn out_of_bounds_range_rejected() { - let tmp = tempfile::tempdir().unwrap(); - let p = tmp.path().join("a.bin"); - std::fs::write(&p, b"abcdef").unwrap(); - let t = LocalFileTransport::open(&p).unwrap(); - let dummy = Url::parse("file:///dummy").unwrap(); - let mut buf: Vec = Vec::new(); - let err = t - .get_range(&dummy, None, 5, 10, &mut buf) - .await - .unwrap_err(); - let msg = format!("{err}"); - assert!(msg.contains("exceeds"), "msg: {msg}"); - } - - #[tokio::test] - async fn missing_file_rejected_at_open() { - let err = LocalFileTransport::open(Path::new("/nonexistent/zzz.zip")).unwrap_err(); - let msg = format!("{err}"); - assert!(msg.contains("not accessible"), "msg: {msg}"); - } -} diff --git a/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs b/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs index 5277eacf3..9a6d8330b 100644 --- a/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs +++ b/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs @@ -10,7 +10,6 @@ //! downloading the multi-GB payload. pub mod detect; -pub mod local_transport; pub mod manifest; pub mod remote_zip; pub mod selector; diff --git a/sdk/model-manager/crates/core/src/source/ai_hub/remote_zip.rs b/sdk/model-manager/crates/core/src/source/ai_hub/remote_zip.rs index 4a8b47423..b90b33b46 100644 --- a/sdk/model-manager/crates/core/src/source/ai_hub/remote_zip.rs +++ b/sdk/model-manager/crates/core/src/source/ai_hub/remote_zip.rs @@ -29,12 +29,15 @@ //! extra + comment. //! - Local file header: 0x04034b50, fixed 30 bytes + name + extra. +use std::path::{Path, PathBuf}; use std::sync::Arc; +use async_trait::async_trait; +use tokio::io::{AsyncSeekExt, AsyncWrite, AsyncWriteExt}; use url::Url; use crate::error::{Error, Result}; -use crate::transport::HttpTransport; +use crate::transport::{HeadInfo, HttpTransport}; const EOCD_SIG: u32 = 0x0605_4b50; const ZIP64_EOCD_LOCATOR_SIG: u32 = 0x0706_4b50; @@ -317,12 +320,98 @@ fn read_u64(buf: &[u8], off: usize) -> u64 { ]) } +/// [`HttpTransport`] adapter that serves a single local file. Used by the +/// LocalFS source to feed [`fetch_central_directory`] when the archive is +/// already on disk; the `Url` argument is ignored. +#[derive(Debug)] +pub struct LocalFileTransport { + path: PathBuf, + len: u64, +} + +impl LocalFileTransport { + pub fn open(path: &Path) -> Result { + let meta = std::fs::metadata(path).map_err(|e| { + Error::Hub(format!( + "local archive {} is not accessible: {e}", + path.display() + )) + })?; + if !meta.is_file() { + return Err(Error::Hub(format!( + "local archive {} is not a regular file", + path.display() + ))); + } + Ok(Self { + path: path.to_path_buf(), + len: meta.len(), + }) + } +} + +#[async_trait] +impl HttpTransport for LocalFileTransport { + async fn head(&self, _url: &Url, _auth: Option<&str>) -> Result { + Ok(HeadInfo { + size: self.len, + accepts_ranges: true, + etag: None, + }) + } + + async fn get_range( + &self, + _url: &Url, + _auth: Option<&str>, + offset: u64, + len: u64, + sink: &mut (dyn AsyncWrite + Unpin + Send), + ) -> Result<()> { + if len == 0 { + return Ok(()); + } + let end = offset + .checked_add(len) + .ok_or_else(|| Error::Hub(format!("range {offset}+{len} overflows u64")))?; + if end > self.len { + return Err(Error::Hub(format!( + "local range {offset}+{len} exceeds archive size {}", + self.len + ))); + } + + let mut file = tokio::fs::File::open(&self.path).await?; + file.seek(std::io::SeekFrom::Start(offset)).await?; + let mut remaining = len; + let mut buf = vec![0u8; 64 * 1024]; + while remaining > 0 { + let want = remaining.min(buf.len() as u64) as usize; + let n = tokio::io::AsyncReadExt::read(&mut file, &mut buf[..want]).await?; + if n == 0 { + return Err(Error::Hub(format!( + "local archive {}: unexpected EOF at offset {}", + self.path.display(), + offset + (len - remaining) + ))); + } + sink.write_all(&buf[..n]) + .await + .map_err(|e| Error::Http(format!("write sink: {e}")))?; + remaining -= n as u64; + } + sink.flush() + .await + .map_err(|e| Error::Http(format!("flush sink: {e}")))?; + Ok(()) + } +} + #[cfg(test)] mod tests { use super::*; use crate::transport::{ReqwestTransport, TransportConfig}; use std::io::Write as IoWrite; - use std::sync::Arc; use std::time::Duration; use wiremock::matchers::{method, path}; use wiremock::{Mock, MockServer, Request, ResponseTemplate}; @@ -440,4 +529,36 @@ mod tests { let msg = format!("{err}"); assert!(msg.contains("EOCD"), "unexpected error: {msg}"); } + + // ---------------- LocalFileTransport ---------------- + + #[tokio::test] + async fn local_transport_serves_ranges_and_rejects_oob() { + let tmp = tempfile::tempdir().unwrap(); + let p = tmp.path().join("a.bin"); + std::fs::write(&p, b"abcdef").unwrap(); + let t = LocalFileTransport::open(&p).unwrap(); + let dummy = Url::parse("file:///dummy").unwrap(); + + let info = t.head(&dummy, None).await.unwrap(); + assert_eq!(info.size, 6); + assert!(info.accepts_ranges); + + let mut buf = Vec::new(); + t.get_range(&dummy, None, 2, 4, &mut buf).await.unwrap(); + assert_eq!(buf, b"cdef"); + + let mut buf = Vec::new(); + let err = t + .get_range(&dummy, None, 5, 10, &mut buf) + .await + .unwrap_err(); + assert!(format!("{err}").contains("exceeds")); + } + + #[test] + fn local_transport_open_rejects_missing() { + let err = LocalFileTransport::open(Path::new("/nonexistent/zzz.zip")).unwrap_err(); + assert!(format!("{err}").contains("not accessible")); + } } diff --git a/sdk/model-manager/crates/core/src/source/localfs.rs b/sdk/model-manager/crates/core/src/source/localfs.rs index faf2ba2fb..4b9238776 100644 --- a/sdk/model-manager/crates/core/src/source/localfs.rs +++ b/sdk/model-manager/crates/core/src/source/localfs.rs @@ -25,8 +25,7 @@ use crate::manifest::{ModelFileInfo, ModelManifest, ModelType}; use crate::manifest_builder::{infer_manifest_from_names, ManifestHint}; use crate::transport::HttpTransport; -use super::ai_hub::local_transport::LocalFileTransport; -use super::ai_hub::remote_zip::{fetch_central_directory, Method}; +use super::ai_hub::remote_zip::{fetch_central_directory, LocalFileTransport, Method}; use super::ai_hub::{classify_from_metadata_json, prepare_flat_entries}; use super::{BytesSource, FileSpec, ModelSource, Plan}; From 99901b48633408692dddcb1be92d8ff4add861d8 Mon Sep 17 00:00:00 2001 From: Mengsheng Wu Date: Wed, 29 Jul 2026 16:47:17 +0800 Subject: [PATCH 09/14] refactor(sdk): rename ai_hub/manifest to ai_hub/dto Signed-off-by: Mengsheng Wu --- .../crates/core/src/source/ai_hub/{manifest.rs => dto.rs} | 0 sdk/model-manager/crates/core/src/source/ai_hub/mod.rs | 6 +++--- sdk/model-manager/crates/core/src/source/ai_hub/selector.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) rename sdk/model-manager/crates/core/src/source/ai_hub/{manifest.rs => dto.rs} (100%) diff --git a/sdk/model-manager/crates/core/src/source/ai_hub/manifest.rs b/sdk/model-manager/crates/core/src/source/ai_hub/dto.rs similarity index 100% rename from sdk/model-manager/crates/core/src/source/ai_hub/manifest.rs rename to sdk/model-manager/crates/core/src/source/ai_hub/dto.rs diff --git a/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs b/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs index 9a6d8330b..030c88c38 100644 --- a/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs +++ b/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs @@ -10,7 +10,7 @@ //! downloading the multi-GB payload. pub mod detect; -pub mod manifest; +pub mod dto; pub mod remote_zip; pub mod selector; @@ -26,7 +26,7 @@ use crate::error::{Error, Result}; use crate::manifest::{ModelFileInfo, ModelManifest, ModelType}; use crate::transport::{HttpTransport, ReqwestTransport}; -use self::manifest::{ +use self::dto::{ ChipsetInfo, InfoJson, ManifestModelEntry, ModelReleaseAssets, PlatformInfo, ReleaseManifest, }; use self::remote_zip::{fetch_central_directory, Method, ZipEntry}; @@ -558,7 +558,7 @@ fn classify_ai_hub(info: Option<&InfoJson>, entry: &ManifestModelEntry) -> Model #[cfg(test)] mod tests { - use self::manifest::ManifestUrls; + use self::dto::ManifestUrls; use super::*; fn entry(id: &str, domain: &str) -> ManifestModelEntry { diff --git a/sdk/model-manager/crates/core/src/source/ai_hub/selector.rs b/sdk/model-manager/crates/core/src/source/ai_hub/selector.rs index ce34238b4..1ea697c04 100644 --- a/sdk/model-manager/crates/core/src/source/ai_hub/selector.rs +++ b/sdk/model-manager/crates/core/src/source/ai_hub/selector.rs @@ -17,7 +17,7 @@ //! (`PRECISION_FP16` before `PRECISION_W4A16`, etc.). This is a //! best-effort tiebreaker; precision selection is not exposed over FFI. -use super::manifest::{AssetDetails, ModelReleaseAssets, PlatformInfo}; +use super::dto::{AssetDetails, ModelReleaseAssets, PlatformInfo}; use crate::error::{Error, Result}; /// Runtime string the public bucket uses for Genie-compatible assets. @@ -162,7 +162,7 @@ impl std::fmt::Display for UnavailableChipset { #[cfg(test)] mod tests { use super::*; - use crate::source::ai_hub::manifest::ChipsetInfo; + use crate::source::ai_hub::dto::ChipsetInfo; fn platform(entries: &[(&str, &[&str])]) -> PlatformInfo { PlatformInfo { From 42463dec9dc75ba615ff777b3e62811f8686a99e Mon Sep 17 00:00:00 2001 From: Mengsheng Wu Date: Thu, 30 Jul 2026 22:09:58 +0800 Subject: [PATCH 10/14] refactor(sdk): share entrypoint/basename helpers, drop C smoke test Signed-off-by: Mengsheng Wu --- .../crates/core/src/manifest_builder.rs | 38 +-- .../crates/core/src/source/ai_hub/mod.rs | 46 +--- .../crates/core/src/source/dockerhub.rs | 6 +- .../crates/core/src/source/localfs.rs | 94 ++----- .../crates/core/src/source/mod.rs | 49 +++- sdk/model-manager/tests/test_model_manager.c | 232 ------------------ sdk/src/CMakeLists.txt | 12 - 7 files changed, 75 insertions(+), 402 deletions(-) delete mode 100644 sdk/model-manager/tests/test_model_manager.c diff --git a/sdk/model-manager/crates/core/src/manifest_builder.rs b/sdk/model-manager/crates/core/src/manifest_builder.rs index 6049d008c..7f8d5a5c7 100644 --- a/sdk/model-manager/crates/core/src/manifest_builder.rs +++ b/sdk/model-manager/crates/core/src/manifest_builder.rs @@ -8,7 +8,6 @@ //! pipeline can operate uniformly. use std::collections::HashMap; -use std::path::Path; use serde::Deserialize; @@ -37,40 +36,9 @@ pub struct ManifestHint { /// head as the recommended pick. pub const QUANT_PRIORITY: &[&str] = &["Q4_0", "Q4_K_M", "Q8_0"]; -/// Infer a manifest by scanning `src_dir` for model files. -pub fn infer_manifest_from_dir( - name: &str, - src_dir: &Path, - hint: ManifestHint, -) -> Result { - let mut file_names: Vec = Vec::new(); - for entry in std::fs::read_dir(src_dir)?.flatten() { - let ft = match entry.file_type() { - Ok(t) => t, - Err(_) => continue, - }; - if !ft.is_file() { - continue; - } - if let Some(n) = entry.file_name().to_str().map(str::to_string) { - file_names.push(n); - } - } - - let mut sizes: HashMap = HashMap::new(); - for n in &file_names { - let size = std::fs::metadata(src_dir.join(n)) - .map(|m| m.len() as i64) - .unwrap_or(0); - sizes.insert(n.clone(), size); - } - - infer_manifest_from_names(name, &file_names, &sizes, hint) -} - -/// Same logic as [`infer_manifest_from_dir`] but driven by an explicit -/// file list and size map — useful when the caller already has the remote -/// listing in hand (e.g. `HfHub::model_info`). +/// Infer a manifest from an explicit list of filenames + their sizes. +/// Used when the caller already holds the listing (HF `model_info`, +/// LocalFS `read_dir`, etc.). pub fn infer_manifest_from_names( name: &str, file_names: &[String], diff --git a/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs b/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs index 030c88c38..b4faf1320 100644 --- a/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs +++ b/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs @@ -32,7 +32,7 @@ use self::dto::{ use self::remote_zip::{fetch_central_directory, Method, ZipEntry}; use self::selector::{match_asset, UnavailableChipset}; -use super::{BytesSource, FileSpec, ModelSource, Plan}; +use super::{basename, BytesSource, FileSpec, ModelSource, Plan}; const MANIFEST_FILENAME: &str = "manifest.json"; const PLATFORM_FILENAME: &str = "platform.json"; @@ -265,13 +265,12 @@ impl ModelSource for AiHubSource { let raw_entries = fetch_central_directory(&self.transport, &download_url).await?; let flat_entries = prepare_flat_entries(&raw_entries)?; - // Lex-first `.bin` matches the Go CLI's `ExtractFlat`, so a - // cache populated by either agent is interchangeable. - let entrypoint_name = flat_entries - .iter() - .find(|(name, _)| name.to_ascii_lowercase().ends_with(".bin")) - .map(|(name, _)| name.clone()) - .ok_or_else(|| Error::Hub("no .bin shard in archive".to_string()))?; + // Lex-first `.bin` matches the Go CLI's `ExtractFlat`. + let (model_file, extra_files) = super::split_entrypoint_and_extras( + &flat_entries, + || "no .bin shard in archive".to_string(), + |e| e.uncompressed_size as i64, + )?; let precision_label = asset .precision @@ -279,33 +278,6 @@ impl ModelSource for AiHubSource { .unwrap_or(&asset.precision) .to_string(); - // Each bucket holds a single file's own size — sibling sizes live in - // extra_files. Aggregating here would double-count via total_size(). - let entrypoint_size = flat_entries - .iter() - .find(|(name, _)| name == &entrypoint_name) - .map(|(_, e)| e.uncompressed_size as i64) - .unwrap_or(0); - - let mut model_file: HashMap = HashMap::new(); - model_file.insert( - "N/A".to_string(), - ModelFileInfo { - name: entrypoint_name.clone(), - downloaded: true, - size: entrypoint_size, - }, - ); - let extra_files: Vec = flat_entries - .iter() - .filter(|(name, _)| name != &entrypoint_name) - .map(|(name, e)| ModelFileInfo { - name: name.clone(), - downloaded: true, - size: e.uncompressed_size as i64, - }) - .collect(); - // `domain` alone cannot distinguish Qwen2.5-VL from text-only // LLMs (both report MODEL_DOMAIN_GENERATIVE_AI), so we also // read the per-model info.json. Fetch failure is non-fatal: @@ -418,10 +390,6 @@ pub(crate) fn prepare_flat_entries(raw: &[ZipEntry]) -> Result String { - path.rsplit(['/', '\\']).next().unwrap_or("").to_string() -} - fn is_macos_metadata(path: &str) -> bool { let normalized = path.replace('\\', "/"); if normalized.starts_with("__MACOSX/") || normalized.contains("/__MACOSX/") { diff --git a/sdk/model-manager/crates/core/src/source/dockerhub.rs b/sdk/model-manager/crates/core/src/source/dockerhub.rs index 864705d9b..4ac89e992 100644 --- a/sdk/model-manager/crates/core/src/source/dockerhub.rs +++ b/sdk/model-manager/crates/core/src/source/dockerhub.rs @@ -36,7 +36,7 @@ use crate::manifest::{ModelFileInfo, ModelManifest, ModelType}; use crate::manifest_builder::extract_quant; use crate::transport::{build_tls_config, HttpTransport, USER_AGENT}; -use super::{BytesSource, FileSpec, ModelSource, Plan}; +use super::{basename, BytesSource, FileSpec, ModelSource, Plan}; pub const DEFAULT_REGISTRY_ENDPOINT: &str = "https://registry-1.docker.io"; pub const DEFAULT_AUTH_ENDPOINT: &str = "https://auth.docker.io/token"; @@ -539,10 +539,6 @@ fn build_plan( Ok(Plan { manifest, files }) } -fn basename(path: &str) -> String { - path.rsplit(['/', '\\']).next().unwrap_or(path).to_string() -} - #[cfg(test)] mod tests { use super::*; diff --git a/sdk/model-manager/crates/core/src/source/localfs.rs b/sdk/model-manager/crates/core/src/source/localfs.rs index 4b9238776..d2391d753 100644 --- a/sdk/model-manager/crates/core/src/source/localfs.rs +++ b/sdk/model-manager/crates/core/src/source/localfs.rs @@ -248,45 +248,14 @@ impl LocalFsSource { self.source_dir.display() ))); } - // Lex-first `.bin` mirrors the remote AiHub puller and the Go - // CLI's ExtractFlat — a model populated by either path is then - // interchangeable on disk. + // Lex-first `.bin` mirrors the remote AiHub puller. entries.sort_by(|a, b| a.0.cmp(&b.0)); - let entrypoint = entries - .iter() - .find(|(n, _)| n.to_ascii_lowercase().ends_with(".bin")) - .map(|(n, _)| n.clone()) - .ok_or_else(|| { - Error::Hub(format!( - "AI Hub directory {} has no .bin shard", - self.source_dir.display() - )) - })?; - - // Each bucket holds a single file's own size; total_size() sums them. - let entrypoint_size = entries - .iter() - .find(|(n, _)| n == &entrypoint) - .map(|(_, s)| *s as i64) - .unwrap_or(0); - let mut model_file: HashMap = HashMap::new(); - model_file.insert( - "N/A".to_string(), - ModelFileInfo { - name: entrypoint.clone(), - downloaded: true, - size: entrypoint_size, - }, - ); - let extra_files: Vec = entries - .iter() - .filter(|(n, _)| n != &entrypoint) - .map(|(n, s)| ModelFileInfo { - name: n.clone(), - downloaded: true, - size: *s as i64, - }) - .collect(); + let display = self.source_dir.display().to_string(); + let (model_file, extra_files) = super::split_entrypoint_and_extras( + &entries, + || format!("AI Hub directory {display} has no .bin shard"), + |s| *s as i64, + )?; let model_type = std::fs::read(self.source_dir.join(AIHUB_METADATA_FILE)) .ok() @@ -350,41 +319,12 @@ impl LocalFsSource { .or_else(|| self.hint.model_type.clone()) .unwrap_or(ModelType::Llm); - let entrypoint = flat - .iter() - .find(|(name, _)| name.to_ascii_lowercase().ends_with(".bin")) - .map(|(name, _)| name.clone()) - .ok_or_else(|| { - Error::Hub(format!( - "AI Hub archive {} has no .bin shard", - zip_path.display() - )) - })?; - - let entrypoint_size = flat - .iter() - .find(|(name, _)| name == &entrypoint) - .map(|(_, e)| e.uncompressed_size as i64) - .unwrap_or(0); - - let mut model_file: HashMap = HashMap::new(); - model_file.insert( - "N/A".to_string(), - ModelFileInfo { - name: entrypoint.clone(), - downloaded: true, - size: entrypoint_size, - }, - ); - let extra_files: Vec = flat - .iter() - .filter(|(name, _)| name != &entrypoint) - .map(|(name, e)| ModelFileInfo { - name: name.clone(), - downloaded: true, - size: e.uncompressed_size as i64, - }) - .collect(); + let zip_display = zip_path.display().to_string(); + let (model_file, extra_files) = super::split_entrypoint_and_extras( + &flat, + || format!("AI Hub archive {zip_display} has no .bin shard"), + |e| e.uncompressed_size as i64, + )?; let manifest = ModelManifest { name: self.model_name.clone(), @@ -805,14 +745,14 @@ mod tests { fs::create_dir_all(&safetensors).unwrap(); fs::write(safetensors.join("config.json"), b"{}").unwrap(); fs::write(safetensors.join("model.safetensors"), b"x").unwrap(); - assert!( - format!("{}", detect_local_kind(&safetensors).unwrap_err()).contains("safetensors") - ); + assert!(format!("{}", detect_local_kind(&safetensors).unwrap_err()).contains("safetensors")); let unknown = tmp.path().join("unk"); fs::create_dir_all(&unknown).unwrap(); fs::write(unknown.join("readme.txt"), b"hi").unwrap(); let msg = format!("{}", detect_local_kind(&unknown).unwrap_err()); - assert!(msg.contains("AI Hub extracted") && msg.contains("HF GGUF") && msg.contains(".zip")); + assert!( + msg.contains("AI Hub extracted") && msg.contains("HF GGUF") && msg.contains(".zip") + ); } } diff --git a/sdk/model-manager/crates/core/src/source/mod.rs b/sdk/model-manager/crates/core/src/source/mod.rs index 30a63ee17..05b366c40 100644 --- a/sdk/model-manager/crates/core/src/source/mod.rs +++ b/sdk/model-manager/crates/core/src/source/mod.rs @@ -20,13 +20,58 @@ pub mod dockerhub; pub mod hf; pub mod localfs; +use std::collections::HashMap; use std::path::PathBuf; use async_trait::async_trait; use url::Url; -use crate::error::Result; -use crate::manifest::ModelManifest; +use crate::error::{Error, Result}; +use crate::manifest::{ModelFileInfo, ModelManifest}; + +/// Last path component of `path`, treating both `/` and `\` as separators. +/// Empty path returns an empty string. +pub(crate) fn basename(path: &str) -> String { + path.rsplit(['/', '\\']).next().unwrap_or("").to_string() +} + +/// Split a flat list of `(name, T)` entries into a `model_file` map keyed +/// by `"N/A"` (AI Hub / QAIRT layout) and an `extra_files` vec. The +/// entrypoint is the lex-first entry whose name ends in `.bin`. +/// +/// `size_of` extracts the on-disk size for each entry (kept generic so +/// remote `ZipEntry` and local `(name, u64)` tuples both work). +pub(crate) fn split_entrypoint_and_extras( + entries: &[(String, T)], + missing_bin_err: impl FnOnce() -> String, + size_of: impl Fn(&T) -> i64, +) -> Result<(HashMap, Vec)> { + let entrypoint_idx = entries + .iter() + .position(|(name, _)| name.to_ascii_lowercase().ends_with(".bin")) + .ok_or_else(|| Error::Hub(missing_bin_err()))?; + let (entry_name, entry_val) = &entries[entrypoint_idx]; + let mut model_file = HashMap::new(); + model_file.insert( + "N/A".to_string(), + ModelFileInfo { + name: entry_name.clone(), + downloaded: true, + size: size_of(entry_val), + }, + ); + let extra_files: Vec = entries + .iter() + .enumerate() + .filter(|(i, _)| *i != entrypoint_idx) + .map(|(_, (name, val))| ModelFileInfo { + name: name.clone(), + downloaded: true, + size: size_of(val), + }) + .collect(); + Ok((model_file, extra_files)) +} #[async_trait] pub trait ModelSource: Send + Sync { diff --git a/sdk/model-manager/tests/test_model_manager.c b/sdk/model-manager/tests/test_model_manager.c deleted file mode 100644 index 0e21f342d..000000000 --- a/sdk/model-manager/tests/test_model_manager.c +++ /dev/null @@ -1,232 +0,0 @@ -// Copyright (c) 2026 Qualcomm Technologies, Inc. and/or its subsidiaries. -// SPDX-License-Identifier: BSD-3-Clause - -/** - * End-to-end test for the model manager C API. - * - * Build (after cmake -DGENIEX_MODEL_MANAGER=ON): - * - * cmake --build --target test_model_manager - * - * Run: - * LD_LIBRARY_PATH=/src GENIEX_DATADIR=/tmp/geniex-test \ - * .//src/test_model_manager - * - * Two test modes: - * 1. LocalFS (always runs): creates a dummy geniex.json and model file in /tmp, - * then exercises the full CRUD + path resolution flow. - * 2. HuggingFace (optional, set GENIEX_TEST_HF=1): downloads a real model from HF. - * Requires network access and a geniex.json in the target HF repo. - */ - -#include -#include -#include -#include - -#ifdef _WIN32 -#include -#define mkdir(path, mode) _mkdir(path) -#endif - -#include "geniex.h" -#include "geniex_model.h" - -/* ---- helpers ---- */ - -#define CHECK(call) \ - do { \ - int32_t _rc = (call); \ - if (_rc != GENIEX_SUCCESS) { \ - fprintf(stderr, "FAIL %s (rc=%d)\n", #call, _rc); \ - return 1; \ - } \ - printf("OK %s\n", #call); \ - } while (0) - -#define EXPECT_FAIL(call, expected_rc) \ - do { \ - int32_t _rc = (call); \ - if (_rc != (expected_rc)) { \ - fprintf(stderr, "FAIL %s expected rc=%d got rc=%d\n", #call, (expected_rc), _rc); \ - return 1; \ - } \ - printf("OK %s (expected failure rc=%d)\n", #call, _rc); \ - } while (0) - -static bool progress_cb(const geniex_FileProgress* files, int32_t file_count, void* user_data) { - int* counter = (int*)user_data; - (*counter)++; - for (int i = 0; i < file_count; i++) { - printf(" progress[%d]: %s %lld / %lld\n", - i, - files[i].file_name ? files[i].file_name : "(null)", - (long long)files[i].downloaded_bytes, - (long long)files[i].total_bytes); - } - return true; /* never cancel */ -} - -static void mkdir_p(const char* path) { - char tmp[512]; - snprintf(tmp, sizeof(tmp), "%s", path); - for (char* p = tmp + 1; *p; p++) { - if (*p == '/') { - *p = '\0'; - mkdir(tmp, 0755); - *p = '/'; - } - } - mkdir(tmp, 0755); -} - -static void write_file(const char* path, const char* content) { - FILE* f = fopen(path, "w"); - if (!f) { - perror(path); - exit(1); - } - fputs(content, f); - fclose(f); -} - -/* ---- LocalFS test ---- */ - -static int test_localfs(const char* data_dir) { - printf("\n=== LocalFS test ===\n"); - - /* 1. Create a fake model source directory */ - const char* src_dir = "/tmp/geniex-localfs-src/qualcomm/TestModel-GGUF"; - mkdir_p(src_dir); - - /* Fake GGUF file */ - char gguf_path[512]; - snprintf(gguf_path, sizeof(gguf_path), "%s/model-Q4_K_M.gguf", src_dir); - write_file(gguf_path, "fake gguf content"); - - /* geniex.json manifest */ - char manifest_path[512]; - snprintf(manifest_path, sizeof(manifest_path), "%s/geniex.json", src_dir); - write_file(manifest_path, - "{" - "\"Name\":\"qualcomm/TestModel-GGUF\"," - "\"ModelName\":\"test-1b\"," - "\"ModelType\":\"llm\"," - "\"PluginId\":\"llama_cpp\"," - "\"ModelFile\":{\"Q4_K_M\":{\"Name\":\"model-Q4_K_M.gguf\",\"Downloaded\":true,\"Size\":17}}," - "\"MMProjFile\":{\"Name\":\"\",\"Downloaded\":false,\"Size\":0}," - "\"TokenizerFile\":{\"Name\":\"\",\"Downloaded\":false,\"Size\":0}," - "\"ExtraFiles\":[]" - "}"); - - /* 2. Pull from LocalFS */ - int progress_calls = 0; - geniex_ModelPullInput pull_input; - memset(&pull_input, 0, sizeof(pull_input)); - pull_input.struct_size = sizeof(pull_input); - pull_input.model_name = "qualcomm/TestModel-GGUF"; - pull_input.quant = NULL; - pull_input.hub = GENIEX_HUB_LOCALFS; - pull_input.local_path = "/tmp/geniex-localfs-src/qualcomm/TestModel-GGUF"; - pull_input.hf_token = NULL; - pull_input.on_progress = progress_cb; - pull_input.user_data = &progress_calls; - CHECK(geniex_model_pull(&pull_input)); - if (progress_calls == 0) { - fprintf(stderr, "FAIL progress callback never invoked\n"); - return 1; - } - printf(" progress callback invoked %d time(s) ✓\n", progress_calls); - - /* 3. List */ - geniex_ModelListDetailedOutput list = {0}; - CHECK(geniex_model_list_detailed(&list)); - printf(" cached models: %d\n", list.count); - if (list.count < 1) { - fprintf(stderr, "FAIL expected at least 1 cached model\n"); - geniex_model_list_detailed_free(&list); - return 1; - } - printf(" [0] %s\n", list.models[0].name); - geniex_model_list_detailed_free(&list); - - /* 4. Get type */ - geniex_ModelType mtype; - CHECK(geniex_model_get_type("qualcomm/TestModel-GGUF", &mtype)); - if (mtype != GENIEX_MODEL_TYPE_LLM) { - fprintf(stderr, "FAIL expected GENIEX_MODEL_TYPE_LLM (%d), got %d\n", GENIEX_MODEL_TYPE_LLM, mtype); - return 1; - } - printf(" model type: LLM ✓\n"); - - /* 5. Get paths */ - geniex_ModelPaths paths = {0}; - CHECK(geniex_model_get_paths("qualcomm/TestModel-GGUF", &paths)); - printf(" model_path: %s\n", paths.model_path ? paths.model_path : "(null)"); - printf(" model_dir: %s\n", paths.model_dir ? paths.model_dir : "(null)"); - printf(" model_name: %s\n", paths.model_name ? paths.model_name : "(null)"); - printf(" plugin_id: %s\n", paths.plugin_id ? paths.plugin_id : "(null)"); - if (!paths.model_path || strstr(paths.model_path, "model-Q4_K_M.gguf") == NULL) { - fprintf(stderr, "FAIL model_path does not contain expected filename\n"); - geniex_model_paths_free(&paths); - return 1; - } - printf(" model_path contains 'model-Q4_K_M.gguf' ✓\n"); - geniex_model_paths_free(&paths); - - /* 6. Get paths with explicit quant */ - CHECK(geniex_model_get_paths("qualcomm/TestModel-GGUF:Q4_K_M", &paths)); - geniex_model_paths_free(&paths); - - /* 7. Error case: unknown quant */ - EXPECT_FAIL(geniex_model_get_paths("qualcomm/TestModel-GGUF:Q8_0", &paths), GENIEX_ERROR_COMMON_INVALID_INPUT); - - /* 8. Remove */ - CHECK(geniex_model_remove("qualcomm/TestModel-GGUF")); - - /* 9. Verify gone */ - geniex_ModelListDetailedOutput list2 = {0}; - CHECK(geniex_model_list_detailed(&list2)); - if (list2.count != 0) { - fprintf(stderr, "FAIL expected 0 models after remove, got %d\n", list2.count); - geniex_model_list_detailed_free(&list2); - return 1; - } - printf(" list after remove: 0 ✓\n"); - geniex_model_list_detailed_free(&list2); - - printf("=== LocalFS test PASSED ===\n"); - return 0; -} - -/* ---- alias test ---- */ - -static int test_alias(void) { - printf("\n=== Alias test ===\n"); - char* full = NULL; - CHECK(geniex_model_resolve_alias("qwen3", &full)); - printf(" qwen3 -> %s\n", full); - geniex_free(full); - - /* unknown alias should fail */ - EXPECT_FAIL(geniex_model_resolve_alias("nonexistent_model_xyz_abc", &full), GENIEX_ERROR_COMMON_INVALID_INPUT); - printf("=== Alias test PASSED ===\n"); - return 0; -} - -/* ---- main ---- */ - -int main(void) { - const char* data_dir = getenv("GENIEX_DATADIR"); - - printf("=== geniex_model_init ===\n"); - CHECK(geniex_model_init(data_dir)); - - if (test_alias()) return 1; - if (test_localfs(data_dir ? data_dir : "/tmp/geniex-test")) return 1; - - CHECK(geniex_model_deinit()); - - printf("\n=== ALL TESTS PASSED ===\n"); - return 0; -} diff --git a/sdk/src/CMakeLists.txt b/sdk/src/CMakeLists.txt index 6c6976709..7d2a8b0e3 100644 --- a/sdk/src/CMakeLists.txt +++ b/sdk/src/CMakeLists.txt @@ -78,18 +78,6 @@ target_include_directories(geniex PUBLIC install(FILES "${CMAKE_SOURCE_DIR}/model-manager/include/geniex_model.h" DESTINATION include) -if(GENIEX_BENCHMARK) - add_executable(test_model_manager - "${CMAKE_SOURCE_DIR}/model-manager/tests/test_model_manager.c" - ) - target_include_directories(test_model_manager PRIVATE - ${COMMON_INCLUDE} - "${CMAKE_SOURCE_DIR}/model-manager/include" - ) - target_link_libraries(test_model_manager PRIVATE geniex) - add_test(NAME test_model_manager COMMAND test_model_manager) -endif() - install(TARGETS geniex LIBRARY DESTINATION lib RUNTIME DESTINATION lib) From e29c5776b9bfac3b7484e76ccf30f6795e6b24d8 Mon Sep 17 00:00:00 2001 From: Mengsheng Wu Date: Thu, 30 Jul 2026 22:10:55 +0800 Subject: [PATCH 11/14] refactor(sdk): compress dockerhub layer plumbing via add_layer helper Signed-off-by: Mengsheng Wu --- .../crates/core/src/source/dockerhub.rs | 114 +++++++----------- 1 file changed, 44 insertions(+), 70 deletions(-) diff --git a/sdk/model-manager/crates/core/src/source/dockerhub.rs b/sdk/model-manager/crates/core/src/source/dockerhub.rs index 4ac89e992..67ae96ea6 100644 --- a/sdk/model-manager/crates/core/src/source/dockerhub.rs +++ b/sdk/model-manager/crates/core/src/source/dockerhub.rs @@ -416,107 +416,81 @@ fn build_plan( ))); } + // Add a layer as both a `FileSpec` (byte-level fetch) and the + // `ModelFileInfo` the caller stores in the manifest, resolving the + // display name from either the OCI filepath annotation or `fallback`. let mut files: Vec = Vec::new(); - let mut push = |name: String, desc: &Descriptor| -> Result<()> { + let mut add_layer = |desc: &Descriptor, fallback: String| -> Result { + let name = if is_layer_per_file { + desc.annotations + .get(ANNOTATION_FILEPATH) + .filter(|fp| !fp.is_empty()) + .map(|fp| basename(fp)) + .unwrap_or(fallback) + } else { + fallback + }; files.push(FileSpec { - name, + name: name.clone(), size: desc.size, bytes: BytesSource::Http { url: blob_url(&desc.digest)?, auth: token.clone(), }, }); - Ok(()) - }; - - let layer_name = |desc: &Descriptor, fallback: String| -> String { - if is_layer_per_file { - if let Some(fp) = desc.annotations.get(ANNOTATION_FILEPATH) { - if !fp.is_empty() { - return basename(fp); - } - } - } - fallback + Ok(ModelFileInfo { + name, + downloaded: true, + size: desc.size as i64, + }) }; - // GGUF weight(s): single file -> "model.gguf"; multiple -> numbered - // shards, matching the Go CLI's `unpackGGUFs` naming exactly so a - // cache populated by either agent is interchangeable. - let mut model_file: HashMap = HashMap::new(); + // GGUF weight(s): single -> "model.gguf"; multi -> "model-NNNNN-of-MMMMM.gguf" + // (matches the Go CLI's unpackGGUFs naming). let mut extra_files: Vec = Vec::new(); let n = gguf_layers.len(); - let mut entrypoint: Option = None; - for (i, desc) in gguf_layers.iter().enumerate() { + let mut gguf_entries = gguf_layers.iter().enumerate().map(|(i, desc)| { let fallback = if n == 1 { "model.gguf".to_string() } else { format!("model-{:05}-of-{:05}.gguf", i + 1, n) }; - let name = layer_name(desc, fallback); - push(name.clone(), desc)?; - let info = ModelFileInfo { - name, - downloaded: true, - size: desc.size as i64, - }; - if i == 0 { - entrypoint = Some(info); - } else { - extra_files.push(info); - } + add_layer(desc, fallback) + }); + let entrypoint = gguf_entries + .next() + .expect("gguf_layers is non-empty (checked above)")?; + for rest in gguf_entries { + extra_files.push(rest?); } - let quant = if !config.quantization.is_empty() { - config.quantization.to_ascii_uppercase() - } else if let Some(entry) = &entrypoint { - extract_quant(&entry.name).unwrap_or_else(|| "DEFAULT".to_string()) - } else { - "DEFAULT".to_string() - }; - model_file.insert(quant, entrypoint.expect("gguf_layers is non-empty")); let mmproj_file = if let Some(desc) = mmproj_layers.first() { - let name = layer_name(desc, "model.mmproj".to_string()); - push(name.clone(), desc)?; - ModelFileInfo { - name, - downloaded: true, - size: desc.size as i64, - } + add_layer(desc, "model.mmproj".to_string())? } else { ModelFileInfo::default() }; - for desc in &chat_template_layers[..chat_template_layers.len().min(1)] { - let name = layer_name(desc, "template.jinja".to_string()); - push(name.clone(), desc)?; - extra_files.push(ModelFileInfo { - name, - downloaded: true, - size: desc.size as i64, - }); + // Docker's OCI convention allows at most one chat template and one license. + if let Some(desc) = chat_template_layers.first() { + extra_files.push(add_layer(desc, "template.jinja".to_string())?); } - for desc in &license_layers[..license_layers.len().min(1)] { - let name = layer_name(desc, "LICENSE".to_string()); - push(name.clone(), desc)?; - extra_files.push(ModelFileInfo { - name, - downloaded: true, - size: desc.size as i64, - }); + if let Some(desc) = license_layers.first() { + extra_files.push(add_layer(desc, "LICENSE".to_string())?); } for desc in &model_file_layers { let short = desc.digest.rsplit(':').next().unwrap_or(&desc.digest); let fallback = format!("file-{}", &short[..short.len().min(12)]); - let name = layer_name(desc, fallback); - push(name.clone(), desc)?; - extra_files.push(ModelFileInfo { - name, - downloaded: true, - size: desc.size as i64, - }); + extra_files.push(add_layer(desc, fallback)?); } + let quant = if !config.quantization.is_empty() { + config.quantization.to_ascii_uppercase() + } else { + extract_quant(&entrypoint.name).unwrap_or_else(|| "DEFAULT".to_string()) + }; + let mut model_file: HashMap = HashMap::new(); + model_file.insert(quant, entrypoint); + let model_type = if mmproj_file.downloaded { ModelType::Vlm } else { From 55dadf0e5cddf0988afa7e9f8954bd55d8613156 Mon Sep 17 00:00:00 2001 From: Mengsheng Wu Date: Thu, 30 Jul 2026 22:14:50 +0800 Subject: [PATCH 12/14] refactor(sdk): add Error::InvalidUrl and parse_manifest helper Signed-off-by: Mengsheng Wu --- sdk/model-manager/crates/core/src/error.rs | 28 +++++++++++++++++++ .../crates/core/src/source/ai_hub/mod.rs | 27 +++++------------- .../crates/core/src/source/dockerhub.rs | 18 ++++-------- .../crates/core/src/source/hf.rs | 6 ++-- sdk/model-manager/crates/ffi/src/types.rs | 3 +- 5 files changed, 45 insertions(+), 37 deletions(-) diff --git a/sdk/model-manager/crates/core/src/error.rs b/sdk/model-manager/crates/core/src/error.rs index b2c4bea1b..fd2fc0872 100644 --- a/sdk/model-manager/crates/core/src/error.rs +++ b/sdk/model-manager/crates/core/src/error.rs @@ -80,6 +80,16 @@ pub enum Error { #[error("http error: {0}")] Http(String), + /// URL syntax error. `context` names what was being built + /// (`"HF endpoint"`, `"docker manifest url"`, …) so the log line is + /// actionable without pattern-matching on the underlying message. + #[error("invalid url ({context}): {source}")] + InvalidUrl { + context: String, + #[source] + source: url::ParseError, + }, + #[error("download cancelled")] Cancelled, @@ -92,3 +102,21 @@ pub enum Error { #[error("could not infer manifest from directory: {0}")] ManifestInferenceFailed(String), } + +impl Error { + /// Wrap a `url::ParseError` with `context` describing the URL role. + pub fn invalid_url(context: impl Into, source: url::ParseError) -> Self { + Error::InvalidUrl { + context: context.into(), + source, + } + } +} + +/// Parse a serde-JSON document, tagging failures with `what` for log grep. +pub fn parse_manifest<'a, T: serde::Deserialize<'a>>( + what: &'static str, + bytes: &'a [u8], +) -> Result { + serde_json::from_slice(bytes).map_err(|source| Error::ManifestParse { what, source }) +} diff --git a/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs b/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs index b4faf1320..06d1b25bf 100644 --- a/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs +++ b/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs @@ -22,7 +22,7 @@ use std::time::{Duration, SystemTime}; use async_trait::async_trait; use url::Url; -use crate::error::{Error, Result}; +use crate::error::{parse_manifest, Error, Result}; use crate::manifest::{ModelFileInfo, ModelManifest, ModelType}; use crate::transport::{HttpTransport, ReqwestTransport}; @@ -89,10 +89,7 @@ async fn fetch_platform_info( transport, ) .await?; - serde_json::from_slice(&platform_bytes).map_err(|source| Error::ManifestParse { - what: "platform.json", - source, - }) + parse_manifest("platform.json", &platform_bytes) } /// List every chipset AI Hub publishes assets for, with its canonical @@ -184,11 +181,7 @@ impl ModelSource for AiHubSource { &self.transport, ) .await?; - let release_manifest: ReleaseManifest = - serde_json::from_slice(&manifest_bytes).map_err(|source| Error::ManifestParse { - what: "manifest.json", - source, - })?; + let release_manifest: ReleaseManifest = parse_manifest("manifest.json", &manifest_bytes)?; // Match by display_name first, then fall back to the snake_case // `id`, so callers can use either "Llama-v3.2-3B-Instruct" or @@ -220,11 +213,8 @@ impl ModelSource for AiHubSource { // release-assets.json is per-model with a URL that rotates each // release, so it's fetched uncached. let release_assets_bytes = fetch_direct(release_assets_url, &self.transport).await?; - let release_assets: ModelReleaseAssets = serde_json::from_slice(&release_assets_bytes) - .map_err(|source| Error::ManifestParse { - what: "release-assets.json", - source, - })?; + let release_assets: ModelReleaseAssets = + parse_manifest("release-assets.json", &release_assets_bytes)?; let platform = fetch_platform_info(&self.cfg, &self.transport).await?; @@ -253,10 +243,7 @@ impl ModelSource for AiHubSource { }; let download_url = Url::parse(&asset.download_url).map_err(|e| { - Error::Hub(format!( - "invalid asset download_url {:?}: {e}", - asset.download_url - )) + Error::invalid_url(format!("asset download_url {:?}", asset.download_url), e) })?; // Only the ZIP64 footer + central directory are fetched here; @@ -449,7 +436,7 @@ fn write_cache(path: &Path, data: &[u8]) { } async fn fetch_direct(url: &str, transport: &Arc) -> Result> { - let parsed = Url::parse(url).map_err(|e| Error::Hub(format!("invalid url {url:?}: {e}")))?; + let parsed = Url::parse(url).map_err(|e| Error::invalid_url(format!("url {url:?}"), e))?; let head = transport.head(&parsed, None).await?; if head.size > MAX_INDEX_BYTES { return Err(Error::Hub(format!( diff --git a/sdk/model-manager/crates/core/src/source/dockerhub.rs b/sdk/model-manager/crates/core/src/source/dockerhub.rs index 67ae96ea6..c477b5d4b 100644 --- a/sdk/model-manager/crates/core/src/source/dockerhub.rs +++ b/sdk/model-manager/crates/core/src/source/dockerhub.rs @@ -31,7 +31,7 @@ use reqwest::Client; use serde::Deserialize; use url::Url; -use crate::error::{Error, Result}; +use crate::error::{parse_manifest, Error, Result}; use crate::manifest::{ModelFileInfo, ModelManifest, ModelType}; use crate::manifest_builder::extract_quant; use crate::transport::{build_tls_config, HttpTransport, USER_AGENT}; @@ -174,7 +174,7 @@ impl DockerHubSource { "{}/v2/{}/manifests/{reference}", self.cfg.registry_endpoint, self.repo )) - .map_err(|e| Error::Hub(format!("invalid docker manifest url: {e}")))?; + .map_err(|e| Error::invalid_url("docker manifest url", e))?; let accept = [ media::OCI_MANIFEST, @@ -222,11 +222,7 @@ impl DockerHubSource { bytes.len() ))); } - let mut parsed: RegistryManifest = - serde_json::from_slice(&bytes).map_err(|source| Error::ManifestParse { - what: "docker registry manifest", - source, - })?; + let mut parsed: RegistryManifest = parse_manifest("docker registry manifest", &bytes)?; if parsed.media_type.is_empty() { parsed.media_type = content_type; } @@ -238,7 +234,7 @@ impl DockerHubSource { "{}/v2/{}/blobs/{digest}", self.cfg.registry_endpoint, self.repo )) - .map_err(|e| Error::Hub(format!("invalid docker blob url: {e}"))) + .map_err(|e| Error::invalid_url("docker blob url", e)) } async fn fetch_blob(&self, digest: &str, token: Option<&str>, cap: u64) -> Result> { @@ -293,11 +289,7 @@ impl ModelSource for DockerHubSource { let config_bytes = self .fetch_blob(&config_desc.digest, token.as_deref(), MAX_CONFIG_BYTES) .await?; - let config_file: DockerConfigFile = - serde_json::from_slice(&config_bytes).map_err(|source| Error::ManifestParse { - what: "docker model config", - source, - })?; + let config_file: DockerConfigFile = parse_manifest("docker model config", &config_bytes)?; let is_layer_per_file = config_desc.media_type == media::CONFIG_V02; build_plan( diff --git a/sdk/model-manager/crates/core/src/source/hf.rs b/sdk/model-manager/crates/core/src/source/hf.rs index c939df02e..24e191e9e 100644 --- a/sdk/model-manager/crates/core/src/source/hf.rs +++ b/sdk/model-manager/crates/core/src/source/hf.rs @@ -50,7 +50,7 @@ impl HfSource { hint: ManifestHint, ) -> Result { let endpoint = Url::parse(endpoint) - .map_err(|e| Error::Hub(format!("invalid HF endpoint {endpoint}: {e}")))?; + .map_err(|e| Error::invalid_url(format!("HF endpoint {endpoint}"), e))?; Ok(Self { repo, endpoint, @@ -63,13 +63,13 @@ impl HfSource { fn api_url(&self) -> Result { self.endpoint .join(&format!("api/models/{}?blobs=true", self.repo)) - .map_err(|e| Error::Hub(format!("join api url for {}: {e}", self.repo))) + .map_err(|e| Error::invalid_url(format!("HF api for {}", self.repo), e)) } fn file_url(&self, name: &str) -> Result { self.endpoint .join(&format!("{}/resolve/main/{name}", self.repo)) - .map_err(|e| Error::Hub(format!("join resolve url for {}/{name}: {e}", self.repo))) + .map_err(|e| Error::invalid_url(format!("HF file {}/{name}", self.repo), e)) } async fn fetch_small(&self, url: &Url, limit: u64) -> Result> { diff --git a/sdk/model-manager/crates/ffi/src/types.rs b/sdk/model-manager/crates/ffi/src/types.rs index f4500c76c..5bdf9e679 100644 --- a/sdk/model-manager/crates/ffi/src/types.rs +++ b/sdk/model-manager/crates/ffi/src/types.rs @@ -72,7 +72,8 @@ pub fn err_to_code(e: &Error) -> i32 { Error::QuantNotFound(_, _) | Error::QuantNotDownloaded(_, _) | Error::InvalidModelName(_) - | Error::InvalidFileName(_) => GENIEX_ERROR_COMMON_INVALID_INPUT, + | Error::InvalidFileName(_) + | Error::InvalidUrl { .. } => GENIEX_ERROR_COMMON_INVALID_INPUT, // Split HTTP status into actionable buckets; everything else (other // statuses, timeout/DNS/proxy, freeform) stays a generic network error. Error::HttpStatus { status, .. } if *status == 401 || *status == 403 => { From c985f770a2e15023a0966c34ce4d11fab1df62cd Mon Sep 17 00:00:00 2001 From: Mengsheng Wu Date: Thu, 30 Jul 2026 22:17:12 +0800 Subject: [PATCH 13/14] refactor(sdk): bundle executor download args into RangeJob/DeflateJob Signed-off-by: Mengsheng Wu --- .../crates/core/src/executor/mod.rs | 125 ++++++++++++------ .../crates/core/src/source/ai_hub/mod.rs | 2 +- 2 files changed, 85 insertions(+), 42 deletions(-) diff --git a/sdk/model-manager/crates/core/src/executor/mod.rs b/sdk/model-manager/crates/core/src/executor/mod.rs index b0accbe81..03fc2242f 100644 --- a/sdk/model-manager/crates/core/src/executor/mod.rs +++ b/sdk/model-manager/crates/core/src/executor/mod.rs @@ -223,17 +223,19 @@ async fn run_one( match spec.bytes.clone() { BytesSource::Http { url, auth } => { download_range_based( - &spec.name, - spec.size, - state, + RangeJob { + name: spec.name.clone(), + size_hint: spec.size, + state, + transport, + chunk_sem, + cancel, + url, + auth, + base_offset: 0, + size_provided: spec.size > 0, + }, dest_dir, - transport, - chunk_sem, - cancel, - url, - auth, - /*base_offset*/ 0, - /*size_provided*/ spec.size > 0, ) .await } @@ -245,8 +247,19 @@ async fn run_one( } => { let size = if len > 0 { len } else { spec.size }; download_range_based( - &spec.name, size, state, dest_dir, transport, chunk_sem, cancel, url, auth, offset, - true, + RangeJob { + name: spec.name.clone(), + size_hint: size, + state, + transport, + chunk_sem, + cancel, + url, + auth, + base_offset: offset, + size_provided: true, + }, + dest_dir, ) .await } @@ -257,16 +270,18 @@ async fn run_one( compressed_len, } => { download_http_deflate( - &spec.name, - spec.size, - compressed_len, - offset, - state, + DeflateJob { + name: spec.name.clone(), + uncompressed_size: spec.size, + compressed_len, + offset, + state, + transport, + cancel, + url, + auth, + }, dest_dir, - transport, - cancel, - url, - auth, ) .await } @@ -423,24 +438,40 @@ async fn decode_local_deflate( Ok(()) } -/// Chunked parallel download for `Http` and `HttpRange`. The only -/// difference between them is the `base_offset` added to every range -/// request: `Http` uses 0, `HttpRange` uses the entry's start inside -/// the outer zip. -#[allow(clippy::too_many_arguments)] -async fn download_range_based( - name: &str, +/// Arguments for [`download_range_based`], grouped so the caller list +/// stays readable and the two [`BytesSource`] variants that dispatch to +/// it (`Http`, `HttpRange`) can share one shape. +struct RangeJob { + name: String, size_hint: u64, state: Arc, - dest_dir: &Path, transport: Arc, chunk_sem: Arc, cancel: CancellationToken, url: url::Url, auth: Option, + /// Added to every range request. `Http` uses 0; `HttpRange` uses the + /// entry's start inside the outer zip. base_offset: u64, + /// True when `size_hint` is authoritative (e.g. from a zip central + /// directory), false when we should HEAD to discover it. size_provided: bool, -) -> Result<()> { +} + +/// Chunked parallel download for `Http` and `HttpRange`. +async fn download_range_based(job: RangeJob, dest_dir: &Path) -> Result<()> { + let RangeJob { + name, + size_hint, + state, + transport, + chunk_sem, + cancel, + url, + auth, + base_offset, + size_provided, + } = job; let size = if size_provided && size_hint > 0 { size_hint } else { @@ -449,7 +480,7 @@ async fn download_range_based( }; state.total_bytes.store(size, Ordering::Relaxed); - let output_path = dest_dir.join(name); + let output_path = dest_dir.join(&name); let marker_path = PathBuf::from(format!("{}{}", output_path.display(), PROGRESS_SUFFIX)); chunk::preallocate(&output_path, size)?; @@ -539,27 +570,39 @@ async fn download_range_based( Ok(()) } -/// DEFLATE-decoding download for AI Hub `.bin` shards. Entry-granular -/// resume: any partial state from a previous crash is discarded and -/// the whole entry is refetched, because flate2 streams aren't -/// seekable mid-entry. -#[allow(clippy::too_many_arguments)] -async fn download_http_deflate( - name: &str, +/// Arguments for [`download_http_deflate`]. +struct DeflateJob { + name: String, uncompressed_size: u64, compressed_len: u64, offset: u64, state: Arc, - dest_dir: &Path, transport: Arc, cancel: CancellationToken, url: url::Url, auth: Option, -) -> Result<()> { +} + +/// DEFLATE-decoding download for AI Hub `.bin` shards. Entry-granular +/// resume: any partial state from a previous crash is discarded and +/// the whole entry is refetched, because flate2 streams aren't +/// seekable mid-entry. +async fn download_http_deflate(job: DeflateJob, dest_dir: &Path) -> Result<()> { + let DeflateJob { + name, + uncompressed_size, + compressed_len, + offset, + state, + transport, + cancel, + url, + auth, + } = job; state .total_bytes .store(uncompressed_size, Ordering::Relaxed); - let output_path = dest_dir.join(name); + let output_path = dest_dir.join(&name); let marker_path = PathBuf::from(format!("{}{}", output_path.display(), PROGRESS_SUFFIX)); // DEFLATE entries are entry-granular: a 0x01 marker means the file diff --git a/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs b/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs index 06d1b25bf..c6f7af4df 100644 --- a/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs +++ b/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs @@ -14,7 +14,7 @@ pub mod dto; pub mod remote_zip; pub mod selector; -use std::collections::{HashMap, HashSet}; +use std::collections::HashSet; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::{Duration, SystemTime}; From eb321fb447597e5d0e06dbe0c7b5de8b32262819 Mon Sep 17 00:00:00 2001 From: Mengsheng Wu Date: Thu, 30 Jul 2026 22:19:01 +0800 Subject: [PATCH 14/14] refactor(sdk): drop num_chunks getter, use div_ceil, flatten info.json fetch Signed-off-by: Mengsheng Wu --- .../crates/core/src/executor/chunk.rs | 22 ++---- .../crates/core/src/source/ai_hub/mod.rs | 74 +++++++++---------- .../crates/core/tests/executor.rs | 2 +- 3 files changed, 45 insertions(+), 53 deletions(-) diff --git a/sdk/model-manager/crates/core/src/executor/chunk.rs b/sdk/model-manager/crates/core/src/executor/chunk.rs index 075fbc70f..32ad5b879 100644 --- a/sdk/model-manager/crates/core/src/executor/chunk.rs +++ b/sdk/model-manager/crates/core/src/executor/chunk.rs @@ -39,12 +39,6 @@ pub struct ChunkPlan { pub chunks: Vec, } -impl ChunkPlan { - pub fn num_chunks(&self) -> usize { - self.chunks.len() - } -} - /// Build a chunk plan for `file_size`. A `file_size` of 0 yields a /// zero-chunk plan; the caller is responsible for still creating an empty /// output file. @@ -62,7 +56,7 @@ pub fn plan_chunks_with_floor(file_size: u64, min_chunk_size: u64) -> ChunkPlan }; let mut chunks = Vec::new(); if file_size > 0 { - let n = (file_size + chunk_size - 1) / chunk_size; + let n = file_size.div_ceil(chunk_size); for i in 0..n { let offset = i * chunk_size; let len = core::cmp::min(chunk_size, file_size - offset); @@ -92,7 +86,7 @@ fn effective_min_chunk_size() -> u64 { /// if the file is missing or length-mismatched (which signals either a /// fresh pull or a chunk-size change since the last attempt). pub fn load_or_init_bitmap(marker_path: &Path, plan: &ChunkPlan) -> Result> { - let expected = plan.num_chunks(); + let expected = plan.chunks.len(); match fs::read(marker_path) { Ok(buf) if buf.len() == expected => Ok(buf), Ok(_) | Err(_) => { @@ -170,7 +164,7 @@ mod tests { #[test] fn tiny_file_is_single_chunk() { let plan = plan_chunks_with_floor(1024, MIN_CHUNK_SIZE); - assert_eq!(plan.num_chunks(), 1); + assert_eq!(plan.chunks.len(), 1); assert_eq!(plan.chunks[0].offset, 0); assert_eq!(plan.chunks[0].len, 1024); assert_eq!(plan.chunk_size, MIN_CHUNK_SIZE); @@ -179,14 +173,14 @@ mod tests { #[test] fn exact_min_chunk_is_one_chunk() { let plan = plan_chunks_with_floor(MIN_CHUNK_SIZE, MIN_CHUNK_SIZE); - assert_eq!(plan.num_chunks(), 1); + assert_eq!(plan.chunks.len(), 1); assert_eq!(plan.chunks[0].len, MIN_CHUNK_SIZE); } #[test] fn thirty_two_mib_splits_into_two() { let plan = plan_chunks_with_floor(2 * MIN_CHUNK_SIZE, MIN_CHUNK_SIZE); - assert_eq!(plan.num_chunks(), 2); + assert_eq!(plan.chunks.len(), 2); assert_eq!(plan.chunks[0].len, MIN_CHUNK_SIZE); assert_eq!(plan.chunks[1].len, MIN_CHUNK_SIZE); } @@ -196,7 +190,7 @@ mod tests { // 4 GiB at the default floor → 128 chunks of 32 MiB each. let size = 4u64 * 1024 * 1024 * 1024; let plan = plan_chunks_with_floor(size, MIN_CHUNK_SIZE); - assert_eq!(plan.num_chunks(), MAX_CHUNKS_PER_FILE as usize); + assert_eq!(plan.chunks.len(), MAX_CHUNKS_PER_FILE as usize); assert_eq!(plan.chunk_size, size / MAX_CHUNKS_PER_FILE); assert_eq!( plan.chunks.iter().map(|c| c.len).sum::(), @@ -209,7 +203,7 @@ mod tests { fn last_chunk_takes_the_remainder() { let size = MIN_CHUNK_SIZE + 123; let plan = plan_chunks_with_floor(size, MIN_CHUNK_SIZE); - assert_eq!(plan.num_chunks(), 2); + assert_eq!(plan.chunks.len(), 2); assert_eq!(plan.chunks[1].len, 123); assert_eq!(plan.chunks[1].offset, MIN_CHUNK_SIZE); } @@ -217,7 +211,7 @@ mod tests { #[test] fn zero_size_yields_no_chunks() { let plan = plan_chunks_with_floor(0, MIN_CHUNK_SIZE); - assert_eq!(plan.num_chunks(), 0); + assert_eq!(plan.chunks.len(), 0); } #[test] diff --git a/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs b/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs index c6f7af4df..745855219 100644 --- a/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs +++ b/sdk/model-manager/crates/core/src/source/ai_hub/mod.rs @@ -163,6 +163,37 @@ impl AiHubSource { transport, } } + + /// Fetch and parse the per-model `info.json`. Returns `None` on any + /// failure (missing URL, fetch error, malformed JSON), logging a + /// warning first — modality classification tolerates absence. + async fn fetch_info_json(&self, entry: &ManifestModelEntry, version: &str) -> Option { + if entry.manifest_urls.info.is_empty() { + return None; + } + let cache_path = self + .cfg + .cache_dir + .join("info") + .join(format!("{}.json", entry.id)); + let bytes = fetch_with_cache( + &entry.manifest_urls.info, + &cache_path, + version, + self.cfg.skip_cache, + &self.transport, + ) + .await + .inspect_err(|e| { + crate::logging::warn(&format!("aihub info.json fetch for {}: {e}", entry.id)); + }) + .ok()?; + serde_json::from_slice::(&bytes) + .inspect_err(|e| { + crate::logging::warn(&format!("aihub info.json parse for {}: {e}", entry.id)); + }) + .ok() + } } #[async_trait] @@ -265,44 +296,11 @@ impl ModelSource for AiHubSource { .unwrap_or(&asset.precision) .to_string(); - // `domain` alone cannot distinguish Qwen2.5-VL from text-only - // LLMs (both report MODEL_DOMAIN_GENERATIVE_AI), so we also - // read the per-model info.json. Fetch failure is non-fatal: - // `classify_ai_hub` falls back to the domain-only signal and - // defaults to LLM if even that is absent. - let info: Option = if entry.manifest_urls.info.is_empty() { - None - } else { - let cache_path = self - .cfg - .cache_dir - .join("info") - .join(format!("{}.json", entry.id)); - match fetch_with_cache( - &entry.manifest_urls.info, - &cache_path, - version, - self.cfg.skip_cache, - &self.transport, - ) - .await - { - Ok(bytes) => match serde_json::from_slice::(&bytes) { - Ok(info) => Some(info), - Err(e) => { - crate::logging::warn(&format!( - "aihub info.json parse for {}: {e}", - entry.id - )); - None - } - }, - Err(e) => { - crate::logging::warn(&format!("aihub info.json fetch for {}: {e}", entry.id)); - None - } - } - }; + // `domain` alone can't distinguish Qwen2.5-VL from text-only LLMs + // (both report MODEL_DOMAIN_GENERATIVE_AI), so we also read the + // per-model info.json. Failure is non-fatal: `classify_ai_hub` + // falls back to the domain-only signal. + let info = self.fetch_info_json(entry, version).await; let model_type = classify_ai_hub(info.as_ref(), entry); let manifest = ModelManifest { name: self.model_name.clone(), diff --git a/sdk/model-manager/crates/core/tests/executor.rs b/sdk/model-manager/crates/core/tests/executor.rs index b916d929e..0dd4ec31b 100644 --- a/sdk/model-manager/crates/core/tests/executor.rs +++ b/sdk/model-manager/crates/core/tests/executor.rs @@ -187,7 +187,7 @@ async fn resume_skips_completed_chunks() { let out = dest.join("f.bin"); std::fs::write(&out, body.clone()).unwrap(); let plan = chunklib::plan_chunks(body.len() as u64); - let mut bitmap = vec![0u8; plan.num_chunks()]; + let mut bitmap = vec![0u8; plan.chunks.len()]; bitmap[0] = 0x01; bitmap[2] = 0x01; std::fs::write(dest.join("f.bin.progress"), &bitmap).unwrap();