Skip to content

Commit d8b87b0

Browse files
fix: harden session cache mtime handling and address PR #90 review
Re-read mtime after parse and only cache when the file was stable during the read; skip cache writes when max_entries is 0. Move get_cached_session to module-level import in projects.py; improve mtime/LRU tests; document concurrent cold-miss behavior. stop search scan after max_results across projects
1 parent abec1a4 commit d8b87b0

5 files changed

Lines changed: 48 additions & 29 deletions

File tree

api/projects.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from models.project import ProjectSessionRowDict, SessionListItemDict
88
from models.session import SessionDict
99
from utils.exclusion_rules import is_session_excluded
10+
from utils.session_cache import get_cached_session
1011
from utils.session_path import get_claude_projects_dir, list_projects, list_sessions, safe_join
1112

1213
projects_bp = Blueprint("projects", __name__)
@@ -80,9 +81,6 @@ def get_project_sessions(project_name: str) -> FlaskReturn:
8081
except ValueError:
8182
return error_response(ErrorCode.INVALID_PATH, "Invalid path", 400)
8283
sessions = list_sessions(project_dir)
83-
# Add summary preview for each session
84-
from utils.session_cache import get_cached_session
85-
8684
rules = current_app.config.get("EXCLUSION_RULES") or []
8785
result: list[ProjectSessionRowDict] = []
8886
for s in sessions:

api/search.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ def search() -> FlaskReturn:
4949
rules = current_app.config.get("EXCLUSION_RULES") or []
5050
results: list[SearchHitDict] = []
5151
for project in projects:
52+
if len(results) >= max_results:
53+
break
5254
sessions = list_sessions(project["path"])
5355
for sess_info in sessions:
5456
if len(results) >= max_results:

tests/test_error_propagation.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,9 +179,7 @@ def test_per_session_error_card_omits_error_detail(self, tmp_path, client, monke
179179
def _boom(*args, **kwargs):
180180
raise KeyError("internal_secret_field_id")
181181

182-
# api/projects.py imports parse_session inside the handler body,
183-
# so patch the source module rather than the consumer.
184-
monkeypatch.setattr("utils.session_cache.get_cached_session", _boom)
182+
monkeypatch.setattr("api.projects.get_cached_session", _boom)
185183

186184
resp = client.get("/api/projects/myproj/sessions")
187185
# Pin the response shape so a future wrapper change (e.g. {"sessions": [...]})

tests/test_session_cache.py

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,25 @@ def counting_parse(p: str):
4848
assert calls == 0
4949

5050

51-
def test_cache_invalidates_on_mtime_change(sample_session: Path) -> None:
51+
def test_cache_invalidates_on_mtime_change(
52+
sample_session: Path, monkeypatch: pytest.MonkeyPatch
53+
) -> None:
5254
path = str(sample_session)
53-
first = get_cached_session(path)
55+
get_cached_session(path)
56+
57+
calls = 0
58+
59+
def counting_parse(p: str):
60+
nonlocal calls
61+
calls += 1
62+
return parse_session(p)
63+
64+
monkeypatch.setattr("utils.session_cache.parse_session", counting_parse)
65+
5466
stat = sample_session.stat()
5567
os.utime(sample_session, (stat.st_mtime + 1, stat.st_mtime + 1))
56-
second = get_cached_session(path)
57-
assert first is not second
68+
get_cached_session(path)
69+
assert calls == 1
5870

5971

6072
def test_cache_normalizes_relative_and_absolute_paths(
@@ -83,7 +95,9 @@ def counting_parse(p: str):
8395
os.chdir(original_cwd)
8496

8597

86-
def test_lru_eviction(sample_session: Path, tmp_path: Path) -> None:
98+
def test_lru_eviction(
99+
sample_session: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
100+
) -> None:
87101
set_max_entries(2)
88102
content = sample_session.read_text(encoding="utf-8")
89103
paths = []
@@ -102,17 +116,13 @@ def counting_parse(p: str):
102116
calls += 1
103117
return parse_session(p)
104118

105-
monkeypatch = pytest.MonkeyPatch()
106119
monkeypatch.setattr("utils.session_cache.parse_session", counting_parse)
107-
try:
108-
get_cached_session(str(paths[2]))
109-
assert calls == 0
110-
get_cached_session(str(paths[1]))
111-
assert calls == 0
112-
get_cached_session(str(paths[0]))
113-
assert calls == 1
114-
finally:
115-
monkeypatch.undo()
120+
get_cached_session(str(paths[2]))
121+
assert calls == 0
122+
get_cached_session(str(paths[1]))
123+
assert calls == 0
124+
get_cached_session(str(paths[0]))
125+
assert calls == 1
116126

117127

118128
def test_set_max_entries_rejects_negative() -> None:

utils/session_cache.py

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,28 @@
1717

1818

1919
def get_cached_session(path: str) -> SessionDict:
20-
"""Return a parsed session, reusing the cache when mtime is unchanged."""
20+
"""Return a parsed session, reusing the cache when mtime is unchanged.
21+
22+
Concurrent requests for different paths proceed in parallel.
23+
Concurrent misses on the *same* path will each parse independently;
24+
the last writer wins. This is safe but may parse the file more than
25+
once under high concurrency for a cold key.
26+
"""
2127
abspath = os.path.abspath(path)
22-
mtime = os.path.getmtime(abspath)
28+
mtime_before = os.path.getmtime(abspath)
2329
with _lock:
2430
hit = _cache.get(abspath)
25-
if hit is not None and hit[0] == mtime:
31+
if hit is not None and hit[0] == mtime_before:
2632
_cache.move_to_end(abspath)
2733
return hit[1]
2834
parsed = parse_session(abspath)
35+
mtime_after = os.path.getmtime(abspath)
2936
with _lock:
30-
_cache[abspath] = (mtime, parsed)
31-
_cache.move_to_end(abspath)
32-
while len(_cache) > _max_entries:
33-
_cache.popitem(last=False)
37+
if _max_entries > 0 and mtime_after == mtime_before:
38+
_cache[abspath] = (mtime_after, parsed)
39+
_cache.move_to_end(abspath)
40+
while len(_cache) > _max_entries:
41+
_cache.popitem(last=False)
3442
return parsed
3543

3644

@@ -41,7 +49,10 @@ def clear_cache() -> None:
4149

4250

4351
def set_max_entries(max_entries: int) -> None:
44-
"""Override the LRU capacity (primarily for tests)."""
52+
"""Override the LRU capacity (primarily for tests).
53+
54+
A value of 0 disables caching entirely — every access will parse from disk.
55+
"""
4556
if max_entries < 0:
4657
raise ValueError(f"max_entries must be non-negative, got {max_entries}")
4758
global _max_entries

0 commit comments

Comments
 (0)