Merge pull request #236 from ggozad/chore/cleanup

Improve test coverage and remove unnecessary defensive code

CHANGELOG.md

@@ -9,6 +9,10 @@
   - Default `None` preserves backwards compatibility (bare state emission)
 - **Page Image Generation Control**: New `generate_page_images` option in `ConversionOptions` to control PDF page image extraction
 
+### Changed
+
+- **CLI Error Handling**: Commands (`rebuild`, `vacuum`, `create-index`, `ask`, `research`) now propagate errors with proper exit codes instead of swallowing exceptions
+
 ### Fixed
 
 - **Embed-only rebuild with changed vector dimensions**: Fixed `haiku-rag rebuild --embed-only` failing when the configured embedding model has different dimensions than the database

haiku_rag_slim/haiku/rag/app.py

@@ -78,12 +78,7 @@ class HaikuRAGApp:
             return
 
         # Connect without going through Store to avoid upgrades/validation writes
-        try:
         db = lancedb.connect(self.db_path)
-            table_names = set(db.table_names())
-        except Exception as e:
-            self.console.print(f"[red]Failed to open database: {e}[/red]")
-            return
 
         versions = get_package_versions()
 
@@ -94,23 +89,16 @@ class HaikuRAGApp:
         table_stats = store.get_stats()
 
         # Read settings after Store init (migrations have run)
-        stored_version = "unknown"
-        embed_provider: str | None = None
-        embed_model: str | None = None
-        vector_dim: int | None = None
-
-        if "settings" in table_names:
         settings_tbl = db.open_table("settings")
         arrow = settings_tbl.search().where("id = 'settings'").limit(1).to_arrow()
-            rows = arrow.to_pylist() if arrow is not None else []
+        rows = arrow.to_pylist()
-            if rows:
         raw = rows[0].get("settings") or "{}"
         data = json.loads(raw) if isinstance(raw, str) else (raw or {})
-                stored_version = str(data.get("version", stored_version))
+        stored_version = str(data.get("version", "unknown"))
         embeddings = data.get("embeddings", {})
         embed_model_obj = embeddings.get("model", {})
-                embed_provider = embed_model_obj.get("provider")
+        embed_provider = embed_model_obj.get("provider", "unknown")
-                embed_model = embed_model_obj.get("name")
+        embed_model = embed_model_obj.get("name", "unknown")
         vector_dim = embed_model_obj.get("vector_dim")
 
         store.close()
@@ -126,31 +114,16 @@ class HaikuRAGApp:
         num_unindexed_rows = table_stats["chunks"].get("num_unindexed_rows", 0)
 
         # Table versions per table (direct API)
-        doc_versions = (
+        doc_versions = len(list(db.open_table("documents").list_versions()))
-            len(list(db.open_table("documents").list_versions()))
+        chunk_versions = len(list(db.open_table("chunks").list_versions()))
-            if "documents" in table_names
-            else 0
-        )
-        chunk_versions = (
-            len(list(db.open_table("chunks").list_versions()))
-            if "chunks" in table_names
-            else 0
-        )
 
         self.console.print(
             f"  [repr.attrib_name]haiku.rag version (db)[/repr.attrib_name]: {stored_version}"
         )
-        if embed_provider or embed_model or vector_dim:
-            provider_part = embed_provider or "unknown"
-            model_part = embed_model or "unknown"
         dim_part = f"{vector_dim}" if vector_dim is not None else "unknown"
         self.console.print(
             "  [repr.attrib_name]embeddings[/repr.attrib_name]: "
-                f"{provider_part}/{model_part} (dim: {dim_part})"
+            f"{embed_provider}/{embed_model} (dim: {dim_part})"
-            )
-        else:
-            self.console.print(
-                "  [repr.attrib_name]embeddings[/repr.attrib_name]: unknown"
         )
         self.console.print(
             f"  [repr.attrib_name]documents[/repr.attrib_name]: {num_docs} "
@@ -418,7 +391,6 @@ class HaikuRAGApp:
             read_only=self.read_only,
             before=self.before,
         ) as self.client:
-            try:
             citations = []
             if deep:
                 graph = build_research_graph(config=self.config)
@@ -460,8 +432,6 @@ class HaikuRAGApp:
                 if cite and citations:
                     for renderable in format_citations_rich(citations):
                         self.console.print(renderable)
-            except Exception as e:
-                self.console.print(f"[red]Error: {e}[/red]")
 
     async def research(self, question: str, filter: str | None = None):
         """Run research via the pydantic-graph pipeline.
@@ -476,7 +446,6 @@ class HaikuRAGApp:
             read_only=self.read_only,
             before=self.before,
         ) as client:
-            try:
             self.console.print("[bold cyan]Starting research[/bold cyan]")
             self.console.print(f"[bold blue]Question:[/bold blue] {question}")
             self.console.print()
@@ -545,9 +514,6 @@ class HaikuRAGApp:
                 self.console.print("[bold cyan]Sources:[/bold cyan]")
                 self.console.print(report.sources_summary)
 
-            except Exception as e:
-                self.console.print(f"[red]Error during research: {e}[/red]")
-
     async def rebuild(self, mode: RebuildMode = RebuildMode.FULL):
         async with HaikuRAG(
             db_path=self.db_path,
@@ -556,14 +522,11 @@ class HaikuRAGApp:
             read_only=self.read_only,
             before=self.before,
         ) as client:
-            try:
             documents = await client.list_documents()
             total_docs = len(documents)
 
             if total_docs == 0:
-                    self.console.print(
+                self.console.print("[yellow]No documents found in database.[/yellow]")
-                        "[yellow]No documents found in database.[/yellow]"
-                    )
                 return
 
             mode_desc = {
@@ -583,12 +546,9 @@ class HaikuRAGApp:
             self.console.print(
                 "[bold green]Database rebuild completed successfully.[/bold green]"
             )
-            except Exception as e:
-                self.console.print(f"[red]Error rebuilding database: {e}[/red]")
 
     async def vacuum(self):
         """Run database maintenance: optimize and cleanup table history."""
-        try:
         async with HaikuRAG(
             db_path=self.db_path,
             config=self.config,
@@ -597,15 +557,10 @@ class HaikuRAGApp:
             before=self.before,
         ) as client:
             await client.vacuum()
-            self.console.print(
+        self.console.print("[bold green]Vacuum completed successfully.[/bold green]")
-                "[bold green]Vacuum completed successfully.[/bold green]"
-            )
-        except Exception as e:
-            self.console.print(f"[red]Error during vacuum: {e}[/red]")
 
     async def create_index(self):
         """Create vector index on the chunks table."""
-        try:
         async with HaikuRAG(
             db_path=self.db_path,
             config=self.config,
@@ -637,8 +592,6 @@ class HaikuRAGApp:
             self.console.print(
                 "[bold green]Vector index created successfully.[/bold green]"
             )
-        except Exception as e:
-            self.console.print(f"[red]Error creating index: {e}[/red]")
 
     async def download_models(self):
         """Download Docling, HuggingFace tokenizer, and Ollama models per config."""

haiku_rag_slim/haiku/rag/chat/__init__.py

@@ -16,7 +16,7 @@ def run_chat(
     """
     try:
         from haiku.rag.chat.app import ChatApp
-    except ImportError as e:
+    except ImportError as e:  # pragma: no cover
         raise ImportError(
             "textual is not installed. Please install it with `pip install 'haiku.rag-slim[tui]'` or use the full haiku.rag package."
         ) from e

haiku_rag_slim/haiku/rag/embeddings/__init__.py

@@ -128,7 +128,7 @@ def get_embedder(config: AppConfig = Config) -> EmbedderWrapper:
     if provider == "voyageai":
         try:
             from haiku.rag.embeddings.voyageai import VoyageAIEmbeddingModel
-        except ImportError:
+        except ImportError:  # pragma: no cover
             raise ImportError(
                 "VoyageAI embedder requires the 'voyageai' package. "
                 "Please install haiku.rag with the 'voyageai' extra: "

haiku_rag_slim/haiku/rag/inspector/__init__.py

@@ -1,6 +1,6 @@
 try:
     from haiku.rag.inspector.app import run_inspector
-except ImportError as e:
+except ImportError as e:  # pragma: no cover
     raise ImportError(
         "textual is not installed. Please install it with `pip install 'haiku.rag-slim[tui]'` or use the full haiku.rag package."
     ) from e

haiku_rag_slim/haiku/rag/reranking/__init__.py

@@ -30,7 +30,7 @@ def get_reranker(config: AppConfig = Config) -> RerankerBase | None:
 
             os.environ["TOKENIZERS_PARALLELISM"] = "true"
             reranker = MxBAIReranker()
-        except ImportError:
+        except ImportError:  # pragma: no cover
             reranker = None
 
     elif config.reranking.model and config.reranking.model.provider == "cohere":
@@ -38,7 +38,7 @@ def get_reranker(config: AppConfig = Config) -> RerankerBase | None:
             from haiku.rag.reranking.cohere import CohereReranker
 
             reranker = CohereReranker()
-        except ImportError:
+        except ImportError:  # pragma: no cover
             reranker = None
 
     elif config.reranking.model and config.reranking.model.provider == "vllm":
@@ -49,7 +49,7 @@ def get_reranker(config: AppConfig = Config) -> RerankerBase | None:
             if not base_url:
                 raise ValueError("vLLM reranker requires base_url in reranking.model")
             reranker = VLLMReranker(config.reranking.model.name, base_url)
-        except ImportError:
+        except ImportError:  # pragma: no cover
             reranker = None
 
     elif config.reranking.model and config.reranking.model.provider == "zeroentropy":
@@ -59,7 +59,7 @@ def get_reranker(config: AppConfig = Config) -> RerankerBase | None:
             # Use configured model or default to zerank-1
             model = config.reranking.model.name or "zerank-1"
             reranker = ZeroEntropyReranker(model)
-        except ImportError:
+        except ImportError:  # pragma: no cover
             reranker = None
 
     _reranker_cache[config_id] = reranker

haiku_rag_slim/haiku/rag/reranking/cohere.py

@@ -3,7 +3,7 @@ from haiku.rag.store.models.chunk import Chunk
 
 try:
     import cohere
-except ImportError as e:
+except ImportError as e:  # pragma: no cover
     raise ImportError(
         "cohere is not installed. Please install it with `pip install cohere` or use the cohere optional dependency."
     ) from e

haiku_rag_slim/haiku/rag/store/repositories/settings.py

@@ -85,12 +85,7 @@ class SettingsRepository:
 
         if existing:
             # Preserve existing version if present to avoid interfering with upgrade flow
-            try:
+            existing_settings = json.loads(existing[0].settings)
-                existing_settings = (
-                    json.loads(existing[0].settings) if existing[0].settings else {}
-                )
-            except Exception:
-                existing_settings = {}
             if "version" in existing_settings:
                 current_config["version"] = existing_settings["version"]
 

tests/test_app.py

@@ -506,3 +506,95 @@ async def test_history_nonexistent_db(tmp_path, monkeypatch):
 
     calls = [str(c) for c in mock_print.call_args_list]
     assert any("does not exist" in c for c in calls)
+
+
+@pytest.mark.asyncio
+async def test_init_creates_database(tmp_path, monkeypatch):
+    """Test init creates a new database."""
+    db_path = tmp_path / "new.lancedb"
+    app = HaikuRAGApp(db_path=db_path)
+    mock_print = MagicMock()
+    monkeypatch.setattr(app.console, "print", mock_print)
+
+    assert not db_path.exists()
+    await app.init()
+
+    assert db_path.exists()
+    calls = [str(c) for c in mock_print.call_args_list]
+    assert any("initialized" in c for c in calls)
+
+
+@pytest.mark.asyncio
+async def test_init_existing_database(tmp_path, monkeypatch):
+    """Test init with existing database shows warning."""
+    from haiku.rag.store.engine import Store
+
+    db_path = tmp_path / "existing.lancedb"
+    store = Store(db_path, create=True)
+    store.close()
+
+    app = HaikuRAGApp(db_path=db_path)
+    mock_print = MagicMock()
+    monkeypatch.setattr(app.console, "print", mock_print)
+
+    await app.init()
+
+    calls = [str(c) for c in mock_print.call_args_list]
+    assert any("already exists" in c for c in calls)
+
+
+@pytest.mark.asyncio
+async def test_vacuum(tmp_path, monkeypatch):
+    """Test vacuum operation."""
+    from haiku.rag.store.engine import Store
+
+    db_path = tmp_path / "test.lancedb"
+    store = Store(db_path, create=True)
+    store.close()
+
+    app = HaikuRAGApp(db_path=db_path)
+    mock_print = MagicMock()
+    monkeypatch.setattr(app.console, "print", mock_print)
+
+    await app.vacuum()
+
+    calls = [str(c) for c in mock_print.call_args_list]
+    assert any("Vacuum completed" in c for c in calls)
+
+
+@pytest.mark.asyncio
+async def test_create_index_insufficient_chunks(tmp_path, monkeypatch):
+    """Test create_index with insufficient chunks shows warning."""
+    from haiku.rag.store.engine import Store
+
+    db_path = tmp_path / "test.lancedb"
+    store = Store(db_path, create=True)
+    store.close()
+
+    app = HaikuRAGApp(db_path=db_path)
+    mock_print = MagicMock()
+    monkeypatch.setattr(app.console, "print", mock_print)
+
+    await app.create_index()
+
+    calls = [str(c) for c in mock_print.call_args_list]
+    assert any("Need at least 256 chunks" in c for c in calls)
+
+
+@pytest.mark.asyncio
+async def test_rebuild_empty_database(tmp_path, monkeypatch):
+    """Test rebuild with empty database shows warning."""
+    from haiku.rag.store.engine import Store
+
+    db_path = tmp_path / "test.lancedb"
+    store = Store(db_path, create=True)
+    store.close()
+
+    app = HaikuRAGApp(db_path=db_path)
+    mock_print = MagicMock()
+    monkeypatch.setattr(app.console, "print", mock_print)
+
+    await app.rebuild()
+
+    calls = [str(c) for c in mock_print.call_args_list]
+    assert any("No documents found" in c for c in calls)

tests/test_settings.py

@@ -1,4 +1,7 @@
-from haiku.rag.config import Config
+import pytest
+
+from haiku.rag.config import AppConfig, Config
+from haiku.rag.store.repositories.settings import ConfigMismatchError
 
 
 def test_settings_table_populated_on_store_init(temp_db_path):
@@ -48,3 +51,108 @@ def test_monitor_filter_patterns_config():
     assert isinstance(Config.monitor.ignore_patterns, list)
     assert isinstance(Config.monitor.include_patterns, list)
     assert isinstance(Config.monitor.directories, list)
+
+
+class TestValidateConfigCompatibility:
+    """Tests for validate_config_compatibility method."""
+
+    def test_empty_settings_saves_config(self, temp_db_path):
+        """When settings row is missing, validation saves current config."""
+        from haiku.rag.store.engine import Store
+        from haiku.rag.store.repositories.settings import SettingsRepository
+
+        store = Store(temp_db_path, create=True, skip_validation=True)
+        settings_repo = SettingsRepository(store)
+
+        # Clear settings to simulate empty state
+        store.settings_table.delete("id = 'settings'")
+        assert settings_repo.get_current_settings() == {}
+
+        # Validation should save settings
+        settings_repo.validate_config_compatibility()
+
+        # Now settings should exist
+        saved = settings_repo.get_current_settings()
+        assert saved.get("embeddings", {}).get("model", {}).get("provider") is not None
+        store.close()
+
+    def test_compatible_config_no_error(self, temp_db_path):
+        """Compatible config does not raise error."""
+        from haiku.rag.store.engine import Store
+        from haiku.rag.store.repositories.settings import SettingsRepository
+
+        store = Store(temp_db_path, create=True)
+        settings_repo = SettingsRepository(store)
+
+        # Should not raise - same config
+        settings_repo.validate_config_compatibility()
+        store.close()
+
+    def test_provider_mismatch_raises_error(self, temp_db_path):
+        """Different embedding provider raises ConfigMismatchError."""
+        from haiku.rag.store.engine import Store
+        from haiku.rag.store.repositories.settings import SettingsRepository
+
+        # Create store with default config (ollama)
+        store = Store(temp_db_path, create=True)
+        store.close()
+
+        # Create new config with different provider
+        new_config = AppConfig()
+        new_config.embeddings.model.provider = "openai"
+
+        store2 = Store(temp_db_path, config=new_config, skip_validation=True)
+        settings_repo = SettingsRepository(store2)
+
+        with pytest.raises(ConfigMismatchError) as exc_info:
+            settings_repo.validate_config_compatibility()
+
+        assert "embedding provider" in str(exc_info.value)
+        assert "ollama" in str(exc_info.value)
+        assert "openai" in str(exc_info.value)
+        store2.close()
+
+    def test_model_mismatch_raises_error(self, temp_db_path):
+        """Different embedding model raises ConfigMismatchError."""
+        from haiku.rag.store.engine import Store
+        from haiku.rag.store.repositories.settings import SettingsRepository
+
+        # Create store with default config
+        store = Store(temp_db_path, create=True)
+        store.close()
+
+        # Create new config with different model
+        new_config = AppConfig()
+        new_config.embeddings.model.name = "different-model"
+
+        store2 = Store(temp_db_path, config=new_config, skip_validation=True)
+        settings_repo = SettingsRepository(store2)
+
+        with pytest.raises(ConfigMismatchError) as exc_info:
+            settings_repo.validate_config_compatibility()
+
+        assert "embedding model" in str(exc_info.value)
+        store2.close()
+
+    def test_vector_dim_mismatch_raises_error(self, temp_db_path):
+        """Different vector dimension raises ConfigMismatchError."""
+        from haiku.rag.store.engine import Store
+        from haiku.rag.store.repositories.settings import SettingsRepository
+
+        # Create store with default config
+        store = Store(temp_db_path, create=True)
+        store.close()
+
+        # Create new config with different vector dimension
+        new_config = AppConfig()
+        new_config.embeddings.model.vector_dim = 9999
+
+        store2 = Store(temp_db_path, config=new_config, skip_validation=True)
+        settings_repo = SettingsRepository(store2)
+
+        with pytest.raises(ConfigMismatchError) as exc_info:
+            settings_repo.validate_config_compatibility()
+
+        assert "vector dimension" in str(exc_info.value)
+        assert "9999" in str(exc_info.value)
+        store2.close()