mirror of
https://github.com/Mesh-LLM/mesh-llm.git
synced 2026-08-08 22:23:19 -04:00
fix(moa): /v1/models advertises quant-suffix IDs that route back
PR #566 review feedback (Apr 2026): > Some IDs in /v1/models dropped quant suffixes. Other endpoints > used the full model refs. > [...] > Direct calls from Carrack to worker-hosted models didn't work. > Carrack to Lemony 35B returned HTTP 404. Reproduced on a 2-node mesh (Mac M4 Max + Mac Studio M3 Ultra): * M4 served `Qwen/Qwen2.5-3B-Instruct-GGUF:qwen2.5-3b-instruct-q4_k_m`. * Studio served `unsloth/Qwen3-0.6B-GGUF:BF16`. * M4 `/v1/models` listed: Qwen/Qwen2.5-3B-Instruct-GGUF (quant suffix lost) unsloth/Qwen3-0.6B-GGUF:BF16 (full id) * Calling either listed id directly: local short id \u2192 200 (rewritten via internal alias table) remote short id \u2192 404 (no alias for remote models) The natural client flow \u2014 read `/v1/models`, take an id, call `/v1/chat/completions` with it \u2014 was broken for remote-hosted models, and inconsistent for local ones. ## Two root causes 1. **`quant_selector_from_gguf_file` was uppercase-only** when matching markers like `-Q`, `-BF16`. Real GGUF filenames mix cases (`...-Q4_K_M.gguf` vs `...-q4_k_m.gguf`). The matcher on the read side (`gguf_matches_quant_selector`) was already case-insensitive, so emitting a lowercase selector is safe and keeps the public id round-trippable. 2. **`public_huggingface_model_ref` only handled artifact-as-filename.** Locally-built `ServedModelDescriptor`s set `artifact = model_ref.selector` \u2014 e.g. `"qwen2.5-3b-instruct-q4_k_m"`, a quant selector, not a GGUF filename. `quant_selector_from_gguf_file` returned None for anything not ending `.gguf`, so the public id collapsed to just the repo name. The `public_model_id` selection logic also needed tightening so we fall back to local disk only when the descriptor cannot produce a lossless id (e.g. catalog or local-gguf identities without enough metadata). ## Test (added first, observed failing) `models_list_id_preserves_quant_suffix_when_descriptor_has_no_artifact` in `transport.rs` builds a HuggingFace descriptor with no `artifact` field and asserts the resulting public id either matches the internal model_name verbatim or carries a non-empty quant tag. Pre-fix the public id collapsed to bare repo and the test failed. ## Fix * `model-ref/src/lib.rs::quant_selector_from_gguf_file` \u2014 lowercases the stem before matching markers so lowercase-quant filenames like `qwen2.5-3b-instruct-q4_k_m.gguf` extract `q4_k_m` instead of None. Returns the slice from the original stem so display casing is preserved. * `transport.rs::public_huggingface_model_ref` \u2014 accepts artifact values that are already a quant selector (no `.gguf` suffix) in addition to GGUF filenames. The selector now round-trips through the resolver. * `transport.rs::public_model_id` \u2014 prefers the descriptor only when its identity carries enough information to produce a lossless id (HuggingFace needs an artifact; Catalog needs a canonical_ref). Otherwise falls back to the on-disk file, then the model_name itself \u2014 never silently drops information. ## Validation `cargo test -p mesh-llm-host-runtime --lib` \u2014 1435/1435 pass. `cargo test -p model-ref` \u2014 10/10 pass. Live 2-node mesh (M4 gateway + Studio peer): `/v1/models` now reports both models with their full ids: Qwen/Qwen2.5-3B-Instruct-GGUF:q4_k_m unsloth/Qwen3-0.6B-GGUF:BF16 Direct `/v1/chat/completions` calls with either listed id return 200 and real inference for both local and remote models. `model: "mesh"` (MoA) still works end-to-end on the same setup, returning a real fanout response (`Tokyo` from the capital-of-japan prompt). `cargo clippy -p mesh-llm-host-runtime --all-targets -- -D warnings` \u2014 clean. `cargo fmt --all -- --check` \u2014 clean.
This commit is contained in:
Michael Neale
parent
64c4e0ec83
commit
f3355bfd34
2 changed files with 113 additions and 6 deletions
|
|
@ -3928,12 +3928,45 @@ fn descriptor_for_model<'a>(
|
|||
}
|
||||
|
||||
fn public_model_id(model_name: &str, descriptor: Option<&mesh::ServedModelDescriptor>) -> String {
|
||||
// A descriptor with an `artifact` field has enough information to
|
||||
// produce a public ID that round-trips to the same model. Without
|
||||
// it, the HuggingFace path collapses to just the repo name and
|
||||
// silently drops the quant-tag suffix the resolver needs (PR #566
|
||||
// review feedback — "some IDs in /v1/models dropped quant
|
||||
// suffixes"). Only use the descriptor-derived id when it can be
|
||||
// lossless; otherwise prefer the on-disk file (authoritative for
|
||||
// local models), and finally the internal model_name (which
|
||||
// always carries the quant suffix our resolver knows how to
|
||||
// route).
|
||||
if let Some(descriptor) = descriptor {
|
||||
return public_model_id_from_identity(&descriptor.identity)
|
||||
.unwrap_or_else(|| model_name.to_string());
|
||||
if descriptor_can_produce_lossless_id(&descriptor.identity) {
|
||||
if let Some(id) = public_model_id_from_identity(&descriptor.identity) {
|
||||
return id;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
public_model_id_from_local_path(model_name).unwrap_or_else(|| model_name.to_string())
|
||||
if let Some(id) = public_model_id_from_local_path(model_name) {
|
||||
return id;
|
||||
}
|
||||
|
||||
model_name.to_string()
|
||||
}
|
||||
|
||||
/// A descriptor identity carries enough information for
|
||||
/// `public_model_id_from_identity` to produce an ID that round-trips
|
||||
/// to the same model. For HuggingFace that means the `artifact` field
|
||||
/// (the GGUF file name) is present so the quant selector can be
|
||||
/// derived. Catalog identities always carry a `canonical_ref` with the
|
||||
/// selector baked in.
|
||||
fn descriptor_can_produce_lossless_id(identity: &mesh::ServedModelIdentity) -> bool {
|
||||
match identity.source_kind {
|
||||
mesh::ModelSourceKind::HuggingFace => identity.artifact.is_some(),
|
||||
mesh::ModelSourceKind::Catalog => identity.canonical_ref.is_some(),
|
||||
mesh::ModelSourceKind::LocalGguf
|
||||
| mesh::ModelSourceKind::DirectUrl
|
||||
| mesh::ModelSourceKind::Unknown => false,
|
||||
}
|
||||
}
|
||||
|
||||
fn public_model_id_from_identity(identity: &mesh::ServedModelIdentity) -> Option<String> {
|
||||
|
|
@ -3972,7 +4005,16 @@ fn public_model_id_from_local_path(model_name: &str) -> Option<String> {
|
|||
}
|
||||
|
||||
fn public_huggingface_model_ref(repo: &str, artifact: Option<&str>) -> Option<String> {
|
||||
let selector = artifact.and_then(model_ref::quant_selector_from_gguf_file);
|
||||
// `artifact` can be either a GGUF filename (e.g. `Falcon-Q4_K_M.gguf`)
|
||||
// or an already-extracted quant selector (e.g. `Q4_K_M` or
|
||||
// `qwen2.5-3b-instruct-q4_k_m`, when the descriptor was built from
|
||||
// a parsed `ModelRef::selector`). Handle both — if the artifact
|
||||
// looks like a quant selector use it directly; otherwise try to
|
||||
// pull a selector out of the filename.
|
||||
let selector = artifact.and_then(|a| {
|
||||
model_ref::quant_selector_from_gguf_file(a)
|
||||
.or_else(|| (!a.is_empty() && !a.ends_with(".gguf")).then(|| a.to_string()))
|
||||
});
|
||||
Some(model_ref::format_model_ref(repo, None, selector.as_deref()))
|
||||
}
|
||||
|
||||
|
|
@ -4385,6 +4427,62 @@ mod tests {
|
|||
assert_eq!(body["data"][0]["owned_by"], "mesh-llm");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn models_list_id_preserves_quant_suffix_when_descriptor_has_no_artifact() {
|
||||
// Regression for PR #566 review feedback: the gateway's view of a
|
||||
// model's public ID must include enough information to route a
|
||||
// request back to that exact model. When a `ServedModelDescriptor`
|
||||
// for a HuggingFace model has no `artifact` field (because the
|
||||
// descriptor was built without inspecting the GGUF file on disk),
|
||||
// `public_huggingface_model_ref` collapses the public ID to just
|
||||
// the repo name — dropping the quant-tag suffix the internal
|
||||
// `model_name` carries. The model is then advertised in `/v1/models`
|
||||
// under a shorter ID than the resolver knows how to route.
|
||||
//
|
||||
// Symptom on a real 2-node mesh: the studio's Qwen3-0.6B-GGUF
|
||||
// shows as `unsloth/Qwen3-0.6B-GGUF:BF16` (descriptor has
|
||||
// artifact), but the gateway-local Qwen2.5-3B-Instruct-GGUF
|
||||
// shows as `Qwen/Qwen2.5-3B-Instruct-GGUF` (descriptor has no
|
||||
// artifact). A client doing the natural thing — read /v1/models,
|
||||
// call /v1/chat/completions with the listed id — then 404s on
|
||||
// remote models because the resolver doesn't know the short id.
|
||||
//
|
||||
// Acceptable behaviour: the public ID either round-trips to the
|
||||
// same model, OR includes the quant suffix the internal name
|
||||
// carries.
|
||||
let models = vec!["Qwen/Qwen2.5-3B-Instruct-GGUF:qwen2.5-3b-instruct-q4_k_m".to_string()];
|
||||
let descriptor = mesh::ServedModelDescriptor {
|
||||
identity: mesh::ServedModelIdentity {
|
||||
model_name: models[0].clone(),
|
||||
source_kind: mesh::ModelSourceKind::HuggingFace,
|
||||
repository: Some("Qwen/Qwen2.5-3B-Instruct-GGUF".to_string()),
|
||||
// No artifact — this is the field whose absence loses the
|
||||
// quant suffix.
|
||||
artifact: None,
|
||||
..Default::default()
|
||||
},
|
||||
..Default::default()
|
||||
};
|
||||
let descriptors = vec![descriptor];
|
||||
|
||||
let body = models_list_json(&models, &descriptors);
|
||||
let public_id = body["data"][0]["id"].as_str().unwrap_or_default();
|
||||
|
||||
// The public ID must NOT silently drop the quant suffix that the
|
||||
// internal model_name carries. Acceptable IDs:
|
||||
// * the full internal name, OR
|
||||
// * the repo with a quant tag we can route back to.
|
||||
assert!(
|
||||
public_id == models[0]
|
||||
|| public_id
|
||||
.strip_prefix("Qwen/Qwen2.5-3B-Instruct-GGUF:")
|
||||
.is_some_and(|tag| !tag.is_empty()),
|
||||
"public id must keep enough information to route back; got {public_id:?}, \
|
||||
internal model_name was {:?}",
|
||||
models[0]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn models_list_uses_catalog_model_ref_ids() {
|
||||
let models = vec!["Falcon-H1-1.5B-Instruct-Q4_K_M".to_string()];
|
||||
|
|
|
|||
|
|
@ -113,10 +113,19 @@ pub fn quant_selector_from_gguf_file(file: &str) -> Option<String> {
|
|||
stem = prefix;
|
||||
}
|
||||
|
||||
// Markers are matched case-insensitively. Real-world GGUF filenames
|
||||
// are inconsistent: `Qwen3-32B-Q4_K_M.gguf` (uppercase markers) is
|
||||
// common, but so is `qwen2.5-3b-instruct-q4_k_m.gguf` (all
|
||||
// lowercase). The matcher in `gguf_matches_quant_selector` already
|
||||
// lowercases both sides, so emitting a lowercase selector here is
|
||||
// safe and keeps the public ID round-trippable.
|
||||
let stem_lower = stem.to_ascii_lowercase();
|
||||
for marker in [
|
||||
"-UD-", ".UD-", "-IQ", ".IQ", "-Q", ".Q", "-BF16", ".BF16", "-F16", ".F16", "-F32", ".F32",
|
||||
"-ud-", ".ud-", "-iq", ".iq", "-q", ".q", "-bf16", ".bf16", "-f16", ".f16", "-f32", ".f32",
|
||||
] {
|
||||
if let Some(pos) = stem.rfind(marker) {
|
||||
if let Some(pos) = stem_lower.rfind(marker) {
|
||||
// Use the original casing for the returned slice so callers
|
||||
// that build a public ID keep the model's preferred display.
|
||||
return Some(stem[pos + 1..].to_string());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue