perf: add mtime-invalidated LRU session cache; route APIs through it

[clean6378-max-it]

Closes #82

Summary

Adds an mtime-invalidated, LRU-bounded in-memory session cache so repeated API reads stop re-parsing JSONL from disk. Routes all parse-backed API handlers through get_cached_session() with no JSON response changes.

Sprint item #4 (5 pt) — Wednesday PR 1 of 2. Merge before PR 2 (benchmark regression gate #6) so baselines capture cached-path performance.

Problem

Every session-touching request called parse_session(filepath) from disk. get_session() and get_session_stats() parsed the same file twice; search() re-parsed the entire corpus per query. No in-memory cache, no mtime check, no reuse across requests.

Changes

New: utils/session_cache.py

• Key: absolute file path; value: (mtime, SessionDict)
• Invalidate on os.path.getmtime() change (edits picked up without restart)
• LRU eviction via OrderedDict (default max 200 entries)
• threading.Lock for --debug reloader / threaded safety
• Parsing happens outside the lock so concurrent misses on different files don't serialize
• Public API: get_cached_session(path), clear_cache(), set_max_entries() (tests)

Wired routes

• api/sessions.py — get_session + get_session_stats share one cached parse
• api/search.py — per-session loop uses cache (big win for repeated searches)
• api/projects.py — get_project_sessions uses cache (get_projects still uses quick_session_info)
• api/export_api.py — single-session export uses cache

Tests & benchmarks

• tests/test_session_cache.py — cache hit, mtime invalidation, LRU eviction
• tests/benchmarks/test_cache_bench.py — cold parse vs warm hit
• Updated monkeypatch targets in test_api_routes.py and test_error_propagation.py

Acceptance criteria

• utils/session_cache.py stores parsed SessionDict keyed by absolute path
• Cache invalidated when os.path.getmtime() changes
• Configurable max size (default 200) with LRU eviction
• api/sessions.py — get_session + get_session_stats share one cached parse
• api/search.py uses cache
• api/projects.py and api/export_api.py use cache
• Repeated-access benchmark in tests/benchmarks/
• test_parse_memory.py still passes
• Identical JSON responses; all tests green

Test plan

• pytest -q — 382 passed
• pytest tests/benchmarks/test_parse_memory.py -q
• pytest tests/benchmarks/test_cache_bench.py --benchmark-only
• mypy -p api -p utils -p models
• ruff check .

Out of scope

• Benchmark regression gate (Wednesday PR 2 — Add CLI + Web GUI for browsing Claude Code sessions #6)
• quick_session_info lightweight cache
• Bulk export path (utils/export_engine) — still calls parse_session directly

Notes

Cache benchmark (local, medium corpus): warm hit ~11µs vs cold parse ~1.45ms (~135× faster).

Summary by CodeRabbit

Release Notes

• New Features

  • Added a thread-safe in-memory session cache with mtime-based change detection, LRU eviction (200-entry default), and controls to clear or resize the cache.

• Performance Improvements

  • Session details, stats, project session listings, and search now reuse cached session data when unchanged.

• Bug Fixes

  • Strengthened error-handling coverage to prevent exception details or internal strings from leaking on failures.

• Tests

  • Expanded cache tests (cold/warm, invalidation, path normalization, eviction, validation, OSError resilience) and added cache benchmarks.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2ccaf047-8434-46a1-ac61-1156779969e6

📥 Commits

Reviewing files that changed from the base of the PR and between d8b87b0 and fe31c5b.

📒 Files selected for processing (2)

• tests/test_session_cache.py
• utils/session_cache.py

🚧 Files skipped from review as they are similar to previous changes (2)

• utils/session_cache.py
• tests/test_session_cache.py

---

📝 Walkthrough

Walkthrough

Adds utils/session_cache.py, a thread-safe in-memory LRU cache (default 200 entries) keyed by absolute file path with mtime-based invalidation. All four API modules (sessions.py, search.py, projects.py, export_api.py) replace direct parse_session calls with get_cached_session. New unit tests, benchmark tests, and updated monkeypatch targets accompany the change.

Changes

Session Cache Layer and API Wiring

Layer / File(s)	Summary
session_cache.py: state, get_cached_session, clear_cache, set_max_entries utils/session_cache.py	New module with an OrderedDict protected by a threading.Lock. get_cached_session resolves the absolute path, checks the stored mtime for a cache hit, updates LRU ordering on hit, or parses from disk and evicts the oldest entry when capacity is exceeded. clear_cache and set_max_entries support testing and configuration.
API route modules wired to get_cached_session api/sessions.py, api/search.py, api/projects.py, api/export_api.py	Each module swaps its parse_session import for get_cached_session from utils.session_cache and replaces the direct parse call at the session-load site. Route signatures, URL patterns, and all downstream logic are unchanged.
test_session_cache.py: correctness, hit, mtime, path normalization, LRU eviction, validation, error handling tests/test_session_cache.py	New unit suite validates data equivalence with direct parse, absence of reparse on cache hit via monkeypatched call counter, mtime-triggered invalidation, path normalization across relative and absolute forms, LRU eviction when cache size is set to 2 with three loaded files, input validation for negative max entries, and robustness when os.path.getmtime raises OSError.
Benchmark: cold parse vs warm cache hit tests/benchmarks/test_cache_bench.py	New pytest-benchmark file measures get_cached_session latency for cold (cache cleared before call) and warm (cache primed before call) scenarios using the existing parse_medium_file fixture.
Updated monkeypatch targets in existing API tests tests/test_api_routes.py, tests/test_error_propagation.py	Existing 500-error and error-propagation tests updated to inject failures via api.sessions.get_cached_session and api.projects.get_cached_session instead of the removed parse_session references.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• cppalliance/claude-code-chat-browser#26: Introduced sanitized error responses for parse failures in the same API routes whose monkeypatch targets this PR updates.
• cppalliance/claude-code-chat-browser#42: Both PRs modify session-parsing paths in search, sessions, and export_session; retrieved PR focuses on structured ErrorCode handling while main PR swaps from parse_session to get_cached_session.
• cppalliance/claude-code-chat-browser#44: Both PRs touch api/sessions.py's get_session_stats parsing and exclusion logic at the same function.

Suggested reviewers

• timon0305
• wpak-ai

Poem

🐇 Hoppin' through the cache with glee,
No more parsing files times three!
mtime changes? I'll know right away,
LRU evicts what shouldn't stay.
OrderedDict keeps the order neat —
warm cache hits make parsing sweet! 🥕

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR implements the core requirements from issue #82: session cache module with mtime-invalidated LRU eviction, thread safety, and integration into all parse-backed API modules.	Address critical race condition in mtime handling: capture mtime both before and after parsing, only caching when mtimes match to prevent stale data from concurrent file edits.
Docstring Coverage	⚠️ Warning	Docstring coverage is 12.90% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: adding an mtime-invalidated LRU session cache and routing APIs through it.
Out of Scope Changes check	✅ Passed	All changes align with issue #82 scope: session cache implementation, API integration, and benchmarks. No out-of-scope modifications detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/session-cache

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

tests/test_session_cache.py (1)

31-90: ⚡ Quick win

Add one test for relative-vs-absolute path aliasing.

The suite doesn’t yet assert the “absolute-path key” contract directly. Add a test that warms cache via a relative path and verifies absolute-path access is a hit (no reparse), so key normalization regressions are caught.

Suggested test

+def test_relative_and_absolute_paths_share_cache_entry(
+    sample_session: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    rel_path = os.path.relpath(sample_session)
+    abs_path = str(sample_session.resolve())
+
+    get_cached_session(rel_path)
+    calls = 0
+
+    def counting_parse(p: str):
+        nonlocal calls
+        calls += 1
+        return parse_session(p)
+
+    monkeypatch.setattr("utils.session_cache.parse_session", counting_parse)
+    get_cached_session(abs_path)
+    assert calls == 0

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_session_cache.py` around lines 31 - 90, Add a new test function to
validate the "absolute-path key" contract for path normalization. The test
should follow a similar pattern to test_cache_hit_avoids_reparse by using
monkeypatch to count parse_session calls, but instead of testing cache hits for
the same path, it should warm the cache with a relative path to the
sample_session file, then call get_cached_session with the absolute path of the
same file and verify that the parse_session call count remains zero, confirming
that relative and absolute paths resolve to the same cache key and prevent
unnecessary reparses.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/benchmarks/test_cache_bench.py`:
- Around line 21-23: The cold() benchmark function includes the clear_cache()
call within its body, which means the benchmark timing measures both
cache-clearing overhead and actual parsing time together. Move the clear_cache()
call outside of the cold() function so that it executes before the benchmark
measurement begins, leaving only the get_cached_session(path) call to be
measured. This ensures the cold benchmark metric reflects only the actual
cache-miss parse cost without the reset overhead, making cold-vs-warm
comparisons accurate.

In `@tests/test_session_cache.py`:
- Around line 54-56: The test uses a fixed sleep duration to force mtime
changes, which is flaky on filesystems with coarse mtime resolution. Replace the
time.sleep(0.05) call with an explicit mtime manipulation using os.utime() on
the session file at path to ensure a deterministic modification time change,
then call sample_session.touch() and verify the cached session is updated. This
removes the timing dependency and makes the test reliable across different
filesystem types.

In `@utils/session_cache.py`:
- Around line 43-49: The set_max_entries function does not validate the
max_entries parameter before use. If a negative value is passed, the while loop
condition while len(_cache) > _max_entries will perpetually evaluate to true
when the cache contains entries, causing the loop to continuously call popitem
until the cache is empty, at which point popitem raises a KeyError. Add input
validation at the beginning of the set_max_entries function to ensure
max_entries is a non-negative integer, and raise an appropriate exception (such
as ValueError) if an invalid value is provided.

---

Nitpick comments:
In `@tests/test_session_cache.py`:
- Around line 31-90: Add a new test function to validate the "absolute-path key"
contract for path normalization. The test should follow a similar pattern to
test_cache_hit_avoids_reparse by using monkeypatch to count parse_session calls,
but instead of testing cache hits for the same path, it should warm the cache
with a relative path to the sample_session file, then call get_cached_session
with the absolute path of the same file and verify that the parse_session call
count remains zero, confirming that relative and absolute paths resolve to the
same cache key and prevent unnecessary reparses.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 51b61091-441c-4eeb-ab7c-a6ff72001b94

📥 Commits

Reviewing files that changed from the base of the PR and between def3280 and 40cb91a.

📒 Files selected for processing (9)

• api/export_api.py
• api/projects.py
• api/search.py
• api/sessions.py
• tests/benchmarks/test_cache_bench.py
• tests/test_api_routes.py
• tests/test_error_propagation.py
• tests/test_session_cache.py
• utils/session_cache.py

[clean6378-max-it]

@CodeRabbit, please review this PR in more depth

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read all changed files
echo "=== utils/session_cache.py ==="
cat utils/session_cache.py

echo ""
echo "=== api/sessions.py ==="
cat api/sessions.py

echo ""
echo "=== api/search.py ==="
cat api/search.py

echo ""
echo "=== api/projects.py ==="
cat api/projects.py

echo ""
echo "=== api/export_api.py ==="
cat api/export_api.py

echo ""
echo "=== tests/test_session_cache.py ==="
cat tests/test_session_cache.py

echo ""
echo "=== tests/benchmarks/test_cache_bench.py ==="
cat tests/benchmarks/test_cache_bench.py

echo ""
echo "=== tests/test_api_routes.py (relevant section) ==="
grep -n "get_cached_session\|parse_session\|PARSE_ERROR\|500\|parse_failure" tests/test_api_routes.py | head -60

echo ""
echo "=== tests/test_error_propagation.py ==="
cat tests/test_error_propagation.py

echo ""
echo "=== utils/jsonl_parser.py (to understand parse_session signature) ==="
cat utils/jsonl_parser.py

echo ""
echo "=== utils/__init__.py or similar ==="
ls utils/

Length of output: 52141

---

---

@clean6378-max-it Here is an in-depth review of PR #90.

---

In-Depth Review: utils/session_cache.py & Consumers

🔴 Bug: Stale mtime Stored After Concurrent File Mutation

File: utils/session_cache.py, Lines 20–35

The mtime is captured before parse_session runs, then stored after it finishes. If the file is modified during parsing, the cache stores the pre-modification mtime alongside data that may be from either version of the file:

mtime = os.path.getmtime(abspath)          # ① mtime snapshot
with _lock:
    hit = _cache.get(abspath)
    if hit is not None and hit[0] == mtime:
        ...
parsed = parse_session(abspath)            # ② file may mutate here
with _lock:
    _cache[abspath] = (mtime, parsed)      # ③ old mtime stored with uncertain data

The safest fix is to re-read mtime after parsing and store that:

def get_cached_session(path: str) -> SessionDict:
    abspath = os.path.abspath(path)
    mtime_before = os.path.getmtime(abspath)
    with _lock:
        hit = _cache.get(abspath)
        if hit is not None and hit[0] == mtime_before:
            _cache.move_to_end(abspath)
            return hit[1]
    parsed = parse_session(abspath)
    mtime_after = os.path.getmtime(abspath)
    with _lock:
        # Only cache if the file was stable during the parse.
        # If mtime changed, a re-parse will happen on next access, which is correct.
        if mtime_after == mtime_before:
            _cache[abspath] = (mtime_after, parsed)
            _cache.move_to_end(abspath)
            while len(_cache) > _max_entries:
                _cache.popitem(last=False)
    return parsed

This avoids ever storing a stale mtime and also prevents the subtle case where two concurrent misses on the same path race to write.

---

🟡 Issue: Deferred Import Inside Route Handler in api/projects.py

File: api/projects.py, Line ~87

Every other consumer (api/sessions.py, api/search.py, api/export_api.py) imports get_cached_session at module level, but get_project_sessions defers it:

`@projects_bp.route`("/api/projects/<path:project_name>/sessions")
def get_project_sessions(project_name: str) -> FlaskReturn:
    ...
    # Add summary preview for each session
    from utils.session_cache import get_cached_session  # ← should be top-level

This adds a (cheap but unnecessary) import resolution on every request and is inconsistent with the rest of the module. Move it to the top of the file alongside the other imports.

---

🟡 Issue: set_max_entries(0) Creates a Write-Then-Evict No-Op Cache

File: utils/session_cache.py, set_max_entries / get_cached_session

set_max_entries(0) passes the >= 0 guard. Every subsequent get_cached_session call will:

1. Miss the cache (it's empty)
2. Parse the file
3. Store the result (len == 1 > 0)
4. Immediately evict it in the while loop

This silently turns the cache into a pass-through with extra overhead. The behaviour should either be documented explicitly or guarded:

def set_max_entries(max_entries: int) -> None:
    """Override the LRU capacity (primarily for tests).

    A value of 0 disables caching entirely — every access will parse from disk.
    """
    if max_entries < 0:
        raise ValueError(f"max_entries must be non-negative, got {max_entries}")

---

🟡 Test Quality: test_lru_eviction Instantiates pytest.MonkeyPatch Directly

File: tests/test_session_cache.py, Lines ~90–105

monkeypatch = pytest.MonkeyPatch()          # ← non-idiomatic
monkeypatch.setattr("utils.session_cache.parse_session", counting_parse)
try:
    ...
finally:
    monkeypatch.undo()

The monkeypatch fixture should be injected as a parameter, consistent with the other tests in the same file:

def test_lru_eviction(sample_session: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
    ...
    monkeypatch.setattr("utils.session_cache.parse_session", counting_parse)
    ...
    # no try/finally needed — pytest handles teardown

---

🟡 Test Quality: test_cache_invalidates_on_mtime_change Checks Object Identity, Not Re-Parse

File: tests/test_session_cache.py, Lines ~58–65

second = get_cached_session(path)
assert first is not second   # ← always True; parse_session always returns a new dict

This assertion is vacuously true because parse_session constructs a new dict each call. It doesn't verify that the cache actually missed and re-parsed. A more meaningful test:

def test_cache_invalidates_on_mtime_change(
    sample_session: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
    path = str(sample_session)
    get_cached_session(path)

    calls = 0
    original = parse_session

    def counting_parse(p: str):
        nonlocal calls
        calls += 1
        return original(p)

    monkeypatch.setattr("utils.session_cache.parse_session", counting_parse)

    stat = sample_session.stat()
    os.utime(sample_session, (stat.st_mtime + 1, stat.st_mtime + 1))
    get_cached_session(path)
    assert calls == 1, "Expected one re-parse after mtime change"

---

🟡 Design: Concurrent Same-File Cache Misses Cause Double Parsing

File: utils/session_cache.py

The PR intentionally releases the lock around parse_session to allow cross-file concurrency. However, two threads requesting the same path simultaneously will both miss the cache, both parse the file, and the second writer silently overwrites the first. This is safe (both parsed the same bytes) but wasteful. Consider noting it in the docstring:

def get_cached_session(path: str) -> SessionDict:
    """Return a parsed session, reusing the cache when mtime is unchanged.

    Concurrent requests for different paths proceed in parallel.
    Concurrent misses on the *same* path will each parse independently;
    the last writer wins. This is safe but may parse the file more than
    once under high concurrency for a cold key.
    """

---

🟡 Pre-existing: Outer Loop Continues After max_results in api/search.py

File: api/search.py, Lines ~42–50

Not introduced by this PR, but worth noting for a follow-up:

for project in projects:
    sessions = list_sessions(project["path"])       # called even when results full
    for sess_info in sessions:
        if len(results) >= max_results:
            break

After the inner break, the outer loop continues calling list_sessions() for every remaining project. Adding an outer guard would avoid redundant I/O:

for project in projects:
    if len(results) >= max_results:
        break
    ...

---

✅ What Works Well

• mtime-invalidated invalidation is a solid and simple correctness model.
• OrderedDict LRU is clean and idiomatic; no third-party dependency needed.
• Lock scope is correctly minimised — the lock is never held during parse_session.
• Module-level _PARSE_ERRORS tuple in api/sessions.py avoids repeating the exception list.
• test_cache_hit_avoids_reparse and test_cache_normalizes_relative_and_absolute_paths are well-written.
• Error propagation regression tests (test_error_propagation.py) are thorough and correctly updated to patch get_cached_session instead of parse_session.
• Benchmark fixture design (test_cache_bench.py) correctly uses setup=clear_cache for cold-parse timing.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@utils/session_cache.py`:
- Around line 35-37: The os.path.getmtime(abspath) call can raise an OSError if
the file is removed or renamed after parse_session(abspath) succeeds, causing
the request to fail even though valid parsed data already exists. Wrap the
os.path.getmtime(abspath) call in a try-except block that catches OSError, and
when caught, skip the cache write logic and return the parsed value directly
since it is already valid.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1ff3e6cd-426d-4de3-9cb0-e9731794299d

📥 Commits

Reviewing files that changed from the base of the PR and between abec1a4 and d8b87b0.

📒 Files selected for processing (5)

• api/projects.py
• api/search.py
• tests/test_error_propagation.py
• tests/test_session_cache.py
• utils/session_cache.py

🚧 Files skipped from review as they are similar to previous changes (3)

• api/projects.py
• tests/test_session_cache.py
• api/search.py