fix(#89): wire MCP recalc into companion invalidation, normalize recalc SSE shape, guard manifest-less roots (#128)

Three review findings on PR #128:

1. /internal/notify now handles kind="recalc" like project_reset — invalidate
   the result/compare LRUs and reload buckaroo sessions. An MCP-driven recalc
   (the primary path) arrives via _notify, so without this the companion kept
   serving stale viewer state that only the in-process /api/recalc route cleaned
   up (#80's communication-gap class).

2. Both recalc emitters publish through a single _recalc_sse_event(remap, step)
   helper, so the SSE event is the same {kind, remap, step} shape regardless of
   surface. The MCP _notify now forwards report.checkpoint_step too; the notify
   handler republishes normalized instead of the raw payload.model_dump() that
   buried remap under extra and dropped step.

3. recalc()'s root guard checks the manifest, not the directory. verdicts (and
   the cone) are keyed by manifest-bearing entries, so a dir-without-manifest
   root (a crashed mid-build leftover) used to enter the cone and KeyError the
   walk at verdicts[hash]; it is now dropped and the run reports clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: paddymul

src/tallyman_companion/app.py

@@ -234,6 +234,19 @@ def _invalidate_reset_caches() -> None:
     cached_result_expr.cache_clear()
 
 
+def _recalc_sse_event(remap: dict, step: int | None) -> dict:
+    """The recalc SSE event, in one canonical shape: ``{kind, remap, step}``.
+
+    Both recalc emitters publish through this — the in-process ``/api/recalc``
+    route and the cross-process ``/internal/notify`` path an MCP-driven recalc
+    arrives on — so the event a frontend sees can't depend on which surface
+    triggered it (the two used to drift: the notify path republished the raw
+    ``payload.model_dump()``, burying ``remap`` under ``extra`` and dropping
+    ``step``).
+    """
+    return {"kind": "recalc", "remap": remap, "step": step}
+
+
 def _annotate_entries(project: str, entries: list[dict]) -> list[dict]:
     """Add alias/version fields and sort: named entries (by alias) first, then scratch."""
     aliases = load_aliases(project)
@@ -1220,7 +1233,7 @@ async def api_recalc(project: str, payload: dict | None = None):
             _invalidate_reset_caches()
             if buckaroo:
                 buckaroo.reload_project_sessions(project)
-            await publish({"kind": "recalc", "remap": report.remap, "step": report.checkpoint_step})
+            await publish(_recalc_sse_event(report.remap, report.checkpoint_step))
         return asdict(report)
 
     # ------------------------------------------------------------------
@@ -1239,18 +1252,37 @@ async def notify(payload: NotifyPayload):
             payload.kind,
             getattr(payload, "hash", None),
         )
-        if payload.kind in ("post_processing_changed", "summary_stat_changed", "display_changed", "project_reset"):
-            if payload.kind == "project_reset":
-                # Out-of-process CLI `reset-to` ran reset_to in the CLI process, so
-                # this long-lived companion's result/compare LRUs are still warm and
-                # may point at snapshots the prune deleted. Clear them here, matching
-                # the in-server /api/reset path (#80). Not gated on `buckaroo`: the
-                # LRUs are process memory independent of the buckaroo subprocess.
+        if payload.kind in (
+            "post_processing_changed",
+            "summary_stat_changed",
+            "display_changed",
+            "project_reset",
+            "recalc",
+        ):
+            if payload.kind in ("project_reset", "recalc"):
+                # project_reset: out-of-process CLI `reset-to` ran reset_to in the
+                # CLI process, so this long-lived companion's result/compare LRUs
+                # are still warm and may point at snapshots the prune deleted.
+                # recalc: an MCP-driven catalog_recalc (arriving here via _notify,
+                # not the in-process /api/recalc route) re-pointed aliases and
+                # rebuilt entries. Either way, clear the LRUs and reload sessions —
+                # the same cleanup /api/recalc and /api/reset do — so the MCP path
+                # doesn't leave the viewer serving stale state (#80's communication
+                # gap). Not gated on `buckaroo`: the LRUs are process memory
+                # independent of the buckaroo subprocess.
                 _invalidate_reset_caches()
             if buckaroo:
                 reloaded = buckaroo.reload_project_sessions(project_name)
                 log.info("reloaded klasses in %d buckaroo session(s) for project %s", reloaded, project_name)
-        await publish(payload.model_dump())
+        # Republish a recalc in the canonical {kind, remap, step} shape so the
+        # cross-process notify path emits the identical event /api/recalc does;
+        # other kinds pass through their raw payload unchanged.
+        if payload.kind == "recalc":
+            extra = payload.extra or {}
+            event = _recalc_sse_event(extra.get("remap", {}), extra.get("step"))
+        else:
+            event = payload.model_dump()
+        await publish(event)
         return {"ok": True, "subscribers": len(subscribers), "project": project_name}
 
     @app.get("/api/projects")

src/tallyman_mcp/server.py

@@ -941,7 +941,10 @@ def catalog_recalc(roots: list[str] | None = None, dry_run: bool = True) -> dict
         }
     report = recalc(project, roots, dry_run=dry_run)
     if not dry_run and report.remap:
-        _notify("recalc", extra={"remap": report.remap})
+        # Forward both the remap and the checkpoint step so the companion can
+        # republish the same normalized {kind, remap, step} SSE event the
+        # in-process /api/recalc route emits (see app._recalc_sse_event).
+        _notify("recalc", extra={"remap": report.remap, "step": report.checkpoint_step})
     return asdict(report)
 
 

src/tallyman_xorq/recalc.py

@@ -42,7 +42,7 @@
 
 from tallyman_core import aliases
 from tallyman_core.entry_config import carry_forward_entry_config
-from tallyman_core.paths import entry_dir, project_dir
+from tallyman_core.paths import entry_dir, entry_manifest_path, project_dir
 from tallyman_xorq.build import BuildError, build_and_persist
 from tallyman_xorq.dependents import DependencyCycleError, build_dag, descendant_cone
 from tallyman_xorq.portable import PLACEHOLDER
@@ -125,9 +125,13 @@ def recalc(project: str, roots: list[str], *, dry_run: bool = False) -> RecalcRe
     ``reset_to`` undoes atomically. Stops at the first build failure (the prefix
     stays committed); never raises on a build error.
     """
-    # Drop roots that name no on-disk entry (a caller mistake) so the walk never
-    # indexes a missing verdict; an all-bogus root set yields an empty report.
-    roots = [r for r in roots if entry_dir(project, r).exists()]
+    # Drop roots that name no live entry (a caller mistake, or a crashed
+    # mid-build leftover dir) so the walk never indexes a missing verdict; an
+    # all-bogus root set yields an empty report. Gate on the manifest, not the
+    # directory: `verdicts` (and the cone) are keyed by manifest-bearing entries
+    # (scan/list_entries), so a dir-without-manifest root would otherwise enter
+    # the cone and KeyError the walk at verdicts[hash].
+    roots = [r for r in roots if entry_manifest_path(project, r).exists()]
     try:
         cone = descendant_cone(project, roots)
     except DependencyCycleError as exc: