The first M01 trial with the memory bank on was 26 turns on a GPU host. It
accepted every turn and reported "complete". It also wrote two memories and
no summary, and logged 180 `database is locked` errors, while derived status
still read `idle`.
The cause was a single uncommitted UPDATE. Retrieval bumped each used
memory's counter before the model call, and the turn commits only after the
reply has streamed. SQLite has one writer, so the turn held the write lock for
the whole reply. Every post-turn memory, summary and status write in that
window waited out the five-second timeout and failed. Recording the failure
needed a write as well, and without a rollback first it raised
PendingRollbackError. The loss therefore reached the log and never reached
the status the Insights panel reads, which F08 forbids. The draco run never
hit this because the bank was off there.
- `retrieve_memories` now only reads. `record_use` writes the counters in the
turn's single commit, so a turn that never lands counts nothing.
- The post-turn task's outer handler rolls back before it records a failure.
The harness could not have caught any of this. It read three prompt sections
under names the builder does not use: `memories` (really `used_memories`),
`story_history` (really `history`/`recent_history`), and a `knowledge` prefix
that matched the fixed instruction section instead of the imported passages.
Memory tokens read 0 whatever the prompt held, and the in-history and
in-memories recall checks could never come out true. The labels are now
constants, pinned by a test against a prompt the real builder assembled.
The harness also stops at the first sign of failed post-turn work. It checks
/derived and new server.log lines after every turn, keeps its log position
across --resume, and waits for background work to settle before its final
checks. A run with no memories or no summaries now ends "failed", not
"complete".
Both new application tests fail on fec46f6: the lock probe sees
`database is locked`, and memory status stays `idle`. The full backend suite
passes (1392 passed, 17 skipped). A 26-turn re-run against the same host had
0 lock errors, wrote 7 memories and 2 summaries, and used them in the prompt
from turn 8.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0136VBTMUKWYeU6G9HgbDbND
161 lines
5.2 KiB
Python
161 lines
5.2 KiB
Python
"""Switching embedding models must re-embed the bank.
|
|
|
|
Vectors from two different models are not comparable. They live in
|
|
different spaces and often have different widths. Changing the model must
|
|
discard the stored vectors and let the post-turn pass rebuild them.
|
|
|
|
This worked while the vectors lived in `memories.embedding`. The settings
|
|
route nulled that column, and the embed queue picked up the rows. Migration
|
|
38 moved the vectors to `embedding_blob` and added an `embedded` flag beside
|
|
them, but the bulk clear kept nulling only the old column. The blob
|
|
survived, the flag stayed true, and `_embed_pending` (which filters on
|
|
`embedded IS FALSE`) never saw the rows. The bank kept ranking against the
|
|
previous model's vectors.
|
|
|
|
Nothing reports this failure. `cosine` returns 0.0 on a width mismatch, so a
|
|
different-width model scores every memory zero, and retrieval silently
|
|
returns whichever rows sort first. A same-width model scores plausible
|
|
garbage instead.
|
|
|
|
python -m pytest tests/test_embedding_model_switch.py -v
|
|
"""
|
|
import asyncio
|
|
|
|
import pytest
|
|
from fastapi import Depends
|
|
from fastapi.testclient import TestClient
|
|
|
|
from app import auth, limits, memorybank, models
|
|
from app.database import Base, SessionLocal, engine, get_db
|
|
from app.main import app
|
|
|
|
DIMS = 8
|
|
|
|
|
|
@pytest.fixture()
|
|
def client(monkeypatch):
|
|
Base.metadata.create_all(bind=engine)
|
|
memorybank._vector_cache.clear()
|
|
setup = SessionLocal()
|
|
user = models.User(is_guest=False, email="switch@example.com")
|
|
setup.add(user)
|
|
setup.flush()
|
|
setup.add(models.Settings(
|
|
user_id=user.id, api_key="enc:dummy", model="test-model",
|
|
embedding_model="model-a",
|
|
))
|
|
adventure = models.Adventure(
|
|
user_id=user.id, title="Cave", script_state={}, memory_bank_enabled=True
|
|
)
|
|
setup.add(adventure)
|
|
setup.flush()
|
|
for i in range(5):
|
|
memory = models.Memory(adventure_id=adventure.id, text=f"Memory {i}")
|
|
memorybank.set_vector(memory, [float(i)] + [0.0] * (DIMS - 1))
|
|
setup.add(memory)
|
|
setup.commit()
|
|
adv_id, user_id = adventure.id, user.id
|
|
setup.close()
|
|
|
|
monkeypatch.setattr(limits, "check_row_cap", lambda *a, **k: None)
|
|
|
|
def _current_user(db=Depends(get_db)):
|
|
return db.get(models.User, user_id)
|
|
|
|
app.dependency_overrides[auth.get_current_user] = _current_user
|
|
c = TestClient(app)
|
|
c.adv_id = adv_id
|
|
try:
|
|
yield c
|
|
finally:
|
|
app.dependency_overrides.clear()
|
|
memorybank._vector_cache.clear()
|
|
Base.metadata.drop_all(bind=engine)
|
|
|
|
|
|
def memories(db):
|
|
return db.query(models.Memory).order_by(models.Memory.id).all()
|
|
|
|
|
|
def test_the_bank_starts_embedded(client):
|
|
db = SessionLocal()
|
|
try:
|
|
rows = memories(db)
|
|
assert len(rows) == 5
|
|
assert all(m.embedded for m in rows)
|
|
assert all(m.embedding_blob for m in rows)
|
|
finally:
|
|
db.close()
|
|
|
|
|
|
def test_changing_the_model_clears_every_vector(client):
|
|
r = client.put("/api/settings", json={"embedding_model": "model-b"})
|
|
assert r.status_code == 200, r.text
|
|
|
|
db = SessionLocal()
|
|
try:
|
|
rows = memories(db)
|
|
assert [m.embedding_blob for m in rows] == [None] * 5, \
|
|
"the blob survived the model change"
|
|
assert not any(m.embedded for m in rows), \
|
|
"`embedded` stayed true, so nothing will ever re-embed these"
|
|
finally:
|
|
db.close()
|
|
|
|
|
|
def test_cleared_memories_are_queued_for_re_embedding(client):
|
|
"""The `embedded` flag is not cosmetic. It is the only condition
|
|
`_embed_pending` filters on, so this test confirms the bank actually
|
|
recovers."""
|
|
client.put("/api/settings", json={"embedding_model": "model-b"})
|
|
|
|
db = SessionLocal()
|
|
try:
|
|
pending = (
|
|
db.query(models.Memory)
|
|
.filter(models.Memory.embedded.is_(False),
|
|
models.Memory.forgotten.is_(False))
|
|
.all()
|
|
)
|
|
assert len(pending) == 5
|
|
finally:
|
|
db.close()
|
|
|
|
|
|
def test_retrieval_uses_no_stale_vector_after_the_switch(client, monkeypatch):
|
|
"""Until the re-embed runs, the bank must return nothing rather than
|
|
ranking against the old model's vectors."""
|
|
client.put("/api/settings", json={"embedding_model": "model-b"})
|
|
|
|
class Embedder:
|
|
async def embed(self, texts):
|
|
return [[1.0] + [0.0] * (DIMS - 1) for _ in texts]
|
|
|
|
monkeypatch.setattr(memorybank, "embedding_provider", lambda s: Embedder())
|
|
|
|
db = SessionLocal()
|
|
try:
|
|
adventure = db.get(models.Adventure, client.adv_id)
|
|
settings = db.query(models.Settings).first()
|
|
result = asyncio.run(
|
|
memorybank.retrieve_memories(adventure, settings)
|
|
)
|
|
assert result["used"] == []
|
|
finally:
|
|
db.close()
|
|
|
|
|
|
def test_an_unrelated_settings_change_keeps_the_vectors(client):
|
|
"""Only an embedding-model change may clear the bank. Re-embedding costs
|
|
an API call per memory."""
|
|
r = client.put("/api/settings", json={"model": "some-other-chat-model"})
|
|
assert r.status_code == 200, r.text
|
|
|
|
db = SessionLocal()
|
|
try:
|
|
rows = memories(db)
|
|
assert all(m.embedded for m in rows)
|
|
assert all(m.embedding_blob for m in rows)
|
|
finally:
|
|
db.close()
|