Skip to content

Commit 01f2483

Browse files
paddymulclaude
paddymul
and
claude
committed
docs(cache): ADRs for measured result-cache rubric and content-addressed source identity
Two decision records spun out of the cost/size cache-rubric discussion (#30, #31): - adr-result-cache-cost-rubric: replace the structural cache_worthy test with a measured value-per-byte model (cache permissively, bound by a raw-data-relative budget, evict cheapest-to-recompute first). Supersedes the structural classifier; rules out divergence/sub-expression caching. - adr-source-identity-content-hash: switch from_project/from_catalog to normalize_read_path_md5sum so content_hash survives reset-to, cross-machine transport, and in-place result regeneration — the precondition for the cache being keyed on a reset-stable content_hash. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent b4e75ac commit 01f2483

2 files changed

Lines changed: 207 additions & 0 deletions

File tree

‎plans/adr-result-cache-cost-rubric.md‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
# ADR: Result-cache admission and eviction by measured cost vs size
2+
3+
- **Status:** Proposed (2026-06-10)
4+
- **Context ticket:** buckaroo-data/tallyman#30 (measured cost/size cache rubric; supersedes #12, feeds #10 and #21)
5+
- **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`
6+
- **Related ADR:** `plans/adr-source-identity-content-hash.md` (why `content_hash` is a stable cache key)
7+
8+
## Problem
9+
10+
`cache_worthy` decides whether to materialise an entry's result by scanning the
11+
serialized xorq build for ops (`classify_build` in `result_cache.py:51`): cache
12+
if the expression contains a Join / Aggregate / Sort / Window / UDF or reads a
13+
non-parquet source, otherwise treat it as cheap and skip the extra copy.
14+
15+
This is a *structural* test, and it misclassifies a common authoring workflow.
16+
The motivating case: a trips ⋈ stations join (~2.5M rows, 100s of MB) followed
17+
by *iterative computed-column additions* — add a distance metric, tweak it, add
18+
another derived column. Every one of those descendant entries inherits the
19+
upstream `Join` in its op graph, so `classify_build` marks all of them
20+
"worthy". Each iteration writes a 100+ MB cache for a result that is trivial to
21+
recompute (a row-wise scalar pass over an already-cheap input). The cache fills
22+
with low-value copies.
23+
24+
The structural test cannot tell *"expensive to produce"* from *"contains an
25+
expensive op upstream but is cheap to recompute"*. It also cannot tell a
26+
big-cheap result from a small-expensive one — exactly the distinction that
27+
matters. Concretely, two results in the same lineage:
28+
29+
- the 2.5M-row computed-column intermediate: ~150 MB, recompute ≈ re-reading the
30+
source (I/O-bound on the same bytes) — caching saves ~nothing;
31+
- the ~6.4k-row route aggregate at the end: <1 MB, recompute requires rescanning
32+
2.5M rows + join + distance + aggregate — caching saves almost all of it.
33+
34+
Profiling the live path (see the buckaroo xorq-search investigation) showed
35+
per-view cost is dominated by re-compiling the large expression DAG to SQL on
36+
every execute; the win from caching a *reduced* result is that the viewed/diffed
37+
expression collapses to `read_parquet ⋈ read_parquet`, cheap to both compile and
38+
execute. The win exists only for results that reduce data; it is absent for
39+
passthroughs whose output is as large as their input.
40+
41+
## Decision
42+
43+
Replace the structural admission test with a measured cost-vs-size model, and
44+
move the admit/skip decision off a per-entry gate and onto a budget with
45+
cost-aware eviction.
46+
47+
1. **Measure at build, always.** Every entry is executed on add (we never add an
48+
expression without executing it; unbound expressions are out of scope), so
49+
the cost is already available. Record both `execute_seconds` (already in
50+
`manifest.json`) and a new `compile_seconds` — the expr→backend-plan step
51+
(the `to_sqlglot`/sqlize path), timed separately from execution. Record the
52+
materialised result's size in bytes.
53+
54+
2. **Admit permissively; do not gate per entry.** There is no structural or
55+
cost-floor admission test. Any built result is a cache candidate. The
56+
structural `classify_build` / `cache_worthy` path is removed (this supersedes
57+
#12, which sought to make that classifier robust — there is nothing left to
58+
robustify).
59+
60+
3. **Rank by value-per-byte.** A cache's value is what reading it back saves
61+
over recomputing: `value ≈ recompute_cost − cache_read_cost`, divided by
62+
bytes occupied. This declines to ~0 for cheap row-wise passthroughs whose
63+
output ≈ input size (the 2.5M intermediate) and is high for reducing /
64+
aggregating expressions (the 6.4k aggregate). `recompute_cost` is
65+
`compile_seconds + execute_seconds`; `cache_read_cost` is estimated from the
66+
result's byte size.
67+
68+
4. **Bound by a budget relative to raw data.** Cap the project's total result
69+
cache at K× the size of `data/` (start at 2×, matching #10). When the cache
70+
exceeds the budget, evict lowest value-per-byte first. Evicted results
71+
self-heal: `ensure_result` / `cached_result_expr` recompute from the build on
72+
the next miss. This is the eviction half of #10; admission and eviction now
73+
share one cost model.
74+
75+
5. **Key on `content_hash`, never mtime.** The cache key is the entry's
76+
`content_hash`, frozen at build time (`build.py:271`) and stable across
77+
reset-to and reload. No modification-time comparison appears on the
78+
correctness path. (`content_hash` being a *content*-stable identifier is the
79+
subject of the related ADR.)
80+
81+
Sub-expression / "cache at the divergence point" caching is explicitly **out of
82+
scope**. In the motivating lineage the divergence point is the 2.5M-row join —
83+
big and cheap to recompute — so caching there is the opposite of what the cost
84+
model wants. v0 caches per-entry results only.
85+
86+
## Alternatives considered
87+
88+
- **Keep the structural classifier (status quo).** Mis-caches the
89+
iterative-computed-column workflow (Join upstream ⇒ every descendant worthy)
90+
and cannot separate big-cheap from small-expensive. This is the bug.
91+
- **Make the structural classifier robust (#12).** Fixes silent
92+
misclassification from regex drift but keeps the structural model, so it still
93+
caches the 2.5M intermediates. Necessary only if we stay structural; this ADR
94+
removes that need.
95+
- **Cache at the divergence / lowest-common-ancestor sub-expression.** Would
96+
materialise the large shared intermediate (2.5M-row join) once and let both
97+
metric variants read it. Rejected: that intermediate is precisely what we do
98+
*not* want cached — it is large and cheap to recompute, and the variants
99+
diverge *on* it (haversine vs manhattan), not below it.
100+
- **Per-entry cost-floor admission gate.** Redundant once a budget plus
101+
value-per-byte eviction exists; a permissive admit + eviction expresses the
102+
same intent without a second threshold to tune.
103+
104+
## Consequences
105+
106+
- Storage is bounded by a multiple of raw data size instead of growing with edit
107+
count. The iterative-column workflow stops bloating the cache.
108+
- route_distance diffs stay fast: both 6.4k endpoints are small and
109+
high-value, so they are cached, and the diff is `read ⋈ read`.
110+
- Requires new instrumentation: `compile_seconds` in the build path and result
111+
byte-size in the manifest. Accurate cache accounting is a prerequisite for
112+
eviction — the `/api/disk_usage` pill currently omits `result_cache/` and
113+
`diff_stat_cache/` (noted in #10) and must be corrected.
114+
- Eviction depends on correct recompute self-healing, which already exists.
115+
- The `value ≈ recompute_cost − cache_read_cost` estimate is a heuristic; the
116+
read-cost term is approximated from bytes rather than measured. Acceptable
117+
because misranking only costs a recompute, never correctness.
118+
- Scope is `result_cache` only. Buckaroo's own stat caches
119+
(`.buckaroo_stat_cache`, `diff_stat_cache`) are out of scope; tallyman will
120+
only hint to Buckaroo (separate sketch).

‎plans/adr-source-identity-content-hash.md‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
# ADR: Content-address source reads so entry identity survives reset and transport
2+
3+
- **Status:** Proposed (2026-06-10)
4+
- **Context ticket:** buckaroo-data/tallyman#30 (precondition for the result-cache rubric's reset-stable `content_hash` key)
5+
- **Affected code:** `src/tallyman_xorq/io.py` (`from_project`, `from_catalog`)
6+
- **Related ADR:** `plans/adr-result-cache-cost-rubric.md` (relies on `content_hash` being a stable cache key)
7+
8+
## Problem
9+
10+
An entry's `content_hash` is xorq's build hash, and the README states the
11+
contract "same code + same inputs → same hash → same entry dir (idempotent)".
12+
That contract is weaker than it reads, because of how xorq hashes a parquet
13+
source by default.
14+
15+
`xo.deferred_read_parquet` defaults to `normalize_method=normalize_read_path_stat`
16+
(`xorq/python/xorq/common/utils/defer_utils.py:235`), which contributes
17+
`(st_mtime, st_size, st_ino)` to the hash, not the file's content
18+
(`defer_utils.py:84`). The build-hash tokenizer confirms this: for an absolute
19+
path that exists it calls `read.normalize_method(p)`
20+
(`xorq/python/xorq/common/utils/dasher/_relations.py:85`), and the compiler's
21+
canonicalization defaults to the same stat method (`ibis_yaml/compiler.py:261`,
22+
`compiler.py:400`). The content-hash variant `normalize_read_path_md5sum` exists
23+
but is opt-in. (Memtables are separately forced to content-md5 at
24+
`compiler.py:602`; only file reads use stat.)
25+
26+
tallyman lands in the stat branch: `from_project` resolves to an **absolute**
27+
path and calls plain `deferred_read_parquet` (`io.py:38`), and its own docstring
28+
notes the build embeds absolute paths. So **`content_hash` depends on the source
29+
file's `(mtime, size, inode)`**, with these consequences:
30+
31+
1. **Reset-to / git checkout forks identity for identical content.** Checkout
32+
writes a new file — new inode, new mtime — so re-deriving an entry after a
33+
reset yields a *different* `content_hash` for the same logical input.
34+
2. **Cross-machine transport forks identity.** `tallyman pack` / `serve` on
35+
another machine sees different mtime+inode, so the same project rebuilds to
36+
different hashes.
37+
3. **In-place result regeneration forks downstream identity.** `ensure_result`
38+
regenerates a cheap entry's `result.parquet` in place
39+
(`result_cache.py:142`), changing its mtime+inode; any child built via
40+
`from_catalog` afterward gets a different hash.
41+
4. **Conversely, a stat-preserving content swap goes stale.** A replacement that
42+
preserves size+mtime+inode keeps the hash, so a genuine content change is
43+
missed.
44+
45+
This does not corrupt an *existing* entry — `content_hash` is frozen into the
46+
dir name at build (`build.py:271`) and never recomputed on load, so caches keyed
47+
on it stay valid through reset and reload. The damage is on *re-derivation*: a
48+
logically-identical rebuild produces a "new" duplicate entry, defeating the
49+
build-time dedup (`build.py:275`) and orphaning the caches keyed on the old
50+
hash.
51+
52+
## Decision
53+
54+
Pass `normalize_method=normalize_read_path_md5sum` in `from_project` and
55+
`from_catalog`, so a source read contributes its content digest to the build
56+
hash instead of filesystem stat.
57+
58+
## Alternatives considered
59+
60+
- **Keep stat hashing (status quo).** Fragile under exactly the operations
61+
tallyman wants to support — reset-to, pack/serve across machines, in-place
62+
regeneration. Rejected.
63+
- **Build-relative `read_path` hashing.** xorq hashes a build-relative path as
64+
the path string alone (`dasher/_relations.py:81`), which is portable but
65+
content-*insensitive*: a content swap at the same relative path is never
66+
detected. Worse than stat for correctness. Rejected.
67+
- **mtime-based cache invalidation layered on top.** Rejected on independent
68+
grounds: git checkout rewrites mtimes to "now" non-monotonically, so any
69+
mtime ordering check is fooled by reset in both directions (false-keep and
70+
false-invalidate). Content-addressing is the only reset-safe key.
71+
72+
## Consequences
73+
74+
- `content_hash` becomes stable across reset, machine, and in-place
75+
regeneration; the README idempotency claim becomes true, and `pack`/`serve`
76+
portability (currently flagged as not-yet-working in `io.py`) gets its
77+
identity precondition.
78+
- This is the precondition the result-cache ADR relies on when it keys caches on
79+
`content_hash` and asserts reset-safety.
80+
- One md5 pass over each source file per build. Cheap — the file is already read
81+
to infer schema — and one-time per content hash.
82+
- This is a tallyman-side usage change only (which `normalize_method` we pass);
83+
**no modification to xorq**. Consistent with xorq's own choice to content-hash
84+
memtables.
85+
- Deliberately recorded as its own ADR, separate from the cache-rubric ADR, so
86+
the source-identity decision can be revisited atomically without reopening the
87+
caching design.

0 commit comments

Comments
 (0)