feat(memory): add durable memory store#181
Conversation
Implement a small SQLite-backed curated memory store for #171 so semantic, episodic, procedural, and rejected-path memories survive process restarts with provenance and supersession state. This keeps boot context durable without storing raw transcripts.
hadamrd
left a comment
hadamrd
left a comment
There was a problem hiding this comment.
[sev2/product] The PR exports a new MemoryStore protocol and adds a FakeMemoryStore contract surface, but the repo has no downstream consumer wired to the store or protocol. Under the no-scaffold-theatre rule, wire this into the maestro/boot memory-loading path in this PR, or quarantine/remove the plug-in surface behind an experimental extra until it has an in-repo consumer.
| """ | ||
|
|
||
|
|
||
| class MemoryStore(Protocol): |
There was a problem hiding this comment.
[sev2/product] The PR exports a new MemoryStore protocol and adds a FakeMemoryStore contract surface, but the repo has no downstream consumer wired to the store or protocol. Under the no-scaffold-theatre rule, wire this into the maestro/boot memory-loading path in this PR, or quarantine/remove the plug-in surface behind an experimental extra until it has an in-repo consumer.
hadamrd
left a comment
hadamrd
left a comment
There was a problem hiding this comment.
[sev2/tests] SqliteMemoryStore.supersede adds a public failure path for missing memory IDs, but the new tests only cover successful supersession. Add an adversarial test that asserts supersede('missing', by_memory_id=...) raises KeyError for the real store and fake, so this public error contract is locked down.
| def list_rejected_paths(self) -> tuple[MemoryItem, ...]: | ||
| return tuple(item for item in self.list_active() if REJECTED_PATH_TAG in item.tags) | ||
|
|
||
| def supersede(self, memory_id: str, *, by_memory_id: str) -> MemoryItem: |
There was a problem hiding this comment.
[sev2/tests] SqliteMemoryStore.supersede adds a public failure path for missing memory IDs, but the new tests only cover successful supersession. Add an adversarial test that asserts supersede('missing', by_memory_id=...) raises KeyError for the real store and fake, so this public error contract is locked down.
hadamrd
left a comment
hadamrd
left a comment
There was a problem hiding this comment.
Manifesto violations:
- [sev2] testing.md#TE-002 —
+ def supersede(self, memory_id: str, *, by_memory_id: str) -> MemoryItem:→ Add a sad-path test for the new public supersede API, e.g. assert SqliteMemoryStore(...).supersede('missing', by_memory_id='mem-new') raises KeyError, and include the fake store in the same contract check.
Address #181 critic feedback by connecting the durable memory store to the curator/status path, reporting real memory health from .forge/memory.db, and covering missing supersession failures. Also create parent directories for the SQLite memory DB.
Ensure superseded memory cannot point at a missing replacement id. Cover both missing source and missing replacement contracts for the real SQLite store and fake store.
Require every MemoryProvenance to carry either an event ref or a task ref, update status fixtures to use valid task provenance, and add negative coverage for missing source provenance.
|
Fresh critic rerun on head 2404c34 after the memory-store fixes found no sev1/sev2 blocker. Local verification: tests/test_memory_store.py, tests/test_cli_status_control_plane.py::TestStatusControlPlane + tests/test_eventlog_sqlite.py, scoped ruff check/format, focused mypy, and pyright. Replay CI is green. |
1 participant
Fixes #171
Summary
Acceptance Criteria
Verification