Skip to content

feat(perf): log the cache admission decision — structural verdict + measured cost side by side (#87) - #94

Merged
paddymul merged 2 commits into
mainfrom
feat/87-cache-admission-instrumentation
Jun 16, 2026
Merged

paddymul merged 2 commits into
mainfrom
feat/87-cache-admission-instrumentation

Conversation

@paddymul

Copy link
Copy Markdown
Contributor

Addresses #87.

What

#30 wants to replace the structural cache_worthy admission test with a measured value-per-byte model, but that flip can't be made on evidence today because the measured side isn't recorded. This logs both verdicts side by side per entry so the disagreement set (entries cache_worthy admits that the cost model rates near-zero — e.g. a Join-descendant computed column) is directly visible.

At build (build.py → manifest.json + BuildResult, logged on tallyman.perf):

  • compile_seconds (new) — the expr→backend-plan (to_sql) step on the author DAG, timed separately from execution. Replace structural cache_worthy with a measured cost/size rubric (supersedes #12) #30's profiling found this dominates per-view cost.
  • the structural cache_worthy verdict + why — classify_build computes it and threw it away; persist it.
  • cache_bytes (new) — the baked snapshot byte-size (the value-per-byte denominator); None for a cheap entry that bakes nothing. Derived the same way the runtime reader locates the snapshot.
  • execute_seconds — already recorded.

The admission decision is logged once per build on the tallyman.perf namespace, both verdicts side by side.

On the runtime read path (result_cache.py):

  • Each cold cached_result_expr read is tagged on tallyman.perf (DEBUG) with the perf(cache): source-cache + baked result-cache, drop blanket result.parquet (#73) #74 path it took — cheap-recompute / baked-read / evicted-self-heal (plus salt-result-parquet and a worthy-recompute-fallback for the lockstep-disagreement case) — and its wall-clock.
  • New baked_snapshot_path(project, content_hash) names the snapshot the read returns (extracted from cached_result_expr's inline derivation, now shared).

Disk accounting (app.py):

Tests

test_build_records_admission_instrumentation / test_build_cheap_entry_records_no_snapshot_bytes (manifest + BuildResult fields, perf admission log), test_cold_read_logs_path_and_wall_time (the three read-path tags + baked_snapshot_path), and test_disk_usage_counts_compute_cache. Committed first so they are visible failing on CI, then the fix lands.

Context

Deliverable 0 of Stage 2 in plans/staged-catalog-perf-reactive.md (the admission-decision half of #60's instrumentation consolidation), and the prerequisite that lets #30 flip from structural to measured by shadow-then-flip. Related: #60, #30, #74, #82.

🤖 Generated with Claude Code

paddymul and others added 2 commits June 16, 2026 14:51
Record both cache verdicts side by side so the #30 structural-vs-measured
flip is data-driven:

- build records compile_seconds, the structural cache_worthy verdict + why,
  and the baked snapshot byte-size in manifest.json + BuildResult, and logs
  the admission decision on the tallyman.perf namespace.
- cached_result_expr tags each cold read with the #74 path it took
  (baked-read / cheap-recompute / evicted-self-heal) + wall-clock on
  tallyman.perf; baked_snapshot_path names the snapshot the read returns.
- disk_usage counts the per-project compute_cache (the byte denominator and
  the #74 disk-usage follow-up).

These assertions fail on main (no compile_seconds key, no baked_snapshot_path,
no compute_cache key, no perf logs).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Persist both cache verdicts per entry so #30's structural→measured flip is
data-driven instead of decided on first principles.

build.py: record the structural cache_worthy verdict + why (classify_build
already computed it and discarded it), compile_seconds (the author DAG's
to_sql / expr→backend-plan step, the dominant per-view cost per #30, timed
separate from execute), and cache_bytes (the baked snapshot size, the
value-per-byte denominator; None for a cheap entry). All land in manifest.json
and BuildResult, and the decision is logged once on the tallyman.perf
namespace with both verdicts side by side.

result_cache.py: tag each cold cached_result_expr read on tallyman.perf
(DEBUG) with the #74 path it took — cheap-recompute / baked-read /
evicted-self-heal (plus salt and the worthiness-disagreement fallback) — and
its reconstruct wall-clock (#82's lever). New baked_snapshot_path() names the
snapshot a cold read returns, extracted from the inline derivation now shared
with the read path.

app.py: /api/disk_usage now counts the per-project compute_cache/ (where #74's
baked results and source-read caches live), which it omitted — supplying the
byte denominator and closing the #74 disk-usage follow-up.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@paddymul
paddymul merged commit ed9ad43 into main Jun 16, 2026
4 checks passed
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
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>
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