perf: graph cache with write-invalidation in build_graph()

[jphein]

Problem

build_graph() scans every drawer's metadata in 1000-item batches on every call — O(n) per graph build with no caching. At 50K+ drawers this costs several seconds per MCP tool call because traverse(), find_tunnels(), and graph_stats() all call build_graph() on every invocation.

Solution

Module-level cache (_graph_cache_nodes, _graph_cache_edges, _graph_cache_time) with a 60-second TTL, following the same pattern as _metadata_cache in mcp_server.py.

• build_graph() returns the cached result if < 60 seconds old
• invalidate_graph_cache() is exported for write operations to call when the palace changes
• Cache is only populated on successful full scans (not on empty collections)

Changes

• mempalace/palace_graph.py — add import time, module-level cache globals, invalidate_graph_cache() function, TTL check at top of build_graph(), cache-store at bottom of build_graph()
• tests/test_palace_graph.py — import invalidate_graph_cache, add setup_method to each test class that exercises build_graph (prevents cross-test cache pollution), add test_cache_returns_same_result and test_invalidate_clears_cache

Test results

23 passed in 0.21s

All 23 tests pass including 2 new cache-specific tests.

[Copilot]

Pull request overview

This PR introduces a short-lived (60s TTL) module-level cache for build_graph() to avoid repeated full metadata scans when multiple MCP tools build the palace graph in quick succession.

Changes:

• Added module-level graph cache state + invalidate_graph_cache() helper in mempalace/palace_graph.py.
• Updated build_graph() to return cached results within TTL and to store results after a scan.
• Added cache-focused tests and per-test cache invalidation to prevent cross-test interference.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
mempalace/palace_graph.py	Adds TTL-based caching for graph builds and an explicit invalidation function.
tests/test_palace_graph.py	Adds cache invalidation in test setup and new tests asserting cache/invalidation behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jphein]

Fixed — removed the stale reference to _metadata_cache. Comment is now self-contained: module-level cache with 60s TTL and write-invalidation. 277e476

[jphein]

Documented — added comment explaining this is intentional for the MCP server's single-palace use case. Callers switching collections should call invalidate_graph_cache() first. 277e476

[jphein]

Fixed — empty graphs are no longer cached (if nodes: guard). New data is picked up immediately when palace is first populated. 277e476

[jphein]

Documented — expanded docstring to explain this verifies intentional single-palace cache behavior. If multi-palace support is needed, cache key should be extended and this test updated. 277e476

[bensig]

Two issues found in code review:

1. No thread safety on cache globals (score 95) — _graph_cache_nodes, _graph_cache_edges, _graph_cache_time are read and written without any lock. Concurrent requests can read stale or partially-written cache. Add a threading.Lock around the read/write paths.

2. Warm cache silently ignores col argument (score 82) — build_graph(col=different_col) returns cached result for the original collection with no indication. Consider keying the cache on the collection identity, or at minimum documenting this behavior in the docstring and logging when cached args are ignored.

[JoyciAkira]

Closing duplicate PR created by network timeout.

[JoyciAkira]

Closing duplicate PR created by network timeout.

[JoyciAkira]

Closing duplicate PR created by network timeout.

[JoyciAkira]

Closing duplicate PR created by network timeout.

[jphein]

@bensig Both issues from your review are already addressed in the current branch:

1. Thread safety — _graph_cache_lock = threading.Lock() guards all cache reads (line 74) and writes (lines 37, 136). Added in commit 277e476 responding to the Copilot review.

2. Warm cache ignores col — Documented in both the docstring ("warm cache ignores col and config arguments — this is intentional for the MCP server's single-palace use case") and an inline comment above the cache check. Callers switching collections should call invalidate_graph_cache() first.

Branch has been rebased on latest develop (v3.2.0). Ready for re-review.

[jphein]

Ping @bensig — the changes you requested on 2026-04-12 have been addressed in the follow-up commit on 2026-04-16 (added threading.Lock around the module-level cache write, which was the main thread-safety concern from your review). Would love to get this back on your radar when you have a moment — happy to iterate further if there's anything still off.

[bensig]

Threading.Lock usage correct. Cache invalidation sound (write-invalidation + 60s TTL fallback). Both original review items addressed. CI green across all platforms.