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

[paddymul]

Stacked on #189: the base is feat/adr-007-009-cache-redesign, not main.

Fixes #204

This closes the issue only once #189 reaches main.

Terms: a worthy entry is one tallyman materializes. Its rows are written once to its snapshot, compute_cache/result_cache/<content_hash>.parquet. A cheap entry has no file of its own and re-runs its small plan on every read. The verdict is decided once, at build, by worthiness.classify_expr, and recorded in the entry's manifest (ADR-008 D4). A heal re-creates a missing snapshot and checks it against the manifest's result_digest (ADR-007 D5).

Problem

cache_worthy (result_cache.py:40-55) read the manifest and, when it was missing, returned whether the snapshot existed. On 1f8cb02, with a probe and then the tests below:

• A worthy aggregate with its manifest and snapshot deleted was reported cheap. cached_result_expr returned its loaded build (the canonical Sort over the Aggregate), so every read re-ran the aggregate, and nothing wrote the snapshot again.
• A cheap child of it failed with "still missing after it was made again" (materialize.py:296). A new child built over it inlined the aggregate as if it were cheap.
• A source version with its manifest and snapshot deleted raised RecursionError. Its build reads its own snapshot, and with no manifest it had no provenance to heal from, so _recreate recursed on it.
• The fallback also worked the other way. A cheap entry with any file at its snapshot path was reported worthy, and a hash whose directory a reset had retired was served from the snapshot the reset left.
• /api/data served a manifest-less entry's rows with a total of 0 (fix(api): guard api_data's manifest read so a missing manifest doesn't 500 (#90) #95), and /api/entry and /api/entry_cache answered 500 on the manifest read.

The choice: reading an entry with no manifest is an error

An entry directory without a manifest is corrupt. Reading one raises a single plain error, and nothing in the read path catches it, softens it, or tells the caller how to repair it. The issue suggested classifying the loaded build when the manifest is missing. This PR doesn't, for these reasons:

1. The verdict is one of four things a read needs from the manifest. Without result_digest a heal cannot be checked (_verify_self_heal returns early when nothing is recorded), so a healed snapshot would be served unverified, which ADR-007 D5 rules out. Without reproducible and unfaithful_heal_digest there is no pin. Without provenance a source version cannot be healed from its clone, which is the RecursionError above. Recomputing the verdict would fix the first and leave the other three.
2. Recomputing re-derives what ADR-008 D4 decides once. It would have worked for the boolean: on 1f8cb02, classify_expr(load_entry_expr(...)) agreed with the recorded verdict for an aggregate, a filter, a cheap child, random(), now(), row_number() and a source version. It would still be a second place that decides the verdict.

The first version of this PR raised a NotAnEntryError whose message explained the state and named a rebuild (catalog_run with the entry's expr.py, or a re-import). The three single-entry routes mapped it to a 404, and _recreate prefixed the child's hash. Review showed the advice was wrong in several cases: it would overwrite a pinned snapshot, it loops for a recipe that reads its own alias, and it builds a different hash once a parent alias has moved on. All of that is gone. The error says what is missing and where:

entry 97cf611c6713 in 'test' has no manifest.json: <home>/projects/test/artifacts/catalog/entries/97cf611c6713

Fix

• result_cache.entry_manifest(project, hash) returns the manifest, or raises a BuildError with the message above when manifest.json is not there. A hash with no directory at all gets the same message.
• cache_worthy returns the manifest's verdict through entry_manifest, and nothing else. It never looks at the snapshot path. ensure_materialized, cached_result_expr, baked_snapshot_path, load_session and primary-key inheritance all go through it, so they raise before loading or writing anything.
• A child whose parent has no manifest raises the parent's error unchanged. _recreate does not catch or rewrap it.
• /api/data, /api/entry, /api/entry_cache and /api/notebook_full answer 500. There is no 404 mapping. /api/data no longer serves the page with a total of 0, which reverses what fix(api): guard api_data's manifest read so a missing manifest doesn't 500 (#90) #95 did for this case.
• These guards around a missing manifest are removed: _snapshot_cache_path's catch-all, _compute_entry_cache's worthy=False and manifest={} fallbacks, the notebook cell's manifest-exists check (which rendered the cell with no metadata and 0 rows), and _load_timeout's exists check and bare except (which used 0 rows).

Unchanged: enumeration still skips a directory without a manifest (list_entries, the checkpoint, catalog_state._retire, recalc roots). That skip defines what is listed as an entry, and it has to pass over a build that is still running. load_entry_expr and materialize don't read the manifest, since a create calls them before it writes one. The startup warm-up catches errors per entry, so it logs the one-line error for a manifest-less directory. No new .execute() or to_pyarrow_batches() site is added in src/ (for #118).

Tests

First red commit, d45bee8, all failing on 1f8cb02 (names as 67648c0 left them):

• test_result_cache.py::test_cache_worthy_errors_on_an_entry_with_no_manifest, over a worthy and a cheap entry, each with and without a file at the snapshot path. It failed with DID NOT RAISE in all four cases.
• test_result_cache.py::test_cache_worthy_errors_on_a_hash_whose_entry_dir_is_gone: a retired entry's snapshot is not served. It failed with DID NOT RAISE.
• test_materialize.py::test_reading_an_entry_that_lost_its_manifest_is_an_error_until_its_recipe_runs_again, for a worthy and a cheap entry: cached_result_expr and ensure_materialized raise, no snapshot is written, and running the recipe again by hand restores the entry under the same hash. It failed with DID NOT RAISE.
• test_materialize.py::test_a_child_of_an_entry_that_lost_its_manifest_raises_that_entrys_error, and a new child's build fails. It failed on the "still missing after it was made again" message.
• test_source_import.py::test_reading_a_source_version_that_lost_its_manifest_and_snapshot_is_an_error, which failed with RecursionError. A re-import by hand writes the version again.
• test_companion.py::test_entry_routes_error_on_an_entry_with_no_manifest, over data, entry and entry_cache. It replaces test_api_data_missing_manifest_serves_page_without_500, which asserted the fix(api): guard api_data's manifest read so a missing manifest doesn't 500 (#90) #95 behaviour this PR reverses.
• test_materialize.py::test_a_failed_retry_of_a_half_built_entry_keeps_its_snapshot (a failed build deletes the snapshot already on disk for its hash — a reset-retired or crashed entry's only copy is lost #193) read the half-built entry through the old fallback. That line now expects the error.

Second red commit, 67648c0, after review, against the first fix c3d7f8b. Every message check became an exact match on the one-line error, so rebuild advice, a separate missing-directory message or a prefixed child hash fails it. The routes expect 500 where the first fix answered 404. test_notebook.py::test_notebook_route_errors_when_a_cells_entry_has_no_manifest is new: the notebook answered 200 with an empty cell.

Guard test in the first fix commit: test_cache_worthy_is_the_manifests_verdict_whatever_is_at_the_snapshot_path. A worthy entry with its snapshot deleted is still worthy, and a cheap entry with a file at its snapshot path is still cheap. It passes on the old code too.

How it was built

1. d45bee8 adds the failing tests. CI run 36017486525: 13 failed and 952 passed, exactly the cases above.
2. c3d7f8b refuses the directory with the explanatory NotAnEntryError. CI run 36018443637 passed.
3. Review of c3d7f8b. The decision was that a read of a manifest-less entry is a plain error with no remedy and no code working around it.
4. 67648c0 changes the tests to that. e7feef7 merges the moved base in, because the PR conflicted with it and GitHub runs no pull_request workflow for a conflicting PR. The conflict was one hunk in api_data: it keeps the manifest read and adds caching: concurrent .execute() on the shared default backend raises "Already borrowed" — api_data has no execute lock (second cause of #79 two-tab 500) #118's execution_lock around the page's execute. CI run 36044243816: 13 failed and 967 passed, and the 13 are the tests from 67648c0.
5. 7ebeaf3 is the fix: 31 lines added and 91 removed. CI run 36044977363: ruff, the fast suite (997 passed, and vitest 15 passed) and the integration suite (7 passed) all pass. The fast count is 17 more than locally because the run tests the merge with the base, which had taken fix(csv): a zone on offset-less CSV text is attached to the wall-clock time (#231) #244 by then.

Checked locally at 7ebeaf3: the fast suite had 980 passed, the integration suite 7 passed, and uvx ruff check passed.

Not in this PR

• A build killed before it writes its manifest reaches this state. The build fills entries/<hash> in place and writes the manifest last, so a Ctrl-C or a crash mid-build leaves the directory this PR treats as corrupt. Building into a staging directory and renaming it into entries/ after the manifest is written would mean only a hand-deleted manifest can produce it.
• A cheap child still reads a manifest-less parent's snapshot when the file is there. _ensure(child) only goes to the parent when a file its plan reads is missing, so with the parent's snapshot on disk the child is served without the parent's manifest being read. With the file gone it raises the parent's error.
• A manifest that exists but cannot be parsed still raises the parse error, as before.
• The Cache page lists a manifest-less directory's snapshot as an orphan and lets the user delete it.
• The known-defects line for cache_worthy falls back to "a snapshot exists" when the manifest is missing — a worthy entry that has lost both is served as cheap #204 in docs/architecture-new.md lives on docs/adr-007-009-implemented and should go when that branch is next updated.
• ruff format would reformat a few lines in build.py, result_cache.py, test_result_cache.py and test_source_import.py that were already unformatted on the base. The repo has no .pre-commit-config.yaml, so I formatted only the lines this PR touches.

🤖 Generated with Claude Code

[paddymul]

Review of c3d7f8b, and what happened to each finding

The review (/code-review) found 14 issues. Three were confirmed with a probe test on c3d7f8b: 1, 3 and 5. The rest came from reading the code.

After the review, the direction changed. Reading an entry directory that has no manifest is now a plain error. It gets no remedy text, no 404 mapping and no wrapper, and no code works around it. Most findings were about the explanatory refusal and the handling around it, so removing that handling resolves them. Red: 67648c0 (CI 36044243816, 13 failed). Fix: 7ebeaf3.

#	Finding	Outcome
1	diff_keys(V2, V3) fails when an older version V1, which isn't part of the diff, has lost its manifest, because primary-key inheritance calls cache_worthy(V1) (probe-confirmed)	Kept as an error, by decision. The suggested fix, treating V1 as having no inherited key and detecting one, works around the corrupt entry.
2	The advice to rebuild with catalog_run overwrites a pinned or unreproducible snapshot	Fixed in 7ebeaf3. The error has no advice.
3	A NotAnEntryError from further down (a parent, a diff, promote-diff) became a 500 while the entry's own became a 404 (probe-confirmed)	Fixed in 7ebeaf3. The 404 mapping is removed, so every route answers 500.
4	The catalog_run advice loops for a recipe that reads its own alias, and builds a different hash once a parent alias has moved on	Fixed in 7ebeaf3. The advice is removed.
5	A cheap child is served while a manifest-less parent's snapshot is on disk, and fails once the file is gone (probe-confirmed)	Not in this PR. It's listed under "Not in this PR" in the description.
6	For a source version, the advice first offers catalog_run on a recipe that can't run	Fixed in 7ebeaf3. The advice is removed.
7	assert "import" in str(info.value) could not fail	Fixed in 67648c0. It's an exact-message assertion now.
8	The 404 detail sent absolute filesystem paths to the client	Fixed in 7ebeaf3. The 404 is gone, and a 500 carries no detail.
9	_recreate re-raised a parent's refusal as NotAnEntryError on a complete child, and added a prefix at each level	Fixed in 7ebeaf3. The rewrap and the class are removed, so the child raises the parent's error unchanged (tested in 67648c0).
10	load_session reports the state as the retryable status error	Declined. It's an error and is reported as one. A separate terminal status would be special handling for it.
11	_recreate's missing-parent-directory check duplicates a branch of entry_manifest	Declined. The check was already on the base. entry_manifest no longer has a separate missing-directory branch.
12	The routes threw away the manifest _require_manifest returned, and _compute_entry_cache kept fallbacks that could no longer be reached	Fixed in 7ebeaf3. _require_manifest is removed, and so are the worthy=False and manifest={} fallbacks and _snapshot_cache_path's catch-all.
13	The build fills entries/<hash> in place, so an interrupted build can reach this state. Build into a staging directory and rename it into place	Not in this PR. It's listed under "Not in this PR".
14	Warm-up logs the full multi-sentence refusal for each manifest-less directory on every start	Partly addressed in 7ebeaf3. The message is one line now, and warm-up still logs it, because it's an error.

The same pass also removed two guards the review didn't list, both in 7ebeaf3. The notebook cell rendered with no metadata and 0 rows when its entry had no manifest (/api/notebook_full answers 500 now, tested in 67648c0). _load_timeout had an exists check and a bare except that used 0 rows.

🤖 Generated with Claude Code