mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(inference): default empty model to reasoning-v1 on OpenHuman backend (#2837)
This commit is contained in:
@@ -32,6 +32,29 @@ fn prompt_guard_user_message(action: PromptEnforcementAction) -> &'static str {
|
||||
}
|
||||
}
|
||||
|
||||
/// Normalize a `model_override` string into the `Option<String>` form the
|
||||
/// downstream config-resolution path expects.
|
||||
///
|
||||
/// `None` → `None`. `Some(non-empty-after-trim)` → `Some(trimmed)`. Anything
|
||||
/// else (`Some("")`, `Some(" ")`, `Some("\t\n")`) collapses to `None` so
|
||||
/// the existing default-model fallback applies instead of overwriting
|
||||
/// `config.default_model` with a blank string that the OpenHuman backend
|
||||
/// would reject with `400 model is required` (Sentry TAURI-RUST-RS).
|
||||
///
|
||||
/// Extracted to keep `agent_chat` and `agent_chat_simple` in lockstep —
|
||||
/// future tweaks (additional log lines, tightening the trim rules) live in
|
||||
/// exactly one place.
|
||||
fn normalize_model_override(opt: Option<String>) -> Option<String> {
|
||||
opt.and_then(|m| {
|
||||
let t = m.trim();
|
||||
if t.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(t.to_string())
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
fn enforce_user_prompt_or_reject(prompt: &str, source: &'static str) -> Result<(), String> {
|
||||
let decision = enforce_prompt_input(
|
||||
prompt,
|
||||
@@ -69,7 +92,10 @@ pub async fn agent_chat(
|
||||
) -> Result<RpcOutcome<String>, String> {
|
||||
enforce_user_prompt_or_reject(message, "local_ai.ops.agent_chat")?;
|
||||
|
||||
if let Some(model) = model_override {
|
||||
// TAURI-RUST-RS: an upstream caller (frontend, JSON-RPC client) can pass
|
||||
// `model_override: Some("")`. See `normalize_model_override` for the
|
||||
// rationale — an empty / whitespace-only override collapses to `None`.
|
||||
if let Some(model) = normalize_model_override(model_override) {
|
||||
config.default_model = Some(model);
|
||||
}
|
||||
if let Some(temp) = temperature {
|
||||
@@ -90,7 +116,8 @@ pub async fn agent_chat_simple(
|
||||
enforce_user_prompt_or_reject(message, "local_ai.ops.agent_chat_simple")?;
|
||||
|
||||
let mut effective = config.clone();
|
||||
if let Some(model) = model_override {
|
||||
// TAURI-RUST-RS: see `normalize_model_override` for the rationale.
|
||||
if let Some(model) = normalize_model_override(model_override) {
|
||||
effective.default_model = Some(model);
|
||||
}
|
||||
if let Some(temp) = temperature {
|
||||
|
||||
@@ -146,3 +146,37 @@ async fn local_ai_assets_status_returns_without_panic() {
|
||||
let config = test_config(&tmp);
|
||||
let _ = local_ai_assets_status(&config).await;
|
||||
}
|
||||
|
||||
// ── normalize_model_override (TAURI-RUST-RS) ───────────────────────────
|
||||
|
||||
#[test]
|
||||
fn normalize_model_override_passthrough_none() {
|
||||
assert_eq!(normalize_model_override(None), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn normalize_model_override_blank_collapses_to_none() {
|
||||
assert_eq!(normalize_model_override(Some(String::new())), None);
|
||||
assert_eq!(normalize_model_override(Some(" ".to_string())), None);
|
||||
assert_eq!(normalize_model_override(Some("\t\n".to_string())), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn normalize_model_override_trims_surrounding_whitespace() {
|
||||
assert_eq!(
|
||||
normalize_model_override(Some(" reasoning-v1 ".to_string())),
|
||||
Some("reasoning-v1".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn normalize_model_override_passes_non_empty_verbatim() {
|
||||
assert_eq!(
|
||||
normalize_model_override(Some("agentic-v1".to_string())),
|
||||
Some("agentic-v1".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
normalize_model_override(Some("hint:reasoning".to_string())),
|
||||
Some("hint:reasoning".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
@@ -15,6 +15,40 @@ use std::path::PathBuf;
|
||||
|
||||
pub const PROVIDER_LABEL: &str = "OpenHuman";
|
||||
|
||||
/// Normalize an inbound `model` argument before forwarding to the OpenHuman backend.
|
||||
///
|
||||
/// The backend rejects a blank `model` field with
|
||||
/// `400 {"success":false,"error":"model is required"}` (Sentry **TAURI-RUST-RS**,
|
||||
/// 163 events / 14d). Empty values reach this layer when a workload routes to
|
||||
/// `<slug>:` with no model after the colon (see the `[config][migrate]`
|
||||
/// rewrites in `src/openhuman/config/schema/load.rs:967`) or when an upstream
|
||||
/// caller passes `model_override: Some("")`.
|
||||
///
|
||||
/// Substitute the canonical default tier so the call succeeds instead of
|
||||
/// failing the wire round-trip. Mirrors the same fallback `make_openhuman_backend`
|
||||
/// already applies when `default_model` is missing
|
||||
/// (`src/openhuman/inference/provider/factory.rs:404`), so behavior stays
|
||||
/// consistent across both entry paths.
|
||||
fn resolve_model(model: &str) -> String {
|
||||
let trimmed = model.trim();
|
||||
if trimmed.is_empty() {
|
||||
// Debug-tier on purpose: the routing-migration path
|
||||
// (`config/schema/load.rs:967`) can hit this on every chat turn for
|
||||
// an affected user (~163 events / 14d on Sentry pre-fix). Warn-tier
|
||||
// here would just move the noise from Sentry to local log dashboards.
|
||||
// Per-process throttling via `Once` was considered — debug is simpler
|
||||
// and gives the same diagnostic when needed (set RUST_LOG=debug).
|
||||
log::debug!(
|
||||
"[providers][openhuman-backend] empty model passed to OpenHuman backend; \
|
||||
substituting default `{}` (TAURI-RUST-RS)",
|
||||
crate::openhuman::config::MODEL_REASONING_V1
|
||||
);
|
||||
crate::openhuman::config::MODEL_REASONING_V1.to_string()
|
||||
} else {
|
||||
trimmed.to_string()
|
||||
}
|
||||
}
|
||||
|
||||
/// Routes chat to `config.api_url` + `/openai` with `Authorization: Bearer` from the `app-session` profile.
|
||||
pub struct OpenHumanBackendProvider {
|
||||
options: ProviderRuntimeOptions,
|
||||
@@ -109,8 +143,9 @@ impl Provider for OpenHumanBackendProvider {
|
||||
) -> anyhow::Result<String> {
|
||||
let token = self.resolve_bearer()?;
|
||||
let inner = self.inner(&token)?;
|
||||
let model = resolve_model(model);
|
||||
inner
|
||||
.chat_with_system(system_prompt, message, model, temperature)
|
||||
.chat_with_system(system_prompt, message, &model, temperature)
|
||||
.await
|
||||
}
|
||||
|
||||
@@ -122,7 +157,8 @@ impl Provider for OpenHumanBackendProvider {
|
||||
) -> anyhow::Result<String> {
|
||||
let token = self.resolve_bearer()?;
|
||||
let inner = self.inner(&token)?;
|
||||
inner.chat_with_history(messages, model, temperature).await
|
||||
let model = resolve_model(model);
|
||||
inner.chat_with_history(messages, &model, temperature).await
|
||||
}
|
||||
|
||||
async fn chat(
|
||||
@@ -133,7 +169,8 @@ impl Provider for OpenHumanBackendProvider {
|
||||
) -> anyhow::Result<ChatResponse> {
|
||||
let token = self.resolve_bearer()?;
|
||||
let inner = self.inner(&token)?;
|
||||
inner.chat(request, model, temperature).await
|
||||
let model = resolve_model(model);
|
||||
inner.chat(request, &model, temperature).await
|
||||
}
|
||||
|
||||
async fn warmup(&self) -> anyhow::Result<()> {
|
||||
@@ -154,6 +191,9 @@ impl Provider for OpenHumanBackendProvider {
|
||||
_temperature: f64,
|
||||
_options: StreamOptions,
|
||||
) -> futures_util::stream::BoxStream<'static, StreamResult<StreamChunk>> {
|
||||
// TODO(stream-support): when streaming is enabled here, route
|
||||
// `_model` through `resolve_model` before forwarding — same blank
|
||||
// model guard as the non-streaming methods (TAURI-RUST-RS).
|
||||
stream::once(async move {
|
||||
Ok(StreamChunk::error(
|
||||
"streaming is not supported for OpenHuman backend provider",
|
||||
@@ -162,3 +202,48 @@ impl Provider for OpenHumanBackendProvider {
|
||||
.boxed()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
// TAURI-RUST-RS regression coverage: the OpenHuman backend rejects an
|
||||
// empty `model` field with 400 `model is required`. `resolve_model` must
|
||||
// intercept blank / whitespace-only values before they hit the wire and
|
||||
// substitute the canonical default tier.
|
||||
|
||||
#[test]
|
||||
fn resolve_model_substitutes_default_for_empty() {
|
||||
assert_eq!(
|
||||
resolve_model(""),
|
||||
crate::openhuman::config::MODEL_REASONING_V1
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_model_substitutes_default_for_whitespace_only() {
|
||||
assert_eq!(
|
||||
resolve_model(" "),
|
||||
crate::openhuman::config::MODEL_REASONING_V1
|
||||
);
|
||||
assert_eq!(
|
||||
resolve_model("\t\n"),
|
||||
crate::openhuman::config::MODEL_REASONING_V1
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_model_trims_surrounding_whitespace() {
|
||||
assert_eq!(resolve_model(" reasoning-v1 "), "reasoning-v1");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_model_preserves_non_empty_value_verbatim() {
|
||||
// Non-empty values are passed through unchanged (after trim) — no
|
||||
// canonicalisation, no remapping. The backend is authoritative over
|
||||
// which model strings it accepts.
|
||||
assert_eq!(resolve_model("agentic-v1"), "agentic-v1");
|
||||
assert_eq!(resolve_model("hint:reasoning"), "hint:reasoning");
|
||||
assert_eq!(resolve_model("some-custom-model"), "some-custom-model");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user