perf(cache): source-cache + baked result-cache, drop blanket result.parquet (#73)

[paddymul]

Addresses #73.

What

Replaces the blanket per-entry result.parquet with a rewrite-then-build caching model. Before building, tallyman rewrites the submitted expression (source_cache.rewrite_for_build):

1. Source-read caching — a snapshot .cache() is injected after each non-parquet file read (read_csv/read_json), so the parse is materialised once and shared across every entry that reads that source (path-only snapshot key). read_parquet/read_delta are exempt.
2. Baked result cache — when the expression is expensive (Aggregate/Join/Sort/window/UDF), a top-level .cache() is baked into the build. The cache node is part of the durable recipe, so every loader — the Buckaroo viewer, diffs, from_catalog chaining — reads the cached result instead of re-running the whole DAG (the read-time-only path perf: viewer /load_expr sends the bare recipe for cache-worthy entries — count + every page re-runs the whole DAG instead of reading result.parquet #71 worked around, fixed at the source).
3. In-memory reads (read_in_memory / ibis.memtable) are a hard BuildError, steering authors back to deferred_read_csv.

result.parquet is no longer written at build. A cheap entry materialises nothing (schema from the expression, row count from a pushdown count()); an expensive entry's rows live in its baked cache. ensure_result regenerates result.parquet on demand for the few callers that need a parquet path (downloads, promote-diff, post-processing), so it's now a regenerable cache, not an immutable artifact.

The content hash is computed from the rewritten expression; xorq_build/ carries the cache nodes while expr.py keeps the author's literal source.

Notable decisions

• from_catalog composes expressions (via cached_result_expr) rather than reading result.parquet, so chaining off a cheap entry works. Consequence: the inter-entry catalog DAG (catalog_parents) goes dark — there's no result.parquet path string to match. Deferred to a future semantic-dependency mechanism (depend on the concept a parent computes, not its materialised hash), per discussion on the issue.
• Co-located caches / result_cache/ retired. xorq's loader (load_expr(cache_dir=…)) rewrites every baked cache node's base path uniformly, so source parses and baked results co-locate in the per-project compute cache (content-addressed, already reset-reconciled). The separate result_cache/ dir is retired (kept as a now-empty vestige so reset/disk-reporting are untouched).
• Internal-lineage view contracts out the injected CachedNode/RemoteTable so it still shows the author's logical pipeline.
• Independent branch off main (does not stack on perf: viewer /load_expr sends the bare recipe for cache-worthy entries — count + every page re-runs the whole DAG instead of reading result.parquet #71).

Migration

build.migrate_drop_result_parquet deletes old build-time result.parquet files once per project (marker-guarded), run at companion create_app. Cheap entries recompute trivially on demand; expensive entries repopulate their cache lazily on first view.

Tests

Full suite: 454 passed. New test_source_cache.py covers injection, parquet exemption, expensive double-bake, and in-memory rejection. Updated result_cache / build / lineage / diff / cache-inspector / paths tests for the new behaviour.

Follow-ups (noted, not in scope)

• Disk-usage reporting still points at the now-empty result_cache/ and doesn't count the compute cache where baked results live.
• The cache-inspector reflects on-demand result.parquet rather than the baked compute-cache results.

Review follow-ups (#74 review)

Filed from the review of this PR; the first is to be reviewed in a separate PR, the rest are deferred:

• previous_version cross-alias ambiguity (src/tallyman_core/aliases.py). version_of_hash returns the first alias (dict order) whose history contains a hash, and previous_version steps back within that one alias. A hash shared across alias histories therefore steps back through whichever alias comes first, which may not be the lineage a from_catalog meant. Not a regression — previous_version is behaviourally identical to the prior inline primary_key._parent_hash — but the new cycle resolver (_resolve_noncyclic_hash) now depends on it for correctness, so the stakes are higher. _resolve_noncyclic_hash has the requested alias in scope and could disambiguate. To be reviewed separately.
• Concurrent on-demand materialisation of the same cold entry races on the output path — caching: concurrent on-demand materialisation of the same cold entry races on the output path — torn result.parquet/snapshot + 500, no single-flight lock #79.
• Reset-to-revision doesn't invalidate the in-process cached_result_expr LRU — caching: reset-to-revision doesn't invalidate the in-process cached_result_expr LRU — a warmed expensive entry then reads a pruned snapshot path and fails #80.
• Scalar-UDF-only entries never bake a result cache (worthiness lockstep gap) — caching: scalar-UDF-only entries never bake a result cache — _is_worthy_expr misses function-named UDF nodes that classify_build sees in YAML #81.

🤖 Generated with Claude Code

[paddymul]

Initial performance findings (ensure_session harness)

Updated the Tier-A perf harness to drive the real ensure_session → /load_expr (production 10s cap) instead of the long-timeout bypass, and re-keyed Tier-B discovery/row-counts on manifest.json since this branch no longer writes result.parquet at build (02361bc). Then compared this branch against main.

Caveat on scope

The comparison runs a reduced, single-parent 1m corpus, not the canonical parking corpus. This branch can't build any entry that combines ≥2 from_catalog parents (union/join) — build_expr raises Multiple backends found — so scripts/rebuild_parking_catalog.py aborts at step 7/15 (all_violations). Filed separately as #75. The numbers below are therefore the entries that build on both sides: the raw from_project reads and single-parent aggregates.

ensure_session (/load_expr) — the production viewer path

entry	rows	main	branch
camera_violations_by_plate (Aggregate)	726,203	0.56s cold / 0.06s warm-reload	0.58s / 0.08s
camera_select_columns (raw read)	1,000,000	0.39s / 0.05s	0.38s / 0.05s

Neutral — within ~20ms. Expected: /load_expr already pushes down and never read result.parquet (#71), so dropping it / baking a result cache doesn't move this path at 1m scale. Neither side trips the 10s cap.

Execute path (load_entry → to_parquet) — what the result.parquet drop touches

entry	rows	main cold/warm	branch cold/warm
camera_violations_by_plate (Aggregate)	726,203	0.13 / 0.11	0.27 / 0.14
camera_select_columns (raw)	1,000,000	0.17 / 0.17	0.17 / 0.17
camera_violations (raw, 20 col)	1,000,000	0.63 / 0.62	0.62 / 0.63

Raw reads identical; the aggregate is ~0.14s slower cold on the branch — main reads a materialised result.parquet (parquet→parquet copy) while the branch materialises through the baked-cache expr. Small absolute cost at this scale.

Bottom line

At the 1m single-parent scale this change is performance-neutral on the production ensure_session path and slightly negative on the cold execute of an aggregate. It can neither confirm nor refute the intended win, which targets large multi-parent aggregates over 100M+ row sources — exactly the entries that (a) can't be rebuilt here (#75) and (b) are where main's /load_expr re-runs the whole DAG (#71). Worth re-running on the full multi-parent corpus once #75 is fixed.

_{Harness: tests/test_perf_tier_a.py (ensure_session, cap-aware), tests/test_perf_integration.py (manifest-keyed). Run: uv run pytest -m perf_tier_a tests/test_perf_tier_a.py -s.}

[paddymul]

Review pass (multi-agent, adversarially verified)

Reviewed across 6 dimensions (source-cache rewrite, recursion/cycles, self-heal + snapshot resolution, concurrency/lifecycle, API/state/lineage, tests/scripts) with each finding re-checked against the code by an independent verifier. 20 findings raised, 17 confirmed, 3 dismissed. Disposition below.

Fixed in this PR

• _import_script leaked a sys.modules entry per reconstruction. Since caching: build_and_persist materialises result.parquet for every entry — replace with at-the-read source caching + cache-worthy-only result materialisation #73, cached_result_expr → _recipe_expr → _import_script runs per-read and registered each recipe under a unique sys.modules name without removing it, pinning the expression graph — unbounded under churn past the LRU's 256. Failing test 01d7fec (seen red on CI: AssertionError: leaked 5 recipe modules), fix d364157 (_recipe_expr pops the module after extracting the expr; _import_script/build path unchanged). Green on c50299a.
• Test gaps (c50299a): the cache_worthy/CachedNode disagreement fallback in cached_result_expr, and read_csv/read_json parity (extracted _should_cache_read so the parity is unit-testable — this xorq exposes no deferred_read_json, so the read_json branch has no end-to-end producer).

Filed as follow-up issues

• caching: concurrent on-demand materialisation of the same cold entry races on the output path — torn result.parquet/snapshot + 500, no single-flight lock #79 — concurrency: concurrent on-demand materialisation of the same cold entry races on the output path (snapshot .tmp collision / to_parquet writing the final path; no single-flight lock). Not blocking — the MCP build typically warms the result before a human opens the UI. Proposed fix: serialize per (project, hash) + atomic temp-then-os.replace.
• caching: reset-to-revision doesn't invalidate the in-process cached_result_expr LRU — a warmed expensive entry then reads a pruned snapshot path and fails #80 — reset-to-revision doesn't invalidate the in-process cached_result_expr LRU, so a warmed expensive entry can read a pruned snapshot path. Low in practice (reset is run out-of-process and the server is restarted after).
• caching: scalar-UDF-only entries never bake a result cache — _is_worthy_expr misses function-named UDF nodes that classify_build sees in YAML #81 — scalar-UDF-only entries never bake a result cache: _is_worthy_expr's "UDF" in type(node).__name__ misses function-named UDF nodes that classify_build sees as op: ScalarUDF. Silent recompute, no crash. To be folded into replacing the regex-based worthiness check.

Noted on the PR body for separate review

• previous_version cross-alias ambiguity (aliases.py) — not a regression (identical to the prior inline _parent_hash), but the new cycle resolver now depends on it. Recorded under "Review follow-ups" in the description.

Low — no action

• api_data first paginated read of a cheap entry materialises the full result.parquet before slicing (one-time per entry/eviction; a design tradeoff of on-demand materialisation, not a regression in steady state).
• _MAX_RECON_DEPTH=64 also caps a legitimate 64+-deep distinct-parent chain (extreme workload; loud BuildError).
• diff._ensure_entry_parquet derives the project from entry.parents[3].name, wrong under a serve-mode path override — latent only; both production callers pass exprs and never reach it.
• Cross-machine clone / project relocation recomputes an expensive entry's baked snapshot (path-structural key); self-heals, consistent with the acknowledged portability caveat (caching: expensive-parent from_catalog children pin the parent's untracked compute_cache snapshot by path — child read fails (no recompute) when it's pruned or uncloned #76, viewer: expensive-parent children render an empty buckaroo grid on a cross-machine clone — /load_expr replays the build, whose snapshot key is bound to the build-time path #77).
• Disk-usage under-counts compute_cache and the cache-inspector reflects on-demand result.parquet — already listed in this PR's "Follow-ups (not in scope)".

Dismissed (3)

• A generic "classify_build vs _is_worthy_expr could diverge" finding — the only realizable instance is the scalar-UDF case (filed as caching: scalar-UDF-only entries never bake a result cache — _is_worthy_expr misses function-named UDF nodes that classify_build sees in YAML #81); no other divergence could be constructed (single expr.yaml, regex over-matches only non-expensive tokens).
• migrate_drop_result_parquet marker race — benign (runs at construction before serving; idempotent, OSError-swallowed).
• "Cache page empty for caching: build_and_persist materialises result.parquet for every entry — replace with at-the-read source caching + cache-worthy-only result materialisation #73 projects" — accurate but an explicitly acknowledged deferred follow-up.

Also independently checked and cleared (no finding): dropping disk_usage.result_cache doesn't break the SPA (Header.tsx reads only formatted.total), and api_notebook_full's total_rows is reset per loop iteration.

CI green on c50299a (ruff · fast · integration).