diff --git a/plans/adr-reactive-catalog-recalc.md b/plans/adr-reactive-catalog-recalc.md new file mode 100644 index 0000000..d737da2 --- /dev/null +++ b/plans/adr-reactive-catalog-recalc.md @@ -0,0 +1,466 @@ +# ADR: Reactive catalog — recalc & staleness over the dependency DAG + +- **Status:** Research snapshot (2026-06-14), reconciled with PR #74 (merged + 2026-06-16) and PRs #103 / #104 (merged 2026-06-17) — see the two + "Reconciliation with PR …" sections below. #74 replaced the chaining/caching + substrate this ADR planned on top of: it dropped the blanket per-entry + `result.parquet` and materializes only at expensive boundaries (the model the + author intended all along), which supersedes the tagged-read spine (findings + #6/#7) and takes cross-entry lineage dark. #104 then removed the last + *on-demand* `result.parquet` layer (`ensure_result` and its consumers), so the + baked `.cache()` snapshot is the only materialized copy and `cached_result_expr` + is the sole read path. #103 dropped xorq's catalog package for a native + tallyman store, dissolving the #48/#52 bug cluster by construction and + decomposing `catalog.yaml` into per-concern tracked files. Separately, the + manifest `parents` edge this ADR called for as a prerequisite already shipped in + #84 (merged via PR #91, before #103): the cross-entry DAG is captured and read + today. The reactive intent and the still-open design questions + (cascade/transaction semantics, archive overlay, trigger model) stand; findings + are annotated inline where #74, #84, #103, or #104 overtook them. +- **Context:** A tallyman project has a few raw sources and many derived + expressions. Today, edits to a raw source or updates to an upstream entry do + not reach the dependents — they go silently stale. We want an on-demand + "recalc" (and/or a scan-on-load that flags staleness) without giving up the + catalog's reproducible, recoverable, git-backed nature. +- **Affected code:** `src/tallyman_xorq/io.py` (`from_catalog`, io.py:63 — + composes the parent via `cached_result_expr` and records the edge with + `parent_capture.note_parent`, io.py:111), + `src/tallyman_xorq/result_cache.py` (`cached_result_expr` at result_cache.py:273, + its memoized resolver `_resolve_result_plan` at :232, `baked_snapshot_path` at + :211, `_resolve_noncyclic_hash` at :117 — `ensure_result` was deleted by #104) + and `src/tallyman_xorq/source_cache.py` (`rewrite_for_build`, source_cache.py:82) + — both added by #74 — `src/tallyman_xorq/parent_capture.py` (the #84 parent-edge + collector), `src/tallyman_core/manifest.py` (`ParentRef` + `Manifest.parents`, + manifest.py:16/:56), `src/tallyman_xorq/lineage.py` (`catalog_parents` / + `catalog_dag`, now manifest-driven, lineage.py:112), + `src/tallyman_xorq/source_identity.py`, `src/tallyman_core/aliases.py` + (`previous_version` at aliases.py:110, now `aliases.jsonl`-backed), + `src/tallyman_companion/app.py` (checkpoint middleware). +- **Related:** buckaroo-data/tallyman#48 (catalog-consistency bug, a + prerequisite for the recovery model — closed by PR #57, then the whole + constraint eliminated by PR #103, which dropped xorq's catalog package), #73/#74 + (the source-cache + baked-result rewrite that replaced this ADR's chaining + substrate — see Reconciliation), #101/#104 (removed the on-demand + `result.parquet` layer), #52/#103 (native `tallyman_core.catalog` store), + #84 (manifest `parents` edge — restores the cross-entry DAG, merged via PR #91), + #82 (deep-chain build-cost measurement), #83 (execution-nondeterminism + soundness), #80 (in-process LRU invalidation on reset), + `plans/native-catalog-store.md` (the PR #103 design + investigation record), + `plans/adr-source-identity-content-hash.md`, + `plans/adr-result-cache-cost-rubric.md`. +- **xorq evidence:** read at xorq 0.3.29 (commit `1f46392e`), under + `~/xorq/python/xorq`. + +## Intent + +The author's framing, paraphrased: a notebook is a handful of raw sources and +many dependent expressions. Two properties make derived work go stale without +anyone noticing. + +1. **Alias dependencies are by-value.** `from_catalog("A")` resolves alias A to + its *current* hash at build time and binds that resolved version. When A is + revised, B keeps reading A's old version. B depends on "the value A had when B + was built," not on "alias A." *[#74/#104: the binding is no longer an embedded + `result.parquet` read — `from_catalog` (io.py:63) composes A's expression via + `cached_result_expr` and records the resolved `{hash, ref, follow}` in + `manifest.parents` (#84). The by-value staleness diagnosis is unchanged; only + the mechanism the value rides on moved from a parquet path to a recorded hash.]* + +2. **xorq invalidates lazily, with no propagation.** A cache key is recomputed + only when an expression executes, by walking to its Read nodes; there is no + "this source changed → invalidate its dependents" mechanism anywhere. + Combined with tallyman's deliberate choice of `SnapshotStrategy` (path-only + keys), an in-place source edit is invisible to identity. + +The goal is to recompute the dependents of a changed source or an updated entry +on demand, while keeping each entry a reproducible, recoverable artifact. + +**Guiding principle (stated 2026-06-14):** use xorq natively wherever possible; +patch in a tallyman-specific solution only when (1) xorq offers no way to do the +thing, or (2) xorq's way is too slow for reasonable interactive use. + +## Reconciliation with PR #74 (merged 2026-06-16) + +This ADR was written believing the per-entry `result.parquet` was a deliberate +truncation boundary worth preserving. That was a misread of the codebase. The +blanket model materialized *every* entry's result out of band regardless of +cost; the author had not realized that, and only ever wanted materialization at +expensive boundaries. Read correctly, the design goal was never "keep the parquet +read and tag it" — it was "stop materializing cheap steps." PR #74 implements +exactly that, so it supersedes this ADR's chaining proposal rather than refining +it. + +**What #74 changed.** `result.parquet` is no longer written at build. A +`rewrite_for_build` step (`source_cache.py`) instead (1) caches each non-parquet +source read (`read_csv`/`read_json`) once, shared across entries, and (2) bakes a +top-level result cache into the durable recipe only for *expensive* entries +(`cache_worthy` — Aggregate/Join/Sort/window/UDF). Cheap entries materialize +nothing. `from_catalog` (`io.py:63`) stopped reading `result.parquet` and now +reconstructs the parent as an expression on the in-process default backend +(`cached_result_expr`, `result_cache.py:273`): an expensive parent resolves to a +bare read of its baked snapshot, a cheap parent re-runs its recipe (pushdown +makes that ~free). At #74's merge `result.parquet` still survived as a +regenerable on-demand artifact (`ensure_result`) for the few callers that needed +a parquet *path* — downloads, promote-diff, post-processing. +*[Superseded by #104: that on-demand layer is gone too. `ensure_result` and the +path helpers (`ENTRY_RESULT_FILENAME`, `entry_result_path`, the manifest +`result_path` field) were deleted; every caller now routes through +`cached_result_expr`. No per-entry `result.parquet` is written at build or on +demand. See "Reconciliation with PR #104" below.]* + +The boundary #74 uses — structural `cache_worthy` (contains +Aggregate/Join/Sort/window/UDF) — is a stopgap, not a settled definition of +"expensive." Whether it should become the measured value-per-byte rubric of +`plans/adr-result-cache-cost-rubric.md` (#30) is decided empirically: instrument +both verdicts, watch where they disagree (a Join-descendant computed column is +structurally worthy but cheap to recompute), and flip when the data says so. The +materialization boundary is policy, not a fixed `cache_worthy`. + +**What this supersedes here.** +- Findings #6 and #7's tagged read of `result.parquet` — there is no per-entry + `result.parquet` read left to tag. The verified-findings note that this spine + "works" still holds in isolation; it was simply not the path taken. +- Recovering cross-entry edges via `walk_nodes(HashingTag)`. #74 took + `catalog_parents` dark: composing the parent's expression leaves no + `result.parquet` path in `expr.yaml`, so the catalog DAG was momentarily a + forest of single-node trees. But going dark was a #74 scope decision, not a + design stance, and the edge is not lost. The fix this ADR proposed — record the + resolved parent hash in the entry's manifest (a `parents` field, parallel to + `sources`) and have `catalog_parents` read the manifest instead of regex-matching + paths — *shipped in #84 (merged via PR #91, ahead of #103)*: `from_catalog` + resolves the parent hash at build and `parent_capture.note_parent` records + `{hash, ref, follow}` (io.py:111), `build_and_persist` persists it into + `Manifest.parents` (`ParentRef`, manifest.py:16/:56), and `catalog_parents` reads + it (lineage.py:112) with the `result.parquet` regex retained only as a pre-#74 + fallback (lineage.py:134). So + the hash-edge DAG and the read-intent are *captured* today. Only the richer + *follow-the-concept* layer (depend on what a parent computes, not its + materialised hash — #73) and the reactive *consumer* (walk dependents to + recompute) remain. See §Open questions (read-intent) and the staged plan's + stage 3. + +**What still stands.** The reactive intent (§Intent) and the staleness diagnosis +(by-value alias deps; xorq's lazy, propagation-free invalidation) are unchanged — +#74 reworked the caching substrate, not reactivity. The open questions move onto +#74's model rather than disappearing: see §Open questions. + +## Reconciliation with PR #104 (merged 2026-06-17) + +#104 (#101) finished what #74 started: it removed the *on-demand* `result.parquet` +layer that #74 had kept as a regenerable artifact. Where #74 stopped writing +`result.parquet` at build but left `ensure_result` to regenerate it lazily, #104 +deleted `ensure_result` outright and repointed its three real consumers +(`post_processing`, `companion.diff.build_diff_expr`, `xorq.diff.full_diff`) at +`cached_result_expr`. It also unwound #102's viewer-side reintroduction +(`ensure_result_build` / `ensure_viewer_expanded_build`): `ensure_session` now +replays the entry's recipe build onto its baked snapshot instead of paging a +materialised parquet. The path plumbing went with it — `ENTRY_RESULT_FILENAME`, +`entry_result_path`, the `.xorq_result_build*` helpers, and the manifest +`result_path` field are all gone (verified absent from the tree). + +**Net effect on this ADR.** The baked `.cache()` snapshot is now the *only* +materialized copy of any entry's rows, and `cached_result_expr` (result_cache.py:273) +is the single read path every surface goes through — the viewer, both diffs, +post-processing, and `from_catalog` parent composition. That sharpens, not +weakens, the reactive design: there is exactly one place to reason about staleness +and one cache to invalidate. Three downstream consequences: + +- **Salt mode no longer routes through an in-place `result.parquet`.** #74's note + that salt-mode reads bypass the baked cache and read an in-place parquet is + obsolete: salt bakes no snapshot and now falls through to plain recompute + (cheap-recompute / worthy-recompute-fallback, result_cache.py:300). +- **Recompute-on-cold-read is now the default for every non-baked entry.** With no + `result.parquet` anywhere, a cheap or salt entry reconstructs its recipe on each + cold read. This *widens* the source-identity drift hazard (§Open questions, + source-identity) and the execution-nondeterminism soundness hole (#83) to a + larger surface, reinforcing both as prerequisites — it does not change their + verdicts. +- **The cache-admin surfaces are correct now.** The disk-usage pill counts + `compute_cache` and dropped the empty `results` bucket (app.py:129); the + result-cache inspector and delete drive off the manifest + the baked snapshot + path, listing only snapshots that exist on disk (app.py:1007 / :1085). These + were #74 follow-ups the staged plan listed as open; #104 closed them. + +## Reconciliation with PR #103 (merged 2026-06-17) + +#103 (#52) replaced xorq's catalog package with a tallyman-native, +content-addressed recipe store (`src/tallyman_core/catalog.py`) and decomposed the +monolithic `catalog.yaml` into per-concern git-tracked files. It deleted +`xorq_catalog.py` (the subprocess that shelled out to `xorq catalog add`) and +added a datafusion `connect()` shim (`backend.py:23`) so the runtime never loads +xorq's 12-module `catalog` package. This ADR's recovery model and several findings +were written against the xorq-catalog substrate; #103 dissolves that substrate. + +**The #48/#52 bug cluster is gone by construction, not fixed in place.** Finding +"#48 — tallyman breaks its own xorq catalog" diagnosed the failure as tallyman +committing `aliases.json` / `alias_history.json` into the xorq catalog repo, which +violated xorq's `Catalog.assert_consistency`, so every `xorq catalog add` after +the first silently failed. With the native store there is no `xorq catalog add` +and no xorq tree contract: tallyman owns the format, so committing any decomposed +sidecar is legal by design. `assert_consistency`-on-every-add is gone; the native +`assert_catalog_consistent` (catalog.py:197, a deny-by-default `TRACKED_SURFACE` +path-allowlist with three guards) runs only at `reset_to` and after rebuild, never +per-add, so #48's failure mode cannot recur. The checkpoint is now the sole zip +writer and sole git transaction (`write_recipe_zip` → `zip_pending_entries`, +catalog.py:126/:159), which also closes the out-of-lock-git-write race #48 rode on. + +**`catalog.yaml` is gone; authored state is decomposed.** The recovery model +(Q5) said "git tracks the small state (alias pointers, xorq catalog zips); heavy +`entries//` dirs are untracked and recovered via the bullpen." Post-#103 the +tracked surface is tallyman's own: `aliases.jsonl`, `notebook.jsonl`, +`chart_specs/*.vl.json`, `display_configs/*.json`, `post_processing/*.py`, +`stats/*.py`, `prompts/.jsonl`, plus the deterministic `entries/.zip` +recipes (allowlist `{expr.py, xorq_build/, schema.json, manifest.json}`) and two +untracked-artifact pointer lists (`entries.jsonl`, `compute_cache.jsonl`). A +`git reset --hard` rolls all of it back together; the heavy entry dirs are still +gitignored and bullpen-recovered. The #49/#54 worry — authored state stored as +`catalog.yaml` keys is dropped when xorq rebuilds `catalog.yaml` on a merge — is +moot: there is no `catalog.yaml` and no xorq merge-rebuild. + +**The manifest `parents` edge — this ADR's prerequisite — was already shipped (in +#84), and #103 makes it durable.** The capture+read landed earlier in #84 (merged +via PR #91, ahead of #103): `parent_capture.py` collects `{hash, ref, follow}` per +direct parent during a build, `build_and_persist` persists it into +`Manifest.parents`, and `catalog_parents` reads it. #103's contribution is +durability: it put `manifest.json` inside the recipe zip's member set +(`RECIPE_TOP_LEVEL` + `RECIPE_TREE_DIRS` = `{expr.py, schema.json, manifest.json, +xorq_build/}`, catalog.py:35), so the edge now survives `reset_to` where a loose +lineage file would not. So "restore the +cross-entry DAG + read-intent" is no longer a pending prerequisite — capture and +read are done (#84) and durable (#103); only the reactive *consumer* (walk +dependents, recompute) remains. + +**What still stands.** The reactive intent and staleness diagnosis are untouched — +#103 reworked the catalog *storage and consistency model*, not reactivity. xorq's +catalog primitives (`HashingTag`, `composed_from`, `Catalog.load`) described under +"What xorq does natively" remain true of xorq-the-library, but tallyman no longer +drives that package at all, so they are no longer candidate mechanisms for +tallyman's lineage (the native-first mapping's `HashingTag` rows — already +superseded by #74 — are now also unreachable through the dropped package). See +`plans/native-catalog-store.md` for the full native-store design and the +decomposition spec. + +## What xorq does natively + +- **Cache strategies.** `ModificationTimeStrategy` keys a cache on the source's + `(mtime, size, inode)` (`caching/strategy.py:89`, `common/utils/defer_utils.py:86`), + so a changed file misses and recomputes. `SnapshotStrategy` keys on the path + string only (`caching/strategy.py:49`), deliberately ignoring source content. + tallyman's entry `content_hash` and `result_cache` use Snapshot. +- **Invalidation is pull-based, at `execute()`.** `cache.set_default` recomputes + the key by walking to the Read nodes; there is no reverse-dependency index, + watcher, or propagation (confirmed across `caching/`). +- **Provenance tagging.** `HashingTag` (`expr/relations.py:120`) tags a node + with catalog provenance — `CatalogTag.SOURCE` / `TRANSFORM` plus `entry_name` + (`catalog/bind.py:9`, `:118`, `:145`) — and is recoverable with `walk_nodes`. + `composed_from` entry metadata records entry→entry edges (`catalog/catalog.py:1281`). +- **Catalog composition.** `Catalog.load` / `bind` / `compose` compose entries, + but every path calls eager `load_expr()` and inlines the upstream recipe + (`catalog/bind.py:115`), wrapping it in `RemoteTable` + `HashingTag`. +- **The catalog is git-backed.** `/artifacts/catalog/` is a git repo; + `xorq catalog add` zips builds; tallyman's `reset_to` sits on top of it. + +## What tallyman did before #74 + +(Pre-#74 state; the `result.parquet` mechanics in the first two bullets were +replaced — see Reconciliation. Retained for the diagnosis it grounds.) + +- `from_catalog` reads the upstream entry's materialized `result.parquet` + (pre-#74 io.py; the read no longer exists in the current tree) as an *anonymous* + `deferred_read_parquet`. This keeps each entry's + graph shallow. The reason on record: long xorq expression graphs are expensive + just to **build/tokenize**, independent of execution, so the parquet read is a + deliberate truncation boundary. The cost: it severs xorq's native dependency + graph — xorq sees a parquet file, not "entry A." + *[Corrected post-#74: the truncation was real, but the blanket per-entry + `result.parquet` behind it was not a wanted design — it materialized every step + out of band regardless of cost, which the author had not realized. #74 keeps + truncation only at expensive boundaries (baked snapshot) and drops it for cheap + entries.]* +- Because the edge is invisible to xorq, tallyman re-derived the cross-entry DAG + by regex-matching `result.parquet` paths inside each `expr.yaml` + (`catalog_parents`). *[Post-#84: `catalog_parents` (now lineage.py:112) + reads `manifest.parents` first; the `result.parquet` path-regex is retained only + as a pre-#74 fallback, lineage.py:134. The old lineage.py:53-64 citation is + stale — those lines are now unrelated `_strip_cache_nodes` code.]* +- Because Snapshot keys are content-blind, tallyman re-adds source-content + sensitivity in `source_identity` (modes `off` / `salt` / `cas`), `off` by + default. + +## Questions answered + +1. **Dependency semantics — follow-alias, pin-hash.** `from_catalog("alias")` + should *follow* the alias head (go stale when it advances); + `from_catalog("")` stays pinned. Requires recording the read-intent + (alias vs hash) at build time, which is not stored today (io.py collapses + both to a path). +2. **Binding through renames — follow.** Track the upstream through renames via + the alias history chain; no stable alias-id exists (aliases.py keys purely on + name). Unalias/delete prompts for what to do with dependents. +3. **Multi-parent ("poisoned") children — offer unbind.** A child that reads a + deleted parent *and* a live one can be **unbound** from the deleted parent: + inline that parent's contribution as a retained snapshot, keep the live + edges. The snapshot becomes source-of-truth (retained in `.cas`), not + evictable cache. +4. **Delete — freeze by default, cascade by option, always notify.** Author's + instinct is git-style cleanliness: deleting an alias means the concept was + flawed, so its descendants are suspect; recovery is real because the catalog + is git-backed + bullpen. Recommended refinements: model deletion as + **archive** (hide, retain bytes, restorable); **derive** "hidden because an + ancestor is archived" rather than stamping `archived` onto each dependent; + **preview the cone** before any cascade. +5. **Recovery model.** The catalog *is* git-backed (corrected mid-discussion — + an earlier claim that it wasn't was wrong). git tracks the small state (alias + pointers, recipe zips); the heavy `entries//` dirs are untracked and + recovered via the **bullpen** during `reset_to`. Recompute-from-recipe is the + recovery story, so recipes must be durable — see #48. *[Post-#103: the tracked + state is tallyman's own native surface (`aliases.jsonl` + the decomposed + sidecars + deterministic `entries/.zip` recipes under a `TRACKED_SURFACE` + allowlist), not xorq catalog zips; the consistency guard is the native + `assert_catalog_consistent`. The recovery story is unchanged in shape.]* +6. **Chaining mechanism — a tagged read of the materialized result.** Replace + the anonymous parquet read with + `deferred_read_parquet(/result.parquet).hashing_tag(CatalogTag.SOURCE, entry_name=…)`. + This records the dependency edge natively (so `catalog_parents` path-matching + becomes a `walk_nodes(HashingTag)`) while keeping the shallow, cheap-to-build + graph the parquet boundary was protecting. + *[Superseded by #74: not adopted. #74 reconstructs the parent expression + (cheap → re-run recipe; expensive → read baked snapshot) instead of a tagged + parquet read, and lets `catalog_parents` go dark rather than switching it to + `walk_nodes(HashingTag)`. See Reconciliation.]* +7. **Native-first mapping (the principle applied):** + - record entry→entry edge → **xorq native** (`HashingTag`); drop the + path-matching in lineage.py. + - within-expression recompute → **xorq native** (`execute()`). + - reference an entry *as a dependency* → **patch**: xorq's `Catalog.load` + deep-loads (too slow, criterion 2), so compose xorq's own + `deferred_read_parquet` + `HashingTag` instead of using `Catalog.load`. + - cross-entry invalidation propagation → **patch** (criterion 1; xorq has none). + - source-change detection → **patch** (criterion 1; xorq's only native option, + `ModificationTimeStrategy`, conflicts with the content-addressed identity + tallyman relies on). + *[#74 update: the first and third rows were superseded. #74 does not record + the edge via `HashingTag` (the edge goes dark, deferred to a + semantic-dependency mechanism), and it references a parent by reconstructing + its expression on the default backend (`cached_result_expr`) — composition, + but not via `HashingTag`. It still avoids `Catalog.load`'s deep-load, so the + criterion-2 read of `Catalog.load` holds. The invalidation and source-change + rows are untouched by #74.]* + +## Verified findings + +- **The tagged-read spine works.** + `deferred_read_parquet(/result.parquet).hashing_tag(CatalogTag.SOURCE, entry_name=…)` + built in 0.019s, stayed shallow (one Read of the result), `head(3).execute()` + returned real data in 0.20s off a 263MB result, and the edge was recovered via + `walk_nodes(HashingTag)` as `('catalog-source', 'af917591da4b')` + (af917591da4b = the `camera_select_columns` entry). + *[#74: this measurement stands on its own, but the spine was not adopted — #74 + reconstructs the parent expression rather than reading and tagging a + `result.parquet`. Kept as the record of what was tested. #104 makes the spine + categorically moot: no `result.parquet` is written for any entry now, on-demand + or otherwise, so there is no artifact left to read and tag.]* +- **xorq composition deep-loads.** `_resolve_source` for a `CatalogEntry` calls + eager `load_expr()` then wraps the result in `RemoteTable` (bind.py:115–118); + `compose` / `bind` inline upstream recipes via `replace_unbound`. So + referencing an entry through xorq's catalog API rebuilds its full graph — the + cost the parquet boundary was avoiding. (Slowness *at depth* is structural plus + author-reported; not yet timed on a deep chain.) +- **#48 — tallyman breaks its own xorq catalog.** tallyman commits + `aliases.json` / `alias_history.json` into the xorq catalog repo, which + violates xorq's `Catalog.assert_consistency`. After the first entry, every + `xorq catalog add` fails with an opaque empty error, so 17 of 18 entry recipes + never reach git — undermining the recompute-from-recipe recovery model above. + Filed as buckaroo-data/tallyman#48. + *[#48 was closed by PR #57, then the whole failure class was eliminated by + PR #103: xorq's catalog package and its `assert_consistency` are dropped, there + is no `xorq catalog add` subprocess, and aliases live in a tracked `aliases.jsonl` + (aliases.py:37) — no `aliases.json`. The original line citations (aliases.py:30-35, + app.py:545-566, the xorq `catalog.py:768-785`) are stale; the native consistency + guard is `assert_catalog_consistent` (catalog.py:197), run only at reset/rebuild.]* + +## Open questions + +- **Source-identity mode — resolved (2026-06-16): cas is required, not optional.** + cas is the only mode that both detects drift *and* recovers original bytes + faithfully (each read pinned to a content-addressed clone); salt detects drift + but recovers against the *current* file (unfaithful); off does neither. #74 + turned this from a tuning question into a prerequisite: `cached_result_expr` + reconstructs on every cold read (a cheap entry re-runs its recipe; an expensive + entry self-heals an evicted snapshot), so under `off` a cold read of an entry + whose source changed in place silently serves the new bytes under the entry's + old `content_hash`. Only cas (frozen `.cas` clone) keeps "this hash ⇒ these + bytes" true through reconstruction, which the reactive staleness signal depends + on. The native alternative, `ModificationTimeStrategy`, is unreachable for + identity (the hash is computed in a hardwired `SnapshotStrategy` context, + `provenance_utils.py:24`) and hostile to reset-to (it forks on the mtime/inode + bump every `reset_to` / checkout causes). So cas is the decision; the only open + part is the disk-cost mitigation the source-identity ADR already lists (reflink + on Linux, `.cas` GC). *[#74 special-cased salt by routing its reads around the + baked cache; #104 then removed the in-place `result.parquet` entirely, so a + salted entry bakes no snapshot and falls through to plain recompute via + `cached_result_expr` (cheap-recompute / worthy-recompute-fallback, + result_cache.py:300). The recompute-on-cold-read default that makes cas + mandatory now covers every cheap and salt entry, not just expensive evictions — + widening the hazard, not changing the verdict.]* +- **Read-intent recording — capture shipped (#84, merged via PR #91); consumption + still open.** `from_catalog("alias")` should follow the alias head; + `from_catalog("")` should stay pinned. This is now *recorded* at build: + `parent_capture.note_parent(content_hash, ref=alias_or_hash, follow=not is_hash)` + (io.py:111) captures `{hash, ref, follow}` — the resolved edge, the original + argument, and whether it was an alias — and `build_and_persist` persists it into + `Manifest.parents` (`ParentRef`, manifest.py:16/:56). What remains is the + *reactive consumer*: nothing yet reads `follow` to decide a dependent is stale + when its alias advances. The alias-history walk both this and #74's cycle-breaker + (`_resolve_noncyclic_hash`, result_cache.py:117) rely on is still ambiguous + (`version_of_hash` returns the first alias in iteration order, now aliases.py:101 + over `aliases.jsonl`); the recorded `ref` disambiguates it (thread the requested + alias through `previous_version`, aliases.py:110, rather than scanning). That + ambiguity is also flagged out-of-scope in `plans/native-catalog-store.md`. A + stable alias-id (question 2's "no stable alias-id") is deferred until usage shows + real cross-alias collisions. +- **Cascade failure / transaction semantics.** When a mid-DAG recompute fails: + stop, skip-and-continue, or all-or-nothing. +- **Archive overlay — derived vs stored** (recommended derived; to confirm). +- **Trigger model.** Button vs scan-on-load vs auto-cascade-on-revise (leaning + button + scan; explicit over implicit). +- **Determinism — resolved (2026-06-16): detect-and-warn, don't enforce.** The + original claim ("re-running `expr.py` with any nondeterminism yields a new + hash") only holds for *structural* nondeterminism — a Python literal baked into + the graph (`now()`, `random()`), which changes the re-derived hash and is + detectable by the predicate "reconstructed hash ≠ stored `content_hash` while + the source digest is unchanged" (cheap; folds into the stage-2 instrumentation). + *Execution* nondeterminism (`.sample()`, `ibis.now()`, unordered `.limit()`, + impure UDF) leaves the graph — and the hash — unchanged while the bytes differ, + so the hash can't see it and cas can't fix it (it is not source drift). The v0 + build-time lint on known-nondeterministic ops **shipped (#88, + `_NONDETERMINISTIC_OPS` at build.py:261, surfaced via the MCP server and + companion)**; the durable fix (it also breaks #30's evict-and-recompute + faithfulness) is #83, still open. No hard enforcement (see + `plans/adr-source-identity-content-hash.md`). +- **Deep-chain build cost (now #82).** Originally framed as timing eager + `load_expr` against the tagged-read reference; #74 took the composition fork, so + the measurement now targets `cached_result_expr` reconstruction at depth — + expensive parents truncate (bare snapshot read), cheap parents compose the full + graph. #104 made `cached_result_expr` the *sole* read path (every viewer, diff, + and post-processing surface routes through it), so this measurement now covers + every read, not just parent composition — strengthening #82's relevance. Filed + as #82; gates stage 3 of the staged plan. +- **In-process cache invalidation on reset (#80).** `cached_result_expr` is backed + by a module-level LRU that reset-to-revision does not invalidate, so a warmed + expensive entry can serve a pruned snapshot path after a reset. The #103 + hardening pass moved that memo onto `_resolve_result_plan` + (`@functools.lru_cache`, result_cache.py:232) with `cache_clear` / `cache_info` + re-exposed on the public `cached_result_expr` name, and fixed the related + warm-then-evicted dangling-path bug by re-checking `path.exists()` on every call + (result_cache.py:321), which now runs on every call (warm hits included), so the + concrete dangling-path failure is defended at the read site. #80 (and its sibling + #96, the LRU surviving a compute-cache prune) — `reset_to` does not call + `cache_clear` — are still open as filed; a reactive recalc/staleness layer has to + treat this in-process cache as one more thing to invalidate. Filed #80/#96. + +## Not in scope here + +No implementation. #48 was a prerequisite for the recovery model (closed by #57, +then the constraint removed by #103). The per-expression cache-metadata tab and +the catalog-sidebar tweaks are separate work in progress on another branch. diff --git a/plans/adr-result-cache-cost-rubric.md b/plans/adr-result-cache-cost-rubric.md index 3ec3322..316b8d5 100644 --- a/plans/adr-result-cache-cost-rubric.md +++ b/plans/adr-result-cache-cost-rubric.md @@ -1,16 +1,29 @@ # ADR: Result-cache admission and eviction by measured cost vs size -- **Status:** Proposed (2026-06-10) +- **Status:** Proposed (2026-06-10); reconciled with PR #74 (merged 2026-06-16) + and PRs #103/#104 (merged 2026-06-17). #74 shipped the *structural* `cache_worthy` + test as the live admission rule and moved the materialised result from + `result_cache/` into the per-project `compute_cache`. #104 then removed the + on-demand `result.parquet` layer entirely — the baked `.cache()` snapshot is the + sole materialised copy and cheap entries recompute via `cached_result_expr`. + Separately, #87 (merged via PR #94, ahead of #103/#104) persisted the structural + verdict (`cache_worthy` / `cache_worthy_why`) and the measured inputs + (`compile_seconds`, `cache_bytes`) into the manifest, so the instrumentation this + ADR needs now exists. This ADR's measured value-per-byte model is still the + target; the path to it is shadow-then-flip — see Decision §2. - **Context ticket:** buckaroo-data/tallyman#30 (measured cost/size cache rubric; supersedes #12, feeds #10 and #21) -- **Affected code:** `src/tallyman_xorq/result_cache.py` (`classify_build`, `cache_worthy`, `cached_result_expr`, `ensure_result`), `src/tallyman_xorq/build.py` (`build_and_persist`, manifest write), `src/tallyman_core/manifest.py` +- **Affected code:** `src/tallyman_xorq/result_cache.py` (`classify_build` at :61, `cache_worthy` at :98, `cached_result_expr` at :273, its memoized resolver `_resolve_result_plan` at :233 (lru_cache at :232), `baked_snapshot_path` at :211 — `ensure_result` was deleted by #104), `src/tallyman_xorq/build.py` (`build_and_persist`, manifest write), `src/tallyman_core/manifest.py` (`compile_seconds` / `cache_worthy` / `cache_worthy_why` / `cache_bytes`, manifest.py:47-50) - **Related ADR:** `plans/adr-source-identity-content-hash.md` (why `content_hash` is a stable cache key) ## Problem `cache_worthy` decides whether to materialise an entry's result by scanning the -serialized xorq build for ops (`classify_build` in `result_cache.py:51`): cache -if the expression contains a Join / Aggregate / Sort / Window / UDF or reads a -non-parquet source, otherwise treat it as cheap and skip the extra copy. +serialized xorq build for ops (`classify_build` in `result_cache.py:61`): cache +if the expression contains a Join / Aggregate / Sort / Window / UDF, otherwise +treat it as cheap and skip the extra copy. (A non-parquet source read — CSV / +JSON — is no longer itself worthy: since #73 the parse is cached at the read by the +injected source-cache node, so only expensive *work* downstream earns a result +snapshot. `_EXPENSIVE_OPS`, result_cache.py:48.) This is a *structural* test, and it misclassifies a common authoring workflow. The motivating case: a trips ⋈ stations join (~2.5M rows, 100s of MB) followed @@ -51,11 +64,21 @@ cost-aware eviction. (the `to_sqlglot`/sqlize path), timed separately from execution. Record the materialised result's size in bytes. -2. **Admit permissively; do not gate per entry.** There is no structural or - cost-floor admission test. Any built result is a cache candidate. The - structural `classify_build` / `cache_worthy` path is removed (this supersedes - #12, which sought to make that classifier robust — there is nothing left to - robustify). +2. **Admit permissively; do not gate per entry — reached by shadow-then-flip.** + The target is no structural or cost-floor admission test: any built result is a + cache candidate, ranked by value-per-byte. But #74 shipped the structural + `classify_build` / `cache_worthy` test as the live admission rule, so rather + than remove it blind, keep it live and compute the measured value-per-byte *in + shadow* alongside it (the admission-decision telemetry — staged plan deliverable + 0). The *recording* half of that telemetry shipped in #87 (PR #94): the manifest + now persists the structural verdict (`cache_worthy` / `cache_worthy_why`) next to + the measured inputs (`compile_seconds`, `cache_bytes`, `execute_seconds`), so + the verdict is no longer computed and thrown away — only the side-by-side + comparison and flip logic remain to build. Watch the disagreement set — entries + `cache_worthy` admits that the cost model rates near-zero (the 2.5M + intermediate) — and flip to measured-only when the data confirms it. This still + supersedes #12 (no point robustifying a classifier we are removing); it just + removes it on evidence, not on first principles. 3. **Rank by value-per-byte.** A cache's value is what reading it back saves over recomputing: `value ≈ recompute_cost − cache_read_cost`, divided by @@ -68,12 +91,15 @@ cost-aware eviction. 4. **Bound by a budget relative to raw data.** Cap the project's total result cache at K× the size of `data/` (start at 2×, matching #10). When the cache exceeds the budget, evict lowest value-per-byte first. Evicted results - self-heal: `ensure_result` / `cached_result_expr` recompute from the build on - the next miss. This is the eviction half of #10; admission and eviction now - share one cost model. + self-heal through `cached_result_expr` alone (#104 deleted `ensure_result`): an + evicted baked snapshot is repopulated by `baked.count().execute()` and re-read + on the next miss (result_cache.py:322, under the `path.exists()` guard at :321). + This is the eviction half of #10; + admission and eviction now share one cost model. 5. **Key on `content_hash`, never mtime.** The cache key is the entry's - `content_hash`, frozen at build time (`build.py:271`) and stable across + `content_hash`, frozen at build time (`content_hash = build_path.name`, + `build.py:403`) and stable across reset-to and reload. No modification-time comparison appears on the correctness path. (`content_hash` being a *content*-stable identifier is the subject of the related ADR.) @@ -108,10 +134,21 @@ model wants. v0 caches per-entry results only. - route_distance diffs stay fast: both 6.4k endpoints are small and high-value, so they are cached, and the diff is `read ⋈ read`. - Requires new instrumentation: `compile_seconds` in the build path and result - byte-size in the manifest. Accurate cache accounting is a prerequisite for - eviction — the `/api/disk_usage` pill currently omits `result_cache/` and - `diff_stat_cache/` (noted in #10) and must be corrected. -- Eviction depends on correct recompute self-healing, which already exists. + byte-size in the manifest (the admission-decision telemetry — staged plan + deliverable 0). **Both shipped** — the manifest carries `compile_seconds` and + `cache_bytes` (manifest.py:47-50, #87 via PR #94). Accurate cache accounting is a + prerequisite for eviction: #87 added the `compute_cache` byte count to + `_compute_disk_usage` (app.py:129) and #104 dropped the now-empty `results` + bucket, so the `/api/disk_usage` pill is correct. Only the ranking/eviction logic + on top of these fields is unbuilt. +- Eviction depends on correct recompute self-healing, which already exists — with + one soundness hole #74 widened and #104 widened further: an entry with + *execution* nondeterminism (`.sample()`, `ibis.now()`, unordered `.limit()`, + impure UDF) recomputes to *different* bytes than were evicted, silently, because + its `content_hash` is structural and can't see the difference. #104 made + recompute the default read path for *every* cheap and salt entry (not just + expensive evictions), so this hole now covers a wider surface. Such an entry must + be retained, not evicted-and-recomputed. Tracked as #83. - The `value ≈ recompute_cost − cache_read_cost` estimate is a heuristic; the read-cost term is approximated from bytes rather than measured. Acceptable because misranking only costs a recompute, never correctness. diff --git a/plans/adr-source-identity-content-hash.md b/plans/adr-source-identity-content-hash.md index ecb84aa..141bcd6 100644 --- a/plans/adr-source-identity-content-hash.md +++ b/plans/adr-source-identity-content-hash.md @@ -3,12 +3,19 @@ - **Status:** Accepted (2026-06-18 — default flipped `off` → `cas` in #86, with `cp --reflink=auto` on Linux and `.cas` GC wired into `reset_to`; supersedes the 2026-06-10 Proposed draft below. See the reconstruction caveat under - Consequences.) + Consequences.) Reconciled with PR #74 (merged 2026-06-16) and PRs #103/#104 + (merged 2026-06-17): #104 removed the on-demand `result.parquet`, so + `from_catalog` composes the parent via `cached_result_expr` and + recompute-on-cold-read is now the default for every cheap and salt entry — + which strengthened the case for `cas` before the flip landed. - **Context ticket:** buckaroo-data/tallyman#30 (precondition for the result-cache rubric's content-stable `content_hash` key) - **Affected code:** `src/tallyman_xorq/source_identity.py` (new), `src/tallyman_xorq/io.py` (`from_project`), `src/tallyman_xorq/build.py` - (`build_and_persist`), `src/tallyman_core/manifest.py` (`sources` map) + (`build_and_persist`, source-digest collect at build.py:355-360), + `src/tallyman_core/manifest.py` (`sources` map at manifest.py:53; the #84 + `parents` edge sits alongside it via the same collector pattern, + `parent_capture.py`) - **Related ADR:** `plans/adr-result-cache-cost-rubric.md` (relies on `content_hash` being a content-stable cache key) - **Evidence:** `tests/test_cache_lab.py` (marker `cache_lab`), reports under @@ -31,7 +38,7 @@ disproved both halves: than stat-fragility: identity is *blind to data changes*. The cache lab's `test_append_invalidates` shows it — append rows to `trips.parquet`, rebuild the same code, get the same `content_hash`, and the build-time - dedup (`build.py:286`) returns the stale entry's result. + dedup (`build.py:410`, `if target.exists()`) returns the stale entry's result. Path-only identity means: an in-place content change is invisible (stale results served under the old hash — the dangerous direction); a @@ -62,13 +69,28 @@ new digest → new path → new hash. Every xorq-level key derived from the expression — the build hash, `ParquetSnapshotCache` keys, anything future — becomes content-honest with no further guards. -`from_catalog` is unchanged: it already reads -`entries//result.parquet`, so the parent's identity is in the -path by construction. +`from_catalog` no longer reads a parent `result.parquet` (that read existed when +this ADR was drafted; #74 replaced it with parent-expression composition and #104 +removed the on-demand parquet outright). It now resolves the parent's +`content_hash` and composes the parent via `cached_result_expr` (io.py:63), +recording the resolved `{hash, ref, follow}` in `manifest.parents` (#84). The +parent's identity is carried by that recorded content hash, not a path — which is +exactly the content-honest identity this ADR establishes for source reads, applied +to entry reads. Implemented behind `TALLYMAN_SOURCE_IDENTITY=off|cas|salt` (default `off`) so the cache lab can benchmark strategies; accepting this ADR means flipping the default to `cas` after the consequences below are addressed. +PR #74 (merged 2026-06-16) raised the stakes from "should we" to "we must": +it reconstructs an entry on every cold read (`cached_result_expr` re-runs a +cheap entry's recipe, self-heals an expensive entry's evicted snapshot), and +under `off` that cold read pulls the *current* source bytes, so an in-place +edit is served under the entry's old `content_hash`. PR #104 widened the surface: +with the on-demand `result.parquet` gone, recompute-on-cold-read is now the +default for *every* cheap and salt entry, so the drift hazard is no longer +confined to expensive evictions. cas is the only mode that keeps reconstruction +faithful, so it is now a prerequisite for the reactive layer, not a tuning choice +(see `plans/adr-reactive-catalog-recalc.md`). ## Measured behavior (cache lab, 150k-row matrix) @@ -82,8 +104,9 @@ flipping the default to `cas` after the consequences below are addressed. | build overhead (memo hit / new content) | — | +13ms / +110ms per 71MB | same | "Self-heal faithful" is load-bearing for the result-cache ADR: its -budget/eviction loop assumes an evicted `result.parquet` recomputes to what -was evicted. Under `cas` the clone is a snapshot of the bytes the entry was +budget/eviction loop assumes an evicted cached result recomputes to what was +evicted (post-#104 that cached result is the baked `.cache()` snapshot, not a +`result.parquet`). Under `cas` the clone is a snapshot of the bytes the entry was built from, so regeneration is faithful even after the user edits the source in place; under `off`/`salt` regeneration silently absorbs the new data under the old identity. @@ -100,14 +123,24 @@ data under the old identity. because it fixes only the entry *name* while every xorq-level key stays path-only: the lab demonstrated two salted entries (same code and path, different data) colliding on one `ParquetSnapshotCache` key, with the new - entry serving the old entry's rows. `result_cache.py` now routes salt-mode - reads around that cache, but the collision class survives for any future - xorq-key surface, and self-heal stays unfaithful. The implementation is - kept in-tree behind the env switch so the lab can re-compare; delete it - when `cas` is made the default. -- **Stat-based identity (mtime/size/inode).** Unreachable for the same - snapshot-normalization reason, and undesirable anyway: forks on touch / - reset / transport, misses stat-preserving swaps. + entry serving the old entry's rows. Salt bakes no snapshot (`rewrite_for_build` + skips the cache under salt); post-#104 a salted entry no longer routes around an + in-place `result.parquet` — that path was deleted — it simply recomputes through + `cached_result_expr` (cheap-recompute / worthy-recompute-fallback, + result_cache.py:300). The collision class survives for any future xorq-key + surface, and self-heal stays unfaithful. The implementation is kept in-tree + behind the env switch so the lab can re-compare; delete it when `cas` is made + the default. +- **Stat-based identity (mtime/size/inode) / `ModificationTimeStrategy`.** + Unreachable for *identity*: the build hash is computed in a hardwired + `SnapshotStrategy().normalization_context()` (`provenance_utils.py:24`), so no + cache-strategy choice reaches it. Undesirable as a cache strategy anyway: it + forks on touch / reset / transport — every `reset_to` and checkout bumps + mtime/inode without changing a byte, so it would treat each as a full + invalidation and trigger a recompute storm in a reset-heavy workflow — and it + misses stat-preserving swaps. So it can't carry identity, and at the cache layer + it fights reset-to; cas keys on content and stays warm across a byte-identical + reset. - **Fix the hash in xorq.** Out of scope: frozen upstream. ## Consequences @@ -131,9 +164,10 @@ data under the old identity. flipping the default: add `cp --reflink=auto` on Linux and document the copy fallback. - `.cas/` accumulates one clone per content version. Manifests now record - each entry's `{rel_path: digest}` map (`manifest.py`), so GC is "delete + each entry's `{rel_path: digest}` map (`manifest.py:53`), so GC is "delete digests unreferenced by any live entry"; wire it into reset/bullpen - maintenance. + maintenance — which post-#103 lives in the native store (`tallyman_core/catalog.py`), + so the GC hook belongs there. - Builds embed `data/.cas/.parquet` paths; UI provenance should display the human name from the manifest `sources` map. - Cross-machine identity (absolute path prefix in the hash) remains broken diff --git a/plans/reactive-staleness-recalc.md b/plans/reactive-staleness-recalc.md new file mode 100644 index 0000000..6397232 --- /dev/null +++ b/plans/reactive-staleness-recalc.md @@ -0,0 +1,281 @@ +# Plan: Reactive consumer — staleness detection & recalc + +- **Status:** Plan (2026-06-18). Implements item #2 of the reactive gap analysis + against `plans/adr-reactive-catalog-recalc.md` (#51): the *consumer* side. + Everything the consumer reads is already captured; nothing reads it yet. +- **Scope:** Detect that an entry is stale relative to its inputs, surface the + stale cone, and recompute it (and its dependents) on demand. Excludes the + trigger-policy and cascade-semantics *decisions* — those are flagged as + decision gates below, not coded blind. +- **Affected code:** `src/tallyman_xorq/parent_capture.py` / + `src/tallyman_core/manifest.py` (`ParentRef`, `Manifest.parents`, + `Manifest.sources`, `Manifest.result_digest` — the captured inputs), + `src/tallyman_core/catalog_state.py` (`read_tallyman_state`, entry + enumeration), `src/tallyman_core/aliases.py` (`get_alias` head resolution), + `src/tallyman_xorq/source_identity.py` (source digesting, cas clones), + `src/tallyman_xorq/build.py` (`build_and_persist` — the recompute primitive), + new `src/tallyman_xorq/dependents.py` and `src/tallyman_xorq/staleness.py`, + `src/tallyman_mcp/server.py` + `src/tallyman_companion/app.py` (surfaces). + +## What remains — sequenced (verified against the tree 2026-06-18) + +The substrate is essentially complete. Every reactive prerequisite in epic #89 +is closed except the two open PRs below, and the worst substrate holes the +consumer would have inherited — #75 (multi-parent backends), #76 (by-path +snapshot pin), #97 (0-row parent), #79 (concurrent materialise race), #81 +(scalar-UDF worthiness), #83 (deferred-nondeterminism faithfulness), #85 +(alias-history disambiguation) — are all **closed**, as is #82 (deep-chain cost, +the stage-3 measurement gate, landed in PR #122). What is left is small and +ordered: + +0. **Land the two open prerequisite PRs.** Both are green and rebase clean on + current main. + - **#124** (#80/#96) — invalidates `_resolve_result_plan` and the companion's + `_build_compare_expr` LRUs on reset. *Load-bearing for this plan:* Stage 2 + step 4 needs the `cache_clear`-on-reset hook, and `_build_compare_expr` has + no per-call self-heal, so a recalc that prunes a snapshot then reads would + otherwise execute over a deleted parquet (the #73/#74 symptom). Merge first. + - **#121** (#88 part 2) — runtime structural-vs-execution drift attribution, + so recipe nondeterminism is not misread as staleness. Completes the + determinism prerequisite. + +1. **Close #115 first — the one genuine hard blocker.** The #86 cas flip made + *build* identity content-aware but did **not** make *cold reconstruction* + content-faithful: `from_project` (`io.py:55-60`) re-digests the **live** file + on every cold read and ignores both the recorded `manifest.sources` digest and + the frozen `.cas/` clone, even while `_RECONSTRUCTING` is set. So a + cold read of a not-yet-recalced downstream entry serves edited bytes under its + *original* `content_hash` — the "this hash ⇒ these bytes" premise this whole + consumer rests on is false the moment a source is edited in place. Both + staleness axes and the recomputed result are untrustworthy until this lands. + Fix: thread the reconstructing entry's recorded `sources` into a contextvar and + resolve `from_project` to `.cas/` (not `digest_for(live)`) during + reconstruction; pin with the assertion #86 omitted (cold read after an + in-place edit returns the *original* rows). + +2. **Build the consumer (this plan), Stages 0→3** — with the corrections under + "Corrections found during verification" below applied. + +3. **Answer the decision gates** (see "Open decisions") — confirm the recommended + defaults, don't default silently. + +**Land-with quality items** (fold each into the stage that touches the same code +path; none blocks correctness): #111 (`set_alias` is repointed before +`carry_forward_entry_config` — recalc's re-point adds a third caller of that +sequence), #117 (scalar-UDF snapshot key is non-deterministic — recalc re-bakes +on replay so it is noise on the determinism axis, not a recompute blocker), #118 +(concurrent `.execute()` on the shared default backend raises "Already borrowed" +— the serial recalc walk doesn't self-collide, but a recalc execute racing a +user read needs a process-wide execute lock before the in-companion Stage-3 +surfaces are reliable). + +**Deferred / out of scope:** #125 (escalate #88 to a build-time `BuildError` — +optional; this plan's separate `nondeterministic` flag already segregates those +entries), #77 (viewer empty grid on a cross-machine clone), and the +archive/delete-cone/multi-parent-unbind semantics (ADR Q3/Q4 — deletion work, +not the revise/source-edit staleness this plan handles). #98 appears obsolete +(the `xorq_catalog.py` yaml round-trip it depends on was removed in the native +recipe-store cut) — verify and close. + +**Corrections found during verification (apply when implementing):** + +- **Entry enumeration:** `read_tallyman_state(project)["entry_hashes"]` lists only + **checkpointed** entries (from `entries.jsonl`); a freshly built, + not-yet-checkpointed entry is on disk but absent. Enumerate via + `build.list_entries(project)` (reads every `manifest.json`) for a complete scan. +- **Source axis:** `source_identity.digest_for` is **stat-memoized** (keyed on + mtime_ns/size/inode in `artifacts/source_digests.json`); a same-stat content + swap returns the stale digest. Set `TALLYMAN_SOURCE_REHASH=1` for a sound + staleness check, or the source-drift axis can false-negative. +- **Cache invalidation:** the public clear is `cached_result_expr.cache_clear()` + (the alias of `_resolve_result_plan.cache_clear`, `result_cache.py:479`), not a + module-level `result_cache.cache_clear`. +- **Recompute primitive:** `build_and_persist(project, code, ...)` takes recipe + **source text**, not an expr or a hash, and does **not** set the alias or + checkpoint — the recalc caller reads the entry's `expr.py`, replays it, then + re-points the alias and checkpoints. Replay yields *advanced* parents only when + the recipe's `from_catalog` argument is the alias **name** (follow=True); a + literal-hash recipe re-pins the same parent (consistent with pinned-child + handling). +- **Stale code refs to clean up while here:** `io.py:83` (the `from_catalog` + docstring) and `parent_capture.py:9` still name `lineage.catalog_parents`, + deleted in #76; `source_identity.py:16` docstring still says `off (default)` + though the code default is `cas`. + +## Why this is the biggest piece + +The substrate shipped (#73/#74/#103/#104/#84/#83/#88) and source-identity now +defaults to `cas` (`source_identity.py:53`). Parent edges are *recorded* +(`manifest.parents`), source digests are *recorded* (`manifest.sources`), and the +executed-bytes digest is *recorded* (`manifest.result_digest`). But **nothing +consumes any of it**: no code compares a recorded parent hash against the current +alias head, nothing compares a recorded source digest against the file on disk, +and `from_catalog`'s docstring (`io.py:83`) still points at a +`lineage.catalog_parents` reader that #76 deleted. The reactive promise — "edit a +source or revise an upstream entry and dependents recompute instead of going +silently stale" — is entirely unbuilt on the read side. This plan builds it. + +## Prerequisite (item #1): a manifest-driven dependents reader + +#76 deleted `lineage.py` (`catalog_parents` / `catalog_dag`) because its only +consumer was the removed lineage views. The edges survive in `manifest.parents`; +the *reader* does not. Rebuild a thin one — this is not the old column-lineage +feature, just the DAG walk the consumer needs. + +New `src/tallyman_xorq/dependents.py`: + +- `parents_of(project, content_hash) -> list[ParentRef]` — read + `manifest.parents` for one entry (None → `[]`). +- `build_dag(project) -> {child_hash: list[ParentRef]}` — enumerate every live + entry hash via `build.list_entries(project)` (reads every `manifest.json`) and + read each manifest's `parents`. Forward edges. (Do **not** enumerate from + `read_tallyman_state(project)["entry_hashes"]` — that lists only *checkpointed* + entries and misses a freshly built, not-yet-checkpointed one.) +- `dependents_index(project) -> {parent_hash: set[child_hash]}` — invert + `build_dag`. This is the reverse index the recalc walk needs; it does not exist + anywhere today. +- `descendant_cone(project, root_hashes) -> ordered list[hash]` — BFS over the + reverse index, topologically ordered (parents before children) so a recalc + rebuilds in dependency order. Detect and report cycles rather than looping + (a `from_catalog`-of-own-alias recipe is already guarded at build, `io.py:100`, + but the reader must not assume acyclicity). + +Tests: a 3-level chain (A→B→C), a diamond (A→B, A→C, B+C→D), a pinned edge +(`follow=False`) and a followed edge (`follow=True`) both appearing in the cone, +and a cycle surfacing as an error not a hang. + +## Stage 1 — staleness predicates (read-only, no recompute) + +New `src/tallyman_xorq/staleness.py`. An entry is **stale** when any input it was +built against no longer matches the current world. Two independent axes, both +computable from already-recorded data: + +1. **Upstream advanced (followed alias).** For each `ParentRef` with + `follow=True`: resolve the alias head now via `aliases.get_alias(project, + ref)`. Stale iff `head != parent.hash`. A `follow=False` (hash-pinned) parent + is *never* stale on this axis by definition (`parent_capture.py:16-19`). +2. **Source drifted.** For each `(rel_path, md5)` in `manifest.sources`: + recompute the current source digest with the same helper `source_identity` + uses at build (`digest_for`, via `io.project_path(rel_path, project)`), compare + to the recorded `md5`. Stale iff they differ. Set `TALLYMAN_SOURCE_REHASH=1` + first — `digest_for` is stat-memoized, so a same-stat in-place content swap + otherwise returns the cached digest and the axis false-negatives. Under `cas` + the recorded digest is the frozen-clone digest, so this is + a faithful "the file the user pointed at changed" signal; under `off` + (`manifest.sources is None`) this axis is unavailable — report `unknown`, not + `fresh` (see Open decision: off-mode). + +`entry_staleness(project, hash) -> StaleVerdict` returns +`{stale: bool, reasons: [{axis, ref|path, was, now}], unknown_axes: [...]}`. + +`scan(project) -> {hash: StaleVerdict}` runs it over every live entry. A +**directly** stale entry (its own inputs moved) is distinguished from a +**transitively** stale entry (a stale ancestor in `dependents.descendant_cone`); +the scan marks both but tags which. This is the "scan-on-load that flags +staleness" the ADR's trigger discussion leans toward. + +`result_digest` is *not* a staleness input — it is the soundness check #83 +already wires at self-heal. The scan should surface a separate +`nondeterministic` flag (recorded `result_digest` present, recompute differs) +distinct from `stale`, because that entry can't be made fresh by recompute and +needs the #83/#30 path, not this one. + +Tests (failing-first, per the TDD rule): build A, alias it, build B +`from_catalog("A")`, re-revise A so the alias advances → `entry_staleness(B)` +reports stale on the followed-alias axis with `was=oldhash now=newhash`. Build +with a literal hash → not stale after A advances. cas mode: mutate the underlying +source file → stale on the source axis; off mode: same mutation → `unknown`. + +## Stage 2 — recalc action (the consumer that acts) + +`recalc(project, roots, *, dry_run) -> RecalcReport`: + +1. `cone = descendant_cone(project, roots)` — topologically ordered. +2. `dry_run=True` returns the cone with each entry's `StaleVerdict` and the + recompute plan (what would rebuild, in order) **without building**. This is + the "preview the cone" the ADR Q4 calls for; it is also the safe default the + surfaces should call first. +3. For a real run, walk the cone in order and rebuild each entry by replaying its + recipe through `build.build_and_persist` — the same primitive `catalog_revise` + uses — so a recomputed entry gets a fresh `content_hash`, fresh + `manifest.parents` (now resolving the *advanced* alias head), and a fresh + `result_digest`. Re-point the followed alias to the new hash via + `aliases.set_alias`. A hash-pinned child is left on its pin (it asked for that + revision). +4. **Invalidate the in-process cache** before/after each rebuild: call + `cached_result_expr.cache_clear()` (the alias of + `_resolve_result_plan.cache_clear`, `result_cache.py:479`). PR #124 already + wires this into `reset_to`, so a checkpoint-wrapped recalc inherits it; the + explicit call covers a rebuild that reads before the wrapping checkpoint + commits. This is the coupling to #80/#96 — see Dependencies. +5. Wrap the whole walk in one checkpoint (`catalog_state.checkpoint_catalog`) so a + recalc is one git transaction and `reset_to` undoes it atomically. + +The cascade-failure behavior (what to do when entry k of the cone fails to +rebuild) is a **decision gate** — do not pick silently. Default the +implementation to **stop-and-report** (leave already-rebuilt entries committed, +halt at the failure, return the report) because it is the least surprising and is +trivially recoverable via `reset_to`; flag the all-or-nothing and +skip-and-continue alternatives in the report for the decision. + +## Stage 3 — surfaces + +- **MCP** (`tallyman_mcp/server.py`): `catalog_scan_staleness(project)` → + the scan map; `catalog_recalc(project, roots, dry_run)` → the report. dry_run + defaults true. +- **Companion** (`tallyman_companion/app.py`): a staleness badge per entry in the + catalog sidebar driven by the scan, and a "recalc" affordance that previews the + cone (dry run) then commits. Scan-on-load wiring fires the scan when a project + is switched in / loaded. +- Keep the surfaces thin: all logic lives in `staleness.py` / `dependents.py` so + both MCP and companion call the same code. + +## Open decisions (gate before coding the stage they touch) + +These are genuinely undecided in #51 and must be answered, not defaulted: + +- **Trigger model** — button vs scan-on-load vs auto-cascade-on-revise. Plan + assumes *scan-on-load (passive flag) + explicit recalc button*; auto-cascade is + out unless chosen. Gates Stage 3. +- **Cascade failure** — stop / skip-and-continue / all-or-nothing. Plan defaults + to stop-and-report. Gates Stage 2 step 4. +- **off-mode source axis** — report `unknown` (plan's choice) or force a one-time + digest backfill so the axis becomes available. Now that `cas` is the default, + off is opt-out; `unknown` is probably fine. Gates Stage 1 axis 2. +- **Pinned-child-of-advanced-alias** — confirm a `follow=False` child stays put on + recalc (plan assumes yes; it asked for that revision). Gates Stage 2 step 3. +- **Multi-parent / archive semantics** (ADR Q3/Q4 — unbind, archive overlay, + delete cone) — *out of scope here*; this plan handles revise/source-edit + staleness, not deletion. Cross-link when that work starts. + +## Dependencies on open issues + +- **#115 (OPEN — hard blocker, land before Stage 1).** Cold reconstruction is not + content-faithful: `from_project` re-digests the live file and ignores the + recorded `manifest.sources` digest / `.cas` clone during reconstruction, so an + edited-in-place source is served under the entry's original `content_hash`. The + staleness scan's "hash ⇒ bytes" premise and a recompute that reads upstream + bytes are both unsound until this is fixed. See "What remains" step 1. +- **#80 / #96 (FIXED by PR #124, pending merge).** `reset_to` and a compute-cache + prune did not invalidate the `_resolve_result_plan` / `_build_compare_expr` + LRUs, so a recalc that rebuilds then reads could serve a pruned snapshot path. + Stage 2 step 4 *needs* the `cache_clear` hook; PR #124 wires it into `reset_to` + and the companion reset paths. Merge before the recalc action lands. +- **#82 (CLOSED — PR #122).** Deep-chain build/reconstruct cost was measured + (the stage-3 gate). Recalc replays recipes through the cone; the measurement + bounds how interactive a large cone's recalc is and whether more baking + boundaries are warranted, but it no longer gates the design. + +## Staging / TDD order (per global rules) + +0. Merge PR #124 (#80/#96) and PR #121 (#88); then fix #115 (digest-pinned + reconstruction) — failing test (cold read after in-place edit returns original + rows) then fix. These precede the consumer. +1. Dependents reader + staleness predicates with **failing** tests committed and + seen red on CI (alias-advance, source-drift, pinned-stays, off=unknown, cycle). +2. Then the reader + `staleness.py` implementation; watch CI go green. +3. `recalc` (dry-run first, then real) + report — failing test then fix. +4. MCP + companion surfaces last, once the trigger/cascade decisions are made, + and after the #118 execute-lock if the recalc runs in the companion process. + +Each test bundle is one commit, separate from its fix, never bundled with it. diff --git a/plans/staged-catalog-perf-reactive.md b/plans/staged-catalog-perf-reactive.md new file mode 100644 index 0000000..bd54018 --- /dev/null +++ b/plans/staged-catalog-perf-reactive.md @@ -0,0 +1,482 @@ +# Plan: three-stage path to reactive catalog recalc + +Status: **proposed (2026-06-14); reconciled with PR #74 (merged 2026-06-16) and +PRs #103 / #104 (merged 2026-06-17).** +Synthesises the decisions from the planning conversation around the +reactive-catalog ADR (`plans/adr-reactive-catalog-recalc.md`). Splits the work +into three sequenced stages. No implementation here. + +**#74 reconciliation.** #74 dropped the blanket per-entry `result.parquet` and +moved to materialization only at expensive boundaries — the model intended all +along; the prior blanket behaviour was a misread of the codebase (see the ADR's +Reconciliation section). Net effect on this plan: stage 1 is unaffected (its +prerequisites closed); stage 2's cache-layer inventory and the build-cost +measurement are updated below (#82); and stage 3's "landable early" tagged-read +spine is removed — #74 took the composition fork instead, so cross-entry lineage +is now a deferred semantic-dependency mechanism, not a `HashingTag` walk. + +**#103 / #104 reconciliation.** Two PRs landed after this plan was written and +reshape Stage 1 and parts of Stages 2/3. **#104** (#101) removed the last +*on-demand* `result.parquet` layer (`ensure_result` and consumers deleted; the +baked `.cache()` snapshot is the only materialized copy; `cached_result_expr` is +the sole read path) and closed the #74 disk-usage / cache-inspector follow-ups +that Stage 2 listed as open. **#103** (#52) replaced xorq's catalog package with a +native `tallyman_core.catalog` store and decomposed `catalog.yaml` into +per-concern tracked files — which *dissolves Stage 1's whole bug cluster by +construction* rather than fixing it within xorq, supersedes the PR #57 mechanism +(aliases moved out to `aliases.jsonl`, not folded into `catalog.yaml`), ships a +catalog rebuild script (`scripts/rebuild_native_catalog.py` — a migration +convenience, not on the critical path; see Stage 1 item 4), and — via #84 — +ships the manifest `parents` edge Stage 3 listed as a prerequisite. Per-item +annotations below; full design in `plans/native-catalog-store.md`. + +The throughline: don't build the reactive layer on machinery that doesn't yet +hold its own invariants, and don't redesign the caching until it's measured. So +stage 1 makes the existing catalog machinery correct and tested, stage 2 maps and +measures the caching, and stage 3 builds reactivity on top — using stage 2's +numbers to settle the ADR's open design questions. + +Operating constraints (stated 2026-06-14): single user, one active parking +catalog, everything `--no-sync`. All perf work runs on this Mac alone; no +distributed or CI perf runs. These narrow the scope in places noted below. + +--- + +## Stage 1 — catalog ↔ xorq coexistence, correct and tested + +The existing machinery has a cluster of bugs where tallyman and the xorq catalog +disagree about what is durable. Stage 1 closes that cluster and proves it closed, +then produces a clean parking catalog for stage 2 to measure. + +### The bug cluster + +> **Superseded by PR #103.** This cluster was framed as "make tallyman and the +> xorq catalog agree about what is durable." #103 dropped xorq's catalog package +> instead: there is no `xorq catalog add` subprocess, no xorq `assert_consistency`, +> and no `catalog.yaml`. Tallyman owns the format end-to-end, so the disagreement +> that produced #48 cannot recur — the cluster is dissolved by construction, not +> reconciled. The descriptions below are the pre-103 diagnosis; per-bug status +> follows in Work. See `plans/native-catalog-store.md`. + +- **#48** — tallyman commits `aliases.json` / `alias_history.json` into the xorq + catalog repo → `assert_consistency` fails → every `xorq catalog add` after the + first silently no-ops. 17/18 recipes never reached git. +- **#52** — `reset_to` reconciles the untracked tallyman artifacts (entry dirs, + result/compute cache) against the pointer lists via the bullpen, but the + git-tracked xorq `.zip` recipes are reconciled separately by `git reset --hard`, + with no cross-view consistency check. A back→forward reset restores an entry the + durable catalog cannot reproduce and reports success. Compounded by #48 today, + but independent: any failed/interrupted `add` (which `checkpoint_catalog` already + anticipates, `catalog_state.py:277`) triggers it after #48 is fixed. +- **#49 / #54** — authored state (charts, post_processing, stats, notebook, …) + stored as `catalog.yaml` keys is dropped when xorq rebuilds `catalog.yaml` from + entries+aliases on a merge. **Resolution direction settled by #54 (and the #57 + precedent):** this state is shareable catalog content and *belongs* in + `catalog.yaml` so it clones and versions with the catalog — #49's proposed + relocation to an out-of-repo store would stop that. So the fix is on the + *durability* side (make the keys survive xorq's merge-rebuild — e.g. per-entry + state into the entry metadata sidecar, which xorq treats as opaque), not + relocation. **Deprioritised out of stage 1's critical path:** the drop is latent + only under multi-user `pull`/sync, which does not happen single-user / + `--no-sync`. Stays filed (#49, #54); not fixed this stage. + **[Resolved-by-decomposition in PR #103.** The premise — authored state lives as + `catalog.yaml` keys that xorq's merge-rebuild can drop — no longer holds. #103 + decomposed `catalog.yaml` out of existence: charts → `chart_specs/.vl.json`, + display → `display_configs/.json`, post-processing/stats → tracked `.py` + files, notebook → `notebook.jsonl`, aliases → `aliases.jsonl`, per-entry prompts → + `prompts/.jsonl`. Each section is its own git-tracked file (the durability + #49 wanted), achieved by dropping xorq rather than relocating out-of-repo. No + `catalog.yaml`, no merge-rebuild, nothing to drop.]** + +### Work + +1. **✅ DONE (PR #57, merged) — #48 fix, alias bookkeeping in `catalog.yaml`.** + `aliases.py` read/wrote `alias_map` / `alias_history` as `catalog.yaml` keys via + `catalog_state` (no `aliases.json`, no out-of-repo sidecar — #53's alternative + was rejected). The tracked file set was back to xorq's expected set, + `assert_consistency` passed, `xorq catalog add` stopped no-opping. `reset_to`'s + `git reset` rolled the alias keys back with the rest of `catalog.yaml`. **#48 + closed.** `test_reset_rolls_back_alias_state` shipped with the fix. + *[Superseded by PR #103: the `catalog.yaml`-keys mechanism is gone. Aliases now + live in a dedicated tracked `aliases.jsonl` (`aliases.py:37`, `_aliases_file → + catalog_dir/aliases.jsonl`), which #57 could not do because committing a + separate file into the (then xorq) catalog repo broke `assert_consistency` — + the very constraint #103 removed. #48 stays closed; the #57 implementation no + longer exists.]* + +2. **✅ DONE (PR #58, merged) — #52 fix, reset cross-view reconciliation.** + `reset_to` asserts the git-tracked recipe set (`entries/.zip`) equals the + `entry_hashes` pointers after the bullpen reconcile; any pointer with no durable + recipe (or recipe with no pointer) raises instead of returning a + silently-divergent catalog. The open recovery-policy question ("re-register vs + fail loudly") was settled **fail-loud**. **#52 closed.** + *[#103 moved this check into the native store: `reset_to` now calls + `tallyman_core.catalog.assert_catalog_consistent` (catalog.py:197) — a + deny-by-default `TRACKED_SURFACE` allowlist whose pointer/recipe-agreement guard + subsumes the #58 check — and the `entries/.zip` are now tallyman's own + deterministic `write_recipe_zip` output (catalog.py:126), not `xorq catalog add` + output. The fail-loud invariant is preserved; the mechanism is native.]* + +3. **Largely dissolved by PR #103 — re-scope #56.** The silent swallow was a + property of the xorq-subprocess path: `set_alias` discarded the bool from + `xorq catalog add-alias`, and `catalog_registered` tracked whether the + subprocess committed. #103 deleted that subprocess. `catalog_registered` now + means "the entry dir was persisted" — a local fact that succeeds because the + dir is written complete before the pointer is added (`build.py`) — and + `set_alias` no longer wraps a swallowed `add_alias` call. The remaining + observability gap is different: `plans/native-catalog-store.md` notes that a + corrupt tracked JSON surfaces as a bare HTTP 500. Re-scope #56 against the + native store before doing any work; the original swallow it targeted is gone. + +4. **Build the stage-2 corpus FRESH in native format — we do NOT migrate old + catalogs.** Per the project rule (single-user dev repo, no installed base): + there is no migration/upgrade burden, and **rebuilding the corpus from scratch + is always the accepted fix**. So the stage-2 corpus is produced by re-authoring + the entries fresh against the source parquets through the normal build path + (`build_and_persist`) — the same way the originals were created — **not** by + converting the pre-#103 on-disk parking catalogs. The source parquets exist + (`/Users/paddy/code/parking_speeding_data/`, ≈25GB). Truncation is a build-time + parameter of the fresh authoring (`head(1M)` on the source read; see Truncation + design under Deliverable 2), not a property of any conversion step. The pre-#103 + on-disk parking catalogs (`parking_ticket_analysis`, `parking_ticket_analysis_1m`) + are stale and can simply be discarded and rebuilt; their layout is irrelevant. + + `scripts/rebuild_native_catalog.py` (shipped in PR #103) is a *migration* + convenience that re-execs an old catalog's recipes into the native store. It is + **not on the critical path** and we don't need it for our own data — its + build-time alias-revision pinning limit on schema-evolving corpora is a + migration-only concern and **out of scope** under the no-migration rule. (#98 — + the old `rebuild_parking_catalog.py` shelling to `xorq catalog add` — is likewise + moot; #99 — the perf harness reading native `aliases.jsonl` — is already fixed in + code, `b51216f`, and closeable.) See `plans/native-catalog-store.md`. + +### Tests / acceptance + +- ✅ PR #50's failing integration tests (Groups A/B/D) landed on `main` + (`80d2e9b`); #57 made the #48 ones pass and added Group F (alias registration + durability); #58 added `test_reset_surfaces_pointer_without_durable_recipe` for + the reset divergence and made it pass. The TDD red-then-green split is satisfied + for both #48 and #52. *[#103 replaced the xorq-catalog test surface: the + assertions that pinned `xorq catalog add` behavior were deleted or retargeted, + and the native store has its own consistency/durability suite + (`tests/test_catalog_xorq_integration.py` rewritten, plus the recipe-zip / + tracked-tree / reset tests). The #48/#52 invariants are still pinned, now against + the native store. See `plans/native-catalog-store.md`.]* +- A broader end-to-end demo-walk invariant test (genesis → creates → revise → + reset back → forward → back, asserting catalog-opens + recipe-durable + + alias-correct + authored-state-survives at every step) would still be a nice + umbrella over the now-passing per-bug tests, but the core invariants are pinned. + Optional; not blocking stage 2. + +--- + +## Stage 2 — model the expression lifecycle, then measure it + +Premise (corrected 2026-06-14): app performance is dictated by **how many +expressions have to be built, loaded, and re-executed, and when** — not by the +cache *key* strategy (Snapshot vs ModificationTime), which experimentation so far +shows is second-order. So stage 2 is reframed around the expression lifecycle, not +the cache design. + +The xorq question is pragmatic, not a correctness audit. Ignore "are you using it +right" — the only question that matters is the ADR's guiding principle: where xorq +can do what we need *and is fast enough*, lean on it and write no code; where it +can't, or is too slow for interactive use, build our own. Each capability gets a +native-or-build verdict backed by a number, not an opinion. + +### Deliverable 0 — consolidate perf instrumentation, make noise configurable (#60) + +Across tallyman and buckaroo, perf/cache-stat logging has been added and then +turned back off in churn (buckaroo `19abae89` "log cache stats", `72bcf2ad` +bk-flash cross-stream tracing + "console cleanup", and similar on/off cycles). +Stop the churn: **leave the instrumentation in permanently and gate its noisiness +the normal way** — log levels and namespaces, not commenting lines in and out. + +The plumbing already exists and is partly used correctly: `TALLYMAN_LOG_LEVEL` +(`src/tallyman_mcp/server.py:1388`), namespaced loggers (`tallyman.companion`, +`tallyman.buckaroo`, `tallyman_mcp`), and instrumentation already level-gated — +the warmup timing in `app.py:400-432` (`log.debug` per entry at :419, `log.info` +summary at :427), `execute_seconds` in `build.py:493`. The work: + +- Recover the toggled-off perf/cache-stat logs and reinstate them permanently at + the right level (per-event detail at DEBUG, per-action summaries at INFO). +- Route perf logging through a dedicated child namespace (e.g. `tallyman.perf`, + and the buckaroo equivalent) so it is independently dialable without raising + the level of everything else. +- For the buckaroo JS side, gate the bk-flash / cross-stream tracing behind a + debug flag rather than deleting it in a "console cleanup." +- Log the **admission decision** with both verdicts side by side, so the + structural-vs-measured tradeoff (#30) is decidable from data — not just + lifecycle counts. *The recording half of this shipped in #87 (PR #94):* the + manifest now persists the structural `cache_worthy` / `cache_worthy_why` verdict + (no longer computed and thrown away) alongside the measured inputs — + `compile_seconds` (the dominant per-view cost), `cache_bytes` (the + result/snapshot byte-size) at manifest.py:47-50, plus `execute_seconds` at + manifest.py:38. #87 also + added the `compute_cache` byte count to disk accounting (app.py:129), supplying + the byte denominator and closing the #74 disk-usage follow-up. What remains is + the side-by-side comparison/flip logic and the per-read path tags: tag each cold + `cached_result_expr` read with the path it took (`cheap-recompute` / + `worthy-recompute-fallback` / `baked-read` / `evicted-self-heal`, + result_cache.py emits these via its `_cold` helper). Counts alone can't show that + a Join-descendant is structurally worthy but cost-cheap. + +This is a prerequisite for the rest of stage 2: the lifecycle counts in +deliverable 1 and the harness in deliverable 2 read these logs as the in-app +source of build/load/execute events. (buckaroo changes are in scope — it's the +companion's own viewer — and follow buckaroo's build/test conventions.) + +### Deliverable 1 — the expression-lifecycle cost model (write before any perf test) + +> **Update (PR #47, merged):** the *prose* lifecycle model and the refreshed +> cache map shipped as `docs/expression-lifecycle.md` and `docs/caching.md`, both +> already post-#74/#104 accurate (baked snapshot is the only materialized copy, +> baked at build; source-read cache; no `result.parquet`). What remains for this +> deliverable is the *measured* per-action build/load/execute counts, which +> Deliverable 2's harness produces. + +For each user-facing action — open the app, view an entry, revise a source, open a +diff, reset — answer: how many expressions get **built** (graph constructed / +tokenized), how many get **loaded** (from a build dir, the `/load_expr` path), and +how many get **re-executed** (`.execute()`), and which of those were avoidable. +Build/load/execute counts are the explanatory variable; wall-clock is the symptom. + +- **Build cost** dominates with graph depth and count — long xorq graphs are + expensive to construct independent of execution. Pre-#74 `from_catalog` + truncated every parent at a parquet read; #74 truncates only at expensive + parents (baked snapshot) and composes cheap ones, so "how many entries, how deep + each chain, and where the expensive boundaries sit" is the first-order lever + (measured by #82). +- **Load cost** is distinct from build — #34 (the 10s `/load_expr` timeout that + abandons cold loads) is a load-path problem, not a cache-key problem. +- **Re-execution timing** is where caching actually shows up, but the lever is + *when* re-execution is triggered, not how the key is computed — #21 (caches warm + lazily on view, never at build) means re-execution lands at view time and the + user waits. + +The cache layers matter only insofar as they change those counts (a hit skips a +re-execution; a shallow graph skips a build). The catalogue of layers already +exists — merged via PR #47 as `docs/caching.md` ("map every cache in the +tallyman/xorq/buckaroo stack", `xorq` `SnapshotStrategy`, the `.cas` store + +`content_hash`, buckaroo's stat/session caches). **The #74/#104 refresh is already in that merged doc:** the +separate `result_cache/` dir is retired (baked results now co-locate in the +per-project content-addressed `compute_cache`), and a new source-read cache +(snapshot `.cache()` after each `read_csv`/`read_json`, shared across entries, +parquet exempt) was added. #74's two disk-usage / cache-inspector follow-ups are +now closed, across two PRs: #87 (PR #94) added the `compute_cache` byte count to +`_compute_disk_usage` (app.py:129), and #104 dropped the now-empty `results` +bucket and repointed the cache inspector + delete off the manifest `cache_bytes` +and the baked snapshot path, listing only snapshots that exist on disk (app.py:1007 +/ :1085) — there is no on-demand `result.parquet` left to reflect. Reuse the map as +the levers section; the prose lifecycle model on top also shipped +(`docs/expression-lifecycle.md`, PR #47), so what is left for this deliverable is +the *measured* per-action build/load/execute counts — produced by Deliverable 2's +harness, not a re-derived cache-key taxonomy. + +Existing backlog items are findings against this model: #21 (lazy warm), #22 +(checkpoint capture cost per mutating request), #30 (measured cost rubric vs +structural `cache_worthy`), #35 (leftover result dirs). The hypotheses to settle +with numbers: build cost as a function of `from_catalog` chain depth now that #74 +truncates only at expensive boundaries and composes cheap parents (#82 — reframes +the ADR's untimed deep-chain question); how many expressions are needlessly +rebuilt or reloaded per app open; does revising a source re-execute dependents at +all today (ADR says no — fast but stale). + +### Deliverable 2 — performance integration tests (#59) + +> **Update (#59 / #64, both closed):** both tiers shipped — Tier B +> (`tests/test_perf_integration.py`, `perf` marker: execute / page-load / diff, +> cold+warm) and Tier A (`tests/test_perf_tier_a.py`, `perf_tier_a` marker, #64: +> Playwright to `.df-viewer .ag-cell` + backend/total calibration). Three things +> remain: (1) it is **report-only** — promote #45's diff time to a regression gate +> once baselined; (2) the harness still references the OLD +> `scripts/rebuild_parking_catalog.py` for its corpus — repoint it at the +> freshly-built native corpus (Stage 1 item 4; no migration), with truncation done +> at authoring time (`head(1M)` source read); (3) the **#82 deep-chain depth +> measurement does not exist** and gates Stage 3. + +Three measurements, each on both corpus variants (full / truncated) and across a +cold/warm cache axis. Local-only on this Mac, marker-gated off CI (same convention +as `cache_lab`). + +Every measurement records the **build / load / execute counts** alongside +wall-clock — per deliverable 1, the counts are the explanatory variable and the +time is the symptom. A regression in wall-clock should be attributable to a +changed count (one more expression rebuilt, one more cold load), not left as an +unexplained slowdown. + +- **Execution time** — `time expr.execute()`. Trivial; cold and warm. +- **Page load including buckaroo — two tiers.** + - *Tier A (Playwright, ground truth).* Spin up the catalog + buckaroo server, + drive `/load` (`mode: 'buckaroo'`), measure to **data-grid-visible** + (`.df-viewer .ag-cell` first-visible) — that is "loaded." Follow buckaroo's + server-mode examples: `packages/buckaroo-js-core/pw-tests/server-buckaroo-summary.spec.ts`, + `playwright.config.server.ts`, `scripts/test_playwright_server.sh`. Produces + the baseline number. + - *Tier B (python-only proxy).* Call the same backend the buckaroo `/load` + (`mode=buckaroo`) handler dispatches to — load the xorq expr from the entry's + `xorq_build/` dir + compute summary stats — no server, no browser. Fast and + deterministic; this is the test that runs routinely. + - *Calibration (one-time, required).* Run both tiers on the same entry, record + the backend/total ratio, and document what Tier B cannot see (browser render, + network, first-paint JS). For the big parking datasets the backend dominates, + so the proxy is faithful — but the ratio must be written down so the proxy + can't silently drift from reality. +- **Diff performance** — exercise the `catalog_diff` stat path (#45 measured 27s + synchronous on first open for 18.4M vs 1.0M). Define the revision pairs diffed; + cold and warm. + +**Truncation design.** Truncate at the raw-source read (`head(1M)` on the source +parquet) so the DAG shape and build/tokenize cost are identical to the full +corpus and only execution cost drops. Both variants are built fresh in the native +format from the source parquets (Stage 1 item 4 — no migration of old catalogs); +regenerable, not checked in. + +**Gating.** Report-only first (no baselines yet), matching the existing perf-report +suite. Promote a few to regression gates once baselined — #45's diff time is the +obvious first gate. + +**Why this is the point.** Once the harness exists, competing approaches stop +being argued and start being measured. Every native-or-build verdict in this +stage, and every design choice in stage 3, is decided by running both options +through the same harness and reading the build/load/execute counts and wall-clock +off the parking corpus. The harness is the comparison instrument, not just a +regression guard. + +--- + +## Stage 3 — reactive recalc & staleness + +Design captured in `plans/adr-reactive-catalog-recalc.md`. Two of its open +questions are answered by stage 2's measurements, so stage 3 design is gated on +stage 2: + +- deep-chain build cost → #82 (reframed by #74: `cached_result_expr` + reconstruction at depth, not eager `load_expr` against a tagged read). +- `cas` is a prerequisite, not an open question (resolved in the reactive ADR): + #74's recompute-on-cold-read means only cas keeps "this hash ⇒ these bytes" + true. Stage 2 measures only its disk cost, to size the reflink/GC mitigation — + not whether to adopt it. + +**Superseded by #74 — the "landable early" tagged-read spine is dropped.** This +plan previously proposed landing, during the stage-1/2 window, a tagged read +(`deferred_read_parquet(/result.parquet).hashing_tag(…)`) plus a +`walk_nodes(HashingTag)` rewrite of `lineage.py`. #74 took the other fork: +`from_catalog` reconstructs the parent expression (`cached_result_expr`) instead +of reading a `result.parquet`, and `catalog_parents` is allowed to go dark. There +is no per-entry `result.parquet` read left to tag, so neither item can or should +land. The cross-entry DAG restoration this paragraph proposed — record the +resolved parent hash in the manifest (a `parents` field, parallel to `sources`) +and have `catalog_parents` read it — **shipped in #84 (merged via PR #91, ahead of +#103)**: `parent_capture.note_parent` records `{hash, ref, follow}` at build +(io.py:111), `build_and_persist` persists it into `Manifest.parents` +(manifest.py:16/:56), and `catalog_parents` reads it (lineage.py:112), with the +path-regex retained only as a pre-#74 fallback. #103 then made the edge durable +across reset by putting `manifest.json` in the recipe-zip member set +(`RECIPE_TOP_LEVEL` + `RECIPE_TREE_DIRS`). Only the *follow-the-concept* layer +(depend on what a parent +computes, not its materialised hash — #73) and the reactive *consumer* (walk the +recorded edges to recompute dependents) are the genuinely deferred work. + +**Prerequisites — landable now, ahead of the reactive feature** (each filed as +its own issue; the 2026-06-16 design review settled their direction): + +- **Restore the cross-entry DAG + read-intent** — ✅ **DONE (#84, merged via + PR #91):** `Manifest.parents: [{hash, ref, follow}]` is recorded by + `parent_capture` + `build_and_persist`, and `catalog_parents` reads it + (lineage.py:112). #103 made the edge durable across reset by putting + `manifest.json` in the recipe-zip allowlist. What remains is not the capture but + the reactive **consumer** — walk the recorded edges to find and recompute + dependents (the feature itself, below). +- **Fix the alias-history ambiguity** — still OPEN. Thread the requested alias + through `previous_version` / `_resolve_noncyclic_hash` so follow-resolution can't + step back through the wrong lineage. `version_of_hash` still returns the + first-match alias in iteration order (aliases.py:101, now over `aliases.jsonl`); + also flagged out-of-scope in `plans/native-catalog-store.md`. +- **Flip the default to `cas`** (+ reflink on Linux, `.cas` GC) — the identity + prerequisite from #74's recompute-on-cold-read (widened by #104: recompute is now + the default for every cheap and salt entry). +- **Admission-decision telemetry** — deliverable 0 above; gates the #30 flip. + *Recording shipped in #87 (merged via PR #94):* the manifest persists the + structural verdict (`cache_worthy` / `cache_worthy_why`) alongside + `compile_seconds` and `cache_bytes` (manifest.py:47-50); the side-by-side + comparison/flip logic is the remaining work. +- **Determinism lint** — ✅ **SHIPPED (#88):** the build-time hint on + nondeterministic ops landed (`_NONDETERMINISTIC_OPS`, build.py:261; wired into + `build_and_persist` at build.py:370; surfaced via the MCP server and companion; + tests pass). Residual: the runtime structural-case predicate log (reconstructed + hash ≠ stored `content_hash`); the durable execution-nondeterminism fix is still + #83 (open). +- **In-process LRU invalidation on reset** — #80 / #96 (still open). The memo + moved onto `_resolve_result_plan` (`@functools.lru_cache`) in #103's hardening + pass and `reset_to` still does not call `cache_clear`, so as filed both stay + open — but the concrete dangling-path failure is now *mitigated*: the + `path.exists()` re-check + self-heal runs in the public `cached_result_expr` + body (result_cache.py:321) on every call, warm LRU hits included, so a reset or + prune no longer serves a deleted snapshot. A reactive layer still has to treat + this in-process cache as one more thing to invalidate. + +**Still genuinely open (the reactive feature's design — tracked as epic #89):** +the reactive *consumer* that walks `manifest.parents` to find and recompute +dependents, plus the trigger model (button vs scan-on-load vs auto-cascade — +leaning button + scan), cascade/transaction semantics, and archive overlay +(derived vs stored). Nothing in the tree walks `manifest.parents` to recompute +today — `catalog_dag` / `catalog_parents` feed only the DAG-visualization +endpoints. The DAG and read-intent *capture* are in place (#84, merged via PR #91; +made durable by #103); these staleness-layer decisions sit on top, once `cas` is +the default and the alias-history ambiguity is fixed. + +**Recompute-substrate correctness holes the consumer inherits (filed since the +#103 reconciliation; not yet reflected above).** The consumer recomputes +dependents through the same `cached_result_expr` / `from_catalog` substrate these +bugs sit in, so each effectively gates a *correct* cascade: + +- **#75** — an entry combining ≥2 catalog parents hits "Multiple backends found". + Multi-parent composition (joins/unions across entries) is the normal case a + cascade must rebuild. Appears handled in-tree (the multi-parent self-heal test + passes) but the issue is still open — confirm before relying on it. +- **#76 / #77** — an expensive-parent child pins the parent's untracked + `compute_cache` snapshot *by path*; on a pruned or cross-machine-cloned snapshot + the child read fails (#76) or the viewer shows an empty grid (#77) instead of + recomputing from the parent. Recompute can be "correct" via `cached_result_expr` + while the user-facing grid is still stale/empty. +- **#97** — a 0-row expensive parent bakes no snapshot, so a child's `/load_expr` + replay reads zero files and the self-heal can't repopulate it. Any cascade that + produces a 0-row intermediate breaks every downstream dependent. +- **#79** — concurrent on-demand materialisation of the same cold entry races on + the output path (torn snapshot + 500, no single-flight lock). A cascade or + scan-on-load that warms many entries introduces exactly this concurrency. +- **#81** — scalar-UDF-only entries are misclassified by `_is_worthy_expr` + (source_cache.py) vs `classify_build`, so some expensive entries bake nothing and + recompute on every read. A concrete instance of the #30/#87 worthiness + disagreement; it skews the recompute-cost model the staleness signal relies on. + +None of these are referenced in the plan/ADR today. They don't change the staged +sequencing, but the cascade/transaction design above should treat them as gating +the substrate it runs on. + +--- + +## Cross-stage dependencies + +``` +#48 ✓ ─┐ +#52 ✓ ─┤ native store ✓ ─► build stage-2 corpus FRESH ──► stage 2 corpus +#103 ✓ ─┴─► (no migration of old catalogs — re-author) │ + │ +cache map + lifecycle docs ✓ (PR #47) ──► perf tiers ✓ (#59/#64) ─► numbers + (deep-chain #82 ✗) │ + ┌─────────┘ +substrate (merged: #74 source-cache + baked result, │ + #104 result.parquet removal, #103 native store)│ + ▼ + prerequisites: parents-DAG + read-intent ✓ (#84/PR #91), + determinism lint ✓ (#88), admission telemetry recording ✓ (#87/PR #94); + still open: cas default (#86), previous_version fix (#85), + #30 worthiness flip, #80/#96 LRU-on-reset (mitigated) + │ + ▼ + stage 3 reactive feature (epic #89): consume parents-DAG (recompute + dependents), trigger model, cascade, archive + — gated also by substrate holes #75/#76/#77/#97/#79/#81 +```