Skip to content

fix(memory): coerce legacy float salience on row hydration - #888

Open
djgaasch-oss wants to merge 2 commits into
bradbrok:mainfrom
djgaasch-oss:fix/memory-salience-coercion
Open

fix(memory): coerce legacy float salience on row hydration#888
djgaasch-oss wants to merge 2 commits into
bradbrok:mainfrom
djgaasch-oss:fix/memory-salience-coercion

Conversation

@djgaasch-oss

Copy link
Copy Markdown

Problem

Early PinkyBot builds stored reflection salience as a 0–1 float. The Reflection pydantic model now requires int in [1, 5], so a single legacy row raises ValidationError at hydration and poisons every recall() / memory_query that touches it.

Observed live on the bubbs agent (Jul 16): 3 rows with salience 0.7 / 0.85 / 0.9 made semantic recall completely unusable:

Error executing tool recall: 1 validation error for Reflection
salience
  Input should be a valid integer, got a number with a fractional part

Fix

Add ReflectionStore._coerce_salience() and apply it in _row_to_reflection:

  • 0–1 floats map onto the 1–5 scale (x*5, rounded)
  • stray numerics round and clamp to [1, 5]
  • non-numeric garbage falls back to the default (3)

Reads become tolerant; writes are already validated by pydantic, so no new bad data can enter.

Testing

  • New TestLegacySalienceCoercion: 9 parametrized coercion cases + an end-to-end test that corrupts a stored row to 0.7 and asserts get() hydrates it as 4 instead of raising.
  • Full tests/test_memory_store.py: 120 passed, 1 skipped.

🤖 Opened by Bubbs

djgaasch-oss and others added 2 commits July 16, 2026 14:08
Early builds stored salience as a 0-1 float; the Reflection model now
requires an int in [1, 5], so a single legacy row raised a pydantic
ValidationError and poisoned every recall()/memory_query that touched
it (observed live on bubbs: rows with salience 0.7/0.85/0.9 made
recall() unusable).

Map 0-1 floats onto the 1-5 scale, round stray numerics, clamp to
bounds, and fall back to the default (3) for garbage values.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SQLite preserves per-value type, so legacy rows hydrate as floats and
modern rows as ints. Keying the x5 legacy mapping on isinstance(float)
prevents a valid modern salience of int 1 from being inflated to 5 on
every read. Also treat bools as garbage (default 3) and add regression
tests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@djgaasch-oss

Copy link
Copy Markdown
Author

Pushed a correctness fix to the coercion: the legacy 0-1 → ×5 mapping now applies only to genuine floats. SQLite preserves per-value type (legacy rows hydrate as float, modern rows as int), and without the type check a perfectly valid modern salience of int 1 fell inside 0 <= val <= 1 and was silently inflated to 5 on every read. Bools (int subclass) are treated as garbage → default 3.

New regression tests: (1, 1), (0, 1), (True, 3) param cases + an end-to-end test_modern_salience_one_is_not_inflated. Suite: 124 passed, 1 skipped; ruff clean.

🤖 Opened by Bubbs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant