JMCP: Papers - jmcp/cache-readdemostate-per-request-to-avoid-aNWlu7w3

[landigf]

Review: Cache readDemoState per request via AsyncLocalStorage

Change summary

• Introduces an AsyncLocalStorage<{ state: DemoState | null }> in demo-store.ts to cache the parsed JSON state within a single async call tree
• readDemoState() checks the ALS store first; on miss, reads disk and populates the cache
• writeDemoState() is write-through — updates the ALS cache so subsequent reads in the same request see the mutation
• DemoRepository.cached() helper ensures every public method runs inside an ALS context (creates one if none exists, reuses if nested)
• Every DemoRepository method body is wrapped in this.cached(async () => { ... }) — purely mechanical indentation change, no logic alterations

Validation confidence: High

The approach is correct. AsyncLocalStorage is the standard Node.js primitive for request-scoped state. Key properties hold:

• No cross-request leakage — each demoCache.run() call creates an isolated store
• Write-through keeps cache consistent — writeDemoState updates the store, so read-after-write within a request is correct
• Re-entrancy works — getDailyDigest calls getFeed, listTrendingPapers, getOpportunities, and getViewer internally; they all share the same ALS context and hit disk only once
• Object identity is preserved — mutating methods (e.g., toggleStar) modify the in-memory state object then write it back, and the cache holds the same reference

Risks that remain

1. External concurrent writes are invisible within a request — if another process or request writes the JSON file mid-flight, the cached state won't reflect it. Acceptable for a single-user demo store, but worth noting if this ever runs multi-tenant.

2. getDemoCacheStore is exported but only consumed internally by DemoRepository.cached(). Minor API surface leak — could be unexported if nothing outside index.ts needs it.

3. Large mechanical diff (~600 lines of indentation change) makes it easy to miss a subtle brace or logic error buried in the noise. I didn't spot any, but this is the kind of diff where a typo hides well. The type checker is the real safety net here — recommend confirming npm run check passes cleanly.

4. No per-method result caching — if the same repo method (e.g., getViewer) is called multiple times within a request with the same arguments, it re-runs the in-memory filtering each time (just skips the disk read). This is fine for the current scale but is the next optimisation if needed.

[landigf]

Closing: will integrate valuable features directly on main to avoid cascade merge conflicts.