diff --git a/crates/app-tauri/src/state.rs b/crates/app-tauri/src/state.rs index 21b5a01..b665989 100644 --- a/crates/app-tauri/src/state.rs +++ b/crates/app-tauri/src/state.rs @@ -74,10 +74,10 @@ use infrastructure::{ FsEmbedderPromptStore, FsHandoffStore, FsIssueNumberAllocator, FsIssueStore, FsLiveStateStore, FsMemoryStore, FsModelServerRegistry, FsOrchestratorWatcher, FsPermissionStore, FsProfileStore, FsProjectStore, FsProviderSessionStore, FsSkillStore, FsSprintStore, FsTemplateStore, - FsWindowStateStore, Git2Repository, HeuristicHandoffSummarizer, HttpOpenAiCompatibleProbe, - IdeaiContextStore, InMemoryConversationRegistry, InMemoryMailbox, LlamaCppRuntime, - LocalFileSystem, LocalManagedProcess, LocalProcessSpawner, McpServer, MediatedInbox, - NaiveMemoryRecall, OrchestratorWatchHandle, PortablePtyAdapter, RwFileGuard, + FsWindowStateStore, Git2Repository, HeuristicHandoffSummarizer, HfModelArtifactDownloader, + HttpOpenAiCompatibleProbe, IdeaiContextStore, InMemoryConversationRegistry, InMemoryMailbox, + LlamaCppRuntime, LocalFileSystem, LocalManagedProcess, LocalProcessSpawner, McpServer, + MediatedInbox, NaiveMemoryRecall, OrchestratorWatchHandle, PortablePtyAdapter, RwFileGuard, StructuredSessionFactory, SystemClock, SystemMillisClock, TicketAssistantEnvironmentPreparer, TicketToolProvider, TokioBroadcastEventBus, TokioScheduler, ToolPolicyRegistry, UuidGenerator, VectorMemoryRecall, DEFAULT_OLLAMA_BASE_URL, ONNX_CACHE_SUBDIR, RECOMMENDED_ONNX_MODELS, @@ -1234,15 +1234,23 @@ impl AppState { Arc::clone(&fs_port), app_data_dir.to_string_lossy().into_owned(), )); - let ensure_local_model_server = Arc::new(EnsureLocalModelServer::new( - Arc::clone(&model_server_registry) as Arc, - Arc::new(HttpOpenAiCompatibleProbe::default()) - as Arc, - Arc::new(LocalManagedProcess::new()) as Arc, - Arc::new(LlamaCppRuntime::new()) as Arc, - Arc::clone(&fs_port), - Arc::clone(&events_port), + let model_artifact_downloader = Arc::new(HfModelArtifactDownloader::new( + app_data_dir.join("hf-model-artifacts"), )); + let ensure_local_model_server = Arc::new( + EnsureLocalModelServer::new( + Arc::clone(&model_server_registry) as Arc, + Arc::new(HttpOpenAiCompatibleProbe::default()) + as Arc, + Arc::new(LocalManagedProcess::new()) as Arc, + Arc::new(LlamaCppRuntime::new()) as Arc, + Arc::clone(&fs_port), + Arc::clone(&events_port), + ) + .with_model_artifact_downloader( + model_artifact_downloader as Arc, + ), + ); let model_server_registry_port = Arc::clone(&model_server_registry) as Arc; let list_model_servers = Arc::new(ListModelServers::new(Arc::clone( diff --git a/crates/application/src/error.rs b/crates/application/src/error.rs index f4fae74..491f987 100644 --- a/crates/application/src/error.rs +++ b/crates/application/src/error.rs @@ -213,6 +213,7 @@ fn model_server_error_code(err: &ModelServerError) -> &'static str { ModelServerError::Probe(_) => "probe", ModelServerError::Process(_) => "process", ModelServerError::Store(_) => "store", + ModelServerError::Cancelled => "cancelled", ModelServerError::Timeout => "timeout", } } diff --git a/crates/application/src/model_server.rs b/crates/application/src/model_server.rs index 8b8fe04..679c9af 100644 --- a/crates/application/src/model_server.rs +++ b/crates/application/src/model_server.rs @@ -3,6 +3,7 @@ use std::collections::HashMap; use std::sync::{Arc, Mutex}; use std::time::Duration; +use std::time::Instant as StdInstant; use domain::events::DomainEvent; use domain::model_server::{ @@ -10,7 +11,8 @@ use domain::model_server::{ ModelSource, }; use domain::ports::{ - EventBus, FileSystem, ManagedProcess, ManagedProcessHandle, ModelServerError, ModelServerProbe, + EventBus, FileSystem, ManagedProcess, ManagedProcessHandle, ModelArtifactCancel, + ModelArtifactDownloader, ModelArtifactProgress, ModelServerError, ModelServerProbe, ModelServerRegistry, ModelServerRuntime, ProcessStatus, ProfileStore, RemotePath, }; use domain::{LocalModelServerId, StopPolicy}; @@ -215,8 +217,10 @@ pub struct EnsureLocalModelServer { events: Arc, active: Mutex>, inflight: AsyncMutex>>, + download_cancels: Mutex>, readiness: ReadinessPolicy, hf_download_deadline: Duration, + model_artifact_downloader: Option>, } impl EnsureLocalModelServer { @@ -240,8 +244,10 @@ impl EnsureLocalModelServer { events, active: Mutex::new(HashMap::new()), inflight: AsyncMutex::new(HashMap::new()), + download_cancels: Mutex::new(HashMap::new()), readiness: ReadinessPolicy::default(), hf_download_deadline: DEFAULT_HF_DOWNLOAD_DEADLINE, + model_artifact_downloader: None, } } @@ -259,6 +265,16 @@ impl EnsureLocalModelServer { self } + /// Enables pre-resolution/download of Hugging Face artifacts. + #[must_use] + pub fn with_model_artifact_downloader( + mut self, + downloader: Arc, + ) -> Self { + self.model_artifact_downloader = Some(downloader); + self + } + /// Ensures the server is reachable. /// /// # Errors @@ -332,8 +348,13 @@ impl EnsureLocalModelServer { self.ensure_model_path_accessible(&config).await?; self.ensure_no_active_port_collision(&config).await?; + let spawn_config = match self.prepare_spawn_config(config.clone()).await { + Ok(config) => config, + Err(err) => return self.fail(config.id, err), + }; + self.publish(config.id, ModelServerLifecycleStatus::Starting); - let spec = match self.runtime.build_spawn_spec(&config) { + let spec = match self.runtime.build_spawn_spec(&spawn_config) { Ok(spec) => spec, Err(err) => return self.fail(config.id, err), }; @@ -345,16 +366,71 @@ impl EnsureLocalModelServer { config.id, ActiveServer { handle: handle.clone(), - port: config.endpoint.port, - stop_policy: config.stop_policy, + port: spawn_config.endpoint.port, + stop_policy: spawn_config.stop_policy, }, ); - let hf_source = hf_source(&config); - self.wait_for_started_server(&config, &handle, hf_source) + let hf_source = hf_source(&spawn_config); + self.wait_for_started_server(&spawn_config, &handle, hf_source) .await } + async fn prepare_spawn_config( + &self, + config: LocalModelServerConfig, + ) -> Result { + let Some(ModelSource::HuggingFace { repo }) = config.model.source.clone() else { + return Ok(config); + }; + let Some(downloader) = self.model_artifact_downloader.as_ref() else { + return Ok(config); + }; + + let cancel = ModelArtifactCancel::new(); + self.download_cancels + .lock() + .unwrap() + .insert(config.id, cancel.clone()); + + let events = Arc::clone(&self.events); + let source = repo.as_str().to_owned(); + let server_id = config.id; + let debouncer = Arc::new(Mutex::new(DownloadProgressDebouncer::new( + server_id, + source.clone(), + events, + ))); + let progress = { + let debouncer = Arc::clone(&debouncer); + Arc::new(move |progress: ModelArtifactProgress| { + debouncer.lock().unwrap().on_progress(progress); + }) as Arc + }; + + let result = tokio::time::timeout( + self.hf_download_deadline, + downloader.resolve_hf_model(&repo, progress, cancel.clone()), + ) + .await; + self.download_cancels.lock().unwrap().remove(&config.id); + + let resolution = match result { + Ok(Ok(resolution)) => resolution, + Ok(Err(err)) => return Err(err), + Err(_) => { + cancel.cancel(); + return Err(ModelServerError::Timeout); + } + }; + + let mut resolved = config; + resolved.model.source = Some(ModelSource::LocalPath { + path: resolution.path, + }); + Ok(resolved) + } + async fn wait_for_started_server( &self, config: &LocalModelServerConfig, @@ -468,6 +544,9 @@ impl EnsureLocalModelServer { /// # Errors /// Returns the first process error after attempting every eligible stop. pub async fn stop_on_app_exit(&self) -> Result<(), AppError> { + for cancel in self.download_cancels.lock().unwrap().values() { + cancel.cancel(); + } let entries: Vec<(LocalModelServerId, ActiveServer)> = self .active .lock() @@ -626,6 +705,109 @@ pub fn model_server_error_code(err: &ModelServerError) -> &'static str { ModelServerError::Probe(_) => "probe", ModelServerError::Process(_) => "process", ModelServerError::Store(_) => "store", + ModelServerError::Cancelled => "cancelled", ModelServerError::Timeout => "timeout", } } + +struct DownloadProgressDebouncer { + server_id: LocalModelServerId, + fallback_source: String, + events: Arc, + last_emit: Option, + last_percent: Option, + window_start: StdInstant, + window_count: u32, +} + +impl DownloadProgressDebouncer { + fn new( + server_id: LocalModelServerId, + fallback_source: String, + events: Arc, + ) -> Self { + let now = StdInstant::now(); + Self { + server_id, + fallback_source, + events, + last_emit: None, + last_percent: None, + window_start: now, + window_count: 0, + } + } + + fn on_progress(&mut self, progress: ModelArtifactProgress) { + let percent = progress_percent(&progress); + let final_progress = is_final_progress(&progress); + if !self.should_publish(percent, final_progress) { + return; + } + self.mark_published(percent); + self.events.publish(DomainEvent::ModelServerStatusChanged { + server_id: self.server_id, + status: ModelServerLifecycleStatus::Downloading { + downloaded_bytes: progress.downloaded_bytes, + total_bytes: progress.total_bytes, + percent, + source: progress + .source + .or_else(|| Some(self.fallback_source.clone())), + }, + }); + } + + fn should_publish(&mut self, percent: Option, final_progress: bool) -> bool { + let now = StdInstant::now(); + if now.duration_since(self.window_start) >= Duration::from_secs(1) { + self.window_start = now; + self.window_count = 0; + } + if self.last_emit.is_none() || final_progress { + return true; + } + if self.window_count >= 10 { + return false; + } + if self + .last_emit + .is_some_and(|last| now.duration_since(last) < Duration::from_millis(100)) + { + return false; + } + if self + .last_emit + .is_some_and(|last| now.duration_since(last) >= Duration::from_millis(500)) + { + return true; + } + match (self.last_percent, percent) { + (Some(last), Some(current)) => current - last >= 1.0, + (None, Some(_)) => true, + _ => false, + } + } + + fn mark_published(&mut self, percent: Option) { + self.last_emit = Some(StdInstant::now()); + self.last_percent = percent.or(self.last_percent); + self.window_count += 1; + } +} + +fn progress_percent(progress: &ModelArtifactProgress) -> Option { + let downloaded = progress.downloaded_bytes?; + let total = progress.total_bytes?; + if total == 0 { + return None; + } + Some((downloaded as f32 / total as f32) * 100.0) +} + +fn is_final_progress(progress: &ModelArtifactProgress) -> bool { + matches!( + (progress.downloaded_bytes, progress.total_bytes), + (Some(downloaded), Some(total)) if total > 0 && downloaded == total + ) +} diff --git a/crates/application/tests/model_server.rs b/crates/application/tests/model_server.rs index 535aeff..663f629 100644 --- a/crates/application/tests/model_server.rs +++ b/crates/application/tests/model_server.rs @@ -18,6 +18,7 @@ use domain::model_server::{ }; use domain::ports::{ DirEntry, EventBus, EventStream, FileSystem, FsError, ManagedProcess, ManagedProcessHandle, + ModelArtifactCancel, ModelArtifactDownloader, ModelArtifactProgress, ModelArtifactResolution, ModelServerArgv, ModelServerError, ModelServerProbe, ModelServerRegistry, ModelServerRuntime, ProcessStatus, ProfileStore, RemotePath, SpawnSpec, StoreError, }; @@ -280,6 +281,74 @@ impl ModelServerRuntime for FakeRuntime { } } +#[derive(Clone)] +enum FakeDownloadOutcome { + Resolve { + progress: Vec, + path: &'static str, + cache_hit: bool, + }, + WaitForCancel, + Sleep(Duration), +} + +struct FakeModelArtifactDownloader { + outcome: Mutex, +} + +impl FakeModelArtifactDownloader { + fn new(outcome: FakeDownloadOutcome) -> Self { + Self { + outcome: Mutex::new(outcome), + } + } +} + +#[async_trait] +impl ModelArtifactDownloader for FakeModelArtifactDownloader { + async fn resolve_hf_model( + &self, + repo: &HfModelRef, + progress: Arc, + cancel: ModelArtifactCancel, + ) -> Result { + let outcome = self.outcome.lock().unwrap().clone(); + match outcome { + FakeDownloadOutcome::Resolve { + progress: events, + path, + cache_hit, + } => { + if cancel.is_cancelled() { + return Err(ModelServerError::Cancelled); + } + if !cache_hit { + for event in events { + progress(ModelArtifactProgress { + source: event.source.or_else(|| Some(repo.as_str().to_owned())), + ..event + }); + } + } + Ok(ModelArtifactResolution { + path: ModelPath::new(path).unwrap(), + cache_hit, + }) + } + FakeDownloadOutcome::WaitForCancel => loop { + if cancel.is_cancelled() { + return Err(ModelServerError::Cancelled); + } + tokio::time::sleep(Duration::from_millis(1)).await; + }, + FakeDownloadOutcome::Sleep(duration) => { + tokio::time::sleep(duration).await; + Err(ModelServerError::Probe("unexpected wake".to_owned())) + } + } + } +} + #[derive(Default)] struct FakeFs { existing: Mutex>, @@ -345,6 +414,26 @@ fn ensure( .with_hf_download_deadline(Duration::from_secs(5)) } +fn ensure_with_downloader( + registry: Arc, + probe: Arc, + process: Arc, + fs: Arc, + events: Arc, + downloader: Arc, +) -> EnsureLocalModelServer { + ensure(registry, probe, process, fs, events) + .with_model_artifact_downloader(downloader as Arc) +} + +fn progress(downloaded: Option, total: Option) -> ModelArtifactProgress { + ModelArtifactProgress { + downloaded_bytes: downloaded, + total_bytes: total, + source: None, + } +} + #[tokio::test] async fn reachable_server_is_reused_without_spawn() { let registry = Arc::new(FakeRegistry::default()); @@ -738,6 +827,270 @@ async fn hf_download_phase_fails_when_process_exits() { })); } +#[tokio::test] +async fn hf_downloader_publishes_debounced_progress_and_spawns_resolved_model_path() { + let registry = Arc::new(FakeRegistry::default()); + registry + .save(hf_config( + sid(14), + 8094, + "Qwen/Qwen3-Coder-30B-A3B-Instruct-GGUF", + )) + .await + .unwrap(); + let process = Arc::new(FakeProcess::default()); + let events = Arc::new(FakeEvents::default()); + let downloader = Arc::new(FakeModelArtifactDownloader::new( + FakeDownloadOutcome::Resolve { + progress: vec![ + progress(Some(10), Some(100)), + progress(Some(50), Some(100)), + progress(Some(100), Some(100)), + ], + path: "/cache/model.gguf", + cache_hit: false, + }, + )); + let usecase = ensure_with_downloader( + Arc::clone(®istry), + Arc::new(FakeProbe::new(vec![ + ModelServerStatus::Unreachable, + ModelServerStatus::ReadyStarted, + ])), + Arc::clone(&process), + Arc::new(FakeFs::default()), + Arc::clone(&events), + downloader, + ); + + let out = usecase + .execute(EnsureLocalModelServerInput { server_id: sid(14) }) + .await + .unwrap(); + + assert_eq!(out.ready.status, ModelServerStatus::ReadyStarted); + let statuses: Vec = events + .0 + .lock() + .unwrap() + .iter() + .filter_map(|event| match event { + DomainEvent::ModelServerStatusChanged { status, .. } => Some(status.clone()), + _ => None, + }) + .collect(); + assert!(matches!( + statuses.as_slice(), + [ + ModelServerLifecycleStatus::Probing, + ModelServerLifecycleStatus::Downloading { + downloaded_bytes: Some(10), + total_bytes: Some(100), + percent: Some(10.0), + .. + }, + ModelServerLifecycleStatus::Downloading { + downloaded_bytes: Some(100), + total_bytes: Some(100), + percent: Some(100.0), + .. + }, + ModelServerLifecycleStatus::Starting, + ModelServerLifecycleStatus::Ready { reused: false }, + ] + )); + let spawns = process.spawns.lock().unwrap(); + assert_eq!(spawns.len(), 1); + assert_eq!(spawns[0].args[0..2], ["--model", "/cache/model.gguf"]); + assert!(!spawns[0].args.iter().any(|arg| arg == "-hf")); +} + +#[tokio::test] +async fn hf_downloader_cache_hit_skips_downloading_events() { + let registry = Arc::new(FakeRegistry::default()); + registry + .save(hf_config(sid(15), 8095, "Qwen/Qwen3-Coder:Q4_K_M")) + .await + .unwrap(); + let process = Arc::new(FakeProcess::default()); + let events = Arc::new(FakeEvents::default()); + let downloader = Arc::new(FakeModelArtifactDownloader::new( + FakeDownloadOutcome::Resolve { + progress: Vec::new(), + path: "/cache/q4.gguf", + cache_hit: true, + }, + )); + let usecase = ensure_with_downloader( + Arc::clone(®istry), + Arc::new(FakeProbe::new(vec![ + ModelServerStatus::Unreachable, + ModelServerStatus::ReadyStarted, + ])), + Arc::clone(&process), + Arc::new(FakeFs::default()), + Arc::clone(&events), + downloader, + ); + + usecase + .execute(EnsureLocalModelServerInput { server_id: sid(15) }) + .await + .unwrap(); + + let statuses: Vec = events + .0 + .lock() + .unwrap() + .iter() + .filter_map(|event| match event { + DomainEvent::ModelServerStatusChanged { status, .. } => Some(status.clone()), + _ => None, + }) + .collect(); + assert!(matches!( + statuses.as_slice(), + [ + ModelServerLifecycleStatus::Probing, + ModelServerLifecycleStatus::Starting, + ModelServerLifecycleStatus::Ready { reused: false }, + ] + )); +} + +#[tokio::test] +async fn hf_downloader_unknown_total_publishes_no_percent() { + let registry = Arc::new(FakeRegistry::default()); + registry + .save(hf_config(sid(16), 8096, "Qwen/Qwen3-Coder:Q4_K_M")) + .await + .unwrap(); + let process = Arc::new(FakeProcess::default()); + let events = Arc::new(FakeEvents::default()); + let downloader = Arc::new(FakeModelArtifactDownloader::new( + FakeDownloadOutcome::Resolve { + progress: vec![progress(Some(10), None)], + path: "/cache/q4.gguf", + cache_hit: false, + }, + )); + let usecase = ensure_with_downloader( + Arc::clone(®istry), + Arc::new(FakeProbe::new(vec![ + ModelServerStatus::Unreachable, + ModelServerStatus::ReadyStarted, + ])), + Arc::clone(&process), + Arc::new(FakeFs::default()), + Arc::clone(&events), + downloader, + ); + + usecase + .execute(EnsureLocalModelServerInput { server_id: sid(16) }) + .await + .unwrap(); + + assert!(events.0.lock().unwrap().iter().any(|event| { + matches!( + event, + DomainEvent::ModelServerStatusChanged { + status: ModelServerLifecycleStatus::Downloading { + downloaded_bytes: Some(10), + total_bytes: None, + percent: None, + .. + }, + .. + } + ) + })); +} + +#[tokio::test] +async fn hf_downloader_cancel_fails_without_process_spawn() { + let registry = Arc::new(FakeRegistry::default()); + registry + .save(hf_config(sid(17), 8097, "Qwen/Qwen3-Coder:Q4_K_M")) + .await + .unwrap(); + let process = Arc::new(FakeProcess::default()); + let events = Arc::new(FakeEvents::default()); + let downloader = Arc::new(FakeModelArtifactDownloader::new( + FakeDownloadOutcome::WaitForCancel, + )); + let usecase = Arc::new(ensure_with_downloader( + Arc::clone(®istry), + Arc::new(FakeProbe::new(vec![ModelServerStatus::Unreachable])), + Arc::clone(&process), + Arc::new(FakeFs::default()), + Arc::clone(&events), + downloader, + )); + let task_usecase = Arc::clone(&usecase); + let task = tokio::spawn(async move { + task_usecase + .execute(EnsureLocalModelServerInput { server_id: sid(17) }) + .await + }); + tokio::time::sleep(Duration::from_millis(5)).await; + + usecase.stop_on_app_exit().await.unwrap(); + let err = task.await.unwrap().unwrap_err(); + + assert!(err.to_string().contains("cancelled")); + assert!(process.spawns.lock().unwrap().is_empty()); + assert!(events.0.lock().unwrap().iter().any(|event| { + matches!( + event, + DomainEvent::ModelServerStatusChanged { + status: ModelServerLifecycleStatus::Failed { code, .. }, + .. + } if code == "cancelled" + ) + })); +} + +#[tokio::test] +async fn hf_downloader_timeout_fails_without_process_spawn() { + let registry = Arc::new(FakeRegistry::default()); + registry + .save(hf_config(sid(18), 8098, "Qwen/Qwen3-Coder:Q4_K_M")) + .await + .unwrap(); + let process = Arc::new(FakeProcess::default()); + let events = Arc::new(FakeEvents::default()); + let downloader = Arc::new(FakeModelArtifactDownloader::new( + FakeDownloadOutcome::Sleep(Duration::from_millis(50)), + )); + let usecase = ensure_with_downloader( + Arc::clone(®istry), + Arc::new(FakeProbe::new(vec![ModelServerStatus::Unreachable])), + Arc::clone(&process), + Arc::new(FakeFs::default()), + Arc::clone(&events), + downloader, + ) + .with_hf_download_deadline(Duration::from_millis(1)); + + let err = usecase + .execute(EnsureLocalModelServerInput { server_id: sid(18) }) + .await + .unwrap_err(); + + assert!(err.to_string().contains("timeout")); + assert!(process.spawns.lock().unwrap().is_empty()); + assert!(events.0.lock().unwrap().iter().any(|event| { + matches!( + event, + DomainEvent::ModelServerStatusChanged { + status: ModelServerLifecycleStatus::Failed { code, .. }, + .. + } if code == "timeout" + ) + })); +} + #[tokio::test] async fn missing_registry_entry_is_model_server_not_configured() { let usecase = ensure( diff --git a/crates/domain/src/lib.rs b/crates/domain/src/lib.rs index 3f9b8f2..9d6e603 100644 --- a/crates/domain/src/lib.rs +++ b/crates/domain/src/lib.rs @@ -199,10 +199,11 @@ pub use ports::{ EmbedderEnvReport, EmbedderError, EmbedderProfileStore, EmbedderPromptDismissal, EmbedderPromptStore, EventBus, EventStream, ExitStatus, FileSystem, FsError, GitCommitInfo, GitError, GitFileStatus, GitPort, GraphCommit, IdGenerator, IssueNumberAllocator, IssueStore, - IssueStoreError, LiveStateStore, MemoryError, MemoryQuery, MemoryRecall, MemoryStore, Output, - OutputStream, PermissionStore, PreparedContext, ProcessError, ProcessSpawner, ProfileStore, - ProjectStore, PtyError, PtyHandle, PtyPort, RemoteError, RemoteHost, RemotePath, RuntimeError, - ScheduledTask, Scheduler, SpawnSpec, SprintStore, SprintStoreError, StoreError, + IssueStoreError, LiveStateStore, MemoryError, MemoryQuery, MemoryRecall, MemoryStore, + ModelArtifactCancel, ModelArtifactDownloader, ModelArtifactProgress, ModelArtifactResolution, + Output, OutputStream, PermissionStore, PreparedContext, ProcessError, ProcessSpawner, + ProfileStore, ProjectStore, PtyError, PtyHandle, PtyPort, RemoteError, RemoteHost, RemotePath, + RuntimeError, ScheduledTask, Scheduler, SpawnSpec, SprintStore, SprintStoreError, StoreError, StructuredSessionEnvironment, StructuredSessionEnvironmentPreparer, TemplateStore, WindowStateStore, }; diff --git a/crates/domain/src/ports.rs b/crates/domain/src/ports.rs index a7a4592..5dc68f2 100644 --- a/crates/domain/src/ports.rs +++ b/crates/domain/src/ports.rs @@ -22,6 +22,7 @@ //! Each port is **fine-grained** (Interface Segregation): `FileSystem`, //! `PtyPort`, `ProcessSpawner` are separate, never a `System` god-trait. +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use async_trait::async_trait; @@ -42,7 +43,9 @@ use crate::issue::{ }; use crate::markdown::MarkdownDoc; use crate::memory::{Memory, MemoryIndexEntry, MemoryLink, MemorySlug}; -use crate::model_server::{LocalModelServerConfig, ModelServerEndpoint, ModelServerStatus}; +use crate::model_server::{ + HfModelRef, LocalModelServerConfig, ModelPath, ModelServerEndpoint, ModelServerStatus, +}; use crate::permission::ProjectPermissions; use crate::profile::{AgentProfile, EmbedderProfile}; use crate::project::{Project, ProjectPath}; @@ -582,6 +585,9 @@ pub enum ModelServerError { /// Store operation failed. #[error("store failed: {0}")] Store(String), + /// Operation was cancelled by the caller. + #[error("cancelled")] + Cancelled, /// Readiness timed out. #[error("readiness timed out")] Timeout, @@ -1013,6 +1019,66 @@ pub trait ModelServerProbe: Send + Sync { ) -> Result; } +/// Progress observed while resolving/downloading a model artifact. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ModelArtifactProgress { + /// Downloaded bytes when known. + pub downloaded_bytes: Option, + /// Total bytes when known. + pub total_bytes: Option, + /// Source being resolved, usually the Hugging Face repo reference. + pub source: Option, +} + +/// Resolved local model artifact. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ModelArtifactResolution { + /// Absolute local path to the resolved `.gguf` file. + pub path: ModelPath, + /// `true` when the artifact was already available in cache. + pub cache_hit: bool, +} + +/// Cooperative cancellation token for model artifact resolution. +#[derive(Debug, Clone, Default)] +pub struct ModelArtifactCancel { + cancelled: Arc, +} + +impl ModelArtifactCancel { + /// Creates a non-cancelled token. + #[must_use] + pub fn new() -> Self { + Self::default() + } + + /// Requests cancellation. + pub fn cancel(&self) { + self.cancelled.store(true, Ordering::SeqCst); + } + + /// Returns whether cancellation was requested. + #[must_use] + pub fn is_cancelled(&self) -> bool { + self.cancelled.load(Ordering::SeqCst) + } +} + +/// Resolves or downloads a model artifact before starting a model server. +#[async_trait] +pub trait ModelArtifactDownloader: Send + Sync { + /// Resolves a Hugging Face model to a local artifact path. + /// + /// # Errors + /// [`ModelServerError`] when resolution/download fails or is cancelled. + async fn resolve_hf_model( + &self, + repo: &HfModelRef, + progress: Arc, + cancel: ModelArtifactCancel, + ) -> Result; +} + /// Manages local long-lived child processes. #[async_trait] pub trait ManagedProcess: Send + Sync { diff --git a/crates/infrastructure/src/lib.rs b/crates/infrastructure/src/lib.rs index 23743c2..f33a772 100644 --- a/crates/infrastructure/src/lib.rs +++ b/crates/infrastructure/src/lib.rs @@ -66,7 +66,8 @@ pub use inspector::{ pub use issues::{FsIssueNumberAllocator, FsIssueStore}; pub use mailbox::InMemoryMailbox; pub use model_server::{ - FsModelServerRegistry, HttpOpenAiCompatibleProbe, LlamaCppRuntime, LocalManagedProcess, + FsModelServerRegistry, HfModelArtifactDownloader, HttpOpenAiCompatibleProbe, LlamaCppRuntime, + LocalManagedProcess, }; pub use orchestrator::mcp::{ McpServer, MemoryTransport, StdioTransport, TicketToolError, TicketToolProvider, diff --git a/crates/infrastructure/src/model_server/mod.rs b/crates/infrastructure/src/model_server/mod.rs index 852307f..a09afdb 100644 --- a/crates/infrastructure/src/model_server/mod.rs +++ b/crates/infrastructure/src/model_server/mod.rs @@ -6,15 +6,18 @@ use std::sync::Mutex; use std::time::Duration; use async_trait::async_trait; +use futures_util::StreamExt; use serde::{Deserialize, Serialize}; +use tokio::io::AsyncWriteExt; use tokio::process::{Child, Command}; use domain::model_server::{ - ExecutablePath, LlamaCppOptions, LocalModelRef, LocalModelServerConfig, LocalModelServerKind, - ModelPath, ModelServerEndpoint, ModelSource, + ExecutablePath, HfModelRef, LlamaCppOptions, LocalModelRef, LocalModelServerConfig, + LocalModelServerKind, ModelPath, ModelServerEndpoint, ModelSource, }; use domain::ports::{ - FileSystem, ManagedProcess, ManagedProcessHandle, ModelServerArgv, ModelServerError, + FileSystem, ManagedProcess, ManagedProcessHandle, ModelArtifactCancel, ModelArtifactDownloader, + ModelArtifactProgress, ModelArtifactResolution, ModelServerArgv, ModelServerError, ModelServerProbe, ModelServerRegistry, ModelServerRuntime, ProcessStatus, RemotePath, SpawnSpec, }; @@ -68,6 +71,210 @@ fn is_ready(result: Result) -> bool { .unwrap_or(false) } +/// Hugging Face artifact resolver/downloader backed by an IdeA-owned cache dir. +#[derive(Clone)] +pub struct HfModelArtifactDownloader { + cache_dir: PathBuf, + client: reqwest::Client, +} + +impl HfModelArtifactDownloader { + /// Builds a downloader that stores artifacts under `cache_dir`. + #[must_use] + pub fn new(cache_dir: impl Into) -> Self { + Self { + cache_dir: cache_dir.into(), + client: reqwest::Client::new(), + } + } + + /// Deterministic cache path for a Hugging Face model reference. + #[must_use] + pub fn cache_path_for(&self, repo: &HfModelRef) -> PathBuf { + let (base, quant) = split_hf_ref(repo); + let file_stem = quant.unwrap_or("model"); + self.cache_dir + .join(base.replace('/', "--")) + .join(format!("{}.gguf", safe_cache_component(file_stem))) + } + + async fn resolve_remote_filename(&self, repo: &HfModelRef) -> Result { + let (base, quant) = split_hf_ref(repo); + let url = format!("https://huggingface.co/api/models/{base}"); + let response = self + .client + .get(url) + .send() + .await + .map_err(|e| ModelServerError::Probe(e.to_string()))?; + if !response.status().is_success() { + return Err(ModelServerError::Probe(format!( + "huggingface model metadata returned {}", + response.status() + ))); + } + let metadata: HfModelMetadata = response + .json() + .await + .map_err(|e| ModelServerError::Probe(e.to_string()))?; + select_gguf_file(&metadata.siblings, quant).ok_or_else(|| { + ModelServerError::PathNotAccessible(format!( + "no matching .gguf artifact found for {}", + repo.as_str() + )) + }) + } +} + +#[async_trait] +impl ModelArtifactDownloader for HfModelArtifactDownloader { + async fn resolve_hf_model( + &self, + repo: &HfModelRef, + progress: std::sync::Arc, + cancel: ModelArtifactCancel, + ) -> Result { + if cancel.is_cancelled() { + return Err(ModelServerError::Cancelled); + } + let path = self.cache_path_for(repo); + if path.is_file() { + return Ok(ModelArtifactResolution { + path: model_path_from_pathbuf(path)?, + cache_hit: true, + }); + } + + let filename = self.resolve_remote_filename(repo).await?; + if cancel.is_cancelled() { + return Err(ModelServerError::Cancelled); + } + let (base, _) = split_hf_ref(repo); + let url = format!( + "https://huggingface.co/{base}/resolve/main/{}", + url_path_segment(&filename) + ); + let response = self + .client + .get(url) + .send() + .await + .map_err(|e| ModelServerError::Probe(e.to_string()))?; + if !response.status().is_success() { + return Err(ModelServerError::Probe(format!( + "huggingface artifact download returned {}", + response.status() + ))); + } + if let Some(parent) = path.parent() { + tokio::fs::create_dir_all(parent) + .await + .map_err(|e| ModelServerError::Store(e.to_string()))?; + } + let tmp_path = path.with_extension("gguf.part"); + let mut file = tokio::fs::File::create(&tmp_path) + .await + .map_err(|e| ModelServerError::Store(e.to_string()))?; + let total = response.content_length(); + let mut downloaded = 0_u64; + let mut stream = response.bytes_stream(); + while let Some(chunk) = stream.next().await { + if cancel.is_cancelled() { + let _ = tokio::fs::remove_file(&tmp_path).await; + return Err(ModelServerError::Cancelled); + } + let chunk = chunk.map_err(|e| ModelServerError::Probe(e.to_string()))?; + file.write_all(&chunk) + .await + .map_err(|e| ModelServerError::Store(e.to_string()))?; + downloaded += chunk.len() as u64; + progress(ModelArtifactProgress { + downloaded_bytes: Some(downloaded), + total_bytes: total, + source: Some(repo.as_str().to_owned()), + }); + } + file.flush() + .await + .map_err(|e| ModelServerError::Store(e.to_string()))?; + drop(file); + tokio::fs::rename(&tmp_path, &path) + .await + .map_err(|e| ModelServerError::Store(e.to_string()))?; + Ok(ModelArtifactResolution { + path: model_path_from_pathbuf(path)?, + cache_hit: false, + }) + } +} + +#[derive(Debug, Deserialize)] +struct HfModelMetadata { + #[serde(default)] + siblings: Vec, +} + +#[derive(Debug, Deserialize)] +struct HfSibling { + #[serde(rename = "rfilename")] + filename: String, +} + +fn split_hf_ref(repo: &HfModelRef) -> (&str, Option<&str>) { + repo.as_str() + .split_once(':') + .map_or((repo.as_str(), None), |(base, quant)| (base, Some(quant))) +} + +fn select_gguf_file(siblings: &[HfSibling], quant: Option<&str>) -> Option { + let mut ggufs: Vec<&str> = siblings + .iter() + .map(|sibling| sibling.filename.as_str()) + .filter(|filename| filename.ends_with(".gguf")) + .collect(); + ggufs.sort_unstable(); + let Some(quant) = quant else { + return ggufs.first().map(|filename| (*filename).to_owned()); + }; + let quant = quant.to_ascii_lowercase(); + ggufs + .into_iter() + .find(|filename| filename.to_ascii_lowercase().contains(&quant)) + .map(str::to_owned) +} + +fn safe_cache_component(raw: &str) -> String { + raw.chars() + .map(|ch| match ch { + 'A'..='Z' | 'a'..='z' | '0'..='9' | '.' | '_' | '-' => ch, + _ => '_', + }) + .collect() +} + +fn url_path_segment(raw: &str) -> String { + raw.split('/') + .map(url_component) + .collect::>() + .join("/") +} + +fn url_component(raw: &str) -> String { + raw.bytes() + .flat_map(|byte| match byte { + b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'.' | b'_' | b'-' => { + vec![byte as char] + } + other => format!("%{other:02X}").chars().collect(), + }) + .collect() +} + +fn model_path_from_pathbuf(path: PathBuf) -> Result { + ModelPath::new(path.to_string_lossy().into_owned()) + .map_err(|e| ModelServerError::Invalid(e.to_string())) +} + /// Builds `llama-server` argv without shell interpolation. #[derive(Debug, Default, Clone, Copy)] pub struct LlamaCppRuntime; diff --git a/crates/infrastructure/tests/model_server.rs b/crates/infrastructure/tests/model_server.rs index c9cac18..908a8a5 100644 --- a/crates/infrastructure/tests/model_server.rs +++ b/crates/infrastructure/tests/model_server.rs @@ -7,9 +7,14 @@ use domain::model_server::{ ExecutablePath, HfModelRef, LlamaCppOptions, LocalModelRef, LocalModelServerConfig, LocalModelServerKind, ModelPath, ModelServerEndpoint, ModelSource, StopPolicy, }; -use domain::ports::{FileSystem, ModelServerRegistry, ModelServerRuntime, RemotePath}; +use domain::ports::{ + FileSystem, ModelArtifactCancel, ModelArtifactDownloader, ModelServerRegistry, + ModelServerRuntime, RemotePath, +}; use domain::LocalModelServerId; -use infrastructure::{FsModelServerRegistry, LlamaCppRuntime, LocalFileSystem}; +use infrastructure::{ + FsModelServerRegistry, HfModelArtifactDownloader, LlamaCppRuntime, LocalFileSystem, +}; use uuid::Uuid; struct TempDir(PathBuf); @@ -28,6 +33,10 @@ impl TempDir { fn child(&self, name: &str) -> RemotePath { RemotePath::new(self.0.join(name).to_string_lossy().into_owned()) } + + fn path(&self) -> &std::path::Path { + &self.0 + } } impl Drop for TempDir { @@ -189,3 +198,29 @@ async fn fs_model_server_registry_migrates_v1_json_to_v2() { }) ); } + +#[tokio::test] +async fn hf_model_artifact_downloader_resolves_deterministic_local_cache_hit_without_network() { + let tmp = TempDir::new(); + let downloader = HfModelArtifactDownloader::new(tmp.path()); + let repo = HfModelRef::new("Qwen/Qwen3-Coder:Q4_K_M").unwrap(); + let path = downloader.cache_path_for(&repo); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(&path, b"gguf").unwrap(); + + let resolution = downloader + .resolve_hf_model( + &repo, + Arc::new(|_| panic!("cache hit must not report progress")), + ModelArtifactCancel::new(), + ) + .await + .unwrap(); + + assert!(resolution.cache_hit); + assert_eq!(resolution.path.as_str(), path.to_string_lossy()); + assert_eq!( + path.strip_prefix(tmp.path()).unwrap(), + std::path::Path::new("Qwen--Qwen3-Coder").join("Q4_K_M.gguf") + ); +} diff --git a/frontend/src/features/agents/index.ts b/frontend/src/features/agents/index.ts index d650503..b34d544 100644 --- a/frontend/src/features/agents/index.ts +++ b/frontend/src/features/agents/index.ts @@ -11,9 +11,14 @@ export type { ModelServerLaunchBadgeProps } from "./ModelServerLaunchBadge"; export { correlateModelServerStatus, modelServerOverlayText, + describeModelServerDownload, + formatBytes, useModelServerLaunchState, } from "./modelServerLaunch"; -export type { ModelServerLaunchState } from "./modelServerLaunch"; +export type { + ModelServerLaunchState, + ModelServerDownloadProgress, +} from "./modelServerLaunch"; export { ResumeProjectPanel } from "./ResumeProjectPanel"; export type { ResumeProjectPanelProps } from "./ResumeProjectPanel"; export { useResumeProject } from "./useResumeProject"; diff --git a/frontend/src/features/agents/modelServerLaunch.test.ts b/frontend/src/features/agents/modelServerLaunch.test.ts new file mode 100644 index 0000000..cd53fe5 --- /dev/null +++ b/frontend/src/features/agents/modelServerLaunch.test.ts @@ -0,0 +1,109 @@ +/** + * Ticket #54 F2 — pure download-progress formatting for the model-server overlay. + * These lock the wire→display mapping independently of React: byte formatting, + * the determinate/indeterminate split, and the "no fake %" rule. + */ +import { describe, it, expect } from "vitest"; + +import type { ModelServerStatus } from "@/domain"; +import { + formatBytes, + describeModelServerDownload, + modelServerOverlayText, +} from "./modelServerLaunch"; + +function downloading( + fields: Partial<{ + downloadedBytes: number | null; + totalBytes: number | null; + percent: number | null; + source: string | null; + }> = {}, +): ModelServerStatus { + return { + state: "downloading", + downloadedBytes: null, + totalBytes: null, + percent: null, + source: null, + ...fields, + }; +} + +describe("formatBytes", () => { + it("shows whole bytes and SI-scaled larger units", () => { + expect(formatBytes(0)).toBe("0 o"); + expect(formatBytes(512)).toBe("512 o"); + expect(formatBytes(1000)).toBe("1 Ko"); + expect(formatBytes(1_500_000)).toBe("1.5 Mo"); + expect(formatBytes(4_200_000_000)).toBe("4.2 Go"); + }); + + it("guards non-finite / negative", () => { + expect(formatBytes(-5)).toBe("0 o"); + expect(formatBytes(Number.NaN)).toBe("0 o"); + }); +}); + +describe("describeModelServerDownload", () => { + it("returns null for every non-downloading state", () => { + expect(describeModelServerDownload(undefined)).toBeNull(); + expect(describeModelServerDownload({ state: "probing" })).toBeNull(); + expect(describeModelServerDownload({ state: "starting" })).toBeNull(); + expect( + describeModelServerDownload({ state: "ready", reused: false }), + ).toBeNull(); + }); + + it("exposes percent + a 'X / Y' bytes label when both bounds are known", () => { + const p = describeModelServerDownload( + downloading({ + downloadedBytes: 1_500_000, + totalBytes: 3_000_000, + percent: 50, + source: "unsloth/Qwen3-Coder-30B", + }), + ); + expect(p).toEqual({ + percent: 50, + bytesLabel: "1.5 Mo / 3 Mo", + source: "unsloth/Qwen3-Coder-30B", + }); + }); + + it("clamps percent to 0..100", () => { + expect(describeModelServerDownload(downloading({ percent: 137 }))?.percent).toBe( + 100, + ); + expect(describeModelServerDownload(downloading({ percent: -3 }))?.percent).toBe( + 0, + ); + }); + + it("is indeterminate (percent null) when the total is unknown", () => { + const p = describeModelServerDownload( + downloading({ downloadedBytes: 800_000, totalBytes: null, percent: null }), + ); + expect(p).toEqual({ percent: null, bytesLabel: "800 Ko", source: null }); + }); + + it("has no bytes label when nothing about size is known", () => { + expect(describeModelServerDownload(downloading())).toEqual({ + percent: null, + bytesLabel: null, + source: null, + }); + }); +}); + +describe("modelServerOverlayText (unchanged F1 titles)", () => { + it("keeps the download / loading titles", () => { + expect(modelServerOverlayText(downloading())).toBe( + "Téléchargement du modèle…", + ); + expect(modelServerOverlayText({ state: "starting" })).toBe( + "Chargement du serveur…", + ); + expect(modelServerOverlayText({ state: "ready", reused: true })).toBeNull(); + }); +}); diff --git a/frontend/src/features/agents/modelServerLaunch.ts b/frontend/src/features/agents/modelServerLaunch.ts index a56db3d..37ec6dc 100644 --- a/frontend/src/features/agents/modelServerLaunch.ts +++ b/frontend/src/features/agents/modelServerLaunch.ts @@ -37,9 +37,9 @@ export function correlateModelServerStatus( * - `downloading` → "Téléchargement du modèle…", * - `starting`/`probing` → "Chargement du serveur…". * - * F1 MVP ignores the progress fields (bytes/percent) — the bar/% is the F2 - * stretch. This is the single predicate that decides whether the launch overlay - * covers a cell. + * This is the single predicate that decides whether the launch overlay covers a + * cell (and supplies its title). The download *progress* (bar/%/bytes, F2) is a + * separate, additive concern — see {@link describeModelServerDownload}. */ export function modelServerOverlayText( status: ModelServerStatus | undefined, @@ -56,6 +56,60 @@ export function modelServerOverlayText( } } +/** + * Human-readable byte size, decimal (SI) units so it matches what download UIs + * and llama.cpp report (`1 Mo = 1000 Ko`). Bytes are shown whole; larger units + * keep one decimal, trailing `.0` trimmed. Guards against non-finite/negative + * inputs (→ "0 o"). + */ +export function formatBytes(n: number): string { + if (!Number.isFinite(n) || n <= 0) return "0 o"; + const units = ["o", "Ko", "Mo", "Go", "To"]; + let value = n; + let unit = 0; + while (value >= 1000 && unit < units.length - 1) { + value /= 1000; + unit += 1; + } + const rounded = + unit === 0 ? Math.round(value) : Math.round(value * 10) / 10; + return `${rounded} ${units[unit]}`; +} + +/** + * Download progress detail extracted from a `downloading` status (F2), or `null` + * for any other state (`starting`/`probing`/`ready`/`failed` → no bar). Mirrors + * the wire shape without inventing data: + * - `percent`: the backend's completion % clamped to `0..100` when the total is + * known, else `null` → the overlay shows an **indeterminate** bar (no fake %); + * - `bytesLabel`: `"X Mo / Y Mo"` when both bounds are known, `"X Mo"` when only + * the downloaded amount is, else `null`; + * - `source`: the remote model ref (e.g. HF `namespace/repo`) when present. + */ +export interface ModelServerDownloadProgress { + percent: number | null; + bytesLabel: string | null; + source: string | null; +} + +export function describeModelServerDownload( + status: ModelServerStatus | undefined, +): ModelServerDownloadProgress | null { + if (!status || status.state !== "downloading") return null; + const { downloadedBytes, totalBytes, percent, source } = status; + const bytesLabel = + downloadedBytes != null && totalBytes != null + ? `${formatBytes(downloadedBytes)} / ${formatBytes(totalBytes)}` + : downloadedBytes != null + ? formatBytes(downloadedBytes) + : null; + return { + percent: percent != null ? Math.min(100, Math.max(0, percent)) : null, + bytesLabel, + source: source ?? null, + }; +} + /** What {@link useModelServerLaunchState} exposes to a consumer. */ export interface ModelServerLaunchState { /** diff --git a/frontend/src/features/layout/LayoutGrid.modelServerOverlay.test.tsx b/frontend/src/features/layout/LayoutGrid.modelServerOverlay.test.tsx index beb0e23..df2f5ab 100644 --- a/frontend/src/features/layout/LayoutGrid.modelServerOverlay.test.tsx +++ b/frontend/src/features/layout/LayoutGrid.modelServerOverlay.test.tsx @@ -12,7 +12,17 @@ * event on that server. xterm is stubbed so the terminal mounts under jsdom. */ import { beforeEach, describe, it, expect, vi } from "vitest"; -import { render, screen, waitFor, act } from "@testing-library/react"; +import { render, screen, waitFor, configure } from "@testing-library/react"; + +// Delivery of a `modelServerStatusChanged` event is made deterministic by +// `trackSubscriptions` (no lost event). What remains is *propagation* latency: +// the overlay only shows once the hook's async profile + agent loads resolve, so +// the assertion is a guaranteed-eventual state. Under the full-suite parallel +// run (22 files) the worker's event loop can be starved past the 1000 ms default +// async-util budget — a scheduling delay, not a logic race. A generous timeout +// removes that flake without weakening any assertion (the state is guaranteed to +// arrive; we only wait long enough for it under load). +configure({ asyncUtilTimeout: 5000 }); vi.mock("@xterm/xterm", () => ({ Terminal: class { @@ -108,16 +118,35 @@ function renderGrid(gateways: Gateways) { ); } -async function emit( +/** + * Emit `status` and KEEP re-emitting it until `check` passes (the emit runs + * inside `waitFor`, whose async wrapper already provides `act`). This kills the + * emit-vs-subscribe race at the root — with no need to identify *which* + * `onDomainEvent` subscriber is the overlay hook's. + * + * Why the previous `waitSubscribed(count > 0)` gate was insufficient: several + * components subscribe to the event stream, and the ones outside `LeafView` + * mount *before* the async layout load brings `LeafView` (and its overlay hook) + * into the tree — so the global subscriber count crosses 0 while the overlay + * hook's own handler is not yet registered, and a single emit gated on it is + * still dropped. Re-emitting is safe because the reducer stores an idempotent + * per-server value: whichever attempt lands *after* the overlay hook subscribed + * and its profile/agent loads resolved is the one that makes `check` pass; the + * earlier (possibly dropped) attempts are no-ops. Used for BOTH appearance and + * retraction assertions so no case can regress into the race. + */ +async function emitUntil( systemGateway: MockSystemGateway, status: ModelServerStatus, + check: () => void, ): Promise { - await act(async () => { + await waitFor(() => { systemGateway.emit({ type: "modelServerStatusChanged", serverId: SERVER_ID, status, }); + check(); }); } @@ -147,29 +176,28 @@ describe("LayoutGrid — model-server launch overlay (ticket #54)", () => { }); renderGrid(gateways); - await waitFor(() => expect(screen.getByTestId("layout-leaf")).toBeTruthy()); - - // No overlay before any status. + // The cell exists but no status has been emitted yet → no overlay. + await screen.findByTestId("layout-leaf"); expect(screen.queryByTestId("model-server-overlay")).toBeNull(); // Downloading → overlay with the download wording. - await emit(systemGateway, { - state: "downloading", - downloadedBytes: null, - totalBytes: null, - percent: null, - source: null, - }); - await waitFor(() => - expect(screen.getByTestId("model-server-overlay")).toBeTruthy(), - ); - expect(screen.getByTestId("model-server-overlay").textContent).toContain( - "Téléchargement du modèle", + await emitUntil( + systemGateway, + { + state: "downloading", + downloadedBytes: null, + totalBytes: null, + percent: null, + source: null, + }, + () => + expect( + screen.getByTestId("model-server-overlay").textContent, + ).toContain("Téléchargement du modèle"), ); // Ready → overlay gone. - await emit(systemGateway, { state: "ready", reused: false }); - await waitFor(() => + await emitUntil(systemGateway, { state: "ready", reused: false }, () => expect(screen.queryByTestId("model-server-overlay")).toBeNull(), ); }); @@ -194,27 +222,25 @@ describe("LayoutGrid — model-server launch overlay (ticket #54)", () => { }); renderGrid(gateways); - await waitFor(() => expect(screen.getByTestId("layout-leaf")).toBeTruthy()); - await emit(systemGateway, { state: "probing" }); - await waitFor(() => + await emitUntil(systemGateway, { state: "probing" }, () => expect( screen.getByTestId("model-server-overlay").textContent, ).toContain("Chargement du serveur"), ); - await emit(systemGateway, { state: "starting" }); - expect(screen.getByTestId("model-server-overlay").textContent).toContain( - "Chargement du serveur", + await emitUntil(systemGateway, { state: "starting" }, () => + expect( + screen.getByTestId("model-server-overlay").textContent, + ).toContain("Chargement du serveur"), ); + // No progressbar for a non-download preparing state. + expect(screen.queryByTestId("model-server-progress")).toBeNull(); - await emit(systemGateway, { - state: "failed", - code: "spawn", - message: "boom", - }); - await waitFor(() => - expect(screen.queryByTestId("model-server-overlay")).toBeNull(), + await emitUntil( + systemGateway, + { state: "failed", code: "spawn", message: "boom" }, + () => expect(screen.queryByTestId("model-server-overlay")).toBeNull(), ); }); @@ -255,25 +281,156 @@ describe("LayoutGrid — model-server launch overlay (ticket #54)", () => { }); renderGrid(gateways); - await waitFor(() => - expect(screen.getAllByTestId("layout-leaf").length).toBe(2), + + // Both cells carry the overlay from a single server event (re-emitted until + // both cells' overlay hooks have subscribed and correlated the server). + await emitUntil( + systemGateway, + { + state: "downloading", + downloadedBytes: null, + totalBytes: null, + percent: null, + source: null, + }, + () => + expect(screen.getAllByTestId("model-server-overlay").length).toBe(2), ); - await emit(systemGateway, { - state: "downloading", - downloadedBytes: null, - totalBytes: null, - percent: null, - source: null, - }); - // Both cells carry the overlay from a single server event. - await waitFor(() => - expect(screen.getAllByTestId("model-server-overlay").length).toBe(2), - ); - - await emit(systemGateway, { state: "ready", reused: true }); - await waitFor(() => + await emitUntil(systemGateway, { state: "ready", reused: true }, () => expect(screen.queryAllByTestId("model-server-overlay").length).toBe(0), ); }); }); + +/** Render a single pinned-agent cell and return its system gateway. */ +async function renderSinglePinnedCell(): Promise { + const profileGateway = new MockProfileGateway(); + await profileGateway.saveProfile(localModelProfile("opencode-local")); + const agentGateway = new MockAgentGateway(); + const agent = await agentGateway.createAgent("p1", { + name: "Worker", + profileId: "opencode-local", + }); + const systemGateway = new MockSystemGateway(); + const gateways = makeGateways(agentGateway, profileGateway, systemGateway); + const layout = gateways.layout as MockLayoutGateway; + const leafId = leaves(await layout.loadLayout("p1"))[0].id; + await layout.mutateLayout("p1", { + type: "setCellAgent", + target: leafId, + agent: agent.id, + }); + renderGrid(gateways); + // No subscription gate needed: every emission goes through `emitUntil`, which + // re-emits until the overlay hook has subscribed and correlated the server. + return systemGateway; +} + +describe("LayoutGrid — download progress (ticket #54 F2)", () => { + it("known percent → determinate progressbar + % + formatted bytes + source", async () => { + const systemGateway = await renderSinglePinnedCell(); + + await emitUntil( + systemGateway, + { + state: "downloading", + downloadedBytes: 1_500_000, + totalBytes: 3_000_000, + percent: 50, + source: "unsloth/Qwen3-Coder-30B", + }, + () => + expect( + screen.getByTestId("model-server-progress").getAttribute("aria-valuenow"), + ).toBe("50"), + ); + + const bar = screen.getByTestId("model-server-progress"); + expect(bar.getAttribute("role")).toBe("progressbar"); + expect(bar.getAttribute("aria-valuemin")).toBe("0"); + expect(bar.getAttribute("aria-valuemax")).toBe("100"); + + const label = screen.getByTestId("model-server-progress-label"); + expect(label.textContent).toContain("50 %"); + expect(label.textContent).toContain("1.5 Mo / 3 Mo"); + + expect(screen.getByTestId("model-server-source").textContent).toBe( + "unsloth/Qwen3-Coder-30B", + ); + }); + + it("unknown total → indeterminate bar, NO fake %, shows downloaded bytes", async () => { + const systemGateway = await renderSinglePinnedCell(); + + await emitUntil( + systemGateway, + { + state: "downloading", + downloadedBytes: 800_000, + totalBytes: null, + percent: null, + source: null, + }, + () => expect(screen.getByTestId("model-server-progress")).toBeTruthy(), + ); + + const bar = screen.getByTestId("model-server-progress"); + // Indeterminate: no aria-valuenow set. + expect(bar.hasAttribute("aria-valuenow")).toBe(false); + + const label = screen.getByTestId("model-server-progress-label"); + expect(label.textContent).not.toContain("%"); + expect(label.textContent).toContain("800 Ko"); + // No source line when the backend sends none. + expect(screen.queryByTestId("model-server-source")).toBeNull(); + }); + + it("starting/probing show the title but NO progressbar", async () => { + const systemGateway = await renderSinglePinnedCell(); + + await emitUntil(systemGateway, { state: "starting" }, () => + expect( + screen.getByTestId("model-server-overlay").textContent, + ).toContain("Chargement du serveur"), + ); + expect(screen.queryByTestId("model-server-progress")).toBeNull(); + }); + + it("later progress events update the bar in place", async () => { + const systemGateway = await renderSinglePinnedCell(); + + await emitUntil( + systemGateway, + { + state: "downloading", + downloadedBytes: 1_000_000, + totalBytes: 4_000_000, + percent: 25, + source: null, + }, + () => + expect( + screen.getByTestId("model-server-progress").getAttribute("aria-valuenow"), + ).toBe("25"), + ); + + await emitUntil( + systemGateway, + { + state: "downloading", + downloadedBytes: 3_000_000, + totalBytes: 4_000_000, + percent: 75, + source: null, + }, + () => + expect( + screen.getByTestId("model-server-progress").getAttribute("aria-valuenow"), + ).toBe("75"), + ); + expect( + screen.getByTestId("model-server-progress-label").textContent, + ).toContain("3 Mo / 4 Mo"); + }); +}); diff --git a/frontend/src/features/layout/LayoutGrid.tsx b/frontend/src/features/layout/LayoutGrid.tsx index 730dd61..33d4df5 100644 --- a/frontend/src/features/layout/LayoutGrid.tsx +++ b/frontend/src/features/layout/LayoutGrid.tsx @@ -40,6 +40,7 @@ import { } from "@/features/announcements"; import { modelServerOverlayText, + describeModelServerDownload, useModelServerLaunchState, } from "@/features/agents"; import { leaves, normalizeWeights, resizeAdjacent } from "./layout"; @@ -382,7 +383,12 @@ function LeafView({ const pinnedAgent = agentId ? agents.find((a) => a.id === agentId) : undefined; - const modelServerOverlay = modelServerOverlayText(statusForAgent(pinnedAgent)); + const modelServerStatus = statusForAgent(pinnedAgent); + const modelServerOverlay = modelServerOverlayText(modelServerStatus); + // F2 — download progress (bar/%/bytes/source) when the status carries it; null + // for every non-`downloading` state, so `starting`/`probing` show just the + // title. An indeterminate download (unknown total) yields `percent: null`. + const modelServerDownload = describeModelServerDownload(modelServerStatus); const agentWork = agentId ? workState?.agents.find((row) => row.agentId === agentId) : undefined; @@ -976,16 +982,101 @@ function LeafView({ userSelect: "none", }} > - - {modelServerOverlay} - + {modelServerOverlay} + {modelServerDownload && ( + <> + {/* Determinate bar when the total is known; an indeterminate + track (no aria-valuenow → ARIA indeterminate) otherwise. */} +
+
+
+ {(modelServerDownload.percent != null || + modelServerDownload.bytesLabel) && ( + + {modelServerDownload.percent != null + ? `${Math.round(modelServerDownload.percent)} %` + : ""} + {modelServerDownload.percent != null && + modelServerDownload.bytesLabel + ? " · " + : ""} + {modelServerDownload.bytesLabel ?? ""} + + )} + {modelServerDownload.source && ( + + {modelServerDownload.source} + + )} + + )} +
)} diff --git a/frontend/src/shared/styles/theme.css b/frontend/src/shared/styles/theme.css index 43ea326..e48ebb0 100644 --- a/frontend/src/shared/styles/theme.css +++ b/frontend/src/shared/styles/theme.css @@ -74,3 +74,14 @@ outline-offset: 1px; } } + +/* Indeterminate progress sliver (ticket #54 F2): a model download whose total + size is unknown slides a sliver back and forth instead of showing a fake %. */ +@keyframes model-server-indeterminate { + 0% { + left: -40%; + } + 100% { + left: 100%; + } +}