fix: enable SQLite WAL mode and add consistent LIMIT to KG timeline#136
fix: enable SQLite WAL mode and add consistent LIMIT to KG timeline#136bensig merged 3 commits intoMemPalace:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves KnowledgeGraph SQLite concurrency by enabling WAL journaling and prevents unbounded entity-specific timeline results by applying a consistent limit. It also introduces a substantial test suite and updates Python packaging/tooling configuration.
Changes:
- Enable SQLite WAL mode for KnowledgeGraph connections to reduce
SQLITE_BUSYcontention under concurrent access. - Add
LIMIT 100to the entity-filtered KGtimeline()query to match the global timeline behavior. - Add/expand pytest coverage and migrate packaging metadata (remove
requirements.txt, switch build backend to hatchling, add dependency groups).
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
mempalace/knowledge_graph.py |
Enables WAL on connections; adds LIMIT 100 to entity timeline query. |
mempalace/__init__.py |
Updates package __version__ to 3.0.0. |
pyproject.toml |
Switches build backend to hatchling; defines dev deps and dependency group. |
requirements.txt |
Removed dependency file (impacts existing install/CI flows). |
tests/conftest.py |
Adds shared fixtures to isolate HOME and provide temp palace/KG/Chroma setup. |
tests/test_knowledge_graph.py |
Adds tests covering KG CRUD, queries, invalidation, timeline, stats. |
tests/test_mcp_server.py |
Adds tests for MCP server request dispatch and tool handlers. |
tests/test_searcher.py |
Adds tests for the programmatic search_memories API. |
tests/test_dialect.py |
Adds tests for AAAK Dialect compression/encoding/decoding behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _conn(self): | ||
| return sqlite3.connect(self.db_path, timeout=10) | ||
| conn = sqlite3.connect(self.db_path, timeout=10) | ||
| conn.execute("PRAGMA journal_mode=WAL") |
There was a problem hiding this comment.
WAL mode is a key behavioral change for concurrency, but it isn't currently covered by tests. Consider adding a test that asserts PRAGMA journal_mode returns wal after creating a KnowledgeGraph connection, so regressions (or environments where WAL can't be enabled) are detected early.
| conn.execute("PRAGMA journal_mode=WAL") | |
| journal_mode = conn.execute("PRAGMA journal_mode=WAL").fetchone() | |
| effective_mode = (journal_mode[0] if journal_mode else "").lower() | |
| if effective_mode != "wal": | |
| conn.close() | |
| raise RuntimeError( | |
| f"Failed to enable SQLite WAL mode for knowledge graph at {self.db_path!r}; " | |
| f"effective journal_mode={effective_mode!r}" | |
| ) |
| FROM triples t | ||
| JOIN entities s ON t.subject = s.id | ||
| JOIN entities o ON t.object = o.id | ||
| WHERE (t.subject = ? OR t.object = ?) | ||
| ORDER BY t.valid_from ASC NULLS LAST | ||
| LIMIT 100 | ||
| """, |
There was a problem hiding this comment.
The new LIMIT 100 on the entity-filtered timeline query isn't covered by tests (current tests only assert the global timeline is limited). Add a test that creates >100 triples connected to a single entity and asserts kg.timeline(entity) returns at most 100 rows.
|
Hey Igor — CI is failing, looks like it needs a rebase on main (we've merged a bunch of changes recently). Can you rebase and push? |
- Enable WAL journal mode in _conn() for better concurrent read performance and reduced SQLITE_BUSY risk - Add LIMIT 100 to entity-filtered timeline query (was unbounded, while global timeline already had LIMIT 100) Findings: MemPalace#8 (HIGH — no WAL mode), MemPalace#22 (LOW — inconsistent limits) Includes test infrastructure from PR MemPalace#131. 92 tests pass.
…nused fixture param
Problem
The KnowledgeGraph SQLite database uses the default journal mode (DELETE). Under concurrent access (e.g., two MCP sessions or a hook + MCP), this risks
SQLITE_BUSYerrors since readers block writers.The entity-filtered
timeline()query has noLIMIT, while the global timeline query already hasLIMIT 100. A well-connected entity could return unbounded results.Findings:
HIGH — no WAL mode
LOW — inconsistent limits
Fix
WAL Mode
WAL (Write-Ahead Logging) allows concurrent readers without blocking writers. The pragma only has effect on first connection and persists on the database file.
Consistent LIMIT
Added
LIMIT 100to the entity-filtered timeline query to match the global timeline.92 tests pass (includes test infrastructure from PR #131).