diff --git a/src/pinky_memory/store.py b/src/pinky_memory/store.py index 7f329aec..15e95f5d 100644 --- a/src/pinky_memory/store.py +++ b/src/pinky_memory/store.py @@ -1512,6 +1512,32 @@ def _timeframe_cutoff(timeframe: str) -> datetime: # ── Helpers ── + @staticmethod + def _coerce_salience(raw: object) -> int: + """Coerce a stored salience value onto the 1-5 integer scale. + + Early builds stored salience as a 0-1 float; the Reflection model + now requires an int in [1, 5], so a single legacy row raises a + pydantic ValidationError and poisons every recall()/query that + touches it. Map 0-1 FLOATS onto 1-5, round anything else numeric, + clamp to bounds, and fall back to the default (3) for garbage. + + The legacy x5 mapping applies only to genuine floats: SQLite + preserves column affinity per value, so legacy rows hydrate as + Python floats while modern rows hydrate as ints. A modern, + perfectly valid salience of int 1 must stay 1 — not be mistaken + for legacy 1.0 and inflated to 5. + """ + if isinstance(raw, bool): # bool is an int subclass; treat as garbage + return 3 + if isinstance(raw, float) and 0 <= raw <= 1: + raw = raw * 5 + try: + val = float(raw) # type: ignore[arg-type] + except (TypeError, ValueError): + return 3 + return max(1, min(5, round(val))) + @staticmethod def _row_to_reflection(row: sqlite3.Row) -> Reflection: # Handle optional columns that may not exist in older schemas @@ -1533,7 +1559,7 @@ def _row_to_reflection(row: sqlite3.Row) -> Reflection: content=row["content"], context=row["context"], project=row["project"], - salience=row["salience"], + salience=ReflectionStore._coerce_salience(row["salience"]), active=bool(row["active"]), no_recall=no_recall, supersedes=row["supersedes"], diff --git a/tests/test_memory_store.py b/tests/test_memory_store.py index dda4c5dd..31266a4d 100644 --- a/tests/test_memory_store.py +++ b/tests/test_memory_store.py @@ -1229,3 +1229,54 @@ def test_consolidate_deactivated_ref_cannot_win(self, tmp_path): assert store.get(ref.id).superseded_by == winner.id # ...and must not have gone on to archive the weaker active memory assert store.get(weak.id).active is True + + +class TestLegacySalienceCoercion: + """Rows written by early builds carry 0-1 float salience; hydration must + coerce them onto the 1-5 int scale instead of raising ValidationError + and poisoning every recall()/query that touches the row (#834).""" + + @pytest.mark.parametrize( + "raw,expected", + [ + (0.7, 4), # legacy 0-1 scale -> x5, rounded + (0.9, 4), # round-half-even: 4.5 -> 4 + (1.0, 5), # FLOAT 1.0 is legacy-scale boundary -> maps high + (1, 1), # INT 1 is a valid modern salience -> passes through + (0.0, 1), # legacy float 0.0 clamps to lower bound + (0, 1), # int 0 is not legacy-mapped; just clamps to 1 + (3, 3), # modern ints pass through + (4.4, 4), # stray float on modern scale just rounds + (99, 5), # clamped to upper bound + (None, 3), # garbage falls back to default + (True, 3), # bool is an int subclass but is garbage here + ("high", 3), # garbage falls back to default + ], + ) + def test_coerce_salience(self, raw, expected): + assert ReflectionStore._coerce_salience(raw) == expected + + def test_recall_survives_legacy_float_salience_row(self, tmp_path): + store = _store(tmp_path) + r = store.insert(_fact("legacy row", salience=3)) + # Corrupt the row the way early builds wrote it: 0-1 float scale + with store._lock: + store._conn.execute( + "UPDATE reflections SET salience = 0.7 WHERE id = ?", (r.id,) + ) + store._conn.commit() + got = store.get(r.id) + assert got is not None + assert got.salience == 4 + + def test_modern_salience_one_is_not_inflated(self, tmp_path): + """A valid modern salience of 1 (stored as INTEGER) must hydrate as 1. + + Regression guard: the legacy 0-1 x5 mapping must key off the value + being a float, or int 1 gets mistaken for legacy 1.0 and read back + as 5 on every hydration.""" + store = _store(tmp_path) + r = store.insert(_fact("low-salience row", salience=1)) + got = store.get(r.id) + assert got is not None + assert got.salience == 1