From f7cae4c0a807f6393180d975dcae1cab2da7364d Mon Sep 17 00:00:00 2001 From: Blomios Date: Wed, 15 Jul 2026 08:41:44 +0200 Subject: [PATCH] fix(model-server): deadline de warmup configurable pour le cold-start llama.cpp (#55) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Corrige la régression high « Error on loading local model » : le premier chargement du serveur modèle local (cold-start llama.cpp) dépassait la fenêtre de readiness et échouait. Le warmup dispose désormais d'un deadline par défaut de 600 s, surchargable par config optionnelle `warmup_deadline_secs` (validée dans [30, 1800]). - domain: champ `warmup_deadline_secs: Option` + validation de borne - application: policy de readiness effective (défaut 600 s, override par config) - app-tauri: DTO `warmupDeadlineSecs` - infrastructure: application de la deadline effective au warmup Verdict QA (vert) : domain 252, application 81 + 22 model_server, app-tauri dto_model_servers 6, infrastructure model_server 2, build OK. Contrat readiness couvert sur ports mockés. Caveat : le cold-start end-to-end réel (llama-server) sort du sandbox de test -> vérification manuelle utilisateur restante, non couverte par les tests unitaires. Co-Authored-By: Claude Opus 4.8 --- crates/app-tauri/src/dto.rs | 5 + crates/app-tauri/tests/dto_model_servers.rs | 32 ++++++ crates/application/src/model_server.rs | 30 +++++- crates/application/tests/model_server.rs | 37 +++++++ crates/domain/src/model_server.rs | 97 ++++++++++++++++++- crates/infrastructure/src/model_server/mod.rs | 1 + crates/infrastructure/tests/model_server.rs | 5 +- 7 files changed, 200 insertions(+), 7 deletions(-) diff --git a/crates/app-tauri/src/dto.rs b/crates/app-tauri/src/dto.rs index bf7e91a..f927b1a 100644 --- a/crates/app-tauri/src/dto.rs +++ b/crates/app-tauri/src/dto.rs @@ -1057,6 +1057,9 @@ pub struct ModelServerConfigDto { pub auto_start: bool, /// Stop policy. pub stop_policy: StopPolicyDto, + /// Optional readiness warmup deadline override in seconds. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub warmup_deadline_secs: Option, } impl ModelServerConfigDto { @@ -1080,6 +1083,7 @@ impl ModelServerConfigDto { args: config.args, auto_start: config.auto_start, stop_policy: config.stop_policy.into(), + warmup_deadline_secs: config.warmup_deadline_secs, } } @@ -1122,6 +1126,7 @@ impl ModelServerConfigDto { self.auto_start, self.stop_policy.into(), ) + .and_then(|config| config.with_warmup_deadline_secs(self.warmup_deadline_secs)) .map_err(invalid_domain_error) } } diff --git a/crates/app-tauri/tests/dto_model_servers.rs b/crates/app-tauri/tests/dto_model_servers.rs index 300aa19..4b79093 100644 --- a/crates/app-tauri/tests/dto_model_servers.rs +++ b/crates/app-tauri/tests/dto_model_servers.rs @@ -59,6 +59,7 @@ fn model_server_dto_serialises_flat_camelcase_wire_shape() { assert_eq!(value["contextSize"], 4096); assert_eq!(value["jinja"], true); assert_eq!(value["stopPolicy"], "stopOnAppExit"); + assert!(value.get("warmupDeadlineSecs").is_none()); assert!(value.get("model").is_none(), "domain model must not leak"); assert!( value.get("endpoint").is_none(), @@ -101,6 +102,7 @@ fn model_server_dto_deserialises_flat_wire_shape_and_generates_model_id() { assert_eq!(input.config.options.gpu_layers, Some(35)); assert_eq!(input.config.options.context_size, Some(4096)); assert_eq!(input.config.options.host, "127.0.0.1"); + assert_eq!(input.config.warmup_deadline_secs, None); assert!(!input.config.options.jinja); assert_eq!(input.config.model.served_name, "qwen3-coder"); assert_eq!(input.config.model.label, "qwen3-coder"); @@ -129,12 +131,42 @@ fn model_server_dto_preserves_existing_internal_model_id_on_upsert() { args: Vec::new(), auto_start: true, stop_policy: StopPolicyDto::StopOnAppExit, + warmup_deadline_secs: Some(900), }; let config = dto.into_domain(Some(&existing)).unwrap(); assert_eq!(config.model.id, "stable-internal-id"); assert_eq!(config.model.served_name, "qwen3-coder-updated"); + assert_eq!(config.warmup_deadline_secs, Some(900)); +} + +#[test] +fn model_server_dto_rejects_invalid_warmup_deadline_secs() { + let raw = json!({ + "id": sid(40).to_string(), + "kind": "llamaCpp", + "name": "Local Qwen", + "baseURL": "http://localhost:8085", + "port": 8085, + "modelPath": "/models/qwen.gguf", + "servedModelName": "qwen3-coder", + "host": "127.0.0.1", + "jinja": false, + "args": [], + "autoStart": true, + "stopPolicy": "stopOnAppExit", + "warmupDeadlineSecs": 29 + }); + + let request = SaveModelServerRequestDto { + config: serde_json::from_value(raw).unwrap(), + }; + + assert_eq!( + save_model_server_input(request, None).unwrap_err().code, + "INVALID" + ); } #[test] diff --git a/crates/application/src/model_server.rs b/crates/application/src/model_server.rs index 5903cff..1aa9eba 100644 --- a/crates/application/src/model_server.rs +++ b/crates/application/src/model_server.rs @@ -162,7 +162,7 @@ impl Default for ReadinessPolicy { Self { attempts: 20, backoff: Duration::from_millis(250), - warmup_deadline: Duration::from_secs(120), + warmup_deadline: Duration::from_secs(600), } } } @@ -261,6 +261,15 @@ impl EnsureLocalModelServer { self } + /// Returns the readiness policy effective for a persisted server config. + /// + /// The config may override only the warmup deadline; probe cadence remains + /// owned by the application policy. + #[must_use] + pub fn effective_readiness_policy(&self, config: &LocalModelServerConfig) -> ReadinessPolicy { + self.readiness_for(config) + } + /// Overrides the long Hugging Face download/preparation deadline. #[must_use] pub fn with_hf_download_deadline(mut self, deadline: Duration) -> Self { @@ -440,8 +449,9 @@ impl EnsureLocalModelServer { handle: &ManagedProcessHandle, hf_source: Option, ) -> Result { + let readiness = self.readiness_for(config); let mut attempts = 0usize; - let deadline = Instant::now() + self.readiness.warmup_deadline; + let deadline = Instant::now() + readiness.warmup_deadline; loop { match self.probe.probe(&config.endpoint).await { Err(err) => { @@ -462,7 +472,7 @@ impl EnsureLocalModelServer { match self.process.status(handle).await { Ok(ProcessStatus::Running) => { - if attempts.saturating_add(1) == self.readiness.attempts { + if attempts.saturating_add(1) == readiness.attempts { if let Some(source) = hf_source.as_ref() { self.publish( config.id, @@ -496,14 +506,24 @@ impl EnsureLocalModelServer { self.stop_started_server(config.id, handle).await; return self.fail(config.id, err); } - if !self.readiness.backoff.is_zero() { - tokio::time::sleep(self.readiness.backoff).await; + if !readiness.backoff.is_zero() { + tokio::time::sleep(readiness.backoff).await; } else { tokio::task::yield_now().await; } } } + fn readiness_for(&self, config: &LocalModelServerConfig) -> ReadinessPolicy { + ReadinessPolicy { + warmup_deadline: config + .warmup_deadline_secs + .map(Duration::from_secs) + .unwrap_or(self.readiness.warmup_deadline), + ..self.readiness + } + } + /// Stops active servers whose policy is [`StopPolicy::StopOnAppExit`]. /// /// # Errors diff --git a/crates/application/tests/model_server.rs b/crates/application/tests/model_server.rs index eada9ee..4dcffbd 100644 --- a/crates/application/tests/model_server.rs +++ b/crates/application/tests/model_server.rs @@ -435,6 +435,43 @@ fn progress(downloaded: Option, total: Option) -> ModelArtifactProgres } } +#[test] +fn readiness_policy_default_warmup_deadline_is_ten_minutes() { + assert_eq!( + ModelServerReadinessPolicy::default().warmup_deadline, + Duration::from_secs(600) + ); +} + +#[test] +fn effective_readiness_policy_uses_configured_warmup_deadline_when_present() { + let configured = config(sid(23), 8103, "/models/qwen.gguf", true) + .with_warmup_deadline_secs(Some(900)) + .unwrap(); + let unconfigured = config(sid(24), 8104, "/models/qwen.gguf", true); + let usecase = EnsureLocalModelServer::new( + Arc::new(FakeRegistry::default()), + Arc::new(FakeProbe::new(Vec::new())), + Arc::new(FakeProcess::default()), + Arc::new(FakeRuntime), + Arc::new(FakeFs::default()), + Arc::new(FakeEvents::default()), + ); + + assert_eq!( + usecase + .effective_readiness_policy(&configured) + .warmup_deadline, + Duration::from_secs(900) + ); + assert_eq!( + usecase + .effective_readiness_policy(&unconfigured) + .warmup_deadline, + Duration::from_secs(600) + ); +} + #[tokio::test] async fn reachable_server_is_reused_without_spawn() { let registry = Arc::new(FakeRegistry::default()); diff --git a/crates/domain/src/model_server.rs b/crates/domain/src/model_server.rs index ef34c61..de839b7 100644 --- a/crates/domain/src/model_server.rs +++ b/crates/domain/src/model_server.rs @@ -6,7 +6,7 @@ use std::net::Ipv4Addr; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize}; use crate::error::DomainError; use crate::ids::LocalModelServerId; @@ -283,6 +283,12 @@ pub struct LlamaCppOptions { pub jinja: bool, } +/// Minimum configurable server warmup deadline in seconds. +pub const MIN_WARMUP_DEADLINE_SECS: u64 = 30; + +/// Maximum configurable server warmup deadline in seconds. +pub const MAX_WARMUP_DEADLINE_SECS: u64 = 1_800; + impl Default for LlamaCppOptions { fn default() -> Self { Self { @@ -376,6 +382,27 @@ pub fn validate_free_args(args: &[String]) -> Result<(), DomainError> { Ok(()) } +fn validate_warmup_deadline_secs(value: Option) -> Result<(), DomainError> { + let Some(value) = value else { + return Ok(()); + }; + if !(MIN_WARMUP_DEADLINE_SECS..=MAX_WARMUP_DEADLINE_SECS).contains(&value) { + return Err(DomainError::Invariant(format!( + "modelServer.warmupDeadlineSecs must be between {MIN_WARMUP_DEADLINE_SECS} and {MAX_WARMUP_DEADLINE_SECS}" + ))); + } + Ok(()) +} + +fn deserialize_warmup_deadline_secs<'de, D>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + let value = Option::::deserialize(deserializer)?; + validate_warmup_deadline_secs(value).map_err(serde::de::Error::custom)?; + Ok(value) +} + /// Stop policy for a managed local model server. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -415,6 +442,15 @@ pub struct LocalModelServerConfig { /// Process stop policy. #[serde(default = "default_stop_policy")] pub stop_policy: StopPolicy, + /// Optional readiness warmup deadline override in seconds. + /// + /// Absent means the application-level default applies. + #[serde( + default, + deserialize_with = "deserialize_warmup_deadline_secs", + skip_serializing_if = "Option::is_none" + )] + pub warmup_deadline_secs: Option, } fn default_stop_policy() -> StopPolicy { @@ -467,8 +503,23 @@ impl LocalModelServerConfig { args, auto_start, stop_policy, + warmup_deadline_secs: None, }) } + + /// Returns a copy with a validated readiness warmup deadline override. + /// + /// # Errors + /// Returns [`DomainError`] if the value is outside the supported + /// `[30, 1800]` seconds range. + pub fn with_warmup_deadline_secs( + mut self, + warmup_deadline_secs: Option, + ) -> Result { + validate_warmup_deadline_secs(warmup_deadline_secs)?; + self.warmup_deadline_secs = warmup_deadline_secs; + Ok(self) + } } /// Observed model-server status. @@ -579,6 +630,50 @@ mod tests { assert!(HfModelRef::new("a/b:c:d").is_err()); } + #[test] + fn warmup_deadline_secs_is_optional_and_validated() { + let endpoint = ModelServerEndpoint::new("http://localhost:8080/v1", 8080).unwrap(); + let model = LocalModelRef::new( + "qwen", + "Qwen", + Some(ModelSource::LocalPath { + path: ModelPath::new("/models/qwen.gguf").unwrap(), + }), + "qwen", + ) + .unwrap(); + let config = LocalModelServerConfig::new( + LocalModelServerId::from_uuid(uuid::Uuid::nil()), + LocalModelServerKind::LlamaCpp, + "llama.cpp", + endpoint, + model, + None, + LlamaCppOptions::default(), + Vec::new(), + true, + StopPolicy::StopOnAppExit, + ) + .unwrap(); + + assert_eq!(config.warmup_deadline_secs, None); + assert_eq!( + config + .clone() + .with_warmup_deadline_secs(Some(MIN_WARMUP_DEADLINE_SECS)) + .unwrap() + .warmup_deadline_secs, + Some(MIN_WARMUP_DEADLINE_SECS) + ); + assert!(config + .clone() + .with_warmup_deadline_secs(Some(MIN_WARMUP_DEADLINE_SECS - 1)) + .is_err()); + assert!(config + .with_warmup_deadline_secs(Some(MAX_WARMUP_DEADLINE_SECS + 1)) + .is_err()); + } + #[test] fn validate_free_args_rejects_reserved_flags_token_and_equals() { for flag in [ diff --git a/crates/infrastructure/src/model_server/mod.rs b/crates/infrastructure/src/model_server/mod.rs index a09afdb..a4b8aa8 100644 --- a/crates/infrastructure/src/model_server/mod.rs +++ b/crates/infrastructure/src/model_server/mod.rs @@ -526,6 +526,7 @@ impl From for ModelServersDoc { args: server.args, auto_start: server.auto_start, stop_policy: server.stop_policy, + warmup_deadline_secs: None, }) .collect(), } diff --git a/crates/infrastructure/tests/model_server.rs b/crates/infrastructure/tests/model_server.rs index 908a8a5..e5de42e 100644 --- a/crates/infrastructure/tests/model_server.rs +++ b/crates/infrastructure/tests/model_server.rs @@ -138,7 +138,9 @@ async fn fs_model_server_registry_roundtrips_global_json() { let fs: Arc = Arc::new(LocalFileSystem::new()); let registry = FsModelServerRegistry::new(Arc::clone(&fs), tmp.app_data_dir()); - let first = config(sid(2), 8081); + let first = config(sid(2), 8081) + .with_warmup_deadline_secs(Some(900)) + .unwrap(); registry.save(first.clone()).await.unwrap(); assert_eq!(registry.get(&first.id).await.unwrap(), Some(first.clone())); @@ -190,6 +192,7 @@ async fn fs_model_server_registry_migrates_v1_json_to_v2() { assert_eq!(servers.len(), 1); assert_eq!(servers[0].options, LlamaCppOptions::default()); + assert_eq!(servers[0].warmup_deadline_secs, None); assert_eq!(servers[0].args, vec!["--ctx-size", "4096"]); assert_eq!( servers[0].model.source,