Stop rewriting stored embedding settings on database open
This commit is contained in:
parent
b6bbea3d64
commit
213569601b
2 changed files with 74 additions and 40 deletions
|
|
@ -102,20 +102,21 @@ class SettingsRepository:
|
||||||
await self.store.settings_table.add([settings_record])
|
await self.store.settings_table.add([settings_record])
|
||||||
|
|
||||||
async def validate_config_compatibility(self) -> None:
|
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
|
Opening a database never modifies it. ``vector_dim`` mismatches raise —
|
||||||
the same dimensional space. ``provider`` and ``name`` mismatches are
|
corpus and query vectors must live in the same dimensional space.
|
||||||
treated as soft drift: legitimate when the same model is served by a
|
``provider`` and ``name`` drift (with matching ``vector_dim``) is soft:
|
||||||
different stack (Ollama vs vLLM-via-openai, etc.). Surface the change
|
legitimate when the same model is served by a different stack (Ollama vs
|
||||||
once via a warning and overwrite stored settings so the warning does
|
vLLM-via-openai, etc.). Drift is surfaced via a warning; a writable open
|
||||||
not fire on every subsequent 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()
|
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:
|
if not stored_settings:
|
||||||
await self.save_current_settings()
|
|
||||||
return
|
return
|
||||||
|
|
||||||
current_config = self.store._config.model_dump(mode="json")
|
current_config = self.store._config.model_dump(mode="json")
|
||||||
|
|
@ -155,9 +156,15 @@ class SettingsRepository:
|
||||||
|
|
||||||
if soft_changes:
|
if soft_changes:
|
||||||
logger.warning(
|
logger.warning(
|
||||||
"Embedding identity changed (vector_dim matches, stored settings "
|
"Embedding identity changed (vector_dim matches): %s. If this is "
|
||||||
"will be updated to match current config): %s. If this is not "
|
"intentional, run 'haiku-rag rebuild --set-embedder' to update the "
|
||||||
"intentional, revert your config to match the stored settings.",
|
"stored settings; otherwise revert your config to match the database.",
|
||||||
"; ".join(soft_changes),
|
"; ".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."
|
||||||
|
)
|
||||||
|
|
|
||||||
|
|
@ -48,8 +48,8 @@ class TestValidateConfigCompatibility:
|
||||||
"""Tests for validate_config_compatibility method."""
|
"""Tests for validate_config_compatibility method."""
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_empty_settings_saves_config(self, temp_db_path):
|
async def test_empty_settings_does_not_write(self, temp_db_path):
|
||||||
"""When settings row is missing, validation saves current config."""
|
"""Validation never writes on open, even when the settings row is missing."""
|
||||||
from haiku.rag.store.engine import Store
|
from haiku.rag.store.engine import Store
|
||||||
from haiku.rag.store.repositories.settings import SettingsRepository
|
from haiku.rag.store.repositories.settings import SettingsRepository
|
||||||
|
|
||||||
|
|
@ -60,14 +60,10 @@ class TestValidateConfigCompatibility:
|
||||||
await store.settings_table.delete("id = 'settings'")
|
await store.settings_table.delete("id = 'settings'")
|
||||||
assert await settings_repo.get_current_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()
|
await settings_repo.validate_config_compatibility()
|
||||||
|
|
||||||
# Now settings should exist
|
assert await settings_repo.get_current_settings() == {}
|
||||||
saved = await settings_repo.get_current_settings()
|
|
||||||
assert (
|
|
||||||
saved.get("embeddings", {}).get("model", {}).get("provider") is not None
|
|
||||||
)
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_compatible_config_no_error(self, temp_db_path):
|
async def test_compatible_config_no_error(self, temp_db_path):
|
||||||
|
|
@ -82,15 +78,14 @@ class TestValidateConfigCompatibility:
|
||||||
await settings_repo.validate_config_compatibility()
|
await settings_repo.validate_config_compatibility()
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@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
|
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)
|
Same model served by a different stack (Ollama vs vLLM via openai-compat)
|
||||||
legitimately differs in `provider`. Validation should surface the change
|
legitimately differs in `provider`. A read-only open surfaces the change
|
||||||
once, then trust the user's config as the new source of truth so the
|
but must never modify the stored settings.
|
||||||
warning doesn't fire on every subsequent open.
|
|
||||||
"""
|
"""
|
||||||
from haiku.rag.store.engine import Store
|
from haiku.rag.store.engine import Store
|
||||||
from haiku.rag.store.repositories.settings import SettingsRepository
|
from haiku.rag.store.repositories.settings import SettingsRepository
|
||||||
|
|
@ -107,7 +102,7 @@ class TestValidateConfigCompatibility:
|
||||||
new_config.embeddings.model.provider = "openai"
|
new_config.embeddings.model.provider = "openai"
|
||||||
|
|
||||||
async with Store(
|
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:
|
) as store2:
|
||||||
settings_repo = SettingsRepository(store2)
|
settings_repo = SettingsRepository(store2)
|
||||||
|
|
||||||
|
|
@ -122,25 +117,57 @@ class TestValidateConfigCompatibility:
|
||||||
for r in caplog.records
|
for r in caplog.records
|
||||||
)
|
)
|
||||||
|
|
||||||
# Stored settings now match current
|
# Stored settings are untouched
|
||||||
saved = await settings_repo.get_current_settings()
|
saved = await settings_repo.get_current_settings()
|
||||||
assert saved["embeddings"]["model"]["provider"] == "openai"
|
assert saved["embeddings"]["model"]["provider"] == "ollama"
|
||||||
|
|
||||||
# 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)
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@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
|
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.engine import Store
|
||||||
from haiku.rag.store.repositories.settings import SettingsRepository
|
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)
|
monkeypatch.setattr(logging.getLogger("haiku.rag"), "propagate", True)
|
||||||
|
|
||||||
async with Store(temp_db_path, create=True):
|
async with Store(temp_db_path, create=True):
|
||||||
|
|
@ -150,7 +177,7 @@ class TestValidateConfigCompatibility:
|
||||||
new_config.embeddings.model.name = "different-model"
|
new_config.embeddings.model.name = "different-model"
|
||||||
|
|
||||||
async with Store(
|
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:
|
) as store2:
|
||||||
settings_repo = SettingsRepository(store2)
|
settings_repo = SettingsRepository(store2)
|
||||||
|
|
||||||
|
|
@ -163,7 +190,7 @@ class TestValidateConfigCompatibility:
|
||||||
)
|
)
|
||||||
|
|
||||||
saved = await settings_repo.get_current_settings()
|
saved = await settings_repo.get_current_settings()
|
||||||
assert saved["embeddings"]["model"]["name"] == "different-model"
|
assert saved["embeddings"]["model"]["name"] != "different-model"
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_vector_dim_mismatch_raises_error(self, temp_db_path):
|
async def test_vector_dim_mismatch_raises_error(self, temp_db_path):
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue