Skip to content

refactor(cache): remove the on-demand result.parquet layer, rely on xorq .cache() (#101) - #104

Merged
paddymul merged 9 commits into
mainfrom
feat/remove-ondemand-result-parquet
Jun 17, 2026
Merged

paddymul merged 9 commits into
mainfrom
feat/remove-ondemand-result-parquet

Conversation

@paddymul

Copy link
Copy Markdown
Contributor

Implements the plan in #101.

Problem

#73 stopped writing result.parquet at build but kept ensure_result to regenerate it lazily; #71/#90 moved the viewer and paginated reads onto cached_result_expr. The on-demand result.parquet layer is the last thing standing between the codebase and a single materialised copy — xorq's baked .cache() snapshot, already produced by rewrite_for_build for expensive entries. Cheap entries recompute on read (a pushdown over a columnar source, ~free).

#102 (perf/71-viewer-serve-result-read) merged after the plan was written and re-introduced an on-demand result.parquet build for the viewer (ensure_result_build / ensure_viewer_expanded_build, ensure_session paging over the materialised parquet). Per maintainer direction this PR removes that too — it is part of the same on-demand layer.

Change

  • Delete ensure_result and point its three real consumers at cached_result_expr:
    • post_processing → cached_result_expr(...).execute()
    • companion/diff.build_diff_expr → cached_result_expr directly (drops a double-materialisation)
    • xorq/diff.full_diff → xorq-only; require a_expr/b_expr, drop the pandas/polars file-reading backends and the _ensure_entry_parquet fallback
  • Remove the path plumbing (ENTRY_RESULT_FILENAME, entry_result_path, the manifest result_path field) and repoint the cache-admin surfaces (/api/result_cache inspector + delete, disk-usage results bucket) at the baked snapshot, manifest-driven.
  • Unwind perf(viewer): serve cache-worthy entries from result.parquet + share the path with Tier-B (#71) #102: delete ensure_result_build / ensure_viewer_expanded_build and the .xorq_result_build* path helpers; ensure_session serves the entry's recipe build dir again (a cache-worthy entry's recipe replays onto its baked snapshot — a read, not a recompute).
  • Salt mode: delete the branch; reads fall through the existing recompute paths.
  • Bump the legacy migration marker so a re-sweep clears any lazily-written result.parquet.

Status — TDD split

Per the repo's test-vs-fix rule, the first commit is the failing tests (watched red on CI); the fixes follow in the next commit (CI green). Frontend stale-copy sweep and end-to-end verification ride with the fix commit.

Context

Builds on #73 (drop blanket result.parquet), #71/#90 (read path onto cached_result_expr), #87 (manifest cache_bytes, reused by the repointed inspector). Reconciles with #102.

🤖 Generated with Claude Code

paddymul and others added 3 commits June 16, 2026 19:06
TDD failing-first tests for removing the on-demand result.parquet layer —
both the ensure_result writer and the #102 viewer-from-result.parquet
build (ensure_result_build / ensure_viewer_expanded_build). Each fails on
today's main, where those still materialise a per-entry parquet:

- run_post_processing (cheap + expensive, and under salt) must leave no
  <entry>/result.parquet behind
- build_diff_expr must compose the two cache-resolving exprs directly,
  writing no parquet for either side
- full_diff must require a_expr/b_expr (xorq-only) rather than silently
  materialise via the dropped pandas fallback
- ensure_session must serve a cache-worthy entry's recipe build (replayed
  onto the baked snapshot), writing no result.parquet for the viewer
- result_cache must expose no ensure_result / ensure_result_build /
  ensure_viewer_expanded_build

Fixes follow in the next commit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cache()

Delete every "materialise a per-entry result.parquet on demand" path. The xorq
.cache() snapshot (baked at build for expensive entries) is the single
materialised copy; cheap entries recompute on read. Finishes the #73/#71/#90
arc and unwinds #102's viewer-from-result.parquet layer, which merged after the
plan and was built on the same on-demand parquet.

Source:
- result_cache: delete ensure_result and the salt read branch (salt entries
  fall through to recompute); delete #102's ensure_result_build /
  ensure_viewer_expanded_build.
- post_processing / companion.diff.build_diff_expr: read cached_result_expr.
- xorq.diff.full_diff: xorq-only; require a_expr/b_expr (ValueError if missing);
  drop the pandas/polars file-reading backends, _ensure_entry_parquet and the
  backend kwarg. Call sites drop backend="xorq".
- paths: delete ENTRY_RESULT_FILENAME / entry_result_path and #102's
  .xorq_result_build* names + helpers; trim ENTRY_CACHE_NAMES. manifest: drop
  result_path. core.__init__: drop the exports.
- companion.app: drop the disk-usage "results" bucket; drive the cache inspector
  off the manifest (cache_bytes, #87), filtered to entries that bake a snapshot;
  delete unlinks the baked snapshot.
- buckaroo_lifecycle: ensure_session serves the recipe build again
  (_entry_parquet_exists -> _entry_exists, keyed on the build dir).
- build: bump the migration marker; sweep result.parquet + dead #102 dirs.
- Frontend: reword CachePage/CatalogPage/NotebookPage off "result.parquet".

Tests: retarget the ensure_result/result.parquet fixtures onto
cached_result_expr / baked snapshots (result_cache, diff, cache_inspector,
disk_usage, buckaroo, buckaroo_multi_project, paths, manifest, perf_integration);
migrate the cache lab onto cached_result_expr (snapshot cold-vs-warm).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Regression guard for the behavior bumped in the previous commit:
migrate_drop_result_parquet deletes a legacy per-entry result.parquet and the
dead #71 viewer-read build dirs, keeps the durable xorq_build recipe, writes the
(renamed) marker, and is a no-op on a second run. No prior test covered it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@paddymul

Copy link
Copy Markdown
Contributor Author

Implementation complete. Three commits under the TDD split:

#102 reconciliation

perf/71-viewer-serve-result-read (#102) merged into main after #101 was written and re-introduced an on-demand result.parquet build for the viewer (ensure_result_build / ensure_viewer_expanded_build, ensure_session paging over the materialised parquet). It's part of the same on-demand layer, so this PR removes it too: ensure_session serves the entry's recipe build again (a cache-worthy entry's recipe replays onto its baked snapshot — a read, not a recompute), and the .xorq_result_build* paths/helpers are gone. The bumped migration marker also sweeps any .xorq_result_build* dirs an install picked up while #102 was on main.

Verification (local)

paddymul and others added 6 commits June 16, 2026 19:59
…sure_result text

scripts/perf_report.py: the per-build "result_mb" probe read a result.parquet
that no longer exists (always None); report the baked snapshot size
(res.cache_bytes, #87) instead. Plus two stale ensure_result docstring mentions
reworded to the cached_result_expr heal. No behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ing-first)

Live verification surfaced a regression the suite missed: deleting a baked
snapshot from the cache page, then reading that entry in the same companion
session, 500s with "At least one path is required". The read path's
cached_result_expr is memoised to a deferred_read of the snapshot, and the
evicted-self-heal only fires on a cache miss — so a warm lru dereferences the
just-deleted file. The delete endpoint's own docstring promises a self-heal.

Fix follows.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The result_cache delete endpoint unlinked the baked snapshot but left the
cached_result_expr lru warm — a deferred_read of the file just removed — so the
next viewer read (api_data) and ensure_session's pre-/load_expr heal dereferenced
a missing file and 500'd ("At least one path is required") instead of re-baking.
Clear the lru on delete (no per-key eviction exists; a full clear is cheap for an
admin op — entries are content-addressed, so warm exprs re-reconstruct and the
evicted one re-bakes on next read). Verified live: delete then re-view now
re-bakes the snapshot and returns rows.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…w (failing-first)

GET /api/result_cache keys its listing on manifest.cache_bytes, which is recorded
at build and untouched by a delete. So after the delete endpoint evicts a baked
snapshot, the entry keeps listing — with a stale size and an "on disk" total that
counts bytes already freed — until the entry is next viewed and re-bakes. A second
delete then 404s on a row the page is still showing.

The test builds an expensive entry, deletes its snapshot, and re-lists without an
intervening read (so nothing re-bakes it): the row must drop out and its bytes
must leave the total. Fails today on the phantom row. Fix follows.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
api_result_cache keyed its listing on manifest.cache_bytes, recorded at build and
untouched by a delete. So once the delete endpoint unlinked a baked snapshot, the
entry kept listing — stale size, freed bytes still in the "on disk" total, a delete
button that 404s — until the entry was next viewed and re-baked.

Resolve the snapshot via baked_snapshot_path and skip rows whose file is gone, and
size from the live file so the figure tracks disk. cache_bytes stays as a cheap
pre-filter (None for a cheap entry that bakes nothing), so the recipe re-import runs
only for the few expensive entries it lets through; this admin page isn't on a hot
path. The listing derives the path only (no execute), so it stays read-only and a
deleted snapshot isn't re-baked just by viewing the cache page.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…removal

ensure_session no longer serves a cache-worthy entry from a materialised
result.parquet; it posts the entry's expanded recipe for every entry, and
a cache-worthy entry's recipe replays onto its baked .cache() snapshot.
The docstring still described the removed #71 path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@paddymul
paddymul merged commit ea06845 into main Jun 17, 2026
3 checks passed
paddymul added a commit that referenced this pull request Jun 17, 2026
…(Cut A)

Stand up tallyman_core/catalog.py as the native recipe store and delete the
xorq catalog subprocess. The build now persists a complete entry dir (and
cleans it up on failure); the checkpoint, under the project lock, is the sole
zip writer and sole git transaction.

- catalog.py: write_recipe_zip (deterministic, allowlisted {expr.py, xorq_build/,
  schema.json, manifest.json} under one <hash>/ prefix), tracked_recipe_hashes,
  zip_pending_entries, assert_catalog_consistent (tracked-surface path allowlist
  + pointer/recipe agreement, #52), and the catalog-repo .gitignore.
- catalog_state.py: ensure_catalog_repo writes the .gitignore; checkpoint zips
  pending entries then `git add -A` (was `git add catalog.yaml`); reset_to
  delegates consistency to catalog.assert_catalog_consistent. Drops the inline
  _tracked_recipe_hashes/_assert_catalog_consistent/_TRACKED.
- build.py: drop the `xorq catalog add` subprocess; catalog_registered now means
  "entry dir persisted" (D2). rmtree the entry dir on a load/execute failure so
  a partial build leaves no orphan pointer (D1/Risk #7).
- aliases.py: strip the module-level xorq_catalog import and the add_alias/
  remove_alias subprocess calls (alias state stays in catalog.yaml for now;
  decomposition to aliases.jsonl is the next commit).
- Delete xorq_catalog.py. Delete the all-@integration test_catalog_xorq_integration.py
  (built on Catalog.from_kwargs / alias symlinks) and port its still-valid
  durability guards into test_native_catalog_store.py as fast in-process tests.

The four commit-1a failing-first tests now pass; added allowlist / byte-stable /
expr.py-member / no-xorq-argv / consistency / salt-naming / reset-durability
coverage. The catalog repo's first .gitignore also ignores the #104 migration
marker so `git add -A` stays honest. Full fast suite: 512 passed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
paddymul added a commit that referenced this pull request Jun 17, 2026
…t-parquet

docs: result.parquet-layer removal, as merged in #104 (supersedes #101)
paddymul added a commit that referenced this pull request Jun 17, 2026
PR #103 (native tallyman_core.catalog store) and PR #104 (removed the
on-demand result.parquet layer) merged to main; merge main in and update
the reactive plan set to match the current tree.

- adr-reactive-catalog-recalc.md: add dedicated "Reconciliation with PR
  #104" and "PR #103" sections; correct the deleted ensure_result /
  result.parquet-survives, salt-mode, and catalog_parents-regex claims;
  re-point io.py / result_cache.py / lineage.py / aliases.py citations.
- staged-catalog-perf-reactive.md: Stage 1 bug cluster dissolved-by-
  construction; rebuild script OPEN -> PARTLY DONE; disk-usage/inspector
  follow-ups closed; split the parents-DAG prerequisite (capture done,
  reactive consumer remains); refresh the dependency diagram.
- adr-result-cache-cost-rubric.md / adr-source-identity-content-hash.md:
  strike ensure_result, fix the now-false "from_catalog reads
  result.parquet" claim, repoint drifted line numbers.

Attribution corrected after git archaeology: the manifest `parents` edge
+ parent_capture (#84) shipped via PR #91, and the cache-admission
instrumentation + compute_cache disk counting (#87) via PR #94 -- both
ahead of #103, not part of it. #103 only makes the parents edge durable
across reset (manifest.json in the recipe-zip member set).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
paddymul added a commit that referenced this pull request Jun 18, 2026
…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>
paddymul added a commit that referenced this pull request Jun 18, 2026
PR #103 (native tallyman_core.catalog store) and PR #104 (removed the
on-demand result.parquet layer) merged to main; merge main in and update
the reactive plan set to match the current tree.

- adr-reactive-catalog-recalc.md: add dedicated "Reconciliation with PR
  #104" and "PR #103" sections; correct the deleted ensure_result /
  result.parquet-survives, salt-mode, and catalog_parents-regex claims;
  re-point io.py / result_cache.py / lineage.py / aliases.py citations.
- staged-catalog-perf-reactive.md: Stage 1 bug cluster dissolved-by-
  construction; rebuild script OPEN -> PARTLY DONE; disk-usage/inspector
  follow-ups closed; split the parents-DAG prerequisite (capture done,
  reactive consumer remains); refresh the dependency diagram.
- adr-result-cache-cost-rubric.md / adr-source-identity-content-hash.md:
  strike ensure_result, fix the now-false "from_catalog reads
  result.parquet" claim, repoint drifted line numbers.

Attribution corrected after git archaeology: the manifest `parents` edge
+ parent_capture (#84) shipped via PR #91, and the cache-admission
instrumentation + compute_cache disk counting (#87) via PR #94 -- both
ahead of #103, not part of it. #103 only makes the parents edge durable
across reset (manifest.json in the recipe-zip member set).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
paddymul added a commit that referenced this pull request Jun 18, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant