Repository navigation
fix(163): reads load the frozen build — contract, red tests, and the canonical-read fix - #167
Merged
Merged
Conversation
… audit docs/system-contract.md states how the system is supposed to work, in response to #163: the xorq substrate from first principles, the tallyman primitives, the binding-time rule (names resolve exactly once, at build), the read/write/cache contracts, and invariants I1-I5. Written as the pre-grilling proposal: it describes the post-fix world, with current-main deviations and open questions collected at the end. plans/cache-soundness-audit.md inventories the same bug class beyond #163's axis: the project binding (W1/W2/W8), CSV ingest identity (W3), primary-key lineage (W4), the Buckaroo handoff (B1/B3), and Buckaroo 0.15.4's own caches. W1/W3/B3 verified directly in source. docs/architecture.md gains an index entry pointing at the contract. Refs #163, #166. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nvariants Five tests over the #163 shape (two revisions of one recipe, distinct only by recorded parent) asserting system-contract invariants: a cheap historical read matches its recorded row_count (I1/I4), two worthy revisions resolve distinct snapshots (I1), an untouched entry verifies faithful (I4), a diff of two revisions is not a self-join (I5), and a cold-cache read reproduces build bytes (I2). EXPECTED TO FAIL on this branch: all five fail on main (assert 37 == 200; identical snapshot paths; verify False; all deltas 0.0; heal manufactures head bytes). Per TDD they must be seen failing on CI before the fix lands. The cold-cache failure also demonstrates the taxonomy gap: the heal logs the lineage bug as execution nondeterminism (#83). Refs #163. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… issues plans/adr-read-path-loads-builds.md records the eight decisions from the grilling session over the system contract: D1 one PR + one rebuild; D2 keep the cached_result_expr/baked_snapshot_path facade, replace internals with the canonical read; D3 rebind composition onto the default backend with a one-content-profile assertion; D4 chaining inlines parent cache nodes (pre-heal retired, deep cache-dir rewrite, corpus rebuild); D5 digest order-insensitivity scoped to CSV (#171 tracks the parquet residual); D6 missing/unloadable build is a hard error, no recipe fallback; D7 verify failures to errors.jsonl + SSE plus a scan sweep flag; D8 manifest records the snapshot key, reads assert it. docs/system-contract.md amended to match (fallback clause, open questions 1/3/5 resolved). plans/cache-soundness-audit.md findings linked to their filed issues: #168 (CSV identity), #169 (PK lineage), #170 (klass scan path), #172 (cross-project sessions), buckaroo#955/#956/#957. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three more failing-first tests: a missing xorq_build/ must raise naming the entry (D6 — main silently reconstructs, DID NOT RAISE); a cold load of a chained child's build must heal the worthy parent's snapshot with no pre-heal (D4 — main dies on the bare read: 'At least one path is required'); an unfaithful heal must wipe the entry's .buckaroo_stat_cache (D10 — main only logs, sentinel survives). All three seen failing locally for these exact reasons; 8 red + probe green. Refs #163, plans/adr-read-path-loads-builds.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The probe builds a 250k-group aggregate over a >1MiB parquet source and heals it three times: locally it observes THREE distinct snapshot digests, none reproducing the build digest, with the advisory firing each time. #171's drift is not a tail risk — at scale it fires on every heal. The probe pins the mechanism (verify agrees with bytes; mismatch surfaces the advisory) and passes on main; the empirical result argues for revisiting ADR D5, since D10/D12 would false-positive on every heal of large parquet-sourced entries. ADR gains the round-3 decisions: D9 no cheap-entry digests; D10 unfaithful heal wipes Buckaroo state; D11 second red-test commit; D12 unfaithful entries pinned + badged. Contract open questions 4/7/8 marked resolved. Refs #163, #171. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d chaining, canonical-order bake Implements plans/adr-read-path-loads-builds.md (D1-D12, incl. the D5 amendment) and flips the eight red lineage-faithfulness tests green. Read path (D2/D6): cached_result_expr / baked_snapshot_path internals become the canonical read — ensure_expanded_build -> load_expr(cache_dir=compute cache) -> portable.rewrite_cache_dirs, a deep cache-dir rewrite over xorq's replace_nodes that reaches CachedNodes nested inside CachedNode.parent, which xorq's own shallow rewrite misses. A missing or unloadable build is a hard error naming the entry; recipe re-execution survives only at minting and in the structural-nondeterminism diagnostic. Chaining (D3/D4): tracked/pinned_expr_from_alias return entry_graph_expr — the parent's loaded build, cache node included, rebound onto the process default backend via replace_sources with a one-content-profile guard. Child builds are self-contained and self-healing, so the ensure_session pre-heal and the #75 stripping are deleted. Child manifests still record ancestors' source digests (io._note_parent_sources) so gc_cas keeps a child's CAS leaves alive independently of its parent entry. Canonical-order bake (D5, amended): rewrite_for_build sorts a worthy expression by all sortable columns before the result-cache wrap (author sort keys stay primary, then original_row_order, then schema order; constant projections skipped — datafusion rejects ORDER BY over a bare literal). The #171 probe that showed 3 distinct digests over 3 heals is now a regression test pinning byte-equality. Verification (D7/D8/D10/D12): every self-heal verifies the healed bytes against result_digest before serving; a mismatch records a durable unfaithful_heal error (the pin marking + UI badge source), wipes the entry's .buckaroo_stat_cache, and fires hooks the companion registers for session eviction + an SSE badge event. The build records snapshot_key in the manifest and the read asserts its own derivation matches. catalog_scan_staleness gains verify_results=True (staleness.verify_sweep); _invalidate_reset_caches now also wipes diff_stat_cache/. Test updates riding with the fix (behavior changes, cannot fail-first): the pre-heal unit test now pins the self-contained-build replacement at the same seam; the structural-drift-on-read test pins its inversion (a baked-literal entry reads faithfully from its frozen build); the cross-process heal stub uses the new _ResultPlan shape. Requires a corpus rebuild at landing (chaining and the sort change content hashes) per the project's no-migration policy. Closes #163. Closes #171. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
paddymul
added a commit
that referenced
this pull request
Sep 24, 2026
…hanged Plans and research notes stay point-in-time records. Each of these asserts current behaviour that the read-path fix (#167) or the cache redesign (#189) changed, so each gets a short dated status note saying what no longer holds and where the current behaviour is described. Nothing else in them changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
paddymul
added a commit
that referenced
this pull request
Sep 24, 2026
…hanged Plans and research notes stay point-in-time records. Each of these asserts current behaviour that the read-path fix (#167) or the cache redesign (#189) changed, so each gets a short dated status note saying what no longer holds and where the current behaviour is described. Nothing else in them changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The full #163 arc: state how the system is supposed to work (
docs/system-contract.md), pin it with tests that fail onmain, then land the fix that makes the contract true. Design decisions are recorded inplans/adr-read-path-loads-builds.md(D1–D12); the bug-class survey beyond #163's alias axis isplans/cache-soundness-audit.md.The contract (Part 1: the five xorq ideas tallyman is built on; Part 2: the tallyman primitives; Part 3: the lifecycle; Part 4: invariants I1–I5). The core rule: after a build, the frozen
xorq_build/is the entry's semantics andexpr.pyis documentation. A content hash names a fixed result; names resolve exactly once, at build.The red suite — eight tests over the #163 shape (two revisions of one recipe, distinct entries only because their recorded parent moved), each seen failing on CI with its designed assertion across two red waves:
assert 37 == 200— serves boots-only rows from the live headverify_result_faithfulreturnsFalsen_abs_deltaexactly 0.0expr.pyValueError: At least one path is requiredThe fix (
ae58484):cached_result_expr/baked_snapshot_pathinternals load the frozen build —ensure_expanded_build→load_expr(cache_dir=compute cache)→portable.rewrite_cache_dirs, a deep cache-dir rewrite that reaches cache nodes nested insideCachedNode.parent(xorq's own rewrite is shallow). A missing or unloadable build raises, naming the entry; there is no recipe fallback.tracked_expr_from_aliasreturns the parent's loaded build with its cache node intact, rebound onto the process default backend under a one-content-profile guard. Theensure_sessionpre-heal and the build: #73's expression-level from_catalog breaks any entry combining ≥2 catalog parents — 'Multiple backends found' #75 stripping are deleted. Child manifests still record ancestors' source digests sogc_caskeeps a child's CAS leaves alive.original_row_order, then schema order). The probe is now a regression test pinning byte-equality across heals.result_digestbefore serving; a mismatch records a durableunfaithful_healerror (the pin marking and UI badge source), wipes the entry's stat cache, evicts its Buckaroo session, and pushes an SSE event. Builds recordsnapshot_key; reads assert their derivation matches.catalog_scan_staleness(verify_results=True)sweeps the corpus.Local: fast suite 690 passed / 0 failed, integration 7 passed, ruff clean.
Rebuild required at landing
Chaining and the canonical sort both change content hashes; per the project's no-migration policy the corpus is rebuilt (and Buckaroo bounced,
diff_stat_cache/wiped) instead of migrated.Closes #163. Closes #171. Refs #166, #168–#172, buckaroo#955–957.
🤖 Generated with Claude Code