Dual-gate (Codex + Opus) findings on the Claude Code transcript parse cache:
1. Cache-key staleness hardening: add st_ctime_ns to the (path, mtime_ns, size,
max_messages) key so a rare same-size, same-mtime in-place edit can't serve a
stale parse. Size is already the strong discriminator for append-only JSONL;
ctime_ns is cheap defense-in-depth (a spurious bump costs one harmless re-parse,
never a stale read).
2. Timestamp-fallback bug-for-bug fix: the row-builder guard used identity
(first_ts is None and last_ts is None) but the field assignment uses truthiness
(first_ts or last_ts or _mtime). They disagreed on a falsy 0.0 timestamp
(epoch-0/1970 transcripts), leaving created_at/updated_at/last_message_at = None
instead of the file mtime the pre-cache inline code fell back to. Guard now uses
the same 'not first_ts and not last_ts' test.
Adds 3 regression tests: epoch-0 mtime fallback, the shared-dict read-only
contract (pins the load-bearing invariant), and same-size+mtime ctime invalidation.
The /api/sessions cold build (which a profile switch triggers) was
dominated by get_claude_code_sessions() re-parsing every Claude Code
JSONL transcript line-by-line on each call — 650-1000ms on a 200-file
/ ~130MB tree, repeated on every switch, every 5s CLI-cache expiry, and
every sidebar poll, because the directory is global but the higher
session cache is keyed per active profile.
Memoize the per-file parse by (path, mtime_ns, size) so a warm build
re-stats files instead of re-parsing them; any genuine edit/append
changes the stat signature and invalidates just that file. Also hoist
the loop-invariant get_last_workspace() out of the Claude Code row loop
(it stats config + probes terminal cwd, ~200 redundant calls per build).
Measured /api/sessions cold-rebuild on a 250-session profile: ~670-880ms
-> ~100-200ms (verified in-browser). Output byte-identical.
Credit @rodboev for the browser-level profiling that isolated the real
bottleneck (the prior pre-warm hypothesis could not move it).
Co-authored-by: rodboev <rodboev@users.noreply.github.com>
Codex CORE: the dismissed-set was keyed by bare approval_id, but gateway/run
sources preserve externally-supplied IDs without session namespacing — so two
sessions carrying the same approval_id collide and dismissing in session A hides
session B's still-pending approval. Fix: _approvalDismissKey(sid, approvalId)
builds a session-namespaced composite key, threaded through _isApprovalDismissed,
_markApprovalDismissed, _unmarkApprovalDismissed + all 5 call sites (showApprovalCard,
dismissApprovalCard, respondApproval, no-pending poll cleanup). Updated 4 stale
assertions to the namespaced signatures + added a functional node-driver
cross-session-collision regression (proven non-vacuous).
Co-authored-by: Rod Boev <rod.boev@gmail.com>
Codex reproduced a SILENT regression: the PR's 4096-BYTE read could miss the
continuation marker the old read_text()[:4096] (4096 CHARS) found. Session.save()
writes compression_anchor_summary (ensure_ascii=False) BEFORE parent_session_id,
so a multi-byte emoji summary puts the marker at char offset ~1230 but byte
offset ~4900 — within the old 4096-char window, past a 4096-byte one → continuation
child no longer detected. Fix: read 16384 bytes (4096 chars worst-case for 4-byte
UTF-8) then slice [:4096] chars, keeping the same needle + bounded I/O. Multibyte
regression test added + proven non-vacuous; updated the read-size assertion to <=16384.
Co-authored-by: starship-s <starship-s@users.noreply.github.com>
LLMs routinely emit JSON/YAML *fragments* in fenced blocks — a bare
`"key": "value"` line, snippets with `...`, trailing commas — which
legitimately fail JSON.parse. initTreeViews() stamped an unstyled
`<div class="tree-parse-note">parse failed</div>` below such blocks,
producing bare white text under an otherwise-clean code box.
The note was incidental, not designed: it was bolted onto the YAML
lazy-load commit (29a31a6e), has no CSS, no behavior test, and originally
even fired on *valid* YAML before the CDN script finished loading (later
fixed). The block still renders as syntax-highlighted raw, so the note
added only noise.
Remove the note insertion; fall through silently to raw view. The per-block
Raw/Tree toggle, YAML lazy-load, and the #484 default-view config are all
unchanged. The `parse_failed_note` i18n key is intentionally retained (kept
green by test_chinese_locale.py) to avoid touching all 13 locales.
Verification: ./scripts/test.sh tests/test_issue484_json_tree_viewer.py
tests/test_chinese_locale.py — 29 passed.
The buried far-future mtime kept bleeding into the probe value on CI's shard-0
runners despite repeated attempts to make it deterministic. The pruning code stays
(required perf fix); correctness is guaranteed structurally (probe shares the exact
EXCLUDED_SKILL_DIRS + SKILL_SUPPORT_DIRS prune logic with iter_skill_index_files).
Co-authored-by: Rod Boev <rod.boev@gmail.com>
CI shard-0 still flaked: float int(far_future*1e9) rounding left probe ~38ns above
the threshold because the legitimate vendored/ tree's mtime on CI tracked the
buried-tree creation. Now pin skills_dir/skill/SKILL.md/config.yaml to a known
modest mtime AFTER building the buried far-future node_modules subtree, and assert
probe < far_future - 1s. Deterministic across filesystems; still non-vacuous
(fails with prune disabled even when node_modules itself is stamped far-future).
Co-authored-by: Rod Boev <rod.boev@gmail.com>
The before/after assertion was fragile — creating node_modules between the two
probe calls bumped the (non-pruned) vendored/ ancestor mtime, so 'after==before'
failed on CI's filesystem timing. Rewrote to a timing-independent assertion: a
FAR-FUTURE mtime buried inside the pruned node_modules/ must NOT appear in the
probe value (probe < far_future_ns), which holds even if node_modules itself is
bumped, because the prune removes it from dirnames before the stat loop.
Co-authored-by: Rod Boev <rod.boev@gmail.com>
The pre-existing test asserted the OLD bug: 'add a skill within the 8s TTL ->
still get stale (1,1)'. That stale window is exactly what #4783 fixes — adding a
skill bumps the skills-dir mtime, the per-call probe catches it, and counts now
update immediately (2,2). Updated the assertions to the corrected contract +
added a no-change-serves-cache step. (Test lived in test_profile_skills_stats.py,
not the PR's test_issue4783 file, so the contributor missed it.)
Co-authored-by: Rod Boev <rod.boev@gmail.com>
Codex round-4 found my followlinks=True change introduced a perf regression: the
probe os.walk lacked the .git/.venv/node_modules/site-packages + skill-support-dir
pruning that iter_skill_index_files applies, so a skill vendoring a dependency
tree made this EVERY-CALL probe walk thousands of irrelevant files — defeating the
cache's whole purpose. The probe now prunes dirnames[:] with the same
EXCLUDED_SKILL_DIRS + SKILL_SUPPORT_DIRS sets as the compute path. Regression test
(change deep inside node_modules/ must not move the probe) added + non-vacuous.
Co-authored-by: Rod Boev <rod.boev@gmail.com>
Codex round-3 reproduced: _skill_tree_max_mtime_ns used os.walk(followlinks=False)
while the compute path iter_skill_index_files uses followlinks=True, so an edit to
a symlinked skill's target SKILL.md was invisible to the probe (stale up to TTL).
Added followlinks=True to the probe walk so it descends symlinked skill dirs and
sees target mtime changes. Regression test added + proven non-vacuous.
Co-authored-by: Rod Boev <rod.boev@gmail.com>
Codex re-gate: my prior 'TTL-expired + unchanged-mtime -> refresh TTL, no recompute'
branch could leave counts stale INDEFINITELY for an mtime-PRESERVING out-of-band
change (e.g. a git checkout restoring the old mtime) — worse than master's 8s
recompute. Codex proved it (cached (2,2) stayed (2,2) while a fresh compute returned
(1,2)). Now: within TTL + unchanged mtime serves cached (perf win kept); mtime
changed OR TTL expired both force a full recompute (the TTL is a genuine safety
net). Updated the row-2 test to assert the safety recompute.
Co-authored-by: Rod Boev <rod.boev@gmail.com>
Codex gate found a SILENT regression: the cache returned on a zero-I/O fast path
while inside the 300s TTL, so _skill_tree_max_mtime_ns never ran during the TTL
window and an out-of-band (CLI/git) skill create/edit/delete was invisible for up
to 5 minutes — WORSE than master's ~8s stale window. Restructured so the cheap
stat-only mtime probe runs on EVERY call (prompt change detection); the TTL now
only bounds the expensive SKILL.md recompute. Unchanged mtime still serves cached
+ refreshes TTL with no recompute (the perf win is preserved). Updated 2 tests
that encoded the flawed zero-I/O design + added a within-TTL-change regression
(proven non-vacuous).
Co-authored-by: Rod Boev <rod.boev@gmail.com>
nesquena-hermes