From 213569601ba3ea87b748a7ef96b13574ed2c4a16 Mon Sep 17 00:00:00 2001 From: Yiorgis Gozadinos Date: Fri, 5 Jun 2026 10:35:50 +0300 Subject: [PATCH] Stop rewriting stored embedding settings on database open --- .../haiku/rag/store/repositories/settings.py | 33 +++++--- tests/test_settings.py | 81 ++++++++++++------- 2 files changed, 74 insertions(+), 40 deletions(-) diff --git a/haiku_rag_slim/haiku/rag/store/repositories/settings.py b/haiku_rag_slim/haiku/rag/store/repositories/settings.py index ee4020d2..5d7fd2e1 100644 --- a/haiku_rag_slim/haiku/rag/store/repositories/settings.py +++ b/haiku_rag_slim/haiku/rag/store/repositories/settings.py @@ -102,20 +102,21 @@ class SettingsRepository: await self.store.settings_table.add([settings_record]) async def validate_config_compatibility(self) -> None: - """Validate that the current configuration is compatible with stored settings. + """Validate the current configuration against stored settings without writing. - ``vector_dim`` mismatches raise — corpus and query vectors must live in - the same dimensional space. ``provider`` and ``name`` mismatches are - treated as soft drift: legitimate when the same model is served by a - different stack (Ollama vs vLLM-via-openai, etc.). Surface the change - once via a warning and overwrite stored settings so the warning does - not fire on every subsequent open. + Opening a database never modifies it. ``vector_dim`` mismatches raise — + corpus and query vectors must live in the same dimensional space. + ``provider`` and ``name`` drift (with matching ``vector_dim``) is soft: + legitimate when the same model is served by a different stack (Ollama vs + vLLM-via-openai, etc.). Drift is surfaced via a warning; a writable open + then raises so a write cannot mix embedding identities in the corpus, + while a read-only open continues. Stored settings are reconciled + explicitly via ``haiku-rag rebuild --set-embedder``, never on open. """ stored_settings = await self.get_current_settings() - # If no stored settings, this is a new database - save current config and return + # Nothing stored to validate against — never write on open. if not stored_settings: - await self.save_current_settings() return current_config = self.store._config.model_dump(mode="json") @@ -155,9 +156,15 @@ class SettingsRepository: if soft_changes: logger.warning( - "Embedding identity changed (vector_dim matches, stored settings " - "will be updated to match current config): %s. If this is not " - "intentional, revert your config to match the stored settings.", + "Embedding identity changed (vector_dim matches): %s. If this is " + "intentional, run 'haiku-rag rebuild --set-embedder' to update the " + "stored settings; otherwise revert your config to match the database.", "; ".join(soft_changes), ) - await self.save_current_settings() + if not self.store.is_read_only: + raise ConfigMismatchError( + "Database embedding identity differs from current config " + f"(vector_dim matches): {'; '.join(soft_changes)}. " + "Run 'haiku-rag rebuild --set-embedder' to adopt the current " + "embedder, or revert your config to match the database." + ) diff --git a/tests/test_settings.py b/tests/test_settings.py index 860854b3..050356a9 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -48,8 +48,8 @@ class TestValidateConfigCompatibility: """Tests for validate_config_compatibility method.""" @pytest.mark.asyncio - async def test_empty_settings_saves_config(self, temp_db_path): - """When settings row is missing, validation saves current config.""" + async def test_empty_settings_does_not_write(self, temp_db_path): + """Validation never writes on open, even when the settings row is missing.""" from haiku.rag.store.engine import Store from haiku.rag.store.repositories.settings import SettingsRepository @@ -60,14 +60,10 @@ class TestValidateConfigCompatibility: await store.settings_table.delete("id = 'settings'") assert await settings_repo.get_current_settings() == {} - # Validation should save settings + # Validation must not write — nothing to validate against await settings_repo.validate_config_compatibility() - # Now settings should exist - saved = await settings_repo.get_current_settings() - assert ( - saved.get("embeddings", {}).get("model", {}).get("provider") is not None - ) + assert await settings_repo.get_current_settings() == {} @pytest.mark.asyncio async def test_compatible_config_no_error(self, temp_db_path): @@ -82,15 +78,14 @@ class TestValidateConfigCompatibility: await settings_repo.validate_config_compatibility() @pytest.mark.asyncio - async def test_provider_mismatch_warns_and_syncs( + async def test_provider_drift_read_only_warns_without_writing( self, temp_db_path, caplog, monkeypatch ): - """Provider mismatch with matching vector_dim warns and overwrites stored. + """Provider drift (vector_dim matches) on a read-only store warns and continues. Same model served by a different stack (Ollama vs vLLM via openai-compat) - legitimately differs in `provider`. Validation should surface the change - once, then trust the user's config as the new source of truth so the - warning doesn't fire on every subsequent open. + legitimately differs in `provider`. A read-only open surfaces the change + but must never modify the stored settings. """ from haiku.rag.store.engine import Store from haiku.rag.store.repositories.settings import SettingsRepository @@ -107,7 +102,7 @@ class TestValidateConfigCompatibility: new_config.embeddings.model.provider = "openai" async with Store( - temp_db_path, config=new_config, skip_validation=True + temp_db_path, config=new_config, skip_validation=True, read_only=True ) as store2: settings_repo = SettingsRepository(store2) @@ -122,25 +117,57 @@ class TestValidateConfigCompatibility: for r in caplog.records ) - # Stored settings now match current + # Stored settings are untouched saved = await settings_repo.get_current_settings() - assert saved["embeddings"]["model"]["provider"] == "openai" - - # Second open is silent — stored matches current. - caplog.clear() - with caplog.at_level(logging.WARNING): - await settings_repo.validate_config_compatibility() - assert not any("provider" in r.getMessage() for r in caplog.records) + assert saved["embeddings"]["model"]["provider"] == "ollama" @pytest.mark.asyncio - async def test_model_mismatch_warns_and_syncs( + async def test_provider_drift_writable_raises_without_writing( self, temp_db_path, caplog, monkeypatch ): - """Model name mismatch with matching vector_dim warns and overwrites stored.""" + """Provider drift on a writable store warns and raises, without writing.""" + from haiku.rag.store.engine import Store + from haiku.rag.store.repositories.settings import ( + ConfigMismatchError, + SettingsRepository, + ) + + monkeypatch.setattr(logging.getLogger("haiku.rag"), "propagate", True) + + async with Store(temp_db_path, create=True): + pass + + new_config = AppConfig() + new_config.embeddings.model.provider = "openai" + + async with Store( + temp_db_path, config=new_config, skip_validation=True + ) as store2: + settings_repo = SettingsRepository(store2) + + with caplog.at_level(logging.WARNING): + with pytest.raises(ConfigMismatchError): + await settings_repo.validate_config_compatibility() + + assert any( + "provider" in r.getMessage() + and "ollama" in r.getMessage() + and "openai" in r.getMessage() + for r in caplog.records + ) + + # Stored settings are untouched despite the writable open + saved = await settings_repo.get_current_settings() + assert saved["embeddings"]["model"]["provider"] == "ollama" + + @pytest.mark.asyncio + async def test_model_drift_read_only_warns_without_writing( + self, temp_db_path, caplog, monkeypatch + ): + """Model name drift (vector_dim matches) on a read-only store warns, no write.""" from haiku.rag.store.engine import Store from haiku.rag.store.repositories.settings import SettingsRepository - # See test_provider_mismatch_warns_and_syncs for the propagate=True rationale. monkeypatch.setattr(logging.getLogger("haiku.rag"), "propagate", True) async with Store(temp_db_path, create=True): @@ -150,7 +177,7 @@ class TestValidateConfigCompatibility: new_config.embeddings.model.name = "different-model" async with Store( - temp_db_path, config=new_config, skip_validation=True + temp_db_path, config=new_config, skip_validation=True, read_only=True ) as store2: settings_repo = SettingsRepository(store2) @@ -163,7 +190,7 @@ class TestValidateConfigCompatibility: ) saved = await settings_repo.get_current_settings() - assert saved["embeddings"]["model"]["name"] == "different-model" + assert saved["embeddings"]["model"]["name"] != "different-model" @pytest.mark.asyncio async def test_vector_dim_mismatch_raises_error(self, temp_db_path):