Skip to content

docs(plans): ADR-007, ADR-008, ADR-009 — the cache redesign (proposed, for review) - #184

Closed
paddymul wants to merge 4 commits into
mainfrom
docs/adr-007-009-cache-redesign
Closed

paddymul wants to merge 4 commits into
mainfrom
docs/adr-007-009-cache-redesign

Conversation

@paddymul

@paddymul paddymul commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Three draft ADRs for review, with the scripts they cite. Docs and scripts only; nothing here is implemented, and implementation does not start until all three have been reviewed.

This replaces #180, #181 and #182, which held one ADR each. The first commit is their content unchanged. The second folds in the last answers from the 2026-09-19/20 design session. The third folds in an adversarial review of this PR and Paddy's decisions on its findings. The fourth (f777d06) folds in a second review, which read the ADRs against the code and probed each claim with a script. Its decisions and findings are below.

What to read

File Decides
plans/ADR-007-tallyman-owned-materialization.md Tallyman stops using xorq's cache. It writes its own result files, makes sure every file an entry reads exists before anything runs (snapshots, ordered copies of sources, source clones), and hands Buckaroo things that already exist. A reset no longer manages the cache directory. Start here: its "Governing rule" section sets the priority for all three.
plans/ADR-008-row-order-of-reads.md Every file tallyman writes carries a visible __row_order column, and every page request sorts by it. It also decides what a cheap entry may contain, how sorts and joins stay deterministic, and which reads are build errors.
plans/ADR-009-digest-stability.md A rewritten file is flagged only when the result changed: single-stream materialization, a digest of content instead of file bytes, and a materialized entry's query run twice at create.

Each ADR opens with a Terms section, and a bare label such as "D5" always means that ADR's own decision. Another ADR's decision is always written with its number and what it decides.

Decisions made in the design session

  • Buckaroo is a displayer. It runs queries only for summary stats, sorting and paging. Tallyman runs an entry's query to completion before asking Buckaroo to show it (ADR-007, governing rule and D6).
  • Cohesion and reliability before speed. Where a uniform rule and a faster special case compete, the uniform rule is the decision and the faster path is noted for later.
  • __row_order: int64, 0..N-1, last column, visible. Each materialization overwrites it. A cheap entry that drops it is a build error. Only the exact name is reserved, so a debugging copy such as __row_order_v1 survives (ADR-008 D2, D3, D6).
  • Create runs a materialized entry's query twice and compares digests, so a recipe that is not reproducible is known from birth and its file is pinned (ADR-009 D6).
  • One write at a time per project, through the existing project file lock. Two tallyman servers on one project is unsupported: two tallyman servers on one project is unsupported and nothing detects it — each holds in-process state the other never sees #183 (ADR-007 D11).
  • Files are deleted only by an explicit user action, and a file is written only because something is about to read it. No disk budget yet (ADR-007 D12).
  • Order of work: one commit of failing tests seen red on CI, then the redesign as one change and one corpus rebuild, then the independent bugs (ADR-007 D9). Normal TDD: every test goes in the failing-tests commit, and a test of a function that does not exist yet fails on import, which counts as red.

Buckaroo's half of the paging fix is filed as buckaroo-data/buckaroo#974.

Decisions made in the second review (fourth commit)

  • Two kinds of entry stay. A worthy entry is one tallyman writes a parquet file for at create. A cheap entry has no file of its own, and its plan re-runs on every read. This closes ADR-007 open question 1. The consequence is that the test for "cheap" is now a correctness gate: a cheap entry pages by its parent's __row_order, so the test has to guarantee the column is still present and unique (ADR-008 D4).
  • The natural order is imposed on every order_by. Natural order is a row's position in the file it came from, which __row_order records. Paddy's rule: when a supplied sort is not deterministic, impose the natural order into each order by. It is applied at every sort in a recipe and on every page request, with the remaining sortable columns after it (ADR-008 D10).
  • A sort that is not the recipe's last step is hoisted, or the build fails (ADR-008 D11). To hoist is to lead the top-level sort with the keys of the author's sort from lower in the recipe. If a key did not survive as an output column, the build fails and names the key.
  • A reset leaves compute_cache/ alone, and source clones are parked, not deleted. A clone is the frozen copy of a source file under data/.cas/. Each class of file gets a re-create rule, and a file is cache only if ensure_materialized can re-create it (ADR-007 D13, D14).
  • Staging the landing (one change against several) was ruled out of scope for this round, so ADR-007 D9 stands as written.

What the second review found

Each is backed by a script in this PR or by the source it cites.

  • Joining three entries fails, and ADR-008 D6 said otherwise. a.join(b).join(c) shows the expected columns and passes build_expr, then raises IntegrityError: Name collisions: {'__row_order_right'} from the canonical sort. A join entry that kept __row_order_right fails the same way when joined to a third. The writer now drops that column, and the build turns ibis's message into an instruction (ADR-008 D6).
  • A sort that is not last is discarded. _canonical_sorted extends an author's sort only when it is the top node. After order_by(amount desc), a following mutate, select or filter leaves the rows in parent order, and a top-three entry is written as 40, 60, 50. The entry is still classed worthy, so the author pays for a copy that ignores the sort (ADR-008 D11).
  • A sort that feeds a limit decides which rows the entry holds. order_by(g).limit(1000) with about 15,000 rows tied at the cut returned 5 different row sets in 5 runs on the default connection, and 1 with the natural order appended (ADR-008 D10).
  • A list of relation operations is not enough for "cheap". An unnest inside a select turned 6 rows into 8 with duplicate __row_order. Window functions, random() and a filter against a second file are also plain selections or filters to that list. The test now has three parts (relation allow-list, exactly one file read, no value operation that multiplies rows, depends on row order or is not pure), is computed once at build and recorded in the manifest, and retires the regex over expr.yaml (Cache classifier parses xorq build YAML with regex — silent misclassification if the format drifts #12) (ADR-008 D4).
  • A recipe can read a parquet file without going through ingest. xo.deferred_read_parquet(abs_path) is what tallyman's own hints recommend. It gets no clone, no digest and no ordered copy, so the entry has no __row_order. A raw parquet read becomes a build error (ADR-008 D12). Nine test files call it today.
  • A reset back and then forward breaks entries on today's code. reset_to parks entries and snapshots in the bullpen (the directory a reset moves retired files into) but gc_cas deletes clones outright. After a reset to an earlier step and forward again, a cheap entry fails immediately with At least one path is required although the live source is unchanged, and a worthy entry fails at its next heal. This is independent of this PR, and it is the state the "accepted gap" in ADR-007 D5 would have left reachable. D5 no longer accepts it (ADR-007 D13).
  • Reset was absent from all three ADRs. It keeps a git-tracked list of cache files, prunes to it, and copies files back with a bare shutil.copy2, which is a second, non-atomic writer of snapshots (ADR-007 D14).
  • A single-partition connection does not reach a loaded build by itself. load_expr makes its own backend objects, so a build loaded beside a target_partitions = 1 connection still ran on 14 partitions: 5 distinct digests in 5 runs. Rebinding it with replace_sources, or SET on each backend it made, gave 1 (ADR-009 D1).
  • The klass reload needs the session record that ADR-007 D6 deletes. A klass is a project-authored stat, post-processing or display class. reload_project_sessions finds open grids in that record, and Buckaroo has no route that lists sessions. With derived session ids the reload posts /reload_expr/<id> per entry and treats a 404 as "not open" (ADR-007 D6).
  • Checked and held. polars writes exactly 122,880-row groups at 1, 3 and 14 threads, and scan_parquet().with_row_index() numbers a 184-row-group source in file order, which answers ADR-008 open question 1's "needs checking". The design also closes viewer: expensive-parent children render an empty buckaroo grid on a cross-machine clone — /load_expr replays the build, whose snapshot key is bound to the build-time path #77 (an empty grid on a clone at another path), because the snapshot's name is now the entry's content hash on every machine.
  • Parquet statistics cannot show a sort key is unique. pyarrow writes min, max and null count, and a unique column and one holding a value twice have the same ones. So the tie-break is always appended, and the ADR records why (ADR-008 D5).

Also changed: a create always runs the query and replaces any file at the path, and only a heal uses the existence shortcut (ADR-007 D4). A heal runs the query once (ADR-009 D6). The format version covers the ordered copies polars writes (ADR-009 D3). Under salt identity mode a worthy entry is written without the canonical sort while the early return in rewrite_for_build stays (ADR-007 Consequences).

Not yet confirmed by Paddy

Each is marked "not yet confirmed" in the ADR text, so any can be struck.

Moved out of this set

Moved To
ADR-007 D10, every diff is built as an entry before it is displayed. D10 stays as a stub so that D11 and D12 keep their numbers. Until then the live diff is the one known exception to the governing rule. #188
Non-pure recipes: what an entry inherits from a non-reproducible parent, and where a pinned file lives #185
The waiting that the project-wide write lock causes #186
What else decides a float's low bits #187
Ordering inside window functions and ordered aggregates (ADR-008 D10 leaves them there) #146

Already tracked, and cited: #168 (CSV source identity), #118 (Already borrowed on concurrent reads), #12, #76, #77 and #22.

Not filed as issues, and recorded in the ADRs only: the clone loss across a reset (above), the klass reload gap, and the memory a deep sorted page holds (ADR-008 Consequences; a performance matter, taken up after correctness).

Still open

  1. Parquet sources get an ordered copy carrying __row_order, and original_row_order is renamed to __row_order. Both are adopted as defaults and not confirmed in so many words (ADR-008 open questions 1 and 2).
  2. What the ordered-copy directory is called (ADR-007 open question 3). ADR-007 D13 settles that copies are cache and live under the project.
  3. Deep chains of cheap entries, which nothing cuts (ADR-007 open question 2).

Evidence

Every figure in the ADRs comes from a script in this PR, each of which runs from a clean temp dir:

  • scripts/spike_bare_read_chaining.py (ADR-007 D3)
  • scripts/spike_reset_roundtrip.py (ADR-007 D13, D14)
  • scripts/spike_stream_order.py (ADR-007 open question 1, ADR-009 D1)
  • scripts/spike_window_read_order.py (ADR-008, the problem and the rejected engine setting)
  • scripts/spike_row_order_paging.py (ADR-008 D3, D5, D6)
  • scripts/spike_csv_source_identity.py (ADR-008 D2, D7)
  • scripts/spike_deep_page_memory.py (ADR-008 Consequences)
  • scripts/spike_cheap_classifier.py (ADR-008 D4)
  • scripts/spike_row_order_joins.py (ADR-008 D6)
  • scripts/spike_sort_grafting.py (ADR-008 D5, D10, D11)
  • scripts/spike_ordered_copy_layout.py (ADR-008 D2, ADR-009 D3)
  • scripts/spike_float_aggregate_digest.py (ADR-009 D1)
  • scripts/spike_float_layout_digest.py (ADR-009 D1, D3)
  • scripts/spike_single_partition_loaded_build.py (ADR-009 D1)
  • scripts/spike_logical_digest.py (ADR-009 D2, D3)

Checked locally: ruff check across the repo, and ruff format --check on the fifteen spike scripts. The seven new scripts were each run and reproduce the figures cited. The eight from earlier commits were not re-run in this round. Every script path the ADRs cite resolves, and the source line references added in this round were checked against the code. Docs and scripts only, so pytest was not run and CI was not watched.

🤖 Generated with Claude Code

paddymul and others added 2 commits September 20, 2026 10:29
Brings the three draft ADRs from the 2026-09-18 cache audit, and the
spikes they cite, onto one branch so they can be reviewed together. They
were drafted as separate PRs (#180, #181, #182) and revised there during
the 2026-09-19/20 design session; this commit is their state at the end
of that session, unchanged.

- ADR-007: tallyman owns result materialization (no xorq cache nodes in
  builds); Buckaroo is a displayer; every diff is built as an entry.
- ADR-008: every file carries a visible __row_order column and every page
  request sorts by it.
- ADR-009: materialization runs single-partition, and result_digest is a
  digest of the snapshot's content.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… ADRs

- ADR-009 D6 (new): create runs the query twice and compares digests, so
  a non-reproducible recipe is known from birth and its file is pinned;
  a cheap entry gets the same check and is materialized if it fails.
  Adds a Testing section. The entry's schema is read from the file.
- ADR-007 D11 (new): one write at a time per project, by taking the
  existing project file lock around every write; replaces the per-entry
  lock. Two tallyman servers on one project is unsupported (#183).
- ADR-007 D12 (new): files are deleted only by an explicit user action;
  the startup warm-up stops rewriting deleted files; no disk budget yet.
- ADR-007 D6: tallyman stops remembering Buckaroo sessions and re-posts
  every time, with a session id derived from project, hash and view kind.
- ADR-007 D9: the agreed order of work. Nothing starts before review.
- ADR-007 open question 1: whether two kinds of entry survive. Unanswered.
- Testing sections for ADR-007 and ADR-008, marking which tests are red
  on main today.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
paddymul and others added 2 commits September 20, 2026 11:44
An adversarial review of the three draft ADRs, and Paddy's decisions on
its findings (2026-09-20). Still Proposed; nothing is implemented.

- ADR-007 D10 (every diff is built as an entry) moved out to #188, so the
  set stays about the core structure of the cache. D10 is kept as a stub
  so that references to D11 and D12 stay valid. The live diff is recorded
  as the one known exception to the governing rule.
- ADR-007 D5: the verify sweep is not a caller of ensure_materialized; it
  reads and never writes. The ordered-copy gap is recorded as accepted.
- ADR-007 D6: the session id is derived from project and content hash
  (closes #172); the exact condition under which Buckaroo skips a repeat
  /load_expr; deleting a snapshot ends no session; an unfaithful heal
  posts force_reload, since evict_session worked by forgetting a record
  that D6 removes.
- ADR-007 D11: three limits of the project lock (per-thread re-entrancy,
  recalc granularity left open, reads not covered, #118). #186 tracks the
  waiting it causes.
- ADR-007 D12: a file is written only because something is about to read
  it. The warm-up sentence now states the precise condition.
- ADR-007 D2: memoizing the read stops tables piling up, not footer reads.
- ADR-008 D2, D7: CSVs do not already work this way. The intermediate is
  keyed by path and overwritten in place (#168), so D7 cannot land before
  that fix. CSVs go through source identity and the ordered copy is built
  from the content-addressed clone.
- ADR-008: "repeatable pages" asserts the exact rows; a new test that an
  edited CSV forks the hash; memory at depth recorded under Consequences
  as a performance matter, deferred.
- ADR-009 D1, D3: single-partition execution does not make an ungrouped
  float total a function of the rows alone; it depends on the parent
  file's row-group layout (#187). Row-group size and batch_size are pinned
  and the manifest records a snapshot format version.
- ADR-009 D6: the cheap-entry half moved to #185; the limits of running
  twice are stated, and the #88 lint is mentioned.
- ADR-009: four stale cross-references to ADR-008 fixed.
- All three Testing sections: normal TDD. Every test goes in the
  failing-tests commit; a test of a missing function fails on import.

New evidence scripts, each runnable from a clean temp dir:
scripts/spike_csv_source_identity.py (ADR-008 D2, D7),
scripts/spike_float_layout_digest.py (ADR-009 D1, D3),
scripts/spike_deep_page_memory.py (ADR-008 Consequences).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Keep two kinds of entry (ADR-007 open question 1). ADR-007 gains D13 (a file is cache only if ensure_materialized can re-create it) and D14 (a reset leaves compute_cache alone), closes the ordered-copy gap in D5, and adds the klass hot-reload fix to D6. ADR-008 rewrites D4 as a three-part test for cheap, corrects D6's three-way join claim, and adds D10 (natural order on every order_by), D11 (hoist a non-final sort) and D12 (a raw parquet read is a build error). ADR-009 says how a loaded build gets onto the single-partition connection and extends the format version to ordered copies.

Adds seven evidence scripts under scripts/. Items not yet confirmed by Paddy are marked as such in the ADR text.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@paddymul

Copy link
Copy Markdown
Contributor Author

Contained in #189: this branch is an ancestor of feat/adr-007-009-cache-redesign.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant