Repository navigation
fix(api): guard api_data's manifest read so a missing manifest doesn't 500 (#90) - #95
Merged
Merged
Conversation
build.py creates an entry's build dir before it writes manifest.json, so a half-built or pruned entry passes api_data's build-dir check yet has no manifest. The handler reads manifest.json unguarded for `total`, raising FileNotFoundError. cached_result_expr needs no manifest, so the page should still serve with a best-effort total. Test asserts a 200 with rows; fails today (FileNotFoundError) ahead of the guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t 500 api_data guards on entry_build_dir(...).is_dir() but then read manifest.json unguarded for `total`. build.py creates the build dir before it writes the manifest, so a half-built or pruned entry passes the guard yet has no manifest — read_text() raised FileNotFoundError → 500. cached_result_expr needs no manifest (recipe / baked snapshot), so only `total` depended on it. Guard the read: present manifest → row_count as before; missing → serve the page off the expression with total=0. Mirrors the .exists()-then-read pattern api_entry_detail uses. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A cheap (unsorted) entry paginates by re-executing the recipe per page (no baked snapshot — only Aggregate/Join/Sort entries bake one), so two pages tile the result with no overlap or gap only if the source scan order is stable across executions. The old path sliced one on-disk result.parquet, stable by construction. Assert two pages equal the matching slices of a single full read. Passes today — a characterization lock on the property #92 introduced, not a bug repro. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
paddymul
added a commit
that referenced
this pull request
Sep 24, 2026
…essing its worthiness from the snapshot (#204) cache_worthy fell back to "a snapshot exists" when the manifest was missing. A worthy entry that had lost both was read as cheap: every read re-ran its aggregate, nothing healed the file, and a child of it failed with "still missing after it was made again". A source version in that state recursed without bound, and a cheap entry with a file at its snapshot path was reported worthy. A directory without a manifest is not an entry (ADR-007 D6), and every writer already treats it as absent. The manifest also holds the digest a heal is checked against, the pin and a source version's provenance, so recomputing the verdict would not make the entry servable. So: - result_cache.entry_manifest returns the manifest or raises NotAnEntryError (a BuildError), whose message names the rebuild: run the recipe in expr.py again with catalog_run, or import the file again for a source version. - cache_worthy reads the verdict through it and never looks at the snapshot path, so ensure_materialized, cached_result_expr, baked_snapshot_path, load_session and primary-key inheritance refuse such a directory before loading or writing anything. - _recreate adds the child's hash, so a child's read names the entry to build again. - /api/data, /api/entry and /api/entry_cache answer 404 with the message; /api/data no longer serves the rows with a total of 0 (#95). Adds a guard test that a complete entry's verdict is the manifest's whatever is at the snapshot path; it passes on the old code too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Sep 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two follow-ups to #92 (which moved
/api/dataontocached_result_expr), found while reviewing that PR.Fix — a missing manifest no longer 500s the read
api_dataguards onentry_build_dir(...).is_dir()(app.py:502) but then readmanifest.jsonunguarded fortotal(app.py:512).build.pycreates the build dir (build.py:380-384) before it writes the manifest (build.py:444), so a half-built or pruned entry passes the guard yet has no manifest —read_text()raisesFileNotFoundError→ 500.cached_result_exprneeds no manifest (it reads the recipe / baked snapshot), so onlytotaldepended on it. The read is now guarded: a present manifest givestotalas before; a missing one serves the page off the expression withtotal = 0rather than crashing. This mirrors the.exists()-then-read patternapi_entry_detailalready uses for the latest-entry manifest.Test — cheap-entry pagination consistency across pages
#92 paginates a cheap (unsorted) entry by re-executing the recipe per page, so two pages tile the result correctly only if the source scan order is stable across executions — the old path sliced a single on-disk
result.parquet, stable by construction. The existing cheap-entry test fetched onlyoffset=0; the deep-offset test used an explicitorder_by. Neither covered the re-execution stability the new path relies on.test_api_data_cheap_entry_paginates_consistently_across_pagesasserts two pages equal the matching slices of a single full read, locking that contract.TDD
test(#90): the missing-manifest reproduction, committed first to fail on CI.fix(#90): the guard.test(#90): the pagination-consistency lock (passes today — characterization).🤖 Generated with Claude Code