mirror of
https://github.com/nearai/ironclaw.git
synced 2026-09-02 23:56:24 +08:00
fix(llm): image detail field + /v1 base URL normalization (#2380)
* fix(llm): add image detail field, auto-append /v1 to base URL (#2378, #1934) Set detail: "auto" on ImageUrl construction so providers requiring the field (e.g. MiniMax) no longer reject vision requests. Normalize OpenAI-compatible base URLs by appending /v1 when missing, fixing 404s for local model servers (MLX, vLLM, llama.cpp) using bare URLs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(llm): scope /v1 normalization to bare host-only URLs Address review feedback: - Only append /v1 when the URL has no path component (bare scheme://host[:port]). URLs with existing paths like Zai's /api/paas/v4 or Gemini's /v1beta/openai are now left unchanged. - Use case-insensitive check for /v1 suffix to prevent double-suffixing URLs like http://localhost:8080/V1. - Document why Ollama is intentionally excluded from normalization (uses /api/chat, not /v1/chat/completions). - Add test cases for real provider URLs from providers.json (Zai, Gemini) and case-insensitive /V1. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: update gimli 0.33.1 -> 0.33.0 (yanked crate) gimli v0.33.1 was yanked on crates.io, causing cargo-deny to fail. Downgrade to v0.33.0 which is the latest non-yanked release compatible with wasmtime 43. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: serrrfirat <f@nuff.tech>
This commit is contained in:
4
Cargo.lock
generated
4
Cargo.lock
generated
@@ -3112,9 +3112,9 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "gimli"
|
||||
version = "0.33.1"
|
||||
version = "0.33.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "19e16c5073773ccf057c282be832a59ee53ef5ff98db3aeff7f8314f52ffc196"
|
||||
checksum = "0bf7f043f89559805f8c7cacc432749b2fa0d0a0a9ee46ce47164ed5ba7f126c"
|
||||
dependencies = [
|
||||
"fnv",
|
||||
"hashbrown 0.16.1",
|
||||
|
||||
@@ -43,7 +43,7 @@ pub fn augment_with_attachments(
|
||||
image_parts.push(ContentPart::ImageUrl {
|
||||
image_url: ImageUrl {
|
||||
url: data_url,
|
||||
detail: None,
|
||||
detail: Some("auto".to_string()),
|
||||
},
|
||||
});
|
||||
}
|
||||
@@ -242,6 +242,26 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn image_url_includes_detail_auto() {
|
||||
let mut att = make_attachment(AttachmentKind::Image);
|
||||
att.mime_type = "image/png".to_string();
|
||||
att.data = vec![0x89, 0x50, 0x4E, 0x47]; // fake PNG header
|
||||
|
||||
let result = augment_with_attachments("check", &[att]).unwrap();
|
||||
assert_eq!(result.image_parts.len(), 1);
|
||||
match &result.image_parts[0] {
|
||||
ContentPart::ImageUrl { image_url } => {
|
||||
assert_eq!(
|
||||
image_url.detail.as_deref(),
|
||||
Some("auto"),
|
||||
"detail field must be set to 'auto' for provider compatibility"
|
||||
);
|
||||
}
|
||||
other => panic!("Expected ImageUrl, got: {:?}", other),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn document_with_extracted_text() {
|
||||
let mut att = make_attachment(AttachmentKind::Document);
|
||||
|
||||
@@ -285,7 +285,8 @@ fn create_openai_compat_from_registry(
|
||||
|
||||
let mut builder = openai::Client::builder().api_key(&api_key);
|
||||
if !config.base_url.is_empty() {
|
||||
builder = builder.base_url(&config.base_url);
|
||||
let base_url = normalize_openai_base_url(&config.base_url);
|
||||
builder = builder.base_url(&base_url);
|
||||
}
|
||||
if !extra_headers.is_empty() {
|
||||
builder = builder.http_headers(extra_headers);
|
||||
@@ -707,6 +708,34 @@ pub fn create_gemini_oauth_provider(config: &LlmConfig) -> Result<Arc<dyn LlmPro
|
||||
Ok(Arc::new(provider))
|
||||
}
|
||||
|
||||
/// Normalize an OpenAI-compatible base URL by appending `/v1` when the URL
|
||||
/// contains no path (bare `scheme://host[:port]`).
|
||||
///
|
||||
/// rig-core's `openai::Client` does not auto-append `/v1/` to the base URL,
|
||||
/// so local model servers (MLX, vLLM, llama.cpp) using bare URLs like
|
||||
/// `http://localhost:8080` get 404s. This mirrors the old
|
||||
/// `NearAiChatProvider::api_url()` behavior.
|
||||
///
|
||||
/// URLs that already carry a path — including non-`/v1` versioned paths such
|
||||
/// as Zai's `/api/paas/v4` or Gemini's `/v1beta/openai` — are returned
|
||||
/// unchanged so we don't corrupt provider-specific endpoints.
|
||||
///
|
||||
/// **Note:** This is intentionally applied only to `OpenAiCompletions`-protocol
|
||||
/// providers. Ollama uses `/api/chat` (not `/v1/chat/completions`) and its
|
||||
/// rig-core client handles the path internally, so normalization is not needed.
|
||||
fn normalize_openai_base_url(url: &str) -> String {
|
||||
let trimmed = url.trim_end_matches('/');
|
||||
if trimmed.to_ascii_lowercase().ends_with("/v1") {
|
||||
return trimmed.to_string();
|
||||
}
|
||||
match url::Url::parse(trimmed) {
|
||||
Ok(parsed) if parsed.path().is_empty() || parsed.path() == "/" => {
|
||||
format!("{trimmed}/v1")
|
||||
}
|
||||
_ => trimmed.to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -867,4 +896,59 @@ mod tests {
|
||||
let config = test_llm_config();
|
||||
assert_eq!(config.cheap_model_name(), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_normalize_openai_base_url_appends_v1_for_bare_hosts() {
|
||||
assert_eq!(
|
||||
normalize_openai_base_url("http://localhost:8080"),
|
||||
"http://localhost:8080/v1"
|
||||
);
|
||||
assert_eq!(
|
||||
normalize_openai_base_url("http://localhost:8080/"),
|
||||
"http://localhost:8080/v1"
|
||||
);
|
||||
assert_eq!(
|
||||
normalize_openai_base_url("https://my-server.example.com"),
|
||||
"https://my-server.example.com/v1"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_normalize_openai_base_url_leaves_v1_alone() {
|
||||
assert_eq!(
|
||||
normalize_openai_base_url("http://localhost:8080/v1"),
|
||||
"http://localhost:8080/v1"
|
||||
);
|
||||
assert_eq!(
|
||||
normalize_openai_base_url("http://localhost:8080/v1/"),
|
||||
"http://localhost:8080/v1"
|
||||
);
|
||||
assert_eq!(
|
||||
normalize_openai_base_url("https://api.openai.com/v1"),
|
||||
"https://api.openai.com/v1"
|
||||
);
|
||||
// Case-insensitive: /V1 should not get double-suffixed
|
||||
assert_eq!(
|
||||
normalize_openai_base_url("http://localhost:8080/V1"),
|
||||
"http://localhost:8080/V1"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_normalize_openai_base_url_preserves_existing_paths() {
|
||||
// Non-/v1 versioned paths from real providers must stay unchanged
|
||||
assert_eq!(
|
||||
normalize_openai_base_url("https://api.z.ai/api/paas/v4"),
|
||||
"https://api.z.ai/api/paas/v4"
|
||||
);
|
||||
assert_eq!(
|
||||
normalize_openai_base_url("https://generativelanguage.googleapis.com/v1beta/openai"),
|
||||
"https://generativelanguage.googleapis.com/v1beta/openai"
|
||||
);
|
||||
// Custom subpaths should also stay unchanged
|
||||
assert_eq!(
|
||||
normalize_openai_base_url("https://api.example.com/custom"),
|
||||
"https://api.example.com/custom"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user