diff --git a/api/models.py b/api/models.py index 022d02b3..52cc63cd 100644 --- a/api/models.py +++ b/api/models.py @@ -51,10 +51,10 @@ _CLI_SESSIONS_CACHE = {} # CLI-cache expiry, and on every sidebar poll, because the higher CLI cache is # keyed per active profile while the underlying transcripts never change between # switches. This cache memoizes the EXPENSIVE per-file parse result keyed by the -# file's (resolved path, mtime_ns, size); a warm sidebar build then re-stats the +# file's (path, mtime_ns, size, ctime_ns); a warm sidebar build then re-stats the # files (~4ms for 200) instead of re-parsing them. Any external edit/append to a -# transcript changes mtime_ns/size and transparently invalidates just that one -# file's entry. Bounded so a pathological projects tree can't grow it unbounded. +# transcript changes mtime_ns/size/ctime_ns and transparently invalidates just +# that one file's entry. Bounded so a pathological projects tree can't grow it unbounded. _CLAUDE_CODE_PARSE_CACHE_LOCK = threading.Lock() _CLAUDE_CODE_PARSE_CACHE: "collections.OrderedDict[tuple, tuple]" = collections.OrderedDict() _CLAUDE_CODE_PARSE_CACHE_MAX = 1000 @@ -3892,21 +3892,26 @@ def _parse_claude_code_jsonl(path: Path, *, max_messages: int = CLAUDE_CODE_MAX_ def _parse_claude_code_jsonl_cached( path: Path, *, max_messages: int = CLAUDE_CODE_MAX_MESSAGES_PER_FILE ) -> tuple[list[dict], str | None, float | None, float | None]: - """``_parse_claude_code_jsonl`` memoized by the file's (path, mtime_ns, size). + """``_parse_claude_code_jsonl`` memoized by the file's (path, mtime_ns, size, ctime_ns). The transcript files under ``~/.claude/projects`` are global and rarely change between sidebar builds, but parsing them dominates the cold /api/sessions latency (and repeats on every profile switch). Caching the parse result keyed by the file's stat signature collapses the warm cost to a single ``os.stat`` per file. A genuine append/edit bumps ``mtime_ns``/``size`` - and misses the cache, so staleness is impossible without re-parsing. + /``ctime_ns`` and misses the cache, so staleness is impossible without + re-parsing. ``max_messages`` is part of the key so a caller asking for a different cap never reads a result truncated to a smaller one. """ try: st = path.stat() - key = (str(path), st.st_mtime_ns, st.st_size, int(max_messages)) + # Key on mtime_ns + size + ctime_ns: size is the strong discriminator for + # append-only JSONL (any write changes it), and ctime_ns guards the rare + # same-size, same-mtime in-place edit so a content change can never serve + # a stale parse. A spurious ctime bump only costs one harmless re-parse. + key = (str(path), st.st_mtime_ns, st.st_size, st.st_ctime_ns, int(max_messages)) except OSError: # Can't stat -> fall back to a direct (uncached) parse; it will also # likely fail and return the empty tuple, matching prior behavior. @@ -4008,7 +4013,13 @@ def get_claude_code_sessions(projects_dir: Path | str | None = None, *, max_file if not messages: continue sid = _claude_code_session_id(path) - if first_ts is None and last_ts is None: + # Match the truthiness fallback used in the assignments below: the old + # inline code was ``first_ts or last_ts or path.stat().st_mtime``, which + # also fell back to mtime for a falsy-but-not-None ``0.0`` timestamp + # (epoch-0 / 1970 transcripts). An identity (``is None``) guard would + # leave those rows with ``None`` instead of the file mtime, so use the + # same ``not`` test the assignments use to stay bug-for-bug compatible. + if not first_ts and not last_ts: try: _mtime = path.stat().st_mtime except OSError: diff --git a/tests/test_issue4718_claude_code_parse_cache.py b/tests/test_issue4718_claude_code_parse_cache.py index 2ee14af1..30e00c1b 100644 --- a/tests/test_issue4718_claude_code_parse_cache.py +++ b/tests/test_issue4718_claude_code_parse_cache.py @@ -147,3 +147,98 @@ def test_get_claude_code_sessions_warm_uses_cache(tmp_path, monkeypatch): assert cold_calls == 3 # parsed each file once on the cold build assert calls["n"] == cold_calls # warm build added zero re-parses assert [s["title"] for s in cold] == [s["title"] for s in warm] + + +def test_epoch_zero_timestamps_fall_back_to_mtime(tmp_path): + """A transcript whose timestamps parse to 0.0 still gets a real mtime fallback. + + The cached row-builder guards the mtime fallback with ``not first_ts and not + last_ts`` so a falsy-but-not-None ``0.0`` timestamp (epoch-0 / 1970 + transcript) falls back to the file mtime, matching the pre-cache inline + ``first_ts or last_ts or path.stat().st_mtime``. An identity (``is None``) + guard would have left these rows with ``None``. + """ + import api.models as models + + models.clear_claude_code_parse_cache() + projects_dir = tmp_path / "claude" / "projects" + fixture = projects_dir / "p" / "s.jsonl" + # All message timestamps are epoch 0 -> _parse_claude_code_timestamp -> 0.0. + rows = [ + {"summary": "Epoch QA"}, + {"timestamp": "1970-01-01T00:00:00Z", "message": {"role": "user", "content": "hi"}}, + {"timestamp": "1970-01-01T00:00:00Z", "message": {"role": "assistant", "content": "ok"}}, + ] + _write_jsonl(fixture, rows) + + sessions = models.get_claude_code_sessions(projects_dir=projects_dir) + assert len(sessions) == 1 + s = sessions[0] + # Must NOT be None — falls back to the file mtime (a real positive float). + assert s["created_at"] is not None + assert s["updated_at"] is not None + assert s["last_message_at"] is not None + assert s["created_at"] > 0 + + +def test_parse_cache_dicts_are_read_only_contract(tmp_path): + """Pin the load-bearing invariant: per-message dicts are SHARED across hits. + + The cache returns a shallow ``list(messages)`` copy, so the per-message dicts + are shared between calls. Every production caller treats them as read-only; + this test documents that contract by proving the sharing exists — a future + caller that mutates a returned dict in place would corrupt the cache, and + this test makes that sharing explicit so such a change is a conscious one. + """ + import api.models as models + + models.clear_claude_code_parse_cache() + fixture = tmp_path / "claude" / "projects" / "p" / "s.jsonl" + _write_jsonl(fixture, _rows()) + + first_msgs, *_ = models._parse_claude_code_jsonl_cached(fixture) + second_msgs, *_ = models._parse_claude_code_jsonl_cached(fixture) + # List wrappers are distinct copies (append isolation, covered above)... + assert first_msgs is not second_msgs + # ...but the dict objects are shared (the read-only contract). If a future + # change deep-copies on read, update this assertion deliberately. + assert first_msgs[0] is second_msgs[0] + + +def test_parse_cache_invalidates_on_same_size_mtime_ctime_edit(tmp_path, monkeypatch): + """A same-size, same-mtime in-place edit still misses the cache via ctime_ns.""" + import os + import api.models as models + + models.clear_claude_code_parse_cache() + fixture = tmp_path / "claude" / "projects" / "p" / "s.jsonl" + _write_jsonl(fixture, _rows("aaaaa")) + + calls = {"n": 0} + real = models._parse_claude_code_jsonl + + def _counting(path, **kw): + calls["n"] += 1 + return real(path, **kw) + + monkeypatch.setattr(models, "_parse_claude_code_jsonl", _counting) + + first = models._parse_claude_code_jsonl_cached(fixture) + assert first[0][0]["content"] == "aaaaa" + st = fixture.stat() + + # Rewrite with same byte length and force the SAME mtime back; the os.utime + # call still stamps ctime with the current wall clock, so a real in-place + # edit moves ctime even when size+mtime are unchanged. Sleep so that ctime is + # measurably distinct from the original mtime_ns we restore. + time.sleep(0.02) + _write_jsonl(fixture, _rows("bbbbb")) + os.utime(fixture, ns=(st.st_atime_ns, st.st_mtime_ns)) + st2 = fixture.stat() + assert st2.st_size == st.st_size + assert st2.st_mtime_ns == st.st_mtime_ns + assert st2.st_ctime_ns != st.st_ctime_ns # only ctime moved (the edit signal) + + second = models._parse_claude_code_jsonl_cached(fixture) + assert calls["n"] == 2 # re-parsed despite identical size+mtime (ctime caught it) + assert second[0][0]["content"] == "bbbbb"