Skip to content

Commit 034e4e1

Browse files
committed
fix(dash): discover served model for configured chat endpoint
A configured chat URL (OPENAI_BASE_URL / --chat-url) with no explicit model resolved to the "local-model" placeholder, which 404s on servers that register the model under its real id. Query /v1/models on startup to adopt the served model, mirroring the managed-service path. Gated on a reachable probe so an unreachable endpoint adds no fetch timeout to launch, and an explicit --chat-model/config value still wins. Signed-off-by: Eugene Volen <Eugene.Volen@amd.com>
1 parent 5bcd8b1 commit 034e4e1

2 files changed

Lines changed: 74 additions & 3 deletions

File tree

crates/rocm-dash-tui/src/app/chat.rs

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,8 +282,28 @@ pub(super) fn config_with_chat(
282282
cfg
283283
}
284284

285+
/// Fold a `/v1/models`-discovered model id into a resolved [`LlmConfig`].
286+
///
287+
/// Only overrides when the user did **not** configure a model explicitly (so the
288+
/// resolver filled in the neutral `DEFAULT_CHAT_MODEL` placeholder). An explicit
289+
/// `--chat-model`/config value always wins; a failed discovery (`None`) leaves
290+
/// the placeholder in place so endpoints that ignore the model field keep
291+
/// working. Pure — the unit-test anchor for the startup model-discovery wiring.
292+
pub(super) fn with_discovered_model(
293+
mut llm: crate::llm::LlmConfig,
294+
explicit_model: Option<&str>,
295+
discovered: Option<String>,
296+
) -> crate::llm::LlmConfig {
297+
if explicit_model.is_none()
298+
&& let Some(model) = discovered
299+
{
300+
llm.model = model;
301+
}
302+
llm
303+
}
304+
285305
/// GET `{base}/models` and return the first served model id, or `None`.
286-
async fn fetch_first_model(base_url: &str) -> Option<String> {
306+
pub(super) async fn fetch_first_model(base_url: &str) -> Option<String> {
287307
let url = format!("{}/models", base_url.trim_end_matches('/'));
288308
let client = reqwest::Client::builder()
289309
.timeout(std::time::Duration::from_secs(3))
@@ -408,4 +428,36 @@ mod tests {
408428
let env = services_envelope(serde_json::json!([service("vllm", "", "ready", "m", 100)]));
409429
assert_eq!(pick_managed_chat_endpoint(&env), None);
410430
}
431+
432+
fn placeholder_config() -> crate::llm::LlmConfig {
433+
crate::llm::detected_llm_config(
434+
"http://127.0.0.1:11435/v1",
435+
crate::llm::DEFAULT_CHAT_MODEL,
436+
)
437+
}
438+
439+
#[test]
440+
fn discovered_model_replaces_placeholder_when_none_configured() {
441+
// The 404 regression: no explicit model → the served id must be adopted.
442+
let out = with_discovered_model(
443+
placeholder_config(),
444+
None,
445+
Some("Qwen/Qwen2.5-1.5B-Instruct".to_string()),
446+
);
447+
assert_eq!(out.model, "Qwen/Qwen2.5-1.5B-Instruct");
448+
}
449+
450+
#[test]
451+
fn explicit_model_is_never_overridden() {
452+
let mut cfg = placeholder_config();
453+
cfg.model = "my-model".to_string();
454+
let out = with_discovered_model(cfg, Some("my-model"), Some("served-id".to_string()));
455+
assert_eq!(out.model, "my-model");
456+
}
457+
458+
#[test]
459+
fn failed_discovery_keeps_placeholder() {
460+
let out = with_discovered_model(placeholder_config(), None, None);
461+
assert_eq!(out.model, crate::llm::DEFAULT_CHAT_MODEL);
462+
}
411463
}

crates/rocm-dash-tui/src/app/mod.rs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ mod slash;
3939
mod summary;
4040

4141
use chat::{
42-
build_chat_agent, build_local_agent, detect_local_chat, detect_managed_chat,
43-
persist_chat_endpoint,
42+
build_chat_agent, build_local_agent, detect_local_chat, detect_managed_chat, fetch_first_model,
43+
persist_chat_endpoint, with_discovered_model,
4444
};
4545
use summary::{parse_plan_result, summarize_json_value, summarize_slash_tool};
4646

@@ -1582,6 +1582,9 @@ async fn event_loop(terminal: &mut Tui, args: &ResolvedArgs) -> color_eyre::Resu
15821582
.await
15831583
.unwrap_or(false)
15841584
};
1585+
// The managed branch already carries a `/v1/models`-discovered model; a
1586+
// resolver-produced config from a configured URL does not.
1587+
let had_managed = managed.is_some();
15851588
let llm = managed.or_else(|| {
15861589
crate::llm::resolve_llm_config(
15871590
args.chat_url.as_deref(),
@@ -1594,6 +1597,22 @@ async fn event_loop(terminal: &mut Tui, args: &ResolvedArgs) -> color_eyre::Resu
15941597
probe_ok,
15951598
)
15961599
});
1600+
// A configured URL (env/CLI) with no explicit model resolves to the
1601+
// `local-model` placeholder, which 404s on servers that register the
1602+
// model under its real id. Discover the served model via `/v1/models`,
1603+
// mirroring the managed-service path. Gated on `probe_ok` so an
1604+
// unreachable endpoint never adds the fetch timeout to startup.
1605+
let llm = match llm {
1606+
Some(cfg) if !had_managed && probe_ok && args.chat_model.is_none() => {
1607+
let discovered = fetch_first_model(&cfg.base_url).await;
1608+
Some(with_discovered_model(
1609+
cfg,
1610+
args.chat_model.as_deref(),
1611+
discovered,
1612+
))
1613+
}
1614+
other => other,
1615+
};
15971616
state.set_chat_config(llm, args.chat_auto_consent);
15981617
// No reachable local endpoint AND no key/url configured → the no-key
15991618
// ChatGPT OAuth default (device-code login surfaced in the chat tab).

0 commit comments

Comments
 (0)