Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/pinky_memory/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
ReflectInput,
Reflection,
ReflectionType,
coerce_reflection_type,
)

if TYPE_CHECKING:
Expand All @@ -32,6 +33,16 @@ def _log(msg: str) -> None:
print(msg, file=sys.stderr, flush=True)


def _parse_reflection_type(type_str: str) -> ReflectionType:
"""Coerce a caller-supplied type to ReflectionType, logging when it falls
back to `fact` (callers, including dream runs, occasionally pass a
plausible-sounding but invalid type — see #session_log bug)."""
coerced = coerce_reflection_type(type_str)
if coerced.value != type_str:
_log(f"reflect: invalid type={type_str!r}, defaulting to 'fact'")
return coerced


# Strict agent-name slug for cross-agent memory targets (#614/#145). Mirrors
# the trust-boundary slug discipline tracked in #105 — defends the
# store-factory path resolution against traversal / injection even though the
Expand Down Expand Up @@ -369,7 +380,7 @@ def reflect(
"""
input_data = ReflectInput(
content=content,
type=ReflectionType(type),
type=_parse_reflection_type(type),
context=context,
project=project,
salience=salience,
Expand Down Expand Up @@ -430,7 +441,7 @@ def reflect_for(
s = _resolve_target_store(target_agent)
input_data = ReflectInput(
content=content,
type=ReflectionType(type),
type=_parse_reflection_type(type),
context=context,
project=project,
salience=salience,
Expand Down
11 changes: 10 additions & 1 deletion src/pinky_memory/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
Reflection,
ReflectionLink,
ReflectionType,
coerce_reflection_type,
resolve_preset,
)

Expand Down Expand Up @@ -1527,9 +1528,17 @@ def _row_to_reflection(row: sqlite3.Row) -> Reflection:
source_message_ids = json.loads(raw_msg_ids) if raw_msg_ids else []
next_review_date = row["next_review_date"] if "next_review_date" in keys else None
review_interval_days = row["review_interval_days"] if "review_interval_days" in keys else 7
row_type = row["type"]
coerced_type = coerce_reflection_type(row_type)
if coerced_type.value != row_type:
logger.warning(
"row_to_reflection: invalid stored type=%r for id=%r, defaulting to 'fact'",
row_type,
row["id"],
)
return Reflection(
id=row["id"],
type=ReflectionType(row["type"]),
type=coerced_type,
content=row["content"],
context=row["context"],
project=row["project"],
Expand Down
12 changes: 12 additions & 0 deletions src/pinky_memory/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,18 @@ class ReflectionType(str, Enum):
fact = "fact"


def coerce_reflection_type(type_str: str) -> ReflectionType:
"""Coerce a caller- or DB-supplied type to ReflectionType, defaulting to
`fact` on an unrecognized value instead of raising. Used both when writing
a new reflection (a caller passed a plausible-sounding but invalid type)
and when hydrating a row written before this type was retired/renamed —
a stale value must not crash every future read of that row."""
try:
return ReflectionType(type_str)
except ValueError:
return ReflectionType.fact


class Reflection(BaseModel):
model_config = ConfigDict(extra="ignore")

Expand Down
26 changes: 26 additions & 0 deletions tests/test_memory_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,32 @@ def test_reflect_with_all_fields(self, srv):
assert result["salience"] == 5
assert result["type"] == "project_state"

def test_reflect_invalid_type_falls_back_to_fact(self, srv):
"""Regression: an unrecognized type (e.g. dream-hallucinated 'session_log')
must not raise — it previously crashed the whole dream run."""
result = json.loads(_tools(srv)["reflect"](
content="something the dream agent decided to call session_log",
type="session_log",
))
assert result["stored"] is True
assert result["type"] == "fact"

def test_recall_survives_legacy_invalid_stored_type(self, srv, store):
"""Regression: a row written before a type was retired/renamed (e.g.
'session_log', 'episode', 'nav_audit' from pre-fallback dream runs)
must not crash hydration on read — it previously aborted every
subsequent recall/dream-linking pass that touched the row."""
import sqlite3

stored = json.loads(_tools(srv)["reflect"](content="pre-fallback dream note", type="fact"))
conn = sqlite3.connect(store._db_path)
conn.execute("UPDATE reflections SET type = 'session_log' WHERE id = ?", (stored["id"],))
conn.commit()
conn.close()

result = _parse_recall(_tools(srv)["recall"](query="pre-fallback dream note"))
assert any(r["id"] == stored["id"] and r["type"] == "fact" for r in result["reflections"])

def test_reflect_supersedes(self, srv, store):
# First memory
r1 = json.loads(_tools(srv)["reflect"](content="old fact", type="fact"))
Expand Down