Skip to content

Commit a1a7127

Browse files
paddymulclaude
andcommitted
docs: reconcile reactive plan with post-#103 state; pin no-migration corpus decision
Verified the #103/#104 reconciliation against the merged tree (all code citations accurate bar one drift, fixed: execute_seconds is manifest.py:38, not :47-50) and brought the plan current with what shipped since: - Deliverable 1: prose lifecycle model + refreshed cache map already shipped as docs/expression-lifecycle.md + docs/caching.md (PR #47); fix the stale "docs/caching-architecture branch" reference. - Deliverable 2: record #59/#64 closed and both perf tiers shipped; remaining = regression-gate promotion (#45), #82 deep-chain measurement (gates Stage 3). - Stage 3: determinism lint shipped (#88) -> mark PARTIAL not open; note the #80/#96 dangling-path failure is mitigated by the per-call path.exists self-heal; name epic #89; add the recompute-substrate holes the consumer inherits (#75/#76/#77/#97/#79/#81). Corpus: per the project no-migration rule, build the stage-2 corpus FRESH in native format from the source parquets, not by converting the pre-#103 on-disk catalogs. rebuild_native_catalog.py is a migration convenience, not on the critical path; its alias-revision pinning limit is out of scope. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 3aa55dc commit a1a7127

2 files changed

Lines changed: 127 additions & 51 deletions

File tree

plans/adr-reactive-catalog-recalc.md

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -432,9 +432,11 @@ replaced — see Reconciliation. Retained for the diagnosis it grounds.)
432432
the source digest is unchanged" (cheap; folds into the stage-2 instrumentation).
433433
*Execution* nondeterminism (`.sample()`, `ibis.now()`, unordered `.limit()`,
434434
impure UDF) leaves the graph — and the hash — unchanged while the bytes differ,
435-
so the hash can't see it and cas can't fix it (it is not source drift). v0 is a
436-
build-time lint on known-nondeterministic ops; the durable fix (it also breaks
437-
#30's evict-and-recompute faithfulness) is #83. No hard enforcement (see
435+
so the hash can't see it and cas can't fix it (it is not source drift). The v0
436+
build-time lint on known-nondeterministic ops **shipped (#88,
437+
`_NONDETERMINISTIC_OPS` at build.py:261, surfaced via the MCP server and
438+
companion)**; the durable fix (it also breaks #30's evict-and-recompute
439+
faithfulness) is #83, still open. No hard enforcement (see
438440
`plans/adr-source-identity-content-hash.md`).
439441
- **Deep-chain build cost (now #82).** Originally framed as timing eager
440442
`load_expr` against the tagged-read reference; #74 took the composition fork, so
@@ -451,9 +453,11 @@ replaced — see Reconciliation. Retained for the diagnosis it grounds.)
451453
(`@functools.lru_cache`, result_cache.py:232) with `cache_clear` / `cache_info`
452454
re-exposed on the public `cached_result_expr` name, and fixed the related
453455
warm-then-evicted dangling-path bug by re-checking `path.exists()` on every call
454-
(result_cache.py:321). #80 itself — `reset_to` does not call `cache_clear` — is
455-
still open; a reactive recalc/staleness layer has to treat this in-process cache
456-
as one more thing to invalidate. Filed #80.
456+
(result_cache.py:321), which now runs on every call (warm hits included), so the
457+
concrete dangling-path failure is defended at the read site. #80 (and its sibling
458+
#96, the LRU surviving a compute-cache prune) — `reset_to` does not call
459+
`cache_clear` — are still open as filed; a reactive recalc/staleness layer has to
460+
treat this in-process cache as one more thing to invalidate. Filed #80/#96.
457461

458462
## Not in scope here
459463

plans/staged-catalog-perf-reactive.md

Lines changed: 117 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,9 @@ that Stage 2 listed as open. **#103** (#52) replaced xorq's catalog package with
2424
native `tallyman_core.catalog` store and decomposed `catalog.yaml` into
2525
per-concern tracked files — which *dissolves Stage 1's whole bug cluster by
2626
construction* rather than fixing it within xorq, supersedes the PR #57 mechanism
27-
(aliases moved out to `aliases.jsonl`, not folded into `catalog.yaml`), ships
28-
Stage 1's rebuild script (`scripts/rebuild_native_catalog.py`), and — via #84
27+
(aliases moved out to `aliases.jsonl`, not folded into `catalog.yaml`), ships a
28+
catalog rebuild script (`scripts/rebuild_native_catalog.py` — a migration
29+
convenience, not on the critical path; see Stage 1 item 4), and — via #84
2930
ships the manifest `parents` edge Stage 3 listed as a prerequisite. Per-item
3031
annotations below; full design in `plans/native-catalog-store.md`.
3132

@@ -127,21 +128,27 @@ then produces a clean parking catalog for stage 2 to measure.
127128
corrupt tracked JSON surfaces as a bare HTTP 500. Re-scope #56 against the
128129
native store before doing any work; the original swallow it targeted is gone.
129130

130-
4. **PARTLY DONE (PR #103) — reproducible rebuild script shipped; not yet run on
131-
the real corpus.** No migration/repair — rebuild from scratch by re-execing
132-
recipes through the now-native machinery. `scripts/rebuild_native_catalog.py`
133-
(+ `tests/test_rebuild_native_catalog.py`) reads a project's old catalog across
134-
all three layouts (native `aliases.jsonl`, the `aliases.json` era, and the
135-
`catalog.yaml` era), toposorts the `from_catalog` graph, wipes `catalog/` keeping
136-
`data/`, and re-builds each entry in dependency order. Because a re-exec does
137-
**not** reproduce the old content hashes, it remaps every hash-keyed artifact
138-
old→new, rewrites literal `from_catalog(<hash>)` refs, and re-points aliases as
139-
each revision rebuilds; it clears the `cached_result_expr` memo to avoid
140-
re-execing a child against a pre-wipe parent. Validated against a disposable copy
141-
of one project. **Remaining:** run it on the real `~/.tallyman-notebooks`
142-
projects (`--dry-run` first) and emit the two stage-2 corpus variants (full
143-
≈10M+ rows, truncated ≈1M). **Blocking input:** confirm the parking source
144-
parquet(s) still exist / where. See `plans/native-catalog-store.md`.
131+
4. **Build the stage-2 corpus FRESH in native format — we do NOT migrate old
132+
catalogs.** Per the project rule (single-user dev repo, no installed base):
133+
there is no migration/upgrade burden, and **rebuilding the corpus from scratch
134+
is always the accepted fix**. So the stage-2 corpus is produced by re-authoring
135+
the entries fresh against the source parquets through the normal build path
136+
(`build_and_persist`) — the same way the originals were created — **not** by
137+
converting the pre-#103 on-disk parking catalogs. The source parquets exist
138+
(`/Users/paddy/code/parking_speeding_data/`, ≈25GB). Truncation is a build-time
139+
parameter of the fresh authoring (`head(1M)` on the source read; see Truncation
140+
design under Deliverable 2), not a property of any conversion step. The pre-#103
141+
on-disk parking catalogs (`parking_ticket_analysis`, `parking_ticket_analysis_1m`)
142+
are stale and can simply be discarded and rebuilt; their layout is irrelevant.
143+
144+
`scripts/rebuild_native_catalog.py` (shipped in PR #103) is a *migration*
145+
convenience that re-execs an old catalog's recipes into the native store. It is
146+
**not on the critical path** and we don't need it for our own data — its
147+
build-time alias-revision pinning limit on schema-evolving corpora is a
148+
migration-only concern and **out of scope** under the no-migration rule. (#98
149+
the old `rebuild_parking_catalog.py` shelling to `xorq catalog add` — is likewise
150+
moot; #99 — the perf harness reading native `aliases.jsonl` — is already fixed in
151+
code, `b51216f`, and closeable.) See `plans/native-catalog-store.md`.
145152

146153
### Tests / acceptance
147154

@@ -204,7 +211,8 @@ summary at :427), `execute_seconds` in `build.py:493`. The work:
204211
manifest now persists the structural `cache_worthy` / `cache_worthy_why` verdict
205212
(no longer computed and thrown away) alongside the measured inputs —
206213
`compile_seconds` (the dominant per-view cost), `cache_bytes` (the
207-
result/snapshot byte-size), and `execute_seconds` (manifest.py:47-50). #87 also
214+
result/snapshot byte-size) at manifest.py:47-50, plus `execute_seconds` at
215+
manifest.py:38. #87 also
208216
added the `compute_cache` byte count to disk accounting (app.py:129), supplying
209217
the byte denominator and closing the #74 disk-usage follow-up. What remains is
210218
the side-by-side comparison/flip logic and the per-read path tags: tag each cold
@@ -220,6 +228,13 @@ companion's own viewer — and follow buckaroo's build/test conventions.)
220228

221229
### Deliverable 1 — the expression-lifecycle cost model (write before any perf test)
222230

231+
> **Update (PR #47, merged):** the *prose* lifecycle model and the refreshed
232+
> cache map shipped as `docs/expression-lifecycle.md` and `docs/caching.md`, both
233+
> already post-#74/#104 accurate (baked snapshot is the only materialized copy,
234+
> baked at build; source-read cache; no `result.parquet`). What remains for this
235+
> deliverable is the *measured* per-action build/load/execute counts, which
236+
> Deliverable 2's harness produces.
237+
223238
For each user-facing action — open the app, view an entry, revise a source, open a
224239
diff, reset — answer: how many expressions get **built** (graph constructed /
225240
tokenized), how many get **loaded** (from a build dir, the `/load_expr` path), and
@@ -241,9 +256,9 @@ Build/load/execute counts are the explanatory variable; wall-clock is the sympto
241256

242257
The cache layers matter only insofar as they change those counts (a hit skips a
243258
re-execution; a shallow graph skips a build). The catalogue of layers already
244-
exists — the `docs/caching-architecture` branch ("map every cache in the
259+
exists — merged via PR #47 as `docs/caching.md` ("map every cache in the
245260
tallyman/xorq/buckaroo stack", `xorq` `SnapshotStrategy`, the `.cas` store +
246-
`content_hash`, buckaroo's stat/session caches). **Refresh it for #74/#104:** the
261+
`content_hash`, buckaroo's stat/session caches). **The #74/#104 refresh is already in that merged doc:** the
247262
separate `result_cache/` dir is retired (baked results now co-locate in the
248263
per-project content-addressed `compute_cache`), and a new source-read cache
249264
(snapshot `.cache()` after each `read_csv`/`read_json`, shared across entries,
@@ -253,8 +268,10 @@ now closed, across two PRs: #87 (PR #94) added the `compute_cache` byte count to
253268
bucket and repointed the cache inspector + delete off the manifest `cache_bytes`
254269
and the baked snapshot path, listing only snapshots that exist on disk (app.py:1007
255270
/ :1085) — there is no on-demand `result.parquet` left to reflect. Reuse the map as
256-
the levers section; the deliverable here is the lifecycle model on top, not a
257-
re-derived cache-key taxonomy.
271+
the levers section; the prose lifecycle model on top also shipped
272+
(`docs/expression-lifecycle.md`, PR #47), so what is left for this deliverable is
273+
the *measured* per-action build/load/execute counts — produced by Deliverable 2's
274+
harness, not a re-derived cache-key taxonomy.
258275

259276
Existing backlog items are findings against this model: #21 (lazy warm), #22
260277
(checkpoint capture cost per mutating request), #30 (measured cost rubric vs
@@ -267,6 +284,17 @@ all today (ADR says no — fast but stale).
267284

268285
### Deliverable 2 — performance integration tests (#59)
269286

287+
> **Update (#59 / #64, both closed):** both tiers shipped — Tier B
288+
> (`tests/test_perf_integration.py`, `perf` marker: execute / page-load / diff,
289+
> cold+warm) and Tier A (`tests/test_perf_tier_a.py`, `perf_tier_a` marker, #64:
290+
> Playwright to `.df-viewer .ag-cell` + backend/total calibration). Three things
291+
> remain: (1) it is **report-only** — promote #45's diff time to a regression gate
292+
> once baselined; (2) the harness still references the OLD
293+
> `scripts/rebuild_parking_catalog.py` for its corpus — repoint it at the
294+
> freshly-built native corpus (Stage 1 item 4; no migration), with truncation done
295+
> at authoring time (`head(1M)` source read); (3) the **#82 deep-chain depth
296+
> measurement does not exist** and gates Stage 3.
297+
270298
Three measurements, each on both corpus variants (full / truncated) and across a
271299
cold/warm cache axis. Local-only on this Mac, marker-gated off CI (same convention
272300
as `cache_lab`).
@@ -300,8 +328,9 @@ unexplained slowdown.
300328

301329
**Truncation design.** Truncate at the raw-source read (`head(1M)` on the source
302330
parquet) so the DAG shape and build/tokenize cost are identical to the full
303-
corpus and only execution cost drops. Both variants emitted by the stage-1
304-
rebuild script; regenerable, not checked in.
331+
corpus and only execution cost drops. Both variants are built fresh in the native
332+
format from the source parquets (Stage 1 item 4 — no migration of old catalogs);
333+
regenerable, not checked in.
305334

306335
**Gating.** Report-only first (no baselines yet), matching the existing perf-report
307336
suite. Promote a few to regression gates once baselined — #45's diff time is the
@@ -372,39 +401,82 @@ its own issue; the 2026-06-16 design review settled their direction):
372401
structural verdict (`cache_worthy` / `cache_worthy_why`) alongside
373402
`compile_seconds` and `cache_bytes` (manifest.py:47-50); the side-by-side
374403
comparison/flip logic is the remaining work.
375-
- **Determinism lint** — build-time hint on nondeterministic ops; durable fix #83.
376-
- **In-process LRU invalidation on reset**#80 (the memo moved onto
377-
`_resolve_result_plan` in #103's hardening pass; `reset_to` still does not clear
378-
it).
379-
380-
**Still genuinely open (the reactive feature's design):** the reactive *consumer*
381-
that walks `manifest.parents` to find and recompute dependents, plus the trigger
382-
model (button vs scan-on-load vs auto-cascade — leaning button + scan),
383-
cascade/transaction semantics, and archive overlay (derived vs stored). The DAG
384-
and read-intent *capture* are now in place (#84, merged via PR #91; made durable by
385-
#103); these staleness-layer decisions sit on top, once `cas` is the default and
386-
the alias-history ambiguity is fixed.
404+
- **Determinism lint** — ✅ **SHIPPED (#88):** the build-time hint on
405+
nondeterministic ops landed (`_NONDETERMINISTIC_OPS`, build.py:261; wired into
406+
`build_and_persist` at build.py:370; surfaced via the MCP server and companion;
407+
tests pass). Residual: the runtime structural-case predicate log (reconstructed
408+
hash ≠ stored `content_hash`); the durable execution-nondeterminism fix is still
409+
#83 (open).
410+
- **In-process LRU invalidation on reset**#80 / #96 (still open). The memo
411+
moved onto `_resolve_result_plan` (`@functools.lru_cache`) in #103's hardening
412+
pass and `reset_to` still does not call `cache_clear`, so as filed both stay
413+
open — but the concrete dangling-path failure is now *mitigated*: the
414+
`path.exists()` re-check + self-heal runs in the public `cached_result_expr`
415+
body (result_cache.py:321) on every call, warm LRU hits included, so a reset or
416+
prune no longer serves a deleted snapshot. A reactive layer still has to treat
417+
this in-process cache as one more thing to invalidate.
418+
419+
**Still genuinely open (the reactive feature's design — tracked as epic #89):**
420+
the reactive *consumer* that walks `manifest.parents` to find and recompute
421+
dependents, plus the trigger model (button vs scan-on-load vs auto-cascade —
422+
leaning button + scan), cascade/transaction semantics, and archive overlay
423+
(derived vs stored). Nothing in the tree walks `manifest.parents` to recompute
424+
today — `catalog_dag` / `catalog_parents` feed only the DAG-visualization
425+
endpoints. The DAG and read-intent *capture* are in place (#84, merged via PR #91;
426+
made durable by #103); these staleness-layer decisions sit on top, once `cas` is
427+
the default and the alias-history ambiguity is fixed.
428+
429+
**Recompute-substrate correctness holes the consumer inherits (filed since the
430+
#103 reconciliation; not yet reflected above).** The consumer recomputes
431+
dependents through the same `cached_result_expr` / `from_catalog` substrate these
432+
bugs sit in, so each effectively gates a *correct* cascade:
433+
434+
- **#75** — an entry combining ≥2 catalog parents hits "Multiple backends found".
435+
Multi-parent composition (joins/unions across entries) is the normal case a
436+
cascade must rebuild. Appears handled in-tree (the multi-parent self-heal test
437+
passes) but the issue is still open — confirm before relying on it.
438+
- **#76 / #77** — an expensive-parent child pins the parent's untracked
439+
`compute_cache` snapshot *by path*; on a pruned or cross-machine-cloned snapshot
440+
the child read fails (#76) or the viewer shows an empty grid (#77) instead of
441+
recomputing from the parent. Recompute can be "correct" via `cached_result_expr`
442+
while the user-facing grid is still stale/empty.
443+
- **#97** — a 0-row expensive parent bakes no snapshot, so a child's `/load_expr`
444+
replay reads zero files and the self-heal can't repopulate it. Any cascade that
445+
produces a 0-row intermediate breaks every downstream dependent.
446+
- **#79** — concurrent on-demand materialisation of the same cold entry races on
447+
the output path (torn snapshot + 500, no single-flight lock). A cascade or
448+
scan-on-load that warms many entries introduces exactly this concurrency.
449+
- **#81** — scalar-UDF-only entries are misclassified by `_is_worthy_expr`
450+
(source_cache.py) vs `classify_build`, so some expensive entries bake nothing and
451+
recompute on every read. A concrete instance of the #30/#87 worthiness
452+
disagreement; it skews the recompute-cost model the staleness signal relies on.
453+
454+
None of these are referenced in the plan/ADR today. They don't change the staged
455+
sequencing, but the cascade/transaction design above should treat them as gating
456+
the substrate it runs on.
387457

388458
---
389459

390460
## Cross-stage dependencies
391461

392462
```
393463
#48 ✓ ─┐
394-
#52 ✓ ─┤ rebuild script(#103) ─► run on real corpus ─► stage 2 corpus
395-
#103 ✓ ─┴─► (native store)
396-
397-
cache-architecture map (refresh for #74/#104) ────────────► perf tests ─► numbers
398-
(incl. #82)
399-
┌──────────────────
464+
#52 ✓ ─┤ native store ✓ ─► build stage-2 corpus FRESH ──► stage 2 corpus
465+
#103 ✓ ─┴─► (no migration of old catalogs — re-author)
466+
467+
cache map + lifecycle docs ✓ (PR #47) ──► perf tiers ✓ (#59/#64) ─► numbers
468+
(deep-chain #82 ✗)
469+
┌─────────┘
400470
substrate (merged: #74 source-cache + baked result, │
401471
#104 result.parquet removal, #103 native store)│
402472
403473
prerequisites: parents-DAG + read-intent ✓ (#84/PR #91),
404-
cas default, previous_version fix, admission telemetry
405-
(recording shipped #87/PR #94), determinism lint, #80 LRU-on-reset
474+
determinism lint ✓ (#88), admission telemetry recording ✓ (#87/PR #94);
475+
still open: cas default (#86), previous_version fix (#85),
476+
#30 worthiness flip, #80/#96 LRU-on-reset (mitigated)
406477
407478
408-
stage 3 reactive feature: consume parents-DAG (recompute
479+
stage 3 reactive feature (epic #89): consume parents-DAG (recompute
409480
dependents), trigger model, cascade, archive
481+
— gated also by substrate holes #75/#76/#77/#97/#79/#81
410482
```

0 commit comments

Comments
 (0)