Add in-process LRU cache for graph analysis results (MOO-72 Commit 2) - #10
Conversation
Commit 1A had all three graph routes computing a canonical cache key and returning it as a permanent miss. This wires a real store behind that hook so a repeat request skips the work rather than just reporting a hit. server/lib/graph-cache.js is a Map-based LRU (insertion order gives O(1) recency) bounded by BOTH item count and bytes -- a handful of large repository graphs can exhaust a Railway container well before 200 entries. Overwrites delete before reinserting so recency actually refreshes, and an entry too large to ever fit is rejected rather than flushing the cache on its way out. Cache placement is per-layer, always ahead of that layer's expensive step: - repository: github-analyzer-bridge.js gains resolveGithubRef (ref resolution only) split from fetchAndAnalyzeRepo (tree + blobs + analysis), so the key can be built from one cheap call and the whole fetch/parse phase skipped. analyzeGithubRepo remains as the composed form for /api/analyze-repo. - file: after the GitHub fetch, before workspace staging and pyan3. - function: after symbol resolution, before CodeVisualizer. Correctness details worth calling out: Cache identity now includes request mode and ref. contextIdentityKey() keys on sourceOwner/sourceRepo@sha and deliberately omits both -- correct for "is this the same content", but not sufficient for caching a whole response: a default-branch request and an explicit ref request can resolve to the same SHA while needing different graph.context values. Without this they would share an entry and one would be served the other's context. Python tree-sitter capability is now an active startup probe (a real Language.load of the grammar) rather than a runtime check that the shim loaded. A loaded runtime does not prove the grammar is usable, and the old signal would report "capable" while every Python parse silently fell back to the regex heuristic. The probe is scoped so a missing Python grammar does not disable tree-sitter for every other language. The file layer keys on request.depth ?? 'auto' rather than the resolved depth mode, since chooseDepthMode needs a built graph -- exactly the work a hit must skip. Request-derived means lookup and storage always agree. Responses are rebuilt on every hit rather than replayed: the store holds the GraphIR, not the AdapterResult, so cache.hit and timing reflect the current request and the stored graph is never mutated to carry per-request metadata. Degraded pyan3 output is never cached -- those failures are typically transient, and storing the tree-sitter-only fallback would keep serving it for the full TTL after pyan3 recovered. Tree-sitter degradation is process-permanent and already in the key, so it is cached. CACHE_ENABLED validates strictly as "true"/"false"; a typo is a startup error rather than silently leaving caching on. The cache is process-local: cleared on restart/deploy, not shared across replicas, and assumes a single Railway instance. Stated in the startup log so an operator debugging a stale result sees it. Concurrent identical misses still duplicate work; single-flight is deferred to Commit 4. Tests: +53 (492 -> 545, all passing). Store unit tests cover TTL boundaries, LRU by count and by bytes, overwrite recency, and oversized rejection. Route tests cover key identity across every dimension -- including the mode/ref collision above -- and that rejected requests never reach the cache. An end-to-end suite drives the real handler with a stubbed global fetch and asserts on GitHub call counts, so it proves a hit skips the fetching rather than merely reporting hit: true. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pu8rYHKSahwRh9mu7LcdwC
|
Cache identity currently collapses requests whose returned graphs are not actually identical.
Consequently, the second request can hit the first request's cache entry and receive the first request's case/order in its response metadata. The current equivalent-pattern E2E test checks only key equality and Please make the cache identity and returned representation obey the same equivalence rule—either canonicalize |
…on cache hit A cache entry shared by requests differing only in exclude-pattern case/order/whitespace was serving the first request's original casing in graph.metadata.excludePatterns to all of them, since that field came from whichever request populated the entry rather than the one being served. displayExcludePatterns() rebuilds the display form fresh from the current request on every response, hit or miss.
|
Fixed in c0545f3. Added Added an assertion to the existing "differing only in case/order/whitespace" E2E test confirming both requests get back their own casing ( |
2 participants
Summary
Implements a centralized, in-process LRU cache for GraphIR analysis results across all three graph layers (repository, file, and function). This cache skips expensive GitHub fetches and analysis work for repeat requests, while maintaining correctness through precise cache key identity that accounts for all dimensions affecting the response.
Key Changes
Cache Infrastructure
GraphCacheclass (server/lib/graph-cache.js) implementing an LRU cache with dual constraints: maximum item count and maximum byte size, plus TTL-based expirationserver/index.js) with configurable boundsCache Key Identity (MOO-72 Commit 2 core)
cacheKeyRequestIdentity()function to distinguish requests that resolve to the same commit but require different responses (e.g., default-branch vs explicitref: 'main'requests)requestModeandrequestRef(for branch mode) in cache keys to prevent serving wronggraph.contextvaluesnormalizeExcludePatterns()so requests differing only in case, order, or whitespace collapse to the same keyPYTHON_TREE_SITTER_CAPABLE) probed at import time and included in repository layer keysRoute Handler Integration
graph-repository.js: Ref resolution moved to Phase 0 (before cache lookup) to compute the resolved SHA needed for the cache key; cache checked after validation but before expensive tree/blob fetchinggraph-file.jsandgraph-function.js: Cache lookup positioned after cheap GitHub fetches and symbol resolution but before expensive analysis work (workspace staging, pyan3, CodeVisualizer)AdapterResultresponses from cached graphs, ensuring timing andcache.hitfields are never staleConfiguration
CACHE_ENABLED,CACHE_MAX_ITEMS,CACHE_MAX_BYTES,CACHE_TTL_MSCACHE_ENABLED(must be exactly "true" or "false") to prevent silent misconfigurationTesting
tests/graph-cache.test.mjs: Unit tests for LRU eviction, TTL expiration, byte budgeting, and entry integritytests/graph-cache-routes.test.mjs: Cache identity tests verifying no collisions between semantically different requests and proper collapse of equivalent ones; pre-network ordering tests confirming validation/allowlist rejections never reach the cachetests/graph-cache-e2e.test.mjs: End-to-end tests through real route handlers with stubbed GitHub API, verifying repeat requests skip expensive work (measured by GitHub call count)Notable Implementation Details
GraphIRgraph objects, not fullAdapterResultenvelopes, so request-specific fields (timing, cache metadata) are always freshget()refreshes recency by delete-and-reinsert;set()evicts the first Map entry until constraints are metmaxBytesare rejected without flushing the cacheLanguage.load) at startup, not inferred from runtime availability, ensuring the cache key accurately reflects what analysis actually usedhttps://claude.ai/code/session_01Pu8rYHKSahwRh9mu7LcdwC