fix(llm): auto-append /v1 to LLM_BASE_URL for openai_compatible (#1934)

[ilblackdragon]

Summary

Fixes #1934. Local model servers (MLX, vLLM, llama.cpp) returned 404 when LLM_BASE_URL lacked a /v1 suffix because rig-core's openai client appends /chat/completions directly to the base URL.

• Added normalize_openai_base_url() in src/llm/mod.rs that auto-appends /v1 when not already present
• Applied in create_openai_compat_from_registry()
• 7 unit tests covering bare URL, trailing slash, already-has-/v1, path prefixes, etc.

Test plan

• cargo fmt, cargo clippy --all-features zero warnings
• All tests pass
• Manual: LLM_BACKEND=openai_compatible LLM_BASE_URL=http://localhost:8080 ironclaw succeeds against a local server

🤖 Generated with Claude Code

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[ilblackdragon]

Patch looks small and coherent, and the intent matches the documented local-server use case.

Residual concern: this now rewrites every OpenAI-compatible base URL by appending /v1 unless it already ends in /v1. That is correct for the local servers named in the PR, but it bakes in an assumption about all custom proxies. If you keep this approach, I would at least add a caller-level test around the provider factory so we assert the constructed client base URL for both a bare local endpoint and an already-versioned custom endpoint. Right now the coverage is helper-only.

[ilblackdragon]

Addressed in 25baf2b: added caller-level factory test asserting the constructed OpenAICompatibleProvider base URL for both a bare endpoint (http://localhost:8080 → .../v1) and an already-versioned one (https://proxy/v1 → unchanged, no double suffix).

[serrrfirat]

Review of PR #2310 -- 2 findings (both Medium).

[serrrfirat]

Medium Severity -- False positive path-segment matching

ends_with("/v1") is a substring check on the full string. A URL ending in /apiv1 or /myv1 would incorrectly match and would NOT get /v1 appended, silently sending requests to the wrong endpoint.

Suggested fix: Check the last path segment explicitly:

trimmed.rsplit_once('/').map(|(_, last)| last) == Some("v1")

[serrrfirat]

Medium Severity -- Debug log shows pre-normalization URL

This tracing::debug! log emits config.base_url (the original, pre-normalization URL), but actual requests go to the normalized URL with /v1 appended (line 22 above). During incident response, this mismatch between logged URL and actual request URL is a debugging trap.

Suggested fix: Log the normalized URL, or log both original and normalized values.

[ilblackdragon]

Pushed 5455e556 addressing review findings:

• Fix A: ends_with("/v1") replaced with rsplit_once('/').map(|(_, last)| last) == Some("v1") — prevents false positives on paths like /apiv1, /myv1. Added two regression tests.
• Fix B: tracing::debug! in create_openai_compat_from_registry now logs both original_base_url and normalized_base_url (via client.base_url()).

Quality gate: cargo fmt clean, cargo clippy --all --all-features zero warnings, cargo test --lib -- llm 759/759 pass.

[ilblackdragon]

All prior review feedback has been addressed in the current diff:

• rsplit_once('/').map(|(_, last)| last) == Some("v1") correctly handles false positives like /apiv1 and /myv1 -- two dedicated tests confirm this.
• Debug logging now shows both original_base_url and normalized_base_url -- no more debugging traps.
• The refactoring to extract build_openai_compat_client() enables clean caller-level testing. Three factory tests verify that normalization actually flows through to the constructed rig-core client.
• 9 helper-level normalization tests + 3 caller-level factory tests provide thorough coverage.

One minor observation: normalize_openai_base_url with "http://localhost:8080///" (multiple trailing slashes) normalizes to "http://localhost:8080/v1" via trim_end_matches('/'). This is correct behavior but slightly surprising -- trim_end_matches strips all trailing slashes, so the intermediate empty path segments are removed. The test covers it, so this is fine.

LGTM -- ready to merge.

[ilblackdragon]

Closing — already fixed on staging, and this diff would regress the fix. Issue #1934 is closed.

Shipped via:

• fix(llm): image detail field + /v1 base URL normalization #2380 — primary fix (merged 2026-04-13)
• Commit `fdcbf184` — initial /v1 auto-append for openai_compatible: LLM_BASE_URL requires /v1 suffix after rig-core migration #1934
• Commit `b81aa52b` — combined with image-detail fix
• Commit `8416dd29` — refined to scope /v1 normalization to bare host-only URLs

The staging version of `normalize_openai_base_url` only appends `/v1` when the URL has no path (bare `scheme://host[:port]`), and is case-insensitive on the `/v1` match. This PR appends `/v1` whenever the last path segment isn't `v1`, which would wrongly mutate custom-path providers:

Input	Staging	This PR
`https://api.z.ai/api/paas/v4\`	unchanged	`.../v4/v1` ❌
`https://generativelanguage.googleapis.com/v1beta/openai\`	unchanged	`.../openai/v1` ❌
`https://example.com/apiv1\`	unchanged	`.../apiv1/v1` ❌

The `build_openai_compat_client` extraction + caller-level tests are a nice pattern, but the staging helper already has unit coverage and the policy there is the one we want.