Skip to content

review last commit in Test#2

Merged
chcavignx merged 11 commits intomainfrom
test
Dec 30, 2025
Merged

review last commit in Test#2
chcavignx merged 11 commits intomainfrom
test

Conversation

@chcavignx
Copy link
Copy Markdown
Owner

@chcavignx chcavignx commented Dec 30, 2025

Summary by CodeRabbit

  • New Features

    • Centralized model loading flow with staged progress.
    • Offline model paths made repository-relative for portable offline use.
    • Platform-aware model selection to choose appropriate model sets.
  • Bug Fixes

    • More resilient downloads with resume/retry behavior and clearer errors.
    • TTS model validation now fails early if required model is missing.
    • Added defensive handling for unavailable gated or missing model snapshots.
  • Chores

    • Updated offline setup guidance and improved script documentation.

✏️ Tip: You can customize this high-level summary in your review settings.

Commented out static model path settings in config.yaml to allow dynamic path construction in Python. Updated TTSEngine to raise FileNotFoundError instead of printing a warning when the Piper model is missing, improving error handling.
… when you import it

Split the model lists into two constants:
MODELS_NAMES_BASE: Core models that work on all platforms
MODELS_NAMES_EXTENDED: Larger models for non-Raspberry Pi systems
Created
get_models_to_download()
 function: This function determines at runtime (not import time) which models to download based on whether the code is running on a Raspberry Pi.
Updated
run()
 function: Now calls
get_models_to_download()
 to get the appropriate list of models.
Benefits:

✅ No import-time side effects: The module can now be imported without triggering detect_raspberry_pi_model() and its console output
✅ Testable: The module can be imported in tests without unwanted side effects
✅ Lazy evaluation: Platform detection only happens when
run()
 is actually called
✅ Same functionality: The behavior is identical when the script is run directly
The detection now only happens when you actually run the script, not when you import it!
…tive or environment-aware path for better portability

Revised documentation to recommend using smaller Whisper models for Raspberry Pi 5, clarified caching paths to use $REPO_ROOT instead of $HOME, and updated instructions for local file references and HuggingFace dataset storage. These changes improve clarity and ensure correct offline usage and resource management.
Added descriptive module-level docstrings and standardized shebang lines to scripts in scripts/models/audio. Improved clarity on each script's purpose and ensured consistency across the codebase.
Added docstrings to the download_file and run functions for better clarity. Enhanced error handling in the run function by printing the exception message when Whisper library download fails, providing more informative output.
… when you import it

Split the model lists into two constants:
MODELS: Core models that work on all platforms
MODELS_EXTENDED: Larger models for non-Raspberry Pi systems
Created
get_models_to_download()
 function: This function determines at runtime (not import time) which models to download based on whether the code is running on a Raspberry Pi.
Updated
run()
 function: Now calls
get_models_to_download()
 to get the appropriate list of models.
Benefits:

✅ No import-time side effects: The module can now be imported without triggering detect_raspberry_pi_model() and its console output
✅ Testable: The module can be imported in tests without unwanted side effects
✅ Lazy evaluation: Platform detection only happens when
run()
 is actually called
✅ Same functionality: The behavior is identical when the script is run directly
The detection now only happens when you actually run the script, not when you import it!
Added exception handling for RepositoryNotFoundError and GatedRepoError when downloading models and datasets from Hugging Face. This provides clearer error messages when a model or dataset is missing or gated, improving script robustness.
Introduces load_all.py, a script that sequentially loads Whisper, Fast Whisper, Hugging Face, Vosk, and Piper models for audio processing. This centralizes model initialization for easier management and testing.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 30, 2025

📝 Walkthrough

Walkthrough

Refactors config to use repository-relative paths, adds platform-aware base vs. extended model selection, hardens model download logic with resume/fallbacks and exception handling, adds a load_all orchestrator for sequential model staging, and makes missing Piper TTS model a hard failure.

Changes

Cohort / File(s) Summary
Configuration & Docs
config.yaml, examples/STT/whisper/STT_offline_whisper_rpi5.md
Removes hard-coded download_root/model_path values in config; docs updated to reference $REPO_ROOT-based cache paths and repository-relative workflows.
Orchestration
scripts/models/audio/load_all.py
New script that sequences model downloads (Whisper, Fast Whisper, HuggingFace objects, Vosk, Piper) with staged progress messages and a main entry.
Whisper model management
scripts/models/audio/whisper_objects.py
Splits models into MODELS_BASE/MODELS_EXTENDED, adds get_models_to_download() platform logic, reworks run() to use the selection, and adds download_file() fallback with resume, symlink/dir handling, and improved error reporting.
Fast Whisper model selection
scripts/models/audio/fast_whisper_objects.py
Introduces MODELS_NAMES_BASE and MODELS_NAMES_EXTENDED, adds get_models_to_download() and uses it in run() replacing prior MODELS_NAMES usage.
Hugging Face downloads & resilience
scripts/models/audio/load_huggingface_objects.py
Adds model/dataset existence checks, wraps snapshot_download() in try/except catching RepositoryNotFoundError and GatedRepoError, and adds a main guard and clearer messages.
Vosk & Piper scripts
scripts/models/audio/vosk_models.py, scripts/models/audio/piper_models.py
vosk_models.py docstring expanded; piper_models.py refactors config imports to call load_config()/setup_python_path() directly.
Utilities
scripts/models/audio/models_check.py
Adds shebang and module docstring; shortens internal docstring with no functional change.
Runtime validation
src/audio/voice_agent_offline.py
Piper TTS model absence now raises FileNotFoundError instead of logging a warning, making missing model a hard failure during initialization.

Sequence Diagram

sequenceDiagram
    autonumber
    participant User
    participant LoadAll as load_all.py
    participant Whisper as whisper_objects.py
    participant FastWhisper as fast_whisper_objects.py
    participant HF as load_huggingface_objects.py
    participant Vosk as vosk_models.py
    participant Piper as piper_models.py
    participant Cache as RepoCache

    User->>LoadAll: run()
    LoadAll->>Whisper: run()
    Whisper->>Whisper: get_models_to_download() (platform check)
    Whisper->>Cache: request snapshot_download / download_file()
    alt snapshot_download success
        Cache-->>Whisper: model cached
    else snapshot_download failure
        Whisper->>Whisper: download_file() fallback (resume, mkdir, symlink)
        Cache-->>Whisper: cached or error
    end
    Whisper-->>LoadAll: done

    LoadAll->>FastWhisper: run()
    FastWhisper->>FastWhisper: get_models_to_download()
    FastWhisper->>Cache: snapshot_download()
    Cache-->>FastWhisper: cached
    FastWhisper-->>LoadAll: done

    LoadAll->>HF: run()
    HF->>Cache: snapshot_download() with try/except
    alt RepositoryNotFound/Gated
        HF-->>LoadAll: logged & skipped
    else success
        Cache-->>HF: cached
        HF-->>LoadAll: done
    end

    LoadAll->>Vosk: run()
    Vosk->>Cache: download_and_extract()
    Cache-->>Vosk: extracted
    Vosk-->>LoadAll: done

    LoadAll->>Piper: run()
    Piper->>Cache: download models
    Cache-->>Piper: cached
    Piper-->>LoadAll: done

    LoadAll-->>User: all stages completed
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 I hopped through caches, paths unwound,
From home to REPO_ROOT I found,
Base for Pi, extended for more,
Downloads guarded at each door,
Models march in—hooray, profound! 🎶

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is vague and generic; it lacks descriptive content about the actual changes made. Terms like 'review last commit' and 'in Test' do not convey meaningful information about what was modified. Provide a clear, descriptive title that summarizes the main change, such as 'Refactor model loading and config paths for repository-relative setup' or 'Add offline Whisper support with dynamic path configuration'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Docstrings were successfully generated.

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 17db5db and 45338c2.

📒 Files selected for processing (1)
  • scripts/models/audio/vosk_models.py
🔇 Additional comments (1)
scripts/models/audio/vosk_models.py (1)

2-3: LGTM! Docstring now accurately describes the script's purpose.

The updated docstring correctly identifies this as a Vosk model downloader rather than a Hugging Face model script, addressing the previous review feedback.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
scripts/models/audio/vosk_models.py (3)

19-20: Hard-coded cache path contradicts dynamic path resolution mentioned in the AI summary.

The AI summary indicates that config.yaml changes enable "dynamic path resolution" and "runtime-configurable model management," but this code uses a hard-coded path (~/cache/models/vosk). This inconsistency could lead to models being downloaded to unexpected locations.

Consider using the config.yaml-based path resolution mechanism mentioned in the summary to maintain consistency with other model loaders.


36-44: Zip extraction lacks error handling and removes file unconditionally.

The extraction logic has two issues:

  1. No try-except block around zipfile.ZipFile operations - extraction failures will crash the script
  2. The zip file is removed unconditionally (line 43), even if extraction fails, making it impossible to retry or debug
🔎 Proposed fix with error handling
     # Extract the file
     print(f"Extracting {filename}...")
-    with zipfile.ZipFile(filepath, "r") as zip_ref:
-        zip_ref.extractall(cache_dir)
-    print(f"Extracted {filename} successfully")
-
-    # Remove the zip file
-    os.remove(filepath)
-    print(f"Removed {filename}")
+    try:
+        with zipfile.ZipFile(filepath, "r") as zip_ref:
+            zip_ref.extractall(cache_dir)
+        print(f"Extracted {filename} successfully")
+        
+        # Remove the zip file only after successful extraction
+        os.remove(filepath)
+        print(f"Removed {filename}")
+    except zipfile.BadZipFile as e:
+        print(f"Error: {filename} is not a valid zip file: {e}")
+        raise
+    except Exception as e:
+        print(f"Error extracting {filename}: {e}")
+        raise

29-33: Add timeout to HTTP request to prevent indefinite hangs.

The requests.get() call lacks a timeout parameter. When using stream=True, the connection can hang indefinitely if the server is unresponsive or the connection is slow. For streaming downloads, use a tuple timeout format—connect timeout and read timeout (applied per-chunk)—or a single larger timeout for both phases. A single timeout=300 is acceptable for large file downloads, though timeout=(5, 300) (5s connect, 300s read) provides more control.

-    with requests.get(url, stream=True) as r:
+    with requests.get(url, stream=True, timeout=300) as r:
🧹 Nitpick comments (6)
examples/STT/whisper/STT_offline_whisper_rpi5.md (2)

31-38: Add guidance on setting the REPO_ROOT environment variable.

The documentation references $REPO_ROOT but doesn't explain how users should set it. Consider adding a step like:

export REPO_ROOT=/path/to/AI-Autonomous-Assistant

45-52: Clarify that users need to expand the variable or provide the full path.

The documentation shows "$REPO_ROOT/cache/models/whisper/vocab.bpe" in the Python code snippet. Users should replace this with the actual expanded path (not the literal string with dollar sign) when editing openai_public.py.

scripts/models/audio/fast_whisper_objects.py (1)

40-52: Consider adding error handling like load_huggingface_objects.py.

The snapshot_download() calls at line 50 lack exception handling. For consistency and robustness, consider wrapping them in try/except blocks to catch RepositoryNotFoundError and GatedRepoError as done in load_huggingface_objects.py (lines 34-42, 51-59).

🔎 Proposed enhancement
+from huggingface_hub.utils import GatedRepoError, RepositoryNotFoundError
+
 def run() -> None:
     """Downloads and saves Hugging Face models, tokenizers, processors,
     and their associated datasets to a local backup in your user cache directory."""
     models_to_download = get_models_to_download()
     for model_name in models_to_download:
         if model_exists(model_name, cache_dir):
             print(f"Model {model_name} already exists.")
             continue
         print(f"Downloading and saving {model_name} to {cache_dir}")
-
-        snapshot_download(repo_id=model_name, repo_type="model", cache_dir=cache_dir)
-        print(f"Model saved to: {os.path.join(cache_dir, model_name)}")
+        try:
+            snapshot_download(repo_id=model_name, repo_type="model", cache_dir=cache_dir)
+            print(f"Model saved to: {os.path.join(cache_dir, model_name)}")
+        except RepositoryNotFoundError:
+            print(f"Model {model_name} not found on Hugging Face.")
+        except GatedRepoError:
+            print(f"Model {model_name} is gated and requires authentication.")
     print("All fast-whisper models have been downloaded and saved.")
scripts/models/audio/vosk_models.py (2)

23-44: Consider adding file integrity verification.

The download process does not verify the integrity of downloaded files (e.g., via checksum or size validation). This could result in corrupted or incomplete files being extracted.

Consider adding SHA256 checksum verification if the Vosk model provider publishes checksums, or at minimum validate the file size matches expected values.


47-55: Sequential processing could benefit from progress indication.

The function processes models sequentially, which is appropriate, but for large downloads users might benefit from overall progress tracking (e.g., "Processing model 1 of 3").

🔎 Optional enhancement for progress tracking
 def run():
+    total_models = len(MODELS)
-    for model_name, url in MODELS.items():
-        print(f"Processing {model_name}...")
+    for idx, (model_name, url) in enumerate(MODELS.items(), 1):
+        print(f"Processing {model_name} ({idx}/{total_models})...")
         if model_exists(model_name, cache_dir):
             print(f"Model '{model_name}' already exists, skipping download.")
         else:
             download_and_extract(model_name, url)
         print("---")
     print("All vosk models processed!")
scripts/models/audio/load_all.py (1)

17-30: Consider adding elapsed time tracking for each phase.

For debugging and performance monitoring, tracking how long each phase takes could be valuable, especially since model downloads can be time-consuming.

🔎 Optional enhancement with timing
+import time
+
 def main() -> None:
     """Function to load all models"""
+    start_time = time.time()
     print("==================================================")
     print("🚀 Starting Master Model Loading Process")
     print("==================================================")
 
     print("\n--- Phase 1: Whisper Models ---")
+    phase_start = time.time()
     whisper_objects.run()
+    print(f"Phase completed in {time.time() - phase_start:.2f}s")
 
     # ... repeat for other phases ...
     
     print("\n==================================================")
+    print(f"Total time: {time.time() - start_time:.2f}s")
     print("✅ All model loading tasks completed!")
     print("==================================================")
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6f8739b and e916c7c.

📒 Files selected for processing (10)
  • config.yaml
  • examples/STT/whisper/STT_offline_whisper_rpi5.md
  • scripts/models/audio/fast_whisper_objects.py
  • scripts/models/audio/load_all.py
  • scripts/models/audio/load_huggingface_objects.py
  • scripts/models/audio/models_check.py
  • scripts/models/audio/piper_models.py
  • scripts/models/audio/vosk_models.py
  • scripts/models/audio/whisper_objects.py
  • src/audio/voice_agent_offline.py
🧰 Additional context used
🧬 Code graph analysis (2)
scripts/models/audio/whisper_objects.py (2)
src/utils/sysutils.py (1)
  • detect_raspberry_pi_model (38-50)
scripts/models/audio/models_check.py (1)
  • model_exists (7-21)
scripts/models/audio/piper_models.py (1)
src/utils/config.py (2)
  • load_config (132-176)
  • setup_python_path (184-195)
🔇 Additional comments (12)
scripts/models/audio/models_check.py (1)

1-8: LGTM!

The shebang and docstring additions improve script usability and documentation without changing behavior.

scripts/models/audio/piper_models.py (1)

6-9: LGTM!

The import refactor improves clarity by directly importing the needed functions rather than accessing them through a module object.

src/audio/voice_agent_offline.py (1)

539-542: LGTM!

Raising FileNotFoundError immediately is better than printing a warning and proceeding to fail later. The error message includes the download URL, which is helpful for troubleshooting.

scripts/models/audio/load_huggingface_objects.py (2)

34-42: Good defensive error handling.

The try/except blocks appropriately catch RepositoryNotFoundError and GatedRepoError, allowing the script to continue processing other models instead of failing completely.


51-59: Good defensive error handling.

The try/except blocks appropriately catch RepositoryNotFoundError and GatedRepoError, allowing the script to continue processing other datasets instead of failing completely.

scripts/models/audio/whisper_objects.py (4)

19-38: Platform-aware model selection is well-designed.

Separating MODELS_BASE (for Raspberry Pi) and MODELS_EXTENDED (for other platforms) ensures resource-constrained devices only download what they need.


47-52: LGTM!

The get_models_to_download() function provides clean platform-aware model selection based on Raspberry Pi detection.


55-119: Enhanced download_file() with robust error handling.

The improvements include:

  • Resume capability with Range headers
  • Symlink-aware directory creation
  • Pre-download existence checks via model_exists()
  • Comprehensive exception handling for HTTP, URL, and filesystem errors

121-146: Good fallback strategy for downloads.

The code tries whisper.load_model() first, then falls back to manual download_file() on RuntimeError. This provides resilience against whisper library download failures.

scripts/models/audio/fast_whisper_objects.py (1)

12-37: Consistent platform-aware model selection.

The MODELS_NAMES_BASE/MODELS_NAMES_EXTENDED split with get_models_to_download() follows the same pattern as whisper_objects.py, providing consistency across model loading scripts.

config.yaml (2)

23-23: None handling is already robustly implemented.

The full_model_path property in src/utils/config.py (lines 96–108) explicitly handles the None case with a fallback to models_path / engine / model_name. All code consuming the TTS model path uses this property rather than the raw model_path field, and path validation occurs at load time (line 538). No additional changes are needed.

Likely an incorrect or invalid review comment.


17-17: None handling is already robust via the property fallback.

The download_path property in src/utils/config.py (lines 69–83) has a fallback mechanism that returns self.models_path / engine_name when download_root is None. Since consuming code accesses config.stt.download_path (the property) rather than download_root directly, and the property always returns a valid Path object, no additional None handling is required.

Comment thread scripts/models/audio/load_all.py
Comment thread scripts/models/audio/vosk_models.py Outdated
chcavignx and others added 2 commits December 30, 2025 15:06
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 30, 2025

Note

Docstrings generation - SUCCESS
Generated docstrings for this pull request at #3

coderabbitai Bot added a commit that referenced this pull request Dec 30, 2025
Docstrings generation was requested by @chcavignx.

* #2 (comment)

The following files were modified:

* `scripts/models/audio/fast_whisper_objects.py`
* `scripts/models/audio/load_all.py`
* `scripts/models/audio/load_huggingface_objects.py`
* `scripts/models/audio/models_check.py`
* `scripts/models/audio/whisper_objects.py`
* `src/audio/voice_agent_offline.py`
@coderabbitai coderabbitai Bot mentioned this pull request Dec 30, 2025
@chcavignx chcavignx merged commit 8e38306 into main Dec 30, 2025
10 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant