fix: restore missing storage modules and unblock install/CI for JOSS review

InfantLab · claude · InfantLab · commit 12f3cc9f606b · 2026-05-27T16:17:40.000Z
The published storage package was missing manager.py and the providers/ subpackage: an unanchored `storage/` rule in .gitignore silently excluded them, so fresh clones failed with `No module named videoannotator.storage.manager`. Anchor the rule to `/storage/` and commit the four source files. Also addresses the other reviewer blockers in openjournals/joss-reviews#10182: - bump openai-whisper ==20240930 -> >=20250625 (sdist-only; build deps kept) - repoint reviewer docs from the removed scripts/verify_installation.py to `videoannotator diagnose` - clear pre-existing lint/type failures so CI can go green: ruff fixes, add types-PyYAML/types-requests, and 5 type-only mypy fixes Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
diff --git a/.gitignore b/.gitignore
@@ -165,7 +165,7 @@ test_output/
 data/*
 models/
 outputs/
-storage/
+/storage/
 
 # Jupyter Notebook checkpoints
 *.ipynb_checkpoints
diff --git a/docs/GETTING_STARTED_REVIEWERS.md b/docs/GETTING_STARTED_REVIEWERS.md
@@ -41,7 +41,7 @@ curl -LsSf https://astral.sh/uv/install.sh | sh
 uv sync
 
 # 4. Verify installation
-uv run python scripts/verify_installation.py
+uv run videoannotator diagnose
 ```
 
 **Expected output**: All checks pass ✅ (GPU optional)
diff --git a/docs/installation/troubleshooting.md b/docs/installation/troubleshooting.md
@@ -237,13 +237,13 @@ chmod -R u+rw logs/
 ### Installation Verification Fails
 
 **Symptoms**:
-- `scripts/verify_installation.py` reports failures
+- `videoannotator diagnose` reports failures
 
 **Solution**:
 
 Run with verbose output to see specific issues:
 ```bash
-uv run python scripts/verify_installation.py --verbose
+uv run videoannotator diagnose
 ```
 
 Common fixes:
@@ -652,7 +652,7 @@ vm_stat  # macOS
 uv run python -c "import videoannotator; print(videoannotator.__version__)"
 
 # Installation verification
-uv run python scripts/verify_installation.py --verbose
+uv run videoannotator diagnose
 
 # Check imports
 uv run python -c "
@@ -781,7 +781,7 @@ If you can't resolve the issue:
 2. **Gather diagnostic information**:
    ```bash
    # Run full diagnostic
-   uv run python scripts/verify_installation.py --verbose > diagnostic.txt 2>&1
+   uv run videoannotator diagnose > diagnostic.txt 2>&1
 
    # Include system info
    uname -a >> diagnostic.txt
diff --git a/pyproject.toml b/pyproject.toml
@@ -31,8 +31,8 @@ dependencies = [
     "librosa>=0.10.0",
     "matplotlib>=3.9.2",
     "moviepy>=1.0.3",
-    # Pin to a stable earlier version to avoid installation metadata issues in CI
-    "openai-whisper==20240930",
+    # sdist-only release; built from source at install (see tool.uv.extra-build-dependencies)
+    "openai-whisper>=20250625",
     "numba>=0.60.0", # Ensure Python 3.12 compatibility
     "openpyxl",
     "pandas>=2.2.2",
@@ -149,7 +149,7 @@ videoannotator = "videoannotator.cli:app"
 # uv-native config - empty is fine for now
 
 [tool.uv.extra-build-dependencies]
-openai-whisper = ["setuptools==69.0.3", "wheel"]
+openai-whisper = ["setuptools", "wheel"]
 
 [[tool.uv.index]]
 name = "pytorch-cu124"
@@ -311,6 +311,8 @@ dev = [
     "pytest-asyncio>=1.1.0",
     "pytest-cov>=4.0.0",
     "ruff>=0.14.0",
+    "types-PyYAML>=6.0.0",
+    "types-requests>=2.31.0",
 ]
 
 [[tool.mypy.overrides]]
diff --git a/src/videoannotator/pipelines/audio_processing/laion_voice_pipeline.py b/src/videoannotator/pipelines/audio_processing/laion_voice_pipeline.py
@@ -995,7 +995,7 @@ def _predict_emotions(self, embedding: torch.Tensor) -> dict[str, Any]:
         # Apply softmax across all emotions to get proper probability distribution
         if raw_scores:
             scores_array = np.array(list(raw_scores.values()))
-            max_score = np.max(scores_array)
+            max_score = float(np.max(scores_array))
             exp_scores = np.exp(scores_array - max_score)
             softmax_scores = exp_scores / np.sum(exp_scores)
 
diff --git a/src/videoannotator/pipelines/face_analysis/face_pipeline.py b/src/videoannotator/pipelines/face_analysis/face_pipeline.py
@@ -463,8 +463,9 @@ def _detect_faces_opencv(
     ) -> list[dict[str, Any]]:
         """Detect faces using OpenCV Haar cascades."""
         # Load cascade classifier
+        haarcascades_dir = cv2.data.haarcascades  # type: ignore[attr-defined]
         face_cascade = cv2.CascadeClassifier(
-            cv2.data.haarcascades + "haarcascade_frontalface_default.xml"
+            haarcascades_dir + "haarcascade_frontalface_default.xml"
         )
 
         gray = cv2.cvtColor(frame, cv2.COLOR_BGR2GRAY)
diff --git a/src/videoannotator/pipelines/scene_detection/scene_pipeline.py b/src/videoannotator/pipelines/scene_detection/scene_pipeline.py
@@ -231,6 +231,7 @@ def _classify_scenes(
 
             # Prepare text prompts
             text_prompts = [f"a {prompt}" for prompt in self.config["scene_prompts"]]
+            assert self.clip_tokenizer is not None
             text = self.clip_tokenizer(text_prompts).to(self.device)
 
             classified_segments = []
diff --git a/src/videoannotator/storage/cleanup.py b/src/videoannotator/storage/cleanup.py
@@ -8,10 +8,9 @@
 
 import shutil
 from datetime import datetime, timedelta
-from pathlib import Path
 from typing import Any
 
-from videoannotator.config_env import STORAGE_RETENTION_DAYS, STORAGE_BASE_DIR
+from videoannotator.config_env import STORAGE_BASE_DIR, STORAGE_RETENTION_DAYS
 from videoannotator.database.models import Job, JobStatus
 from videoannotator.storage.file_backend import FileStorageBackend
 from videoannotator.utils.logging_config import get_logger
@@ -107,9 +106,7 @@ def find_old_jobs(retention_days: int | None = None) -> list[Job]:
     days = retention_days if retention_days is not None else STORAGE_RETENTION_DAYS
 
     if days is None or days <= 0:
-        raise ValueError(
-            "Cleanup is disabled (STORAGE_RETENTION_DAYS not set or <= 0)"
-        )
+        raise ValueError("Cleanup is disabled (STORAGE_RETENTION_DAYS not set or <= 0)")
 
     cutoff_date = datetime.now() - timedelta(days=days)
 
@@ -150,7 +147,11 @@ def verify_job_safe_to_delete(job: Job) -> tuple[bool, str]:
         return False, "Job has no completion timestamp"
 
     # Check 3: Completion must be in the past (naive comparison for test compatibility)
-    completed_at = job.completed_at.replace(tzinfo=None) if hasattr(job.completed_at, 'replace') else job.completed_at
+    completed_at = (
+        job.completed_at.replace(tzinfo=None)
+        if hasattr(job.completed_at, "replace")
+        else job.completed_at
+    )
     if completed_at > datetime.now():
         return False, "Job completion timestamp is in the future"
 
diff --git a/src/videoannotator/storage/config.py b/src/videoannotator/storage/config.py
@@ -11,7 +11,7 @@
 import os
 from pathlib import Path
 
-import yaml  # type: ignore
+import yaml
 
 
 def get_storage_root() -> Path:
diff --git a/src/videoannotator/storage/manager.py b/src/videoannotator/storage/manager.py
@@ -0,0 +1,42 @@
+"""Storage manager factory.
+
+This module provides a factory for obtaining the configured storage provider.
+"""
+
+from functools import lru_cache
+
+from videoannotator.storage.config import get_storage_root
+from videoannotator.storage.providers.base import StorageProvider
+from videoannotator.storage.providers.local import LocalStorageProvider
+from videoannotator.utils.logging_config import get_logger
+
+logger = get_logger("storage.manager")
+
+
+@lru_cache
+def get_storage_provider() -> StorageProvider:
+    """Get the configured storage provider instance.
+
+    Returns:
+        StorageProvider: The singleton storage provider instance.
+    """
+    # In the future, we will read the provider type from config.
+    # For now, we default to LocalStorageProvider.
+
+    root_path = get_storage_root()
+    logger.info(f"Initializing storage provider with root: {root_path}")
+
+    provider = LocalStorageProvider(root_path=root_path)
+    provider.initialize()
+
+    # Validate write permissions
+    try:
+        test_file = root_path / ".write_test"
+        test_file.touch()
+        test_file.unlink()
+    except Exception as e:
+        logger.warning(f"Storage root {root_path} is not writable: {e}")
+        # We don't raise here to allow read-only scenarios if intended,
+        # but for a job processor this is likely fatal.
+
+    return provider
diff --git a/src/videoannotator/storage/providers/__init__.py b/src/videoannotator/storage/providers/__init__.py
@@ -0,0 +1,5 @@
+"""Storage providers for VideoAnnotator.
+
+This package contains implementations of the StorageProvider interface
+for different backends (Local, S3, etc.).
+"""
diff --git a/src/videoannotator/storage/providers/base.py b/src/videoannotator/storage/providers/base.py
@@ -0,0 +1,119 @@
+"""Base storage provider interface.
+
+This module defines the abstract base class for storage providers and
+related data structures.
+"""
+
+from abc import ABC, abstractmethod
+from collections.abc import Iterator
+from dataclasses import dataclass
+from enum import Enum
+from pathlib import Path
+from typing import BinaryIO
+
+
+class ArtifactType(str, Enum):
+    """Types of artifacts generated by pipelines."""
+
+    VIDEO = "video"
+    ANNOTATION = "annotation"
+    REPORT = "report"
+    LOG = "log"
+    OTHER = "other"
+
+
+@dataclass
+class JobArtifact:
+    """Represents a file stored in the job storage."""
+
+    job_id: str
+    path: str  # Relative path within the job directory
+    name: str
+    size_bytes: int
+    artifact_type: ArtifactType
+
+
+class StorageProvider(ABC):
+    """Abstract base class for storage providers."""
+
+    @abstractmethod
+    def initialize(self) -> None:
+        """Initialize the storage provider (e.g., create root dir)."""
+        pass
+
+    @abstractmethod
+    def create_job_dir(self, job_id: str) -> None:
+        """Create a directory for a specific job.
+
+        Args:
+            job_id: The unique identifier of the job.
+        """
+        pass
+
+    @abstractmethod
+    def save_file(self, job_id: str, relative_path: str, content: BinaryIO) -> str:
+        """Save a file to the job storage.
+
+        Args:
+            job_id: The unique identifier of the job.
+            relative_path: Path relative to the job directory.
+            content: File content as a binary stream.
+
+        Returns:
+            str: The absolute path or URL of the saved file.
+        """
+        pass
+
+    @abstractmethod
+    def get_file(self, job_id: str, relative_path: str) -> BinaryIO:
+        """Open a file for reading.
+
+        Args:
+            job_id: The unique identifier of the job.
+            relative_path: Path relative to the job directory.
+
+        Returns:
+            BinaryIO: File content as a binary stream.
+        """
+        pass
+
+    @abstractmethod
+    def list_files(self, job_id: str) -> Iterator[JobArtifact]:
+        """List all files for a specific job.
+
+        Args:
+            job_id: The unique identifier of the job.
+
+        Returns:
+            Iterator[JobArtifact]: Iterator of artifact objects.
+        """
+        pass
+
+    @abstractmethod
+    def exists(self, job_id: str, relative_path: str) -> bool:
+        """Check if a file exists.
+
+        Args:
+            job_id: The unique identifier of the job.
+            relative_path: Path relative to the job directory.
+
+        Returns:
+            bool: True if the file exists, False otherwise.
+        """
+        pass
+
+    @abstractmethod
+    def get_absolute_path(self, job_id: str, relative_path: str) -> Path:
+        """Get the absolute local path of a file (if applicable).
+
+        Note: This may raise NotImplementedError for cloud providers that
+        don't have a local representation.
+
+        Args:
+            job_id: The unique identifier of the job.
+            relative_path: Path relative to the job directory.
+
+        Returns:
+            Path: Absolute local path.
+        """
+        pass
diff --git a/src/videoannotator/storage/providers/local.py b/src/videoannotator/storage/providers/local.py
diff --git a/src/videoannotator/utils/size_based_person_analysis.py b/src/videoannotator/utils/size_based_person_analysis.py
diff --git a/tests/unit/storage/test_cleanup.py b/tests/unit/storage/test_cleanup.py
diff --git a/uv.lock b/uv.lock
