fix(cache): a read refuses a directory with no manifest instead of guessing its worthiness from the snapshot (#204)

src/tallyman_companion/app.py

@@ -49,7 +49,7 @@
     read_prompts,
 )
 from tallyman_xorq.primary_key import PrimaryKeySearchTimeout, diff_keys
-from tallyman_xorq.result_cache import cached_result_expr
+from tallyman_xorq.result_cache import cached_result_expr, entry_manifest
 from tallyman_xorq.row_order import page as row_order_page
 
 log = logging.getLogger("tallyman.companion")
@@ -215,14 +215,11 @@ def _snapshot_cache_path(project: str, content_hash: str):
     content hash and the manifest's ``cache_worthy``). It loads and writes
     nothing, so a worthy entry whose file was deleted reports an absent (0 B)
     snapshot rather than being forced to materialise just because someone opened
-    the metadata tab.
+    the metadata tab. An entry without a manifest raises (#204).
     """
-    try:
-        from tallyman_xorq.result_cache import baked_snapshot_path  # noqa: PLC0415
+    from tallyman_xorq.result_cache import baked_snapshot_path  # noqa: PLC0415
 
-        path = baked_snapshot_path(project, content_hash)
-    except Exception as exc:  # noqa: BLE001 — best-effort sizing, never 500 the tab
-        return True, None, f"snapshot path unresolved: {type(exc).__name__}"
+    path = baked_snapshot_path(project, content_hash)
     if path is None:
         return False, None, "cheap entry — a small plan over files that exist, no snapshot"
     return True, path, ""
@@ -334,12 +331,9 @@ def add(key: str, label: str, path: Path, kind: str, reclaimable: bool, detail:
     diff_cache_bytes = sum(d["bytes"] for d in diff_caches)
 
     total_bytes = cache_bytes + artifact_bytes + diff_cache_bytes
-    try:
-        from tallyman_xorq.result_cache import cache_worthy as _cw  # noqa: PLC0415
+    from tallyman_xorq.result_cache import cache_worthy as _cw  # noqa: PLC0415
 
-        worthy = _cw(project, content_hash)
-    except Exception:  # noqa: BLE001
-        worthy = False
+    worthy = _cw(project, content_hash)
 
     # Enrichment (#134): the recipe's raw source files, last-modified, the
     # cheap/expensive reason, and clickable lineage (parents + dependent
@@ -349,11 +343,7 @@ def add(key: str, label: str, path: Path, kind: str, reclaimable: bool, detail:
 
     from tallyman_xorq.dependents import dependents_index, parents_of  # noqa: PLC0415
 
-    manifest: dict = {}
-    try:
-        manifest = json.loads((entry / "manifest.json").read_text())
-    except (OSError, ValueError):
-        pass
+    manifest = json.loads((entry / "manifest.json").read_text())
 
     # The raw bytes this entry holds, which is a question only a source version has an answer to
     # (ADR-011 D6: a computed entry reads aliases, never a file). For one it is the clone of the
@@ -930,13 +920,9 @@ def api_data(project: str, content_hash: str, offset: int = 0, limit: int = 200)
         # the user's keys and then ``__row_order``, so the same request returns the same rows in any process and any
         # cache state. cached_result_expr hands back the result as a live single-backend expression over files that
         # exist (ensure_materialized ran first), so the window pushes down and nothing is written here. total is the
-        # manifest's row_count (recorded at build; api_entry_detail reads it the same way). The manifest is written
-        # after the build dir exists (build.py), so guard the read: a half-built or pruned entry still serves its page
-        # with a best-effort total of 0 rather than 500ing on a missing manifest.
-        manifest_path = entry_dir(project, content_hash) / ENTRY_MANIFEST_FILENAME
-        total = 0
-        if manifest_path.exists():
-            total = json.loads(manifest_path.read_text()).get("row_count") or 0
+        # manifest's row_count, recorded at build. An entry directory without a manifest is corrupt, and this raises
+        # (#204).
+        total = entry_manifest(project, content_hash).row_count or 0
         # The page runs on the process's shared backend, one execution at a time (#118). cached_result_expr may heal,
         # and a heal takes the project lock, which comes before the execution lock, so it runs first.
         expr = row_order_page(cached_result_expr(project, content_hash), offset=offset, limit=limit)
@@ -1145,11 +1131,10 @@ def api_notebook_full(project: str):
             buckaroo_session = None
             if latest is not None:
                 entry = entry_dir(project, latest)
-                if (entry / ENTRY_MANIFEST_FILENAME).exists():
-                    entry_meta = json.loads((entry / ENTRY_MANIFEST_FILENAME).read_text())
-                    schema = json.loads((entry / ENTRY_SCHEMA_FILENAME).read_text())
-                    # #73: row count from the manifest, not a result.parquet.
-                    total_rows = entry_meta.get("row_count", 0)
+                entry_meta = json.loads((entry / ENTRY_MANIFEST_FILENAME).read_text())
+                schema = json.loads((entry / ENTRY_SCHEMA_FILENAME).read_text())
+                # #73: row count from the manifest, not a result.parquet.
+                total_rows = entry_meta.get("row_count", 0)
                 chart_spec = get_chart(project, latest)
                 if buckaroo_available:
                     buckaroo_session = buckaroo.ensure_session(latest, project)

src/tallyman_companion/buckaroo_lifecycle.py

@@ -54,8 +54,7 @@
     entry_stat_cache_dir,
     entry_view_build_dir,
 )
-from tallyman_core.manifest import read_manifest
-from tallyman_core.paths import artifacts_dir, entry_manifest_path, project_dir
+from tallyman_core.paths import artifacts_dir, project_dir
 from tallyman_xorq.row_order import ROW_ORDER
 
 log = logging.getLogger("tallyman.buckaroo")
@@ -480,13 +479,9 @@ def ensure_session(
         return self.load_session(content_hash, project, column_config_overrides)["session_id"]
 
     def _load_timeout(self, project: str, content_hash: str) -> float:
-        row_count = 0
-        mpath = entry_manifest_path(project, content_hash)
-        if mpath.exists():
-            try:
-                row_count = read_manifest(mpath.parent).row_count or 0
-            except Exception:
-                pass
+        from tallyman_xorq.result_cache import entry_manifest  # noqa: PLC0415
+
+        row_count = entry_manifest(project, content_hash).row_count or 0
         return 10.0 + row_count / 1_000_000
 
     def _load_body(self, project: str, content_hash: str, column_config_overrides: dict | None) -> dict:

src/tallyman_xorq/materialize.py

@@ -408,6 +408,8 @@ def ensure_materialized(project: str, content_hash: str) -> None:
 
     Every caller that composes or executes an entry goes through here, so nothing ever runs over a file that is
     missing. When nothing can make a file again, the error names the source file.
+
+    An entry directory without a manifest is corrupt, and this raises before anything is loaded or written (#204).
     """
     _ensure(project, content_hash)
 

src/tallyman_xorq/result_cache.py

@@ -7,7 +7,8 @@
 is a hard error (ADR-006 D6), never a fallback.
 
 There is no xorq cache node in any build (ADR-007 D1). Whether an entry has a file of its own is one recorded fact,
-``manifest.cache_worthy``, decided once at build by ``worthiness.classify_expr`` (ADR-008 D4):
+``manifest.cache_worthy``, decided once at build by ``worthiness.classify_expr`` (ADR-008 D4). An entry directory
+without a manifest is corrupt, and reading it is an error (``entry_manifest``, #204):
 
   * **Worthy** entries do work that is expensive or that cannot inherit a row order (an aggregate, join, sort, window
     function, UDF, union). Tallyman materializes them: ``materialize`` writes
@@ -30,29 +31,41 @@
 import sys
 import time
 from pathlib import Path
-from typing import NamedTuple
+from typing import TYPE_CHECKING, NamedTuple
+
+if TYPE_CHECKING:
+    from tallyman_core.manifest import Manifest
 
 # Perf instrumentation rides a dedicated child namespace so it can be dialed up
 # independently of the rest of tallyman's logging (#60), via TALLYMAN_LOG_LEVEL.
 perf_log = logging.getLogger("tallyman.perf")
 
 
-def cache_worthy(project: str, content_hash: str) -> bool:
-    """Whether the entry is materialized, read from its manifest: the verdict recorded at build (ADR-008 D4).
+def entry_manifest(project: str, content_hash: str) -> Manifest:
+    """The entry's manifest. Reading an entry without one is an error (#204).
 
-    Nothing re-derives it: the manifest is the record, and ``expr.yaml`` is never parsed to work it out. An entry whose
-    manifest is gone (half-built, or pruned) still has its build and may still serve a page (#90), so a snapshot on disk
-    stands in for the verdict: it can only have been written for a worthy entry.
+    The manifest holds what a read needs: the worthy-or-cheap verdict, the digest a heal is checked against, the pin,
+    and a source version's provenance. An entry directory without one is corrupt, and a hash with no directory names
+    no entry. Nothing works around either one.
     """
     from tallyman_core import read_manifest
     from tallyman_core.paths import entry_dir
+    from tallyman_xorq.build import BuildError
 
+    path = entry_dir(project, content_hash)
     try:
-        return bool(read_manifest(entry_dir(project, content_hash)).cache_worthy)
-    except FileNotFoundError:
-        from tallyman_xorq.materialize import snapshot_path
+        return read_manifest(path)
+    except FileNotFoundError as exc:
+        raise BuildError(f"entry {content_hash} in {project!r} has no manifest.json: {path}") from exc
 
-        return snapshot_path(project, content_hash).exists()
+
+def cache_worthy(project: str, content_hash: str) -> bool:
+    """Whether the entry is materialized, read from its manifest: the verdict recorded at build (ADR-008 D4).
+
+    Nothing re-derives it and nothing stands in for it: the manifest is the record, ``expr.yaml`` is never parsed to
+    work it out, and a file at the snapshot path says nothing about it.
+    """
+    return bool(entry_manifest(project, content_hash).cache_worthy)
 
 
 # Entries currently being reconstructed by cached_result_expr, on this call

tests/test_companion.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import pytest
 from fastapi.testclient import TestClient
 
 from tallyman_core import entry_dir
@@ -170,16 +171,15 @@ def test_api_data_cheap_entry_paginates_consistently_across_pages(
     assert page2 == full[50:100]
 
 
-def test_api_data_missing_manifest_serves_page_without_500(
-    fresh_companion_app, project: str, orders_src: str, monkeypatch
+@pytest.mark.parametrize("route", ["data", "entry", "entry_cache"])
+def test_entry_routes_error_on_an_entry_with_no_manifest(
+    fresh_companion_app, project: str, orders_src: str, monkeypatch, route: str
 ):
-    """#90: a missing manifest must not 500 the row read.
+    """#204, reversing #90, which served the row read with a total of 0 here.
 
-    The build dir is written before manifest.json (build.py creates the dir, then
-    writes the manifest after executing), so a half-built or pruned entry can pass
-    the build-dir check yet have no manifest. cached_result_expr needs none — only
-    ``total`` reads it — so the read must be guarded: serve the page with a
-    best-effort total rather than raising FileNotFoundError on the manifest read.
+    An entry directory without a manifest is corrupt. Reading it is a server error: no route answers around it, and
+    none maps it to something softer than a 500. For a worthy entry whose snapshot was gone too, the row read used to
+    re-run its aggregate as if it were cheap.
     """
     monkeypatch.setenv("TALLYMAN_PROJECT", project)
     code = f"""
@@ -188,15 +188,11 @@ def test_api_data_missing_manifest_serves_page_without_500(
 expr = t.select("region", "price", "__row_order")
 """
     h = build_and_persist(project, code, prompt="cols").content_hash
-    (entry_dir(project, h) / "manifest.json").unlink()  # half-built / pruned entry
+    (entry_dir(project, h) / "manifest.json").unlink()
 
-    c = TestClient(fresh_companion_app)
-    r = c.get(f"/{project}/api/data/{h}?offset=0&limit=10")
-    assert r.status_code == 200  # served off the expression, not the manifest
-    body = r.json()
-    assert len(body["data"]) == 10
-    assert set(body["data"][0]) == {"region", "price", "__row_order"}
-    assert body["total"] == 0  # no manifest → best-effort total, not a crash
+    c = TestClient(fresh_companion_app, raise_server_exceptions=False)
+    r = c.get(f"/{project}/api/{route}/{h}")
+    assert r.status_code == 500, (r.status_code, r.text[:500])
 
 
 def test_entry_detail_sidebar_lists_all_entries_with_current_highlighted(

tests/test_materialize.py

@@ -372,15 +372,16 @@ def test_a_failed_first_create_leaves_no_snapshot_and_no_temp_file(project, orde
 
 def test_a_failed_retry_of_a_half_built_entry_keeps_its_snapshot(project, orders_src, monkeypatch):
     """ADR-007 D4, #193. A build killed after it made the entry directory and before it wrote the manifest leaves a
-    directory with no manifest. A snapshot already at the path, such as one a reset left, is still served
-    (``cache_worthy`` falls back to the file). A retry is a create, and one that fails removes the half-built directory
-    and leaves the snapshot as it was."""
+    directory with no manifest. That directory is not an entry (ADR-007 D6), so a read refuses it even with a snapshot
+    already at the path, such as one a reset left (#204). A retry is a create, and one that fails removes the
+    half-built directory and leaves the snapshot as it was."""
     code = _agg_code(project)
     h = build_and_persist(project, code).content_hash
     snap, digest = snapshot_path(project, h), _digest_of(project, h)
     (entry_dir(project, h) / "manifest.json").unlink()
     cached_result_expr.cache_clear()
-    assert len(cached_result_expr(project, h).execute()) > 0
+    with pytest.raises(BuildError, match="has no manifest.json"):
+        cached_result_expr(project, h)
 
     _disk_fills_at(monkeypatch, "first run")
     with pytest.raises(BuildError, match="No space left on device"):
@@ -409,6 +410,61 @@ def test_ensure_materialized_rewrites_a_missing_snapshot_and_verifies_it(project
     assert snapshot_file_digest(snap) == recorded
 
 
+@pytest.mark.parametrize("kind", ["worthy", "cheap"])
+def test_reading_an_entry_that_lost_its_manifest_is_an_error_until_its_recipe_runs_again(project, orders_src, kind):
+    """#204. The manifest holds what a read needs: the worthy-or-cheap verdict, and the digest a heal is checked
+    against. An entry directory without one is corrupt, so a read raises before it loads or writes anything, for a
+    cheap entry as for a worthy one. A worthy entry that had also lost its snapshot used to be read as cheap: every
+    read re-ran its aggregate and nothing made the file again. Running the recipe again by hand writes the entry
+    again, under the same hash."""
+    from tallyman_xorq.materialize import ensure_materialized
+
+    code = _agg_code(project) if kind == "worthy" else _root_code(project)
+    built = build_and_persist(project, code)
+    h = built.content_hash
+    snap = snapshot_path(project, h)
+    (entry_dir(project, h) / "manifest.json").unlink()
+    snap.unlink(missing_ok=True)
+    cached_result_expr.cache_clear()
+
+    for read in (cached_result_expr, ensure_materialized):
+        with pytest.raises(BuildError) as info:
+            read(project, h)
+        assert str(info.value) == f"entry {h} in {project!r} has no manifest.json: {entry_dir(project, h)}"
+    assert not snap.exists()
+
+    assert build_and_persist(project, code).content_hash == h
+    assert read_manifest(entry_dir(project, h)).cache_worthy is (kind == "worthy")
+    assert snap.exists() is (kind == "worthy")
+    assert len(cached_result_expr(project, h).execute()) == built.row_count
+
+
+def test_a_child_of_an_entry_that_lost_its_manifest_raises_that_entrys_error(project, orders_src):
+    """#204. A worthy entry that lost its manifest and its snapshot used to look cheap. A child that reads its snapshot
+    asked for the file, nothing wrote it, and the read failed with "still missing after it was made again"; a new
+    child built over it inlined the aggregate as if it were cheap. Both raise now, with the parent's own error:
+    nothing on the way up catches it or adds to it."""
+    agg = _hash(catalog_create("agg", _agg_code(project)))
+    child = _hash(catalog_create("share", _cheap_child_code("agg")))
+    (entry_dir(project, agg) / "manifest.json").unlink()
+    snapshot_path(project, agg).unlink()
+    cached_result_expr.cache_clear()
+
+    with pytest.raises(BuildError) as info:
+        cached_result_expr(project, child)
+    assert str(info.value) == f"entry {agg} in {project!r} has no manifest.json: {entry_dir(project, agg)}"
+
+    res = catalog_create(
+        "busy",
+        """
+from tallyman_xorq.io import tracked_expr_from_alias
+t = tracked_expr_from_alias("agg")
+expr = t.filter(t.n > 1)
+""",
+    )
+    assert "has no manifest.json" in res.get("error", ""), res
+
+
 def test_files_exist_before_anything_runs(project, orders_src, monkeypatch):
     """ADR-007 D5: with an ancestor's snapshot deleted, opening a descendant rewrites the ancestor first, so no plan
     is ever executed over a file that is missing. This is #76's reproduction."""

tests/test_notebook.py

@@ -234,6 +234,23 @@ def test_notebook_route_renders_cells(fresh_companion_app, project: str, orders_
     assert cells[0]["entry_meta"]["prompt"] == "region totals"
 
 
+def test_notebook_route_errors_when_a_cells_entry_has_no_manifest(
+    fresh_companion_app, project: str, orders_src: str, monkeypatch
+):
+    """#204: an entry directory without a manifest is corrupt, and the notebook reads the head entry of every cell.
+    The cell used to render with no metadata and a row count of 0."""
+    from tallyman_core import entry_dir
+    from tallyman_core.aliases import get_alias
+
+    monkeypatch.setenv("TALLYMAN_PROJECT", project)
+    catalog_create("shoe_sales", _agg_code(project), prompt="region totals")
+    (entry_dir(project, get_alias(project, "shoe_sales")) / "manifest.json").unlink()
+
+    c = TestClient(fresh_companion_app, raise_server_exceptions=False)
+    r = c.get(f"/{project}/api/notebook_full")
+    assert r.status_code == 500, (r.status_code, r.text[:500])
+
+
 def test_api_notebook_reorder(fresh_companion_app, project: str, orders_src: str, monkeypatch):
     monkeypatch.setenv("TALLYMAN_PROJECT", project)
     catalog_create("a", _agg_code(project))