Skip to content

Commit 717c323

Browse files
paddymulclaude
andcommitted
fix(#80): clear companion LRUs on the cross-process reset notify path
A CLI `reset-to` runs `reset_to` (and the prune) in the short-lived CLI process, then signals the long-lived companion via `POST /internal/notify {kind: project_reset}`. That handler only reloaded buckaroo sessions; it never cleared the companion's process-global `_build_compare_expr` / `cached_result_expr` LRUs, so a warmed diff build kept pointing at a snapshot the prune deleted — #80's "Communication gap", the path #80 calls the normal one ("reset is normally run out-of-process"). - Hoist the in-server clear into `_invalidate_reset_caches()` and call it from BOTH reset paths (`/api/reset` and the `project_reset` branch of `/internal/notify`) so they can't drift. The clear is not gated on `buckaroo`: the LRUs are companion process memory, independent of the subprocess. - Reframe the `reset_to` comment (#96): clearing the result-plan memo answers #96's literal "LRU outlives a prune", but for the dangling-read symptom it is belt to cached_result_expr's per-call exists() self-heal (ee0a90a). The load-bearing protection is the `_build_compare_expr` clear — the one LRU that bakes the resolved path into a serialized build with no per-call recheck. Closes #80, #96. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent a965683 commit 717c323

2 files changed

Lines changed: 41 additions & 12 deletions

File tree

src/tallyman_companion/app.py

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,27 @@ def _build_compare_expr(project: str, a_hash: str, b_hash: str, keys: tuple[str,
212212
return build_path, overrides
213213

214214

215+
def _invalidate_reset_caches() -> None:
216+
"""Drop the companion's process-global result/compare caches after a reset.
217+
218+
A reset prunes baked snapshots (``catalog_state.reset_to`` →
219+
``prune_compute_cache``). ``_build_compare_expr`` serializes those snapshot
220+
paths into a build with no per-call ``exists()`` recheck, so a warmed diff
221+
pair would otherwise serve a build over a parquet the prune deleted —
222+
buckaroo reads zero files → empty compare grid (#80). ``cached_result_expr``
223+
self-heals on every read so clearing its memo is hygiene, not load-bearing,
224+
but we clear it for parity. Blunt global clear is correct and cheap: entries
225+
are content-addressed, so the next call rebuilds an identical expression.
226+
227+
Called from BOTH reset paths — the in-server ``/api/reset`` and the
228+
cross-process CLI ``reset-to`` that arrives as a ``project_reset`` on
229+
``/internal/notify`` — so the two can't drift. (The in-server path having the
230+
clear while the notify path lacked it was exactly #80's "Communication gap".)
231+
"""
232+
_build_compare_expr.cache_clear()
233+
cached_result_expr.cache_clear()
234+
235+
215236
def _annotate_entries(project: str, entries: list[dict]) -> list[dict]:
216237
"""Add alias/version fields and sort: named entries (by alias) first, then scratch."""
217238
aliases = load_aliases(project)
@@ -1135,12 +1156,10 @@ async def api_reset(project: str, payload: dict):
11351156
await run_in_threadpool(reset_to, project, ref)
11361157
except RuntimeError as exc:
11371158
raise HTTPException(404, str(exc))
1138-
# reset_to clears the result-plan memo; the compare-expr LRU is a
1139-
# companion-layer cache, so clear it here. It bakes each entry's snapshot
1140-
# path into a serialized build with no per-call exists() recheck, so a
1141-
# reset that pruned a snapshot would otherwise serve a build over a
1142-
# deleted parquet (#80). Cheap: a content-addressed pair rebuilds identically.
1143-
_build_compare_expr.cache_clear()
1159+
# reset_to ran in-process and cleared the result-plan memo; the
1160+
# compare-expr LRU is a companion-layer cache it can't reach, so invalidate
1161+
# the companion caches here (#80). Shared with the cross-process notify path.
1162+
_invalidate_reset_caches()
11441163
reloaded = buckaroo.reload_project_sessions(project) if buckaroo else 0
11451164
step = current_step(project)
11461165
await publish({"kind": "project_reset", "step": step})
@@ -1163,6 +1182,13 @@ async def notify(payload: NotifyPayload):
11631182
getattr(payload, "hash", None),
11641183
)
11651184
if payload.kind in ("post_processing_changed", "summary_stat_changed", "display_changed", "project_reset"):
1185+
if payload.kind == "project_reset":
1186+
# Out-of-process CLI `reset-to` ran reset_to in the CLI process, so
1187+
# this long-lived companion's result/compare LRUs are still warm and
1188+
# may point at snapshots the prune deleted. Clear them here, matching
1189+
# the in-server /api/reset path (#80). Not gated on `buckaroo`: the
1190+
# LRUs are process memory independent of the buckaroo subprocess.
1191+
_invalidate_reset_caches()
11661192
if buckaroo:
11671193
reloaded = buckaroo.reload_project_sessions(project_name)
11681194
log.info("reloaded klasses in %d buckaroo session(s) for project %s", reloaded, project_name)

src/tallyman_core/catalog_state.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -344,12 +344,15 @@ def reset_to(project: str, ref: int | str) -> None:
344344
restore_from_bullpen(project)
345345
_gc_cas_clones(project)
346346
catalog.assert_catalog_consistent(project, set(read_tallyman_state(project)["entry_hashes"]))
347-
# The prune above retires baked snapshots on disk; invalidate the in-process
348-
# plan memo that points at them (#80 / #96). cached_result_expr re-checks a
349-
# baked snapshot's existence on every call, but its memoised _resolve_result_plan
350-
# half caches the resolved path, and downstream caches that bake it in (the
351-
# companion's _build_compare_expr) have no such per-call recheck — so the reset
352-
# layer invalidates the memo rather than relying on the self-heal. Blunt global
347+
# The prune above retires baked snapshots on disk. Clear the in-process
348+
# result-plan memo that resolved their paths: #96's literal complaint is that
349+
# the LRU outlives the prune, so drop it. For the dangling-read *symptom* this
350+
# clear is belt-and-suspenders — cached_result_expr re-checks path.exists()
351+
# and self-heals on every read (ee0a90a), so its own reads already survive a
352+
# prune. The load-bearing protection against the symptom is the companion's
353+
# _build_compare_expr clear (#80): that is the one LRU that bakes the resolved
354+
# path into a serialized build with no per-call recheck, and it lives in the
355+
# companion layer (see app._invalidate_reset_caches), not here. Blunt global
353356
# clear is correct and cheap: entries are content-addressed, so the next read
354357
# rebuilds an identical plan. Lazy import avoids a core->xorq import cycle.
355358
from tallyman_xorq.result_cache import cached_result_expr # noqa: PLC0415

0 commit comments

Comments
 (0)