docs(models): add inline comments throughout models crate

Add concise, high-signal inline comments to function bodies across the
model discovery, download, and caching implementation. Comments explain
intent, edge cases, error handling strategies, and concurrency patterns
rather than restating code behavior.

Key areas covered:
- Catalog lookup (blocking client, redirect limits, JSON parsing)
- Local discovery (filesystem scanning, directory naming conventions)
- Download logic (resume support, Range headers, progress throttling)
- Cache helpers (path resolution, partial file management)
- Metadata cache (TTL, backoff config, cache-first lookup)
- Network utilities (timeouts, exponential backoff, error tracking)
- Worker threads (batch updates, non-blocking sends)

Author: hate

keyless-models/src/catalog.rs

@@ -7,35 +7,46 @@ use keyless_core::error::{KeylessError, KeylessResult};
 /// List public OpenAI Whisper model IDs from Hugging Face.
 pub fn list_openai_whisper_models() -> KeylessResult<Vec<String>> {
     let url = "https://huggingface.co/api/models?search=openai/whisper";
+    // Use blocking client (synchronous API; this function is not async).
+    // Limit redirects to 10 to prevent infinite loops (safety measure).
     let client = reqwest::blocking::Client::builder()
         .redirect(reqwest::redirect::Policy::limited(10))
         .build()
         .map_err(|e| KeylessError::Config(format!("http client: {}", e)))?;
     let mut req = client.get(url);
+    // Add auth header if available (HF token from env); works without token for public models.
     if let Some((h, v)) = crate::net::auth_header() {
         req = req.header(h, v);
     }
+    // Send request and check HTTP status (4xx/5xx treated as errors).
     let resp = req
         .send()
         .map_err(|e| KeylessError::Config(format!("GET {}: {}", url, e)))?
         .error_for_status()
         .map_err(|e| KeylessError::Config(e.to_string()))?;
+    // Read response body as UTF-8 string (assumes HF API returns UTF-8).
     let text = resp
         .text()
         .map_err(|e| KeylessError::Config(format!("read response: {}", e)))?;
+    // Parse JSON into generic Value (flexible parsing; don't assume exact schema).
     let v: serde_json::Value = serde_json::from_str(&text)
         .map_err(|e| KeylessError::Config(format!("parse json: {}", e)))?;
     let mut out: Vec<String> = Vec::new();
+    // Expect response to be array of model objects (handle non-array gracefully).
     if let Some(arr) = v.as_array() {
         for item in arr {
+            // Extract "modelId" field, filter to only OpenAI Whisper models (prefix check).
+            // and_then chains: get("modelId") → as_str() → Some(id) if all succeed.
             if let Some(id) = item.get("modelId").and_then(|s| s.as_str())
                 && id.starts_with("openai/whisper")
             {
                 out.push(id.to_string());
             }
         }
     }
+    // Sort for deterministic output (alphabetical order).
     out.sort();
+    // Remove duplicates (same model ID may appear multiple times in HF response).
     out.dedup();
     Ok(out)
 }

keyless-models/src/discover.rs

@@ -9,26 +9,33 @@ use std::fs;
 pub fn discover_local_whisper_repos() -> Vec<String> {
     let root = crate::hf::keyless_cache_root();
     let mut out = Vec::new();
+    // Return empty list if cache dir doesn't exist or can't be read (graceful degradation).
     let Ok(entries) = fs::read_dir(&root) else {
         return out;
     };
+    // flatten() skips I/O errors on individual entries (best-effort scanning).
     for e in entries.flatten() {
         let name = e.file_name();
+        // Convert OsStr to String (lossy; invalid UTF-8 replaced with �).
         let name = name.to_string_lossy();
-        // Expect <org>--<repo>
+        // Expect <org>--<repo> format (HF model IDs use `/`, but filesystem uses `--` to avoid conflicts).
         if !name.contains("--") {
             continue;
         }
+        // splitn(2, "--") limits to 2 parts (prevents splitting on multiple `--` sequences).
         let mut parts = name.splitn(2, "--");
+        // unwrap_or is safe here: we checked contains("--") above, but defensive fallback handles edge cases.
         let org = parts.next().unwrap_or("");
         let repo = parts.next().unwrap_or("");
+        // Skip malformed directory names (empty org or repo after split).
         if org.is_empty() || repo.is_empty() {
             continue;
         }
         let repo_path = e.path();
         let weights = repo_path.join("model.safetensors");
-        // Mark as installed ONLY when final weights exist (ignore .partial)
+        // Mark as installed ONLY when final weights exist (ignore .partial downloads in progress).
         if weights.exists() {
+            // Convert back to HF model ID format (`org/repo`).
             out.push(format!("{}/{}", org, repo));
         }
     }

keyless-models/src/download.rs

@@ -57,47 +57,55 @@ pub fn ensure_model_cached(
     tx: SyncSender<DownloadEvent>,
     cancel: Arc<AtomicBool>,
 ) {
+    // Try-send for non-blocking start event (may drop if channel full; OK for best-effort).
     let _ = tx.try_send(DownloadEvent::Started {
         model: model_id.clone(),
     });
     info!(model = %model_id, "starting model download");
+    // Clone channel senders for different scopes (closure captures require owned values).
     let tx_clone = tx.clone();
     let tx_for_loop = tx.clone();
     let result: KeylessResult<()> = (|| {
-        // Blocking reqwest client (with timeouts)
+        // Blocking reqwest client (with timeouts); this function is not async.
         let client =
             net::build_blocking_client().map_err(|e| KeylessError::Other(e.to_string()))?;
+        // Optional auth header (HF token from env); works without token for public models.
         let auth = net::auth_header();
 
-        // Ensure repo cache dir
+        // Ensure repo cache dir exists (e.g., ~/.cache/keyless/openai--whisper-base/).
         let repo_dir = hf::keyless_cache_repo_dir(&model_id);
         fs::create_dir_all(&repo_dir).map_err(KeylessError::from)?;
 
-        // Plan line (if size known)
+        // Plan line (if size known from cache): show total expected size to user.
         if let Some(total) = meta::plan_total_bytes(&model_id) {
             let _ = tx_clone.try_send(DownloadEvent::Stage {
                 text: format!("plan: {}", keyless_core::utils::human_size(total)),
             });
         }
 
-        // Small files (with retry/backoff)
+        // Small files (with retry/backoff): fetch into memory, then write atomically.
         for name in ["config.json", "tokenizer.json"] {
+            // Check cancellation before each file (allows prompt abort).
             if cancel.load(Ordering::Relaxed) {
                 return Err(KeylessError::Other("cancelled".into()));
             }
             let dst = repo_dir.join(name);
+            // Skip if already downloaded (idempotent operation).
             if dst.exists() {
                 continue;
             }
             let url = net::hf_resolve_url(&model_id, name);
             let _ = tx_for_loop.try_send(DownloadEvent::Stage {
                 text: format!("downloading {}", name),
             });
+            // Retry with exponential backoff (handles transient network errors).
             let (attempts, initial, max_ms) = meta::backoff_config();
             let (bytes, content_len_hdr) =
                 net::blocking_get_with_backoff(&client, &url, &auth, attempts, initial, max_ms)?;
+            // Write file atomically (create overwrites if exists; we checked above, but defensive).
             let mut f = File::create(&dst).map_err(KeylessError::from)?;
             f.write_all(&bytes).map_err(KeylessError::from)?;
+            // Verify downloaded size matches Content-Length header (catches truncation/corruption).
             if let Some(cl) = content_len_hdr
                 && cl != bytes.len() as u64
             {
@@ -110,25 +118,28 @@ pub fn ensure_model_cached(
             }
         }
 
-        // Large file streaming with progress
+        // Large file streaming with progress: stream chunks to avoid memory issues.
         if cancel.load(Ordering::Relaxed) {
             return Err(KeylessError::Other("cancelled".into()));
         }
         let weights = repo_dir.join("model.safetensors");
         if !weights.exists() {
             let url = net::hf_resolve_url(&model_id, "model.safetensors");
             let mut req = client.get(&url);
+            // Add auth header if available (for private models or rate limit increases).
             if let Some((h, v)) = &auth {
                 req = req.header(h, v.clone());
             }
-            // resume support
+            // Resume support: check for existing partial file to resume download.
             let tmp = repo_dir.join("model.safetensors.partial");
             let mut downloaded: u64 = 0;
+            // Read partial file size to determine resume offset.
             if tmp.exists()
                 && let Ok(m) = std::fs::metadata(&tmp)
             {
                 downloaded = m.len();
             }
+            // Add Range header if resuming (bytes=<offset>- requests from offset to end).
             if downloaded > 0 {
                 req = req.header(reqwest::header::RANGE, format!("bytes={}-", downloaded));
             }
@@ -138,7 +149,7 @@ pub fn ensure_model_cached(
             let status = resp.status();
             let mut file: Box<dyn std::io::Write>;
             if status == reqwest::StatusCode::PARTIAL_CONTENT {
-                // Resume OK (206); keep downloaded as-is and append
+                // Resume OK (206): server honored Range header; append to existing partial file.
                 resp = resp
                     .error_for_status()
                     .map_err(|e| KeylessError::Other(e.to_string()))?;
@@ -149,12 +160,14 @@ pub fn ensure_model_cached(
                         .map_err(KeylessError::from)?,
                 );
             } else if status == reqwest::StatusCode::OK {
-                // Range ignored; restart from 0
+                // Range ignored (200): server doesn't support Range; restart from beginning.
+                // Remove partial file to avoid corruption (truncate would also work).
                 let _ = std::fs::remove_file(&tmp);
                 downloaded = 0;
                 resp = resp
                     .error_for_status()
                     .map_err(|e| KeylessError::Other(e.to_string()))?;
+                // Create new file (truncate overwrites existing; defensive for edge cases).
                 file = Box::new(
                     std::fs::OpenOptions::new()
                         .write(true)
@@ -164,46 +177,61 @@ pub fn ensure_model_cached(
                         .map_err(KeylessError::from)?,
                 );
             } else {
+                // Unexpected status (e.g., 416 Range Not Satisfiable); fail fast.
                 return Err(KeylessError::Other(format!("unexpected status {}", status)));
             }
 
+            // Calculate total size: for 206, add downloaded bytes to Content-Length (partial response).
+            // For 200, Content-Length is full file size (no resume offset).
             let total = if status == reqwest::StatusCode::PARTIAL_CONTENT {
                 resp.content_length()
                     .unwrap_or(0)
                     .saturating_add(downloaded)
             } else {
                 resp.content_length().unwrap_or(0)
             };
+            // Cache total size for future resume attempts (if known).
             if total > 0 {
                 meta::update_saved_size(&model_id, total);
             }
+            // Track time for speed/ETA calculations.
             let start = std::time::Instant::now();
             let mut last_emit = std::time::Instant::now();
+            // 64KB buffer: balances memory usage vs I/O syscall overhead.
             let mut buf = vec![0u8; 64 * 1024];
             loop {
+                // Check cancellation before reading (allows prompt abort on slow networks).
                 if cancel.load(Ordering::Relaxed) {
                     return Err(KeylessError::Other("cancelled".into()));
                 }
                 use std::io::Read;
+                // Read chunk from response stream (may return < buf.len() at end).
                 let n = resp
                     .read(&mut buf)
                     .map_err(|e| KeylessError::Other(format!("read chunk: {}", e)))?;
+                // EOF: n == 0 indicates stream end (normal completion).
                 if n == 0 {
                     break;
                 }
+                // Check cancellation after read (allows abort during write).
                 if cancel.load(Ordering::Relaxed) {
                     return Err(KeylessError::Other("cancelled".into()));
                 }
+                // Write chunk to file (only write n bytes, not full buffer).
                 file.write_all(&buf[..n]).map_err(KeylessError::from)?;
                 downloaded += n as u64;
-                // Throttle progress events to at most every 250ms
+                // Throttle progress events to at most every 250ms (prevents channel saturation).
                 if last_emit.elapsed().as_millis() >= 250 {
                     last_emit = std::time::Instant::now();
                     if total > 0 && downloaded <= total {
+                        // Calculate speed (MB/s) and ETA (seconds remaining).
+                        // max(0.001) prevents division by zero on very fast downloads.
                         let secs = start.elapsed().as_secs_f64().max(0.001);
                         let mbps = (downloaded as f64 / 1_000_000.0) / secs;
+                        // saturating_sub prevents underflow if downloaded > total (shouldn't happen).
                         let left = (total.saturating_sub(downloaded)) as f64;
                         let bps = (downloaded as f64) / secs;
+                        // ETA = remaining_bytes / bytes_per_second; max(0.0) prevents negative ETA.
                         let eta = if bps > 0.0 {
                             (left / bps).max(0.0)
                         } else {
@@ -216,6 +244,7 @@ pub fn ensure_model_cached(
                             eta_s: eta,
                         });
                     } else {
+                        // Total unknown or downloaded exceeds expected (shouldn't happen).
                         let _ = tx_clone.try_send(DownloadEvent::Progress {
                             bytes: downloaded,
                             total: None,
@@ -225,22 +254,25 @@ pub fn ensure_model_cached(
                     }
                 }
             }
+            // Close file handle before rename (ensures all buffers flushed to disk).
             drop(file);
+            // Atomic rename: partial → final (prevents partial files from being used).
             fs::rename(&tmp, &weights).map_err(KeylessError::from)?;
-            // Verify final size if we know total
+            // Verify final size if we know total (catches truncation/corruption).
             if total > 0 {
                 let final_len = std::fs::metadata(&weights)
                     .map_err(KeylessError::from)?
                     .len();
                 if final_len != total {
-                    // clean up and error out; controller will surface error
+                    // Clean up corrupted file (better to have no file than wrong size).
+                    // Controller will surface error to user and allow retry.
                     let _ = std::fs::remove_file(&weights);
                     return Err(KeylessError::Other(format!(
                         "downloaded size mismatch ({} != {}), please retry",
                         final_len, total
                     )));
                 }
-                // Emit a final 100% progress to fully fill the gauge
+                // Emit a final 100% progress to fully fill the gauge (UI completeness).
                 let _ = tx_clone.try_send(DownloadEvent::Progress {
                     bytes: total,
                     total: Some(total),
@@ -252,12 +284,13 @@ pub fn ensure_model_cached(
         Ok(())
     })();
 
-    // Log result for debugging (goes to session.log)
+    // Log result for debugging (goes to session.log; not shown in TUI).
     match &result {
         Ok(()) => info!(model = %model_id, "model download completed successfully"),
         Err(e) => error!(model = %model_id, error = %e, "model download failed"),
     }
 
-    // Ensure the terminal receives completion even when the channel is saturated
+    // Use blocking send for completion event (must be delivered; wait if channel full).
+    // try_send would drop the event if channel saturated, hiding errors from user.
     let _ = tx.send(DownloadEvent::Done(result));
 }

keyless-models/src/hf.rs

@@ -7,6 +7,8 @@ use keyless_core::error::KeylessResult;
 /// Downloads are written here in `<org>--<repo>/` subdirectories.
 /// Env override: `KEYLESS_CACHE_DIR` (primarily for tests).
 pub fn keyless_cache_root() -> std::path::PathBuf {
+    // Use platform-specific cache directory (e.g., ~/.cache/keyless/models on Linux).
+    // Fallback to .cache in current directory if cache_dir() unavailable (rare edge case).
     if let Some(base) = dirs_next::cache_dir() {
         return base.join("keyless").join("models");
     }
@@ -19,29 +21,39 @@ pub fn keyless_cache_root() -> std::path::PathBuf {
 ///
 /// Converts `owner/repo` to `<cache_root>/owner--repo/`.
 pub fn keyless_cache_repo_dir(model_id: &str) -> std::path::PathBuf {
+    // splitn(2, '/') limits to 2 parts (prevents splitting on multiple slashes).
     let mut parts = model_id.splitn(2, '/');
+    // Fallback to "openai" if no org (malformed model_id like "whisper-tiny").
     let org = parts.next().unwrap_or("openai");
-    // Extract repo name from DEFAULT_MODEL_ID for fallback
+    // Extract repo name from DEFAULT_MODEL_ID for fallback (if model_id has no slash).
+    // nth(1) gets second part after split; fallback to "whisper-tiny" if DEFAULT_MODEL_ID malformed.
     let default_repo = keyless_core::config::DEFAULT_MODEL_ID
         .split('/')
         .nth(1)
         .unwrap_or("whisper-tiny");
+    // Use default_repo if model_id has no repo part (e.g., just "openai").
     let repo = parts.next().unwrap_or(default_repo);
+    // Format as "org--repo" (double dash replaces slash for filesystem compatibility).
     keyless_cache_root().join(format!("{}--{}", org, repo))
 }
 
 /// Get the size of a locally cached model's weights file (if present).
 pub fn get_local_model_size(model_id: &str) -> Option<u64> {
     let cache_root = keyless_cache_root();
+    // splitn(2, '/') limits to 2 parts (prevents splitting on multiple slashes).
     let mut parts = model_id.splitn(2, '/');
+    // Empty fallbacks for validation (unlike keyless_cache_repo_dir which uses defaults).
     let org = parts.next().unwrap_or("");
     let repo = parts.next().unwrap_or("");
+    // Return None for malformed model_id (empty org or repo).
     if org.is_empty() || repo.is_empty() {
         return None;
     }
+    // Construct path: cache_root/org--repo/model.safetensors.
     let weights = cache_root
         .join(format!("{}--{}", org, repo))
         .join("model.safetensors");
+    // ok() converts Result to Option; map extracts file size if metadata exists.
     std::fs::metadata(weights).ok().map(|m| m.len())
 }
 
@@ -56,10 +68,13 @@ pub fn get_local_model_size(model_id: &str) -> Option<u64> {
 pub fn delete_partial_file(model_id: &str) -> KeylessResult<bool> {
     let repo_dir = keyless_cache_repo_dir(model_id);
     let partial = repo_dir.join("model.safetensors.partial");
+    // Check existence before deletion (avoids error if file already gone).
     if partial.exists() {
-        std::fs::remove_file(&partial)?; // auto-converts to KeylessError::Io
+        // ? operator auto-converts io::Error to KeylessError::Io via From impl.
+        std::fs::remove_file(&partial)?;
         Ok(true)
     } else {
+        // File doesn't exist; return false (not an error).
         Ok(false)
     }
 }